diff options
| author | 2020-11-20 18:45:35 +0000 | |
|---|---|---|
| committer | 2020-11-20 18:45:35 +0000 | |
| commit | b893f8766c4fe8b494f92b4c9c9097224bb307fc (patch) | |
| tree | 192bedfeab8c8fedaf622e331ca5d8a570838b65 | |
| parent | 60d06cf8df7e2dd62fd1e9049074261709ead816 (diff) | |
| parent | 619b9ab260085f159de2f4f3e822e857d06949d8 (diff) | |
Merge "Revert "Rewrite sbox to use a textproto manifest""
| -rw-r--r-- | android/Android.bp | 1 | ||||
| -rw-r--r-- | android/rule_builder.go | 111 | ||||
| -rw-r--r-- | android/rule_builder_test.go | 70 | ||||
| -rw-r--r-- | cc/gen.go | 6 | ||||
| -rw-r--r-- | cc/gen_test.go | 5 | ||||
| -rw-r--r-- | cmd/sbox/Android.bp | 14 | ||||
| -rw-r--r-- | cmd/sbox/sbox.go | 388 | ||||
| -rw-r--r-- | cmd/sbox/sbox_proto/sbox.pb.go | 233 | ||||
| -rw-r--r-- | cmd/sbox/sbox_proto/sbox.proto | 58 | ||||
| -rw-r--r-- | genrule/Android.bp | 1 | ||||
| -rw-r--r-- | genrule/genrule.go | 9 | ||||
| -rw-r--r-- | genrule/genrule_test.go | 73 |
12 files changed, 229 insertions, 740 deletions
diff --git a/android/Android.bp b/android/Android.bp index 7950fff39..66d361efa 100644 --- a/android/Android.bp +++ b/android/Android.bp @@ -4,7 +4,6 @@ bootstrap_go_package { deps: [ "blueprint", "blueprint-bootstrap", - "sbox_proto", "soong", "soong-android-soongconfig", "soong-env", diff --git a/android/rule_builder.go b/android/rule_builder.go index 3efe9f8c6..86418b2d5 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -20,33 +20,27 @@ import ( "path/filepath" "sort" "strings" - "testing" - "github.com/golang/protobuf/proto" "github.com/google/blueprint" "github.com/google/blueprint/proptools" - "android/soong/cmd/sbox/sbox_proto" "android/soong/shared" ) -const sboxSandboxBaseDir = "__SBOX_SANDBOX_DIR__" -const sboxOutSubDir = "out" -const sboxOutDir = sboxSandboxBaseDir + "/" + sboxOutSubDir +const sboxOutDir = "__SBOX_OUT_DIR__" // RuleBuilder provides an alternative to ModuleContext.Rule and ModuleContext.Build to add a command line to the build // graph. type RuleBuilder struct { - commands []*RuleBuilderCommand - installs RuleBuilderInstalls - temporariesSet map[WritablePath]bool - restat bool - sbox bool - highmem bool - remoteable RemoteRuleSupports - sboxOutDir WritablePath - sboxManifestPath WritablePath - missingDeps []string + commands []*RuleBuilderCommand + installs RuleBuilderInstalls + temporariesSet map[WritablePath]bool + restat bool + sbox bool + highmem bool + remoteable RemoteRuleSupports + sboxOutDir WritablePath + missingDeps []string } // NewRuleBuilder returns a newly created RuleBuilder. @@ -112,14 +106,12 @@ func (r *RuleBuilder) Remoteable(supports RemoteRuleSupports) *RuleBuilder { return r } -// Sbox marks the rule as needing to be wrapped by sbox. The outputDir should point to the output -// directory that sbox will wipe. It should not be written to by any other rule. manifestPath should -// point to a location where sbox's manifest will be written and must be outside outputDir. sbox -// will ensure that all outputs have been written, and will discard any output files that were not -// specified. +// Sbox marks the rule as needing to be wrapped by sbox. The WritablePath should point to the output +// directory that sbox will wipe. It should not be written to by any other rule. sbox will ensure +// that all outputs have been written, and will discard any output files that were not specified. // // Sbox is not compatible with Restat() -func (r *RuleBuilder) Sbox(outputDir WritablePath, manifestPath WritablePath) *RuleBuilder { +func (r *RuleBuilder) Sbox(outputDir WritablePath) *RuleBuilder { if r.sbox { panic("Sbox() may not be called more than once") } @@ -131,7 +123,6 @@ func (r *RuleBuilder) Sbox(outputDir WritablePath, manifestPath WritablePath) *R } r.sbox = true r.sboxOutDir = outputDir - r.sboxManifestPath = manifestPath return r } @@ -429,8 +420,7 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string r.depFileMergerCmd(ctx, depFiles) if r.sbox { - // Check for Rel() errors, as all depfiles should be in the output dir. Errors - // will be reported to the ctx. + // Check for Rel() errors, as all depfiles should be in the output dir for _, path := range depFiles[1:] { Rel(ctx, r.sboxOutDir.String(), path.String()) } @@ -453,60 +443,34 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string commandString := strings.Join(commands, " && ") if r.sbox { - // If running the command inside sbox, write the rule data out to an sbox - // manifest.textproto. - manifest := sbox_proto.Manifest{} - command := sbox_proto.Command{} - manifest.Commands = append(manifest.Commands, &command) - command.Command = proto.String(commandString) - - if depFile != nil { - manifest.OutputDepfile = proto.String(depFile.String()) - } - - // Add copy rules to the manifest to copy each output file from the sbox directory. - // to the output directory. sboxOutputs := make([]string, len(outputs)) for i, output := range outputs { - rel := Rel(ctx, r.sboxOutDir.String(), output.String()) - sboxOutputs[i] = filepath.Join(sboxOutDir, rel) - command.CopyAfter = append(command.CopyAfter, &sbox_proto.Copy{ - From: proto.String(filepath.Join(sboxOutSubDir, rel)), - To: proto.String(output.String()), - }) + sboxOutputs[i] = filepath.Join(sboxOutDir, Rel(ctx, r.sboxOutDir.String(), output.String())) } - // Add a hash of the list of input files to the manifest so that the textproto file - // changes when the list of input files changes and causes the sbox rule that - // depends on it to rerun. - command.InputHash = proto.String(hashSrcFiles(inputs)) - - // Verify that the manifest textproto is not inside the sbox output directory, otherwise - // it will get deleted when the sbox rule clears its output directory. - _, manifestInOutDir := MaybeRel(ctx, r.sboxOutDir.String(), r.sboxManifestPath.String()) - if manifestInOutDir { - ReportPathErrorf(ctx, "sbox rule %q manifestPath %q must not be in outputDir %q", - name, r.sboxManifestPath.String(), r.sboxOutDir.String()) + commandString = proptools.ShellEscape(commandString) + if !strings.HasPrefix(commandString, `'`) { + commandString = `'` + commandString + `'` } - // Create a rule to write the manifest as a the textproto. - WriteFileRule(ctx, r.sboxManifestPath, proto.MarshalTextString(&manifest)) - - // Generate a new string to use as the command line of the sbox rule. This uses - // a RuleBuilderCommand as a convenience method of building the command line, then - // converts it to a string to replace commandString. sboxCmd := &RuleBuilderCommand{} - sboxCmd.Text("rm -rf").Output(r.sboxOutDir) - sboxCmd.Text("&&") sboxCmd.BuiltTool(ctx, "sbox"). + Flag("-c").Text(commandString). Flag("--sandbox-path").Text(shared.TempDirForOutDir(PathForOutput(ctx).String())). - Flag("--manifest").Input(r.sboxManifestPath) + Flag("--output-root").Text(r.sboxOutDir.String()) + + if depFile != nil { + sboxCmd.Flag("--depfile-out").Text(depFile.String()) + } + + // Add a hash of the list of input files to the xbox command line so that ninja reruns + // it when the list of input files changes. + sboxCmd.FlagWithArg("--input-hash ", hashSrcFiles(inputs)) + + sboxCmd.Flags(sboxOutputs) - // Replace the command string, and add the sbox tool and manifest textproto to the - // dependencies of the final sbox rule. commandString = sboxCmd.buf.String() tools = append(tools, sboxCmd.tools...) - inputs = append(inputs, sboxCmd.inputs...) } else { // If not using sbox the rule will run the command directly, put the hash of the // list of input files in a comment at the end of the command line to ensure ninja @@ -926,19 +890,6 @@ func (c *RuleBuilderCommand) NinjaEscapedString() string { return ninjaEscapeExceptForSpans(c.String(), c.unescapedSpans) } -// RuleBuilderSboxProtoForTests takes the BuildParams for the manifest passed to RuleBuilder.Sbox() -// and returns sbox testproto generated by the RuleBuilder. -func RuleBuilderSboxProtoForTests(t *testing.T, params TestingBuildParams) *sbox_proto.Manifest { - t.Helper() - content := ContentFromFileRuleForTests(t, params) - manifest := sbox_proto.Manifest{} - err := proto.UnmarshalText(content, &manifest) - if err != nil { - t.Fatalf("failed to unmarshal manifest: %s", err.Error()) - } - return &manifest -} - func ninjaEscapeExceptForSpans(s string, spans [][2]int) string { if len(spans) == 0 { return proptools.NinjaEscape(s) diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index dc360c35f..c1d552198 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -395,17 +395,16 @@ func TestRuleBuilder(t *testing.T) { }) t.Run("sbox", func(t *testing.T) { - rule := NewRuleBuilder().Sbox(PathForOutput(ctx, ""), - PathForOutput(ctx, "sbox.textproto")) + rule := NewRuleBuilder().Sbox(PathForOutput(ctx)) addCommands(rule) wantCommands := []string{ - "__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output Input __SBOX_SANDBOX_DIR__/out/Output __SBOX_SANDBOX_DIR__/out/SymlinkOutput Text Tool after command2 old cmd", - "command2 __SBOX_SANDBOX_DIR__/out/depfile2 input2 __SBOX_SANDBOX_DIR__/out/output2 tool2", - "command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3", + "__SBOX_OUT_DIR__/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_OUT_DIR__/depfile FlagWithInput=input FlagWithOutput=__SBOX_OUT_DIR__/output Input __SBOX_OUT_DIR__/Output __SBOX_OUT_DIR__/SymlinkOutput Text Tool after command2 old cmd", + "command2 __SBOX_OUT_DIR__/depfile2 input2 __SBOX_OUT_DIR__/output2 tool2", + "command3 input3 __SBOX_OUT_DIR__/output2 __SBOX_OUT_DIR__/output3", } - wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer __SBOX_SANDBOX_DIR__/out/DepFile __SBOX_SANDBOX_DIR__/out/depfile __SBOX_SANDBOX_DIR__/out/ImplicitDepFile __SBOX_SANDBOX_DIR__/out/depfile2" + wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer __SBOX_OUT_DIR__/DepFile __SBOX_OUT_DIR__/depfile __SBOX_OUT_DIR__/ImplicitDepFile __SBOX_OUT_DIR__/depfile2" if g, w := rule.Commands(), wantCommands; !reflect.DeepEqual(g, w) { t.Errorf("\nwant rule.Commands() = %#v\n got %#v", w, g) @@ -452,12 +451,11 @@ type testRuleBuilderModule struct { func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { in := PathsForSource(ctx, t.properties.Srcs) - out := PathForModuleOut(ctx, "gen", ctx.ModuleName()) - outDep := PathForModuleOut(ctx, "gen", ctx.ModuleName()+".d") - outDir := PathForModuleOut(ctx, "gen") - manifestPath := PathForModuleOut(ctx, "sbox.textproto") + out := PathForModuleOut(ctx, ctx.ModuleName()) + outDep := PathForModuleOut(ctx, ctx.ModuleName()+".d") + outDir := PathForModuleOut(ctx) - testRuleBuilder_Build(ctx, in, out, outDep, outDir, manifestPath, t.properties.Restat, t.properties.Sbox) + testRuleBuilder_Build(ctx, in, out, outDep, outDir, t.properties.Restat, t.properties.Sbox) } type testRuleBuilderSingleton struct{} @@ -468,18 +466,17 @@ func testRuleBuilderSingletonFactory() Singleton { func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { in := PathForSource(ctx, "bar") - out := PathForOutput(ctx, "singleton/gen/baz") - outDep := PathForOutput(ctx, "singleton/gen/baz.d") - outDir := PathForOutput(ctx, "singleton/gen") - manifestPath := PathForOutput(ctx, "singleton/sbox.textproto") - testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, manifestPath, true, false) + out := PathForOutput(ctx, "baz") + outDep := PathForOutput(ctx, "baz.d") + outDir := PathForOutput(ctx) + testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, true, false) } -func testRuleBuilder_Build(ctx BuilderContext, in Paths, out, outDep, outDir, manifestPath WritablePath, restat, sbox bool) { +func testRuleBuilder_Build(ctx BuilderContext, in Paths, out, outDep, outDir WritablePath, restat, sbox bool) { rule := NewRuleBuilder() if sbox { - rule.Sbox(outDir, manifestPath) + rule.Sbox(outDir) } rule.Command().Tool(PathForSource(ctx, "cp")).Inputs(in).Output(out).ImplicitDepFile(outDep) @@ -521,10 +518,10 @@ func TestRuleBuilder_Build(t *testing.T) { _, errs = ctx.PrepareBuildActions(config) FailIfErrored(t, errs) - check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraImplicits, extraCmdDeps []string) { + check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraCmdDeps []string) { t.Helper() command := params.RuleParams.Command - re := regexp.MustCompile(" # hash of input list: [a-z0-9]*$") + re := regexp.MustCompile(" (# hash of input list:|--input-hash) [a-z0-9]*") command = re.ReplaceAllLiteralString(command, "") if command != wantCommand { t.Errorf("\nwant RuleParams.Command = %q\n got %q", wantCommand, params.RuleParams.Command) @@ -539,8 +536,7 @@ func TestRuleBuilder_Build(t *testing.T) { t.Errorf("want RuleParams.Restat = %v, got %v", wantRestat, params.RuleParams.Restat) } - wantImplicits := append([]string{"bar"}, extraImplicits...) - if !reflect.DeepEqual(params.Implicits.Strings(), wantImplicits) { + if len(params.Implicits) != 1 || params.Implicits[0].String() != "bar" { t.Errorf("want Implicits = [%q], got %q", "bar", params.Implicits.Strings()) } @@ -562,29 +558,27 @@ func TestRuleBuilder_Build(t *testing.T) { } t.Run("module", func(t *testing.T) { - outFile := filepath.Join(buildDir, ".intermediates", "foo", "gen", "foo") + outFile := filepath.Join(buildDir, ".intermediates", "foo", "foo") check(t, ctx.ModuleForTests("foo", "").Rule("rule"), "cp bar "+outFile, - outFile, outFile+".d", true, nil, nil) + outFile, outFile+".d", true, nil) }) t.Run("sbox", func(t *testing.T) { outDir := filepath.Join(buildDir, ".intermediates", "foo_sbox") - outFile := filepath.Join(outDir, "gen/foo_sbox") - depFile := filepath.Join(outDir, "gen/foo_sbox.d") - manifest := filepath.Join(outDir, "sbox.textproto") + outFile := filepath.Join(outDir, "foo_sbox") + depFile := filepath.Join(outDir, "foo_sbox.d") sbox := filepath.Join(buildDir, "host", config.PrebuiltOS(), "bin/sbox") sandboxPath := shared.TempDirForOutDir(buildDir) - cmd := `rm -rf ` + outDir + `/gen && ` + - sbox + ` --sandbox-path ` + sandboxPath + ` --manifest ` + manifest + cmd := sbox + ` -c 'cp bar __SBOX_OUT_DIR__/foo_sbox' --sandbox-path ` + sandboxPath + " --output-root " + outDir + " --depfile-out " + depFile + " __SBOX_OUT_DIR__/foo_sbox" - check(t, ctx.ModuleForTests("foo_sbox", "").Output("gen/foo_sbox"), - cmd, outFile, depFile, false, []string{manifest}, []string{sbox}) + check(t, ctx.ModuleForTests("foo_sbox", "").Rule("rule"), + cmd, outFile, depFile, false, []string{sbox}) }) t.Run("singleton", func(t *testing.T) { - outFile := filepath.Join(buildDir, "singleton/gen/baz") + outFile := filepath.Join(buildDir, "baz") check(t, ctx.SingletonForTests("rule_builder_test").Rule("rule"), - "cp bar "+outFile, outFile, outFile+".d", true, nil, nil) + "cp bar "+outFile, outFile, outFile+".d", true, nil) }) } @@ -721,16 +715,14 @@ func TestRuleBuilderHashInputs(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Run("sbox", func(t *testing.T) { gen := ctx.ModuleForTests(test.name+"_sbox", "") - manifest := RuleBuilderSboxProtoForTests(t, gen.Output("sbox.textproto")) - hash := manifest.Commands[0].GetInputHash() - - if g, w := hash, test.expectedHash; g != w { - t.Errorf("Expected has %q, got %q", w, g) + command := gen.Output(test.name + "_sbox").RuleParams.Command + if g, w := command, " --input-hash "+test.expectedHash; !strings.Contains(g, w) { + t.Errorf("Expected command line to end with %q, got %q", w, g) } }) t.Run("", func(t *testing.T) { gen := ctx.ModuleForTests(test.name+"", "") - command := gen.Output("gen/" + test.name).RuleParams.Command + command := gen.Output(test.name).RuleParams.Command if g, w := command, " # hash of input list: "+test.expectedHash; !strings.HasSuffix(g, w) { t.Errorf("Expected command line to end with %q, got %q", w, g) } @@ -232,8 +232,7 @@ func genSources(ctx android.ModuleContext, srcFiles android.Paths, var yaccRule_ *android.RuleBuilder yaccRule := func() *android.RuleBuilder { if yaccRule_ == nil { - yaccRule_ = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "yacc"), - android.PathForModuleGen(ctx, "yacc.sbox.textproto")) + yaccRule_ = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "yacc")) } return yaccRule_ } @@ -262,8 +261,7 @@ func genSources(ctx android.ModuleContext, srcFiles android.Paths, deps = append(deps, headerFile) case ".aidl": if aidlRule == nil { - aidlRule = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "aidl"), - android.PathForModuleGen(ctx, "aidl.sbox.textproto")) + aidlRule = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "aidl")) } cppFile := android.GenPathWithExt(ctx, "aidl", srcFile, "cpp") depFile := android.GenPathWithExt(ctx, "aidl", srcFile, "cpp.d") diff --git a/cc/gen_test.go b/cc/gen_test.go index 41ef95c27..4b9a36e6b 100644 --- a/cc/gen_test.go +++ b/cc/gen_test.go @@ -18,8 +18,6 @@ import ( "path/filepath" "strings" "testing" - - "android/soong/android" ) func TestGen(t *testing.T) { @@ -58,14 +56,13 @@ func TestGen(t *testing.T) { }`) aidl := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_shared").Rule("aidl") - aidlManifest := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_shared").Output("aidl.sbox.textproto") libfoo := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_shared").Module().(*Module) if !inList("-I"+filepath.Dir(aidl.Output.String()), libfoo.flags.Local.CommonFlags) { t.Errorf("missing aidl includes in global flags") } - aidlCommand := android.RuleBuilderSboxProtoForTests(t, aidlManifest).Commands[0].GetCommand() + aidlCommand := aidl.RuleParams.Command if !strings.Contains(aidlCommand, "-Isub") { t.Errorf("aidl command for c.aidl should contain \"-Isub\", but was %q", aidlCommand) } diff --git a/cmd/sbox/Android.bp b/cmd/sbox/Android.bp index f5e87c0c2..6fa304eff 100644 --- a/cmd/sbox/Android.bp +++ b/cmd/sbox/Android.bp @@ -14,20 +14,8 @@ blueprint_go_binary { name: "sbox", - deps: [ - "sbox_proto", - "soong-makedeps", - ], + deps: ["soong-makedeps"], srcs: [ "sbox.go", ], } - -bootstrap_go_package { - name: "sbox_proto", - pkgPath: "android/soong/cmd/sbox/sbox_proto", - deps: ["golang-protobuf-proto"], - srcs: [ - "sbox_proto/sbox.pb.go", - ], -} diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go index e04e0a742..65a34fdf4 100644 --- a/cmd/sbox/sbox.go +++ b/cmd/sbox/sbox.go @@ -19,39 +19,41 @@ import ( "errors" "flag" "fmt" - "io" "io/ioutil" "os" "os/exec" + "path" "path/filepath" - "strconv" "strings" "time" - "android/soong/cmd/sbox/sbox_proto" "android/soong/makedeps" - - "github.com/golang/protobuf/proto" ) var ( sandboxesRoot string - manifestFile string + rawCommand string + outputRoot string keepOutDir bool -) - -const ( - depFilePlaceholder = "__SBOX_DEPFILE__" - sandboxDirPlaceholder = "__SBOX_SANDBOX_DIR__" + depfileOut string + inputHash string ) func init() { flag.StringVar(&sandboxesRoot, "sandbox-path", "", "root of temp directory to put the sandbox into") - flag.StringVar(&manifestFile, "manifest", "", - "textproto manifest describing the sandboxed command(s)") + flag.StringVar(&rawCommand, "c", "", + "command to run") + flag.StringVar(&outputRoot, "output-root", "", + "root of directory to copy outputs into") flag.BoolVar(&keepOutDir, "keep-out-dir", false, "whether to keep the sandbox directory when done") + + flag.StringVar(&depfileOut, "depfile-out", "", + "file path of the depfile to generate. This value will replace '__SBOX_DEPFILE__' in the command and will be treated as an output but won't be added to __SBOX_OUT_FILES__") + + flag.StringVar(&inputHash, "input-hash", "", + "This option is ignored. Typical usage is to supply a hash of the list of input names so that the module will be rebuilt if the list (and thus the hash) changes.") } func usageViolation(violation string) { @@ -60,7 +62,11 @@ func usageViolation(violation string) { } fmt.Fprintf(os.Stderr, - "Usage: sbox --manifest <manifest> --sandbox-path <sandboxPath>\n") + "Usage: sbox -c <commandToRun> --sandbox-path <sandboxPath> --output-root <outputRoot> [--depfile-out depFile] [--input-hash hash] <outputFile> [<outputFile>...]\n"+ + "\n"+ + "Deletes <outputRoot>,"+ + "runs <commandToRun>,"+ + "and moves each <outputFile> out of <sandboxPath> and into <outputRoot>\n") flag.PrintDefaults() @@ -97,8 +103,8 @@ func findAllFilesUnder(root string) (paths []string) { } func run() error { - if manifestFile == "" { - usageViolation("--manifest <manifest> is required and must be non-empty") + if rawCommand == "" { + usageViolation("-c <commandToRun> is required and must be non-empty") } if sandboxesRoot == "" { // In practice, the value of sandboxesRoot will mostly likely be at a fixed location relative to OUT_DIR, @@ -108,124 +114,89 @@ func run() error { // and by passing it as a parameter we don't need to duplicate its value usageViolation("--sandbox-path <sandboxPath> is required and must be non-empty") } + if len(outputRoot) == 0 { + usageViolation("--output-root <outputRoot> is required and must be non-empty") + } - manifest, err := readManifest(manifestFile) - - if len(manifest.Commands) == 0 { - return fmt.Errorf("at least one commands entry is required in %q", manifestFile) + // the contents of the __SBOX_OUT_FILES__ variable + outputsVarEntries := flag.Args() + if len(outputsVarEntries) == 0 { + usageViolation("at least one output file must be given") } - // setup sandbox directory - err = os.MkdirAll(sandboxesRoot, 0777) + // all outputs + var allOutputs []string + + // setup directories + err := os.MkdirAll(sandboxesRoot, 0777) if err != nil { - return fmt.Errorf("failed to create %q: %w", sandboxesRoot, err) + return err } - - tempDir, err := ioutil.TempDir(sandboxesRoot, "sbox") + err = os.RemoveAll(outputRoot) if err != nil { - return fmt.Errorf("failed to create temporary dir in %q: %w", sandboxesRoot, err) + return err + } + err = os.MkdirAll(outputRoot, 0777) + if err != nil { + return 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 wipe the temporary - // directory. - defer func() { - // in some cases we decline to remove the temp dir, to facilitate debugging - if !keepOutDir { - os.RemoveAll(tempDir) - } - }() - - // If there is more than one command in the manifest use a separate directory for each one. - useSubDir := len(manifest.Commands) > 1 - var depFiles []string + tempDir, err := ioutil.TempDir(sandboxesRoot, "sbox") - for i, command := range manifest.Commands { - localTempDir := tempDir - if useSubDir { - localTempDir = filepath.Join(localTempDir, strconv.Itoa(i)) - } - depFile, err := runCommand(command, localTempDir) - if err != nil { - // Running the command failed, keep the temporary output directory around in - // case a user wants to inspect it for debugging purposes. Soong will delete - // it at the beginning of the next build anyway. - keepOutDir = true - return err - } - if depFile != "" { - depFiles = append(depFiles, depFile) + for i, filePath := range outputsVarEntries { + if !strings.HasPrefix(filePath, "__SBOX_OUT_DIR__/") { + return fmt.Errorf("output files must start with `__SBOX_OUT_DIR__/`") } + outputsVarEntries[i] = strings.TrimPrefix(filePath, "__SBOX_OUT_DIR__/") } - depFile := manifest.GetOutputDepfile() - if len(depFiles) > 0 && depFile == "" { - return fmt.Errorf("Sandboxed commands used %s but output depfile is not set in manifest file", - depFilePlaceholder) - } + allOutputs = append([]string(nil), outputsVarEntries...) - if depFile != "" { - // Merge the depfiles from each command in the manifest to a single output depfile. - err = rewriteDepFiles(depFiles, depFile) + if depfileOut != "" { + sandboxedDepfile, err := filepath.Rel(outputRoot, depfileOut) if err != nil { - return fmt.Errorf("failed merging depfiles: %w", err) + return err } - } + allOutputs = append(allOutputs, sandboxedDepfile) + rawCommand = strings.Replace(rawCommand, "__SBOX_DEPFILE__", filepath.Join(tempDir, sandboxedDepfile), -1) - return nil -} - -// readManifest reads an sbox manifest from a textproto file. -func readManifest(file string) (*sbox_proto.Manifest, error) { - manifestData, err := ioutil.ReadFile(file) - if err != nil { - return nil, fmt.Errorf("error reading manifest %q: %w", file, err) } - manifest := sbox_proto.Manifest{} - - err = proto.UnmarshalText(string(manifestData), &manifest) if err != nil { - return nil, fmt.Errorf("error parsing manifest %q: %w", file, err) + return fmt.Errorf("Failed to create temp dir: %s", err) } - return &manifest, nil -} - -// runCommand runs a single command from a manifest. If the command references the -// __SBOX_DEPFILE__ placeholder it returns the name of the depfile that was used. -func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, err error) { - rawCommand := command.GetCommand() - if rawCommand == "" { - return "", fmt.Errorf("command is required") - } - - err = os.MkdirAll(tempDir, 0777) - if err != nil { - return "", fmt.Errorf("failed to create %q: %w", tempDir, err) - } - - // Copy in any files specified by the manifest. - err = linkOrCopyFiles(command.CopyBefore, "", tempDir) - if err != nil { - return "", 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 func() { + // in some cases we decline to remove the temp dir, to facilitate debugging + if !keepOutDir { + os.RemoveAll(tempDir) + } + }() - if strings.Contains(rawCommand, depFilePlaceholder) { - depFile = filepath.Join(tempDir, "deps.d") - rawCommand = strings.Replace(rawCommand, depFilePlaceholder, depFile, -1) + if strings.Contains(rawCommand, "__SBOX_OUT_DIR__") { + rawCommand = strings.Replace(rawCommand, "__SBOX_OUT_DIR__", tempDir, -1) } - if strings.Contains(rawCommand, sandboxDirPlaceholder) { - rawCommand = strings.Replace(rawCommand, sandboxDirPlaceholder, 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 outputsVarEntries { + tempOutPath := path.Join(tempDir, outputPath) + tempOutPaths = append(tempOutPaths, tempOutPath) + } + pathsText := strings.Join(tempOutPaths, " ") + rawCommand = strings.Replace(rawCommand, "__SBOX_OUT_FILES__", pathsText, -1) } - // Emulate ninja's behavior of creating the directories for any output files before - // running the command. - err = makeOutputDirs(command.CopyAfter, tempDir) - if err != nil { - return "", err + for _, filePath := range allOutputs { + dir := path.Join(tempDir, filepath.Dir(filePath)) + err = os.MkdirAll(dir, 0777) + if err != nil { + return err + } } commandDescription := rawCommand @@ -234,20 +205,27 @@ func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, er cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - - if command.GetChdir() { - cmd.Dir = tempDir - } err = cmd.Run() if exit, ok := err.(*exec.ExitError); ok && !exit.Success() { - return "", fmt.Errorf("sbox command failed with err:\n%s\n%w\n", commandDescription, err) + return fmt.Errorf("sbox command (%s) failed with err %#v\n", commandDescription, err.Error()) } else if err != nil { - return "", err + return err } - missingOutputErrors := validateOutputFiles(command.CopyAfter, tempDir) - + // validate that all files are created properly + var missingOutputErrors []string + for _, filePath := range allOutputs { + tempPath := filepath.Join(tempDir, filePath) + fileInfo, err := os.Stat(tempPath) + if err != nil { + missingOutputErrors = append(missingOutputErrors, fmt.Sprintf("%s: does not exist", filePath)) + continue + } + if fileInfo.IsDir() { + missingOutputErrors = append(missingOutputErrors, fmt.Sprintf("%s: not a file", filePath)) + } + } if len(missingOutputErrors) > 0 { // find all created files for making a more informative error message createdFiles := findAllFilesUnder(tempDir) @@ -258,7 +236,7 @@ func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, er errorMessage += "in sandbox " + tempDir + ",\n" errorMessage += fmt.Sprintf("failed to create %v files:\n", len(missingOutputErrors)) for _, missingOutputError := range missingOutputErrors { - errorMessage += " " + missingOutputError.Error() + "\n" + errorMessage += " " + missingOutputError + "\n" } if len(createdFiles) < 1 { errorMessage += "created 0 files." @@ -275,183 +253,57 @@ func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, er } } - return "", errors.New(errorMessage) + // Keep the temporary output directory around in case a user wants to inspect it for debugging purposes. + // Soong will delete it later anyway. + keepOutDir = true + return errors.New(errorMessage) } // the created files match the declared files; now move them - err = moveFiles(command.CopyAfter, tempDir, "") - - return depFile, nil -} - -// makeOutputDirs creates directories in the sandbox dir for every file that has a rule to be copied -// out of the sandbox. This emulate's Ninja's behavior of creating directories for output files -// so that the tools don't have to. -func makeOutputDirs(copies []*sbox_proto.Copy, sandboxDir string) error { - for _, copyPair := range copies { - dir := joinPath(sandboxDir, filepath.Dir(copyPair.GetFrom())) - err := os.MkdirAll(dir, 0777) - if err != nil { - return err - } - } - return nil -} - -// validateOutputFiles verifies that all files that have a rule to be copied out of the sandbox -// were created by the command. -func validateOutputFiles(copies []*sbox_proto.Copy, sandboxDir string) []error { - var missingOutputErrors []error - for _, copyPair := range copies { - fromPath := joinPath(sandboxDir, copyPair.GetFrom()) - fileInfo, err := os.Stat(fromPath) - if err != nil { - missingOutputErrors = append(missingOutputErrors, fmt.Errorf("%s: does not exist", fromPath)) - continue - } - if fileInfo.IsDir() { - missingOutputErrors = append(missingOutputErrors, fmt.Errorf("%s: not a file", fromPath)) + for _, filePath := range allOutputs { + tempPath := filepath.Join(tempDir, filePath) + destPath := filePath + if len(outputRoot) != 0 { + destPath = filepath.Join(outputRoot, filePath) } - } - return missingOutputErrors -} - -// linkOrCopyFiles hardlinks or copies files in or out of the sandbox. -func linkOrCopyFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error { - for _, copyPair := range copies { - fromPath := joinPath(fromDir, copyPair.GetFrom()) - toPath := joinPath(toDir, copyPair.GetTo()) - err := linkOrCopyOneFile(fromPath, toPath) - if err != nil { - return fmt.Errorf("error copying %q to %q: %w", fromPath, toPath, err) - } - } - return nil -} - -// linkOrCopyOneFile first attempts to hardlink a file to a destination, and falls back to making -// a copy if the hardlink fails. -func linkOrCopyOneFile(from string, to string) error { - err := os.MkdirAll(filepath.Dir(to), 0777) - if err != nil { - return err - } - - // First try hardlinking - err = os.Link(from, to) - if err != nil { - // Retry with copying in case the source an destination are on different filesystems. - // TODO: check for specific hardlink error? - err = copyOneFile(from, to) + err := os.MkdirAll(filepath.Dir(destPath), 0777) if err != nil { return err } - } - return nil -} - -// copyOneFile copies a file. -func copyOneFile(from string, to string) error { - stat, err := os.Stat(from) - if err != nil { - return err - } - - perm := stat.Mode() - - in, err := os.Open(from) - if err != nil { - return err - } - defer in.Close() - - out, err := os.Create(to) - if err != nil { - return err - } - defer func() { - out.Close() - if err != nil { - os.Remove(to) - } - }() - - _, err = io.Copy(out, in) - if err != nil { - return err - } - - if err = out.Close(); err != nil { - return err - } - - if err = os.Chmod(to, perm); err != nil { - return err - } - - return nil -} - -// moveFiles moves files specified by a set of copy rules. It uses os.Rename, so it is restricted -// to moving files where the source and destination are in the same filesystem. This is OK for -// sbox because the temporary directory is inside the out directory. It updates the timestamp -// of the new file. -func moveFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error { - for _, copyPair := range copies { - fromPath := joinPath(fromDir, copyPair.GetFrom()) - toPath := joinPath(toDir, copyPair.GetTo()) - err := os.MkdirAll(filepath.Dir(toPath), 0777) + // Update the timestamp of the output file in case the tool wrote an old timestamp (for example, tar can extract + // files with old timestamps). + now := time.Now() + err = os.Chtimes(tempPath, now, now) if err != nil { return err } - err = os.Rename(fromPath, toPath) + err = os.Rename(tempPath, destPath) if err != nil { return err } + } - // Update the timestamp of the output file in case the tool wrote an old timestamp (for example, tar can extract - // files with old timestamps). - now := time.Now() - err = os.Chtimes(toPath, now, now) + // Rewrite the depfile so that it doesn't include the (randomized) sandbox directory + if depfileOut != "" { + in, err := ioutil.ReadFile(depfileOut) if err != nil { return err } - } - return nil -} -// Rewrite one or more depfiles so that it doesn't include the (randomized) sandbox directory -// to an output file. -func rewriteDepFiles(ins []string, out string) error { - var mergedDeps []string - for _, in := range ins { - data, err := ioutil.ReadFile(in) + deps, err := makedeps.Parse(depfileOut, bytes.NewBuffer(in)) if err != nil { return err } - deps, err := makedeps.Parse(in, bytes.NewBuffer(data)) + deps.Output = "outputfile" + + err = ioutil.WriteFile(depfileOut, deps.Print(), 0666) if err != nil { return err } - mergedDeps = append(mergedDeps, deps.Inputs...) - } - - deps := makedeps.Deps{ - // Ninja doesn't care what the output file is, so we can use any string here. - Output: "outputfile", - Inputs: mergedDeps, } - return ioutil.WriteFile(out, deps.Print(), 0666) -} - -// joinPath wraps filepath.Join but returns file without appending to dir if file is -// absolute. -func joinPath(dir, file string) string { - if filepath.IsAbs(file) { - return file - } - return filepath.Join(dir, file) + // TODO(jeffrygaston) if a process creates more output files than it declares, should there be a warning? + return nil } diff --git a/cmd/sbox/sbox_proto/sbox.pb.go b/cmd/sbox/sbox_proto/sbox.pb.go deleted file mode 100644 index 6584bdf0b..000000000 --- a/cmd/sbox/sbox_proto/sbox.pb.go +++ /dev/null @@ -1,233 +0,0 @@ -// Code generated by protoc-gen-go. DO NOT EDIT. -// source: sbox.proto - -package sbox_proto - -import ( - fmt "fmt" - proto "github.com/golang/protobuf/proto" - math "math" -) - -// Reference imports to suppress errors if they are not otherwise used. -var _ = proto.Marshal -var _ = fmt.Errorf -var _ = math.Inf - -// This is a compile-time assertion to ensure that this generated file -// is compatible with the proto package it is being compiled against. -// A compilation error at this line likely means your copy of the -// proto package needs to be updated. -const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package - -// A set of commands to run in a sandbox. -type Manifest struct { - // A list of commands to run in the sandbox. - Commands []*Command `protobuf:"bytes,1,rep,name=commands" json:"commands,omitempty"` - // If set, GCC-style dependency files from any command that references __SBOX_DEPFILE__ will be - // merged into the given output file relative to the $PWD when sbox was started. - OutputDepfile *string `protobuf:"bytes,2,opt,name=output_depfile,json=outputDepfile" json:"output_depfile,omitempty"` - XXX_NoUnkeyedLiteral struct{} `json:"-"` - XXX_unrecognized []byte `json:"-"` - XXX_sizecache int32 `json:"-"` -} - -func (m *Manifest) Reset() { *m = Manifest{} } -func (m *Manifest) String() string { return proto.CompactTextString(m) } -func (*Manifest) ProtoMessage() {} -func (*Manifest) Descriptor() ([]byte, []int) { - return fileDescriptor_9d0425bf0de86ed1, []int{0} -} - -func (m *Manifest) XXX_Unmarshal(b []byte) error { - return xxx_messageInfo_Manifest.Unmarshal(m, b) -} -func (m *Manifest) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { - return xxx_messageInfo_Manifest.Marshal(b, m, deterministic) -} -func (m *Manifest) XXX_Merge(src proto.Message) { - xxx_messageInfo_Manifest.Merge(m, src) -} -func (m *Manifest) XXX_Size() int { - return xxx_messageInfo_Manifest.Size(m) -} -func (m *Manifest) XXX_DiscardUnknown() { - xxx_messageInfo_Manifest.DiscardUnknown(m) -} - -var xxx_messageInfo_Manifest proto.InternalMessageInfo - -func (m *Manifest) GetCommands() []*Command { - if m != nil { - return m.Commands - } - return nil -} - -func (m *Manifest) GetOutputDepfile() string { - if m != nil && m.OutputDepfile != nil { - return *m.OutputDepfile - } - return "" -} - -// SandboxManifest describes a command to run in the sandbox. -type Command struct { - // A list of copy rules to run before the sandboxed command. The from field is relative to the - // $PWD when sbox was run, the to field is relative to the top of the temporary sandbox directory. - CopyBefore []*Copy `protobuf:"bytes,1,rep,name=copy_before,json=copyBefore" json:"copy_before,omitempty"` - // If true, change the working directory to the top of the temporary sandbox directory before - // running the command. If false, leave the working directory where it was when sbox was started. - Chdir *bool `protobuf:"varint,2,opt,name=chdir" json:"chdir,omitempty"` - // The command to run. - Command *string `protobuf:"bytes,3,req,name=command" json:"command,omitempty"` - // A list of copy rules to run after the sandboxed command. The from field is relative to the - // top of the temporary sandbox directory, the to field is relative to the $PWD when sbox was run. - CopyAfter []*Copy `protobuf:"bytes,4,rep,name=copy_after,json=copyAfter" json:"copy_after,omitempty"` - // An optional hash of the input files to ensure the textproto files and the sbox rule reruns - // when the lists of inputs changes, even if the inputs are not on the command line. - InputHash *string `protobuf:"bytes,5,opt,name=input_hash,json=inputHash" json:"input_hash,omitempty"` - XXX_NoUnkeyedLiteral struct{} `json:"-"` - XXX_unrecognized []byte `json:"-"` - XXX_sizecache int32 `json:"-"` -} - -func (m *Command) Reset() { *m = Command{} } -func (m *Command) String() string { return proto.CompactTextString(m) } -func (*Command) ProtoMessage() {} -func (*Command) Descriptor() ([]byte, []int) { - return fileDescriptor_9d0425bf0de86ed1, []int{1} -} - -func (m *Command) XXX_Unmarshal(b []byte) error { - return xxx_messageInfo_Command.Unmarshal(m, b) -} -func (m *Command) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { - return xxx_messageInfo_Command.Marshal(b, m, deterministic) -} -func (m *Command) XXX_Merge(src proto.Message) { - xxx_messageInfo_Command.Merge(m, src) -} -func (m *Command) XXX_Size() int { - return xxx_messageInfo_Command.Size(m) -} -func (m *Command) XXX_DiscardUnknown() { - xxx_messageInfo_Command.DiscardUnknown(m) -} - -var xxx_messageInfo_Command proto.InternalMessageInfo - -func (m *Command) GetCopyBefore() []*Copy { - if m != nil { - return m.CopyBefore - } - return nil -} - -func (m *Command) GetChdir() bool { - if m != nil && m.Chdir != nil { - return *m.Chdir - } - return false -} - -func (m *Command) GetCommand() string { - if m != nil && m.Command != nil { - return *m.Command - } - return "" -} - -func (m *Command) GetCopyAfter() []*Copy { - if m != nil { - return m.CopyAfter - } - return nil -} - -func (m *Command) GetInputHash() string { - if m != nil && m.InputHash != nil { - return *m.InputHash - } - return "" -} - -// Copy describes a from-to pair of files to copy. The paths may be relative, the root that they -// are relative to is specific to the context the Copy is used in and will be different for -// from and to. -type Copy struct { - From *string `protobuf:"bytes,1,req,name=from" json:"from,omitempty"` - To *string `protobuf:"bytes,2,req,name=to" json:"to,omitempty"` - XXX_NoUnkeyedLiteral struct{} `json:"-"` - XXX_unrecognized []byte `json:"-"` - XXX_sizecache int32 `json:"-"` -} - -func (m *Copy) Reset() { *m = Copy{} } -func (m *Copy) String() string { return proto.CompactTextString(m) } -func (*Copy) ProtoMessage() {} -func (*Copy) Descriptor() ([]byte, []int) { - return fileDescriptor_9d0425bf0de86ed1, []int{2} -} - -func (m *Copy) XXX_Unmarshal(b []byte) error { - return xxx_messageInfo_Copy.Unmarshal(m, b) -} -func (m *Copy) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { - return xxx_messageInfo_Copy.Marshal(b, m, deterministic) -} -func (m *Copy) XXX_Merge(src proto.Message) { - xxx_messageInfo_Copy.Merge(m, src) -} -func (m *Copy) XXX_Size() int { - return xxx_messageInfo_Copy.Size(m) -} -func (m *Copy) XXX_DiscardUnknown() { - xxx_messageInfo_Copy.DiscardUnknown(m) -} - -var xxx_messageInfo_Copy proto.InternalMessageInfo - -func (m *Copy) GetFrom() string { - if m != nil && m.From != nil { - return *m.From - } - return "" -} - -func (m *Copy) GetTo() string { - if m != nil && m.To != nil { - return *m.To - } - return "" -} - -func init() { - proto.RegisterType((*Manifest)(nil), "sbox.Manifest") - proto.RegisterType((*Command)(nil), "sbox.Command") - proto.RegisterType((*Copy)(nil), "sbox.Copy") -} - -func init() { - proto.RegisterFile("sbox.proto", fileDescriptor_9d0425bf0de86ed1) -} - -var fileDescriptor_9d0425bf0de86ed1 = []byte{ - // 252 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x64, 0x90, 0x41, 0x4b, 0xc3, 0x40, - 0x10, 0x85, 0x49, 0x9a, 0xd2, 0x66, 0x6a, 0x7b, 0x18, 0x3c, 0xec, 0x45, 0x08, 0x01, 0x21, 0x55, - 0xe8, 0xc1, 0x7f, 0x60, 0xf5, 0xe0, 0xc5, 0xcb, 0x1e, 0x45, 0x08, 0xdb, 0x64, 0x97, 0x04, 0x4c, - 0x66, 0xd9, 0xdd, 0x82, 0xfd, 0x57, 0xfe, 0x44, 0xd9, 0x49, 0xea, 0xc5, 0xdb, 0xcc, 0xfb, 0x78, - 0xf3, 0x1e, 0x03, 0xe0, 0x4f, 0xf4, 0x7d, 0xb0, 0x8e, 0x02, 0x61, 0x16, 0xe7, 0xf2, 0x13, 0xd6, - 0xef, 0x6a, 0xec, 0x8d, 0xf6, 0x01, 0xf7, 0xb0, 0x6e, 0x68, 0x18, 0xd4, 0xd8, 0x7a, 0x91, 0x14, - 0x8b, 0x6a, 0xf3, 0xb4, 0x3d, 0xb0, 0xe1, 0x65, 0x52, 0xe5, 0x1f, 0xc6, 0x7b, 0xd8, 0xd1, 0x39, - 0xd8, 0x73, 0xa8, 0x5b, 0x6d, 0x4d, 0xff, 0xa5, 0x45, 0x5a, 0x24, 0x55, 0x2e, 0xb7, 0x93, 0xfa, - 0x3a, 0x89, 0xe5, 0x4f, 0x02, 0xab, 0xd9, 0x8c, 0x8f, 0xb0, 0x69, 0xc8, 0x5e, 0xea, 0x93, 0x36, - 0xe4, 0xf4, 0x1c, 0x00, 0xd7, 0x00, 0x7b, 0x91, 0x10, 0xf1, 0x91, 0x29, 0xde, 0xc2, 0xb2, 0xe9, - 0xda, 0xde, 0xf1, 0xd9, 0xb5, 0x9c, 0x16, 0x14, 0xb0, 0x9a, 0x1b, 0x88, 0x45, 0x91, 0x56, 0xb9, - 0xbc, 0xae, 0xb8, 0x07, 0x76, 0xd7, 0xca, 0x04, 0xed, 0x44, 0xf6, 0xef, 0x76, 0x1e, 0xe9, 0x73, - 0x84, 0x78, 0x07, 0xd0, 0x8f, 0xb1, 0x79, 0xa7, 0x7c, 0x27, 0x96, 0x5c, 0x3b, 0x67, 0xe5, 0x4d, - 0xf9, 0xae, 0x7c, 0x80, 0x2c, 0x3a, 0x10, 0x21, 0x33, 0x8e, 0x06, 0x91, 0x70, 0x10, 0xcf, 0xb8, - 0x83, 0x34, 0x90, 0x48, 0x59, 0x49, 0x03, 0x1d, 0x6f, 0x3e, 0xf8, 0xa1, 0x35, 0x3f, 0xf4, 0x37, - 0x00, 0x00, 0xff, 0xff, 0x95, 0x4d, 0xee, 0x7d, 0x5d, 0x01, 0x00, 0x00, -} diff --git a/cmd/sbox/sbox_proto/sbox.proto b/cmd/sbox/sbox_proto/sbox.proto deleted file mode 100644 index ab95545a5..000000000 --- a/cmd/sbox/sbox_proto/sbox.proto +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2020 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. - -syntax = "proto2"; - -package sbox; -option go_package = "sbox_proto"; - -// A set of commands to run in a sandbox. -message Manifest { - // A list of commands to run in the sandbox. - repeated Command commands = 1; - - // If set, GCC-style dependency files from any command that references __SBOX_DEPFILE__ will be - // merged into the given output file relative to the $PWD when sbox was started. - optional string output_depfile = 2; -} - -// SandboxManifest describes a command to run in the sandbox. -message Command { - // A list of copy rules to run before the sandboxed command. The from field is relative to the - // $PWD when sbox was run, the to field is relative to the top of the temporary sandbox directory. - repeated Copy copy_before = 1; - - // If true, change the working directory to the top of the temporary sandbox directory before - // running the command. If false, leave the working directory where it was when sbox was started. - optional bool chdir = 2; - - // The command to run. - required string command = 3; - - // A list of copy rules to run after the sandboxed command. The from field is relative to the - // top of the temporary sandbox directory, the to field is relative to the $PWD when sbox was run. - repeated Copy copy_after = 4; - - // An optional hash of the input files to ensure the textproto files and the sbox rule reruns - // when the lists of inputs changes, even if the inputs are not on the command line. - optional string input_hash = 5; -} - -// Copy describes a from-to pair of files to copy. The paths may be relative, the root that they -// are relative to is specific to the context the Copy is used in and will be different for -// from and to. -message Copy { - required string from = 1; - required string to = 2; -}
\ No newline at end of file diff --git a/genrule/Android.bp b/genrule/Android.bp index ba6f40d73..ff543a61f 100644 --- a/genrule/Android.bp +++ b/genrule/Android.bp @@ -4,7 +4,6 @@ bootstrap_go_package { deps: [ "blueprint", "blueprint-pathtools", - "sbox_proto", "soong", "soong-android", "soong-shared", diff --git a/genrule/genrule.go b/genrule/genrule.go index b6a5861e5..f85146c6b 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -417,23 +417,18 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { } g.rawCommands = append(g.rawCommands, rawCommand) - // Pick a unique path outside the task.genDir for the sbox manifest textproto, - // a unique rule name, and the user-visible description. - manifestName := g.subDir + "genrule.sbox.textproto" + // Pick a unique rule name and the user-visible description. desc := "generate" name := "generator" if task.shards > 0 { - manifestName = g.subDir + "genrule_" + strconv.Itoa(task.shard) + ".sbox.textproto" desc += " " + strconv.Itoa(task.shard) name += strconv.Itoa(task.shard) } else if len(task.out) == 1 { desc += " " + task.out[0].Base() } - manifestPath := android.PathForModuleOut(ctx, manifestName) - // Use a RuleBuilder to create a rule that runs the command inside an sbox sandbox. - rule := android.NewRuleBuilder().Sbox(task.genDir, manifestPath) + rule := android.NewRuleBuilder().Sbox(task.genDir) cmd := rule.Command() cmd.Text(rawCommand) cmd.ImplicitOutputs(task.out) diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 89ac67585..6779c73e4 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -141,7 +141,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_OUT_DIR__/out", }, { name: "empty location tool2", @@ -150,7 +150,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_OUT_DIR__/out", }, { name: "empty location tool file", @@ -159,7 +159,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_OUT_DIR__/out", }, { name: "empty location tool file fg", @@ -168,7 +168,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_OUT_DIR__/out", }, { name: "empty location tool and tool file", @@ -178,7 +178,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_OUT_DIR__/out", }, { name: "tool", @@ -187,7 +187,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location tool) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_OUT_DIR__/out", }, { name: "tool2", @@ -196,7 +196,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location :tool) > $(out)", `, - expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_OUT_DIR__/out", }, { name: "tool file", @@ -205,7 +205,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location tool_file1) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_OUT_DIR__/out", }, { name: "tool file fg", @@ -214,7 +214,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location :1tool_file) > $(out)", `, - expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_OUT_DIR__/out", }, { name: "tool files", @@ -223,7 +223,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(locations :tool_files) > $(out)", `, - expect: "tool_file1 tool_file2 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 tool_file2 > __SBOX_OUT_DIR__/out", }, { name: "in1", @@ -232,7 +232,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(in) > $(out)", `, - expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 > __SBOX_OUT_DIR__/out", }, { name: "in1 fg", @@ -241,7 +241,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(in) > $(out)", `, - expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 > __SBOX_OUT_DIR__/out", }, { name: "ins", @@ -250,7 +250,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(in) > $(out)", `, - expect: "cat in1 in2 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 in2 > __SBOX_OUT_DIR__/out", }, { name: "ins fg", @@ -259,7 +259,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(in) > $(out)", `, - expect: "cat in1 in2 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 in2 > __SBOX_OUT_DIR__/out", }, { name: "location in1", @@ -268,7 +268,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(location in1) > $(out)", `, - expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 > __SBOX_OUT_DIR__/out", }, { name: "location in1 fg", @@ -277,7 +277,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(location :1in) > $(out)", `, - expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 > __SBOX_OUT_DIR__/out", }, { name: "location ins", @@ -286,7 +286,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(location in1) > $(out)", `, - expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 > __SBOX_OUT_DIR__/out", }, { name: "location ins fg", @@ -295,7 +295,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "cat $(locations :ins) > $(out)", `, - expect: "cat in1 in2 > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat in1 in2 > __SBOX_OUT_DIR__/out", }, { name: "outs", @@ -303,7 +303,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out", "out2"], cmd: "echo foo > $(out)", `, - expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out __SBOX_SANDBOX_DIR__/out/out2", + expect: "echo foo > __SBOX_OUT_DIR__/out __SBOX_OUT_DIR__/out2", }, { name: "location out", @@ -311,7 +311,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out", "out2"], cmd: "echo foo > $(location out2)", `, - expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out2", + expect: "echo foo > __SBOX_OUT_DIR__/out2", }, { name: "depfile", @@ -320,7 +320,7 @@ func TestGenruleCmd(t *testing.T) { depfile: true, cmd: "echo foo > $(out) && touch $(depfile)", `, - expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out && touch __SBOX_DEPFILE__", + expect: "echo foo > __SBOX_OUT_DIR__/out && touch __SBOX_DEPFILE__", }, { name: "gendir", @@ -328,7 +328,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "echo foo > $(genDir)/foo && cp $(genDir)/foo $(out)", `, - expect: "echo foo > __SBOX_SANDBOX_DIR__/out/foo && cp __SBOX_SANDBOX_DIR__/out/foo __SBOX_SANDBOX_DIR__/out/out", + expect: "echo foo > __SBOX_OUT_DIR__/foo && cp __SBOX_OUT_DIR__/foo __SBOX_OUT_DIR__/out", }, { @@ -443,7 +443,7 @@ func TestGenruleCmd(t *testing.T) { allowMissingDependencies: true, - expect: "cat ***missing srcs :missing*** > __SBOX_SANDBOX_DIR__/out/out", + expect: "cat ***missing srcs :missing*** > __SBOX_OUT_DIR__/out", }, { name: "tool allow missing dependencies", @@ -455,7 +455,7 @@ func TestGenruleCmd(t *testing.T) { allowMissingDependencies: true, - expect: "***missing tool :missing*** > __SBOX_SANDBOX_DIR__/out/out", + expect: "***missing tool :missing*** > __SBOX_OUT_DIR__/out", }, } @@ -570,11 +570,20 @@ func TestGenruleHashInputs(t *testing.T) { for _, test := range testcases { t.Run(test.name, func(t *testing.T) { gen := ctx.ModuleForTests(test.name, "") - manifest := android.RuleBuilderSboxProtoForTests(t, gen.Output("genrule.sbox.textproto")) - hash := manifest.Commands[0].GetInputHash() + command := gen.Rule("generator").RuleParams.Command - if g, w := hash, test.expectedHash; g != w { - t.Errorf("Expected has %q, got %q", w, g) + if len(test.expectedHash) > 0 { + // We add spaces before and after to make sure that + // this option doesn't abutt another sbox option. + expectedInputHashOption := " --input-hash " + test.expectedHash + + if !strings.Contains(command, expectedInputHashOption) { + t.Errorf("Expected command \"%s\" to contain \"%s\"", command, expectedInputHashOption) + } + } else { + if strings.Contains(command, "--input-hash") { + t.Errorf("Unexpected \"--input-hash\" found in command: \"%s\"", command) + } } }) } @@ -600,7 +609,7 @@ func TestGenSrcs(t *testing.T) { cmd: "$(location) $(in) > $(out)", `, cmds: []string{ - "bash -c 'out/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", + "bash -c 'out/tool in1.txt > __SBOX_OUT_DIR__/in1.h' && bash -c 'out/tool in2.txt > __SBOX_OUT_DIR__/in2.h'", }, deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"}, files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"}, @@ -614,8 +623,8 @@ func TestGenSrcs(t *testing.T) { shard_size: 2, `, cmds: []string{ - "bash -c 'out/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", - "bash -c 'out/tool in3.txt > __SBOX_SANDBOX_DIR__/out/in3.h'", + "bash -c 'out/tool in1.txt > __SBOX_OUT_DIR__/in1.h' && bash -c 'out/tool in2.txt > __SBOX_OUT_DIR__/in2.h'", + "bash -c 'out/tool in3.txt > __SBOX_OUT_DIR__/in3.h'", }, deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"}, files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"}, @@ -701,7 +710,7 @@ func TestGenruleDefaults(t *testing.T) { } gen := ctx.ModuleForTests("gen", "").Module().(*Module) - expectedCmd := "cp in1 __SBOX_SANDBOX_DIR__/out/out" + expectedCmd := "cp in1 __SBOX_OUT_DIR__/out" if gen.rawCommands[0] != expectedCmd { t.Errorf("Expected cmd: %q, actual: %q", expectedCmd, gen.rawCommands[0]) } |