Revert "Rewrite sbox to use a textproto manifest"
This reverts commit 151b9ff0cf7d22b7257defa6aecc39693f1f2b3c.
Reason for revert: broke builds
Change-Id: I69b3b8795d5a36b4fa0debb1af2d433be3c15d6c
diff --git a/android/Android.bp b/android/Android.bp
index 1733175..7bd1450 100644
--- a/android/Android.bp
+++ b/android/Android.bp
@@ -4,7 +4,6 @@
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 3efe9f8..86418b2 100644
--- a/android/rule_builder.go
+++ b/android/rule_builder.go
@@ -20,33 +20,27 @@
"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 @@
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 @@
}
r.sbox = true
r.sboxOutDir = outputDir
- r.sboxManifestPath = manifestPath
return r
}
@@ -429,8 +420,7 @@
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 @@
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())
- // Replace the command string, and add the sbox tool and manifest textproto to the
- // dependencies of the final sbox rule.
+ 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)
+
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 @@
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 dc360c3..c1d5521 100644
--- a/android/rule_builder_test.go
+++ b/android/rule_builder_test.go
@@ -395,17 +395,16 @@
})
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 @@
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 (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 @@
_, 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 @@
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 @@
}
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 @@
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)
}