From efc1b412f199bbbe2474d4c5396dc4b39a6beff7 Mon Sep 17 00:00:00 2001 From: Jeff Gaston Date: Wed, 29 Mar 2017 17:29:06 -0700 Subject: [PATCH] Have Soong try to enforce that genrules declare all their outputs. This causes Soong to put the outputs of each genrule into a temporary location and copy the declared outputs back to the output directory. This gets the process closer to having an actual sandbox. Bug: 35562758 Test: make Change-Id: I8048fbf1a3899a86fb99d71b60669b6633b07b3e --- Android.bp | 21 +++---- android/config.go | 4 ++ cmd/sbox/Android.bp | 21 +++++++ cmd/sbox/sbox.go | 133 ++++++++++++++++++++++++++++++++++++++++++++ genrule/genrule.go | 46 ++++++++++++--- shared/paths.go | 26 +++++++++ ui/build/Android.bp | 1 + ui/build/build.go | 2 + ui/build/config.go | 8 ++- ui/build/util.go | 13 +++++ 10 files changed, 254 insertions(+), 21 deletions(-) create mode 100644 cmd/sbox/Android.bp create mode 100644 cmd/sbox/sbox.go create mode 100644 shared/paths.go diff --git a/Android.bp b/Android.bp index 9f508d57d..a02926558 100644 --- a/Android.bp +++ b/Android.bp @@ -1,15 +1,3 @@ -// -// WARNING: Modifying this file will NOT automatically regenerate build.ninja.in! -// -// Before modifying this file make sure minibp is up to date: -// 1) "repo sync build/soong" to make sure you have the latest build.ninja.in -// 2) build minibp, which builds automicatically through the normal build steps. For example: -// -// After modifying this file regenerate build.ninja.in and build your changes: -// 1) In your build directory, execute "../bootstrap.bash -r" to regenerate build.ninja.in -// 2) Build again -// - subdirs = [ "androidmk", "cmd/*", @@ -168,6 +156,7 @@ bootstrap_go_package { "blueprint-pathtools", "soong", "soong-android", + "soong-shared", ], srcs: [ "genrule/filegroup.go", @@ -233,6 +222,14 @@ bootstrap_go_package { pluginFor: ["soong_build"], } +bootstrap_go_package { + name: "soong-shared", + pkgPath: "android/soong/shared", + srcs: [ + "shared/paths.go", + ], +} + // // Defaults to enable various configurations of host bionic // diff --git a/android/config.go b/android/config.go index 869a5c431..c3beb08ee 100644 --- a/android/config.go +++ b/android/config.go @@ -52,6 +52,10 @@ type Config struct { *config } +func (c Config) BuildDir() string { + return c.buildDir +} + // A DeviceConfig object represents the configuration for a particular device being built. For // now there will only be one of these, but in the future there may be multiple devices being // built diff --git a/cmd/sbox/Android.bp b/cmd/sbox/Android.bp new file mode 100644 index 000000000..fe4c7bbce --- /dev/null +++ b/cmd/sbox/Android.bp @@ -0,0 +1,21 @@ +// Copyright 2017 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +blueprint_go_binary { + name: "sbox", + srcs: [ + "sbox.go", + ], +} + diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go new file mode 100644 index 000000000..ead34437f --- /dev/null +++ b/cmd/sbox/sbox.go @@ -0,0 +1,133 @@ +// Copyright 2017 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "fmt" + "io/ioutil" + "os" + "os/exec" + "path" + "path/filepath" + "strings" +) + +func main() { + error := run() + if error != nil { + fmt.Fprintln(os.Stderr, error) + os.Exit(1) + } +} + +var usage = "Usage: sbox -c --sandbox-path " + +func usageError(violation string) error { + return fmt.Errorf("Usage error: %s.\n %s", violation, usage) +} + +func run() error { + var outFiles []string + args := os.Args[1:] + + var rawCommand string + var sandboxesRoot string + + for i := 0; i < len(args); i++ { + arg := args[i] + if arg == "--sandbox-path" { + sandboxesRoot = args[i+1] + i++ + } else if arg == "-c" { + rawCommand = args[i+1] + i++ + } else { + outFiles = append(outFiles, arg) + } + } + if len(rawCommand) == 0 { + return usageError("-c is required and must be non-empty") + } + if outFiles == nil { + return usageError("at least one output file must be given") + } + if len(sandboxesRoot) == 0 { + // In practice, the value of sandboxesRoot will mostly likely be at a fixed location relative to OUT_DIR, + // and the sbox executable will most likely be at a fixed location relative to OUT_DIR too, so + // the value of sandboxesRoot will most likely be at a fixed location relative to the sbox executable + // However, Soong also needs to be able to separately remove the sandbox directory on startup (if it has anything left in it) + // and by passing it as a parameter we don't need to duplicate its value + return usageError("--sandbox-path is required and must be non-empty") + } + + os.MkdirAll(sandboxesRoot, 0777) + + tempDir, err := ioutil.TempDir(sandboxesRoot, "sbox") + if err != nil { + return fmt.Errorf("Failed to create temp dir: %s", err) + } + + // In the common case, the following line of code is what removes the sandbox + // If a fatal error occurs (such as if our Go process is killed unexpectedly), + // then at the beginning of the next build, Soong will retry the cleanup + defer os.RemoveAll(tempDir) + + if strings.Contains(rawCommand, "__SBOX_OUT_DIR__") { + rawCommand = strings.Replace(rawCommand, "__SBOX_OUT_DIR__", tempDir, -1) + } + + if strings.Contains(rawCommand, "__SBOX_OUT_FILES__") { + // expands into a space-separated list of output files to be generated into the sandbox directory + tempOutPaths := []string{} + for _, outputPath := range outFiles { + tempOutPath := path.Join(tempDir, outputPath) + tempOutPaths = append(tempOutPaths, tempOutPath) + } + pathsText := strings.Join(tempOutPaths, " ") + rawCommand = strings.Replace(rawCommand, "__SBOX_OUT_FILES__", pathsText, -1) + } + + for _, filePath := range outFiles { + os.MkdirAll(path.Join(tempDir, filepath.Dir(filePath)), 0777) + } + + cmd := exec.Command("bash", "-c", rawCommand) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err = cmd.Run() + if exit, ok := err.(*exec.ExitError); ok && !exit.Success() { + return fmt.Errorf("sbox command %#v failed with err %#v\n", cmd, err) + } else if err != nil { + return err + } + + for _, filePath := range outFiles { + tempPath := filepath.Join(tempDir, filePath) + fileInfo, err := os.Stat(tempPath) + if err != nil { + return fmt.Errorf("command run under sbox did not create expected output file %s", filePath) + } + if fileInfo.IsDir() { + return fmt.Errorf("Output path %s refers to a directory, not a file. This is not permitted because it prevents robust up-to-date checks", filePath) + } + err = os.Rename(tempPath, filePath) + if err != nil { + return err + } + } + // TODO(jeffrygaston) if a process creates more output files than it declares, should there be a warning? + return nil +} diff --git a/genrule/genrule.go b/genrule/genrule.go index 2ff018f5d..b98ccfdba 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -16,11 +16,14 @@ package genrule import ( "fmt" + "path" "strings" "github.com/google/blueprint" "android/soong/android" + "android/soong/shared" + "path/filepath" ) func init() { @@ -32,6 +35,10 @@ var ( pctx = android.NewPackageContext("android/soong/genrule") ) +func init() { + pctx.HostBinToolVariable("sboxCmd", "sbox") +} + type SourceFileGenerator interface { GeneratedSourceFiles() android.Paths GeneratedHeaderDirs() android.Paths @@ -42,7 +49,11 @@ type HostToolProvider interface { } type generatorProperties struct { - // command to run on one or more input files. Available variables for substitution: + // The command to run on one or more input files. Cmd supports substitution of a few variables + // (the actual substitution is implemented in GenerateAndroidBuildActions below) + // + // Available variables for substitution: + // // $(location): the path to the first entry in tools or tool_files // $(location