Merge "Revert "Rewrite sbox to use a textproto manifest"" (b893f876) · Commits · e / os / android_build_soong

android/Android.bp

+0 −1

Original line number	Diff line number	Diff line
		@@ -4,7 +4,6 @@ bootstrap_go_package {
		deps: [
		"blueprint",
		"blueprint-bootstrap",
		"sbox_proto",
		"soong",
		"soong-android-soongconfig",
		"soong-env",

android/rule_builder.go

+31 −80

Original line number	Diff line number	Diff line
		@@ -20,19 +20,14 @@ 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.
		@@ -45,7 +40,6 @@ type RuleBuilder struct {
		highmem bool
		remoteable RemoteRuleSupports
		sboxOutDir WritablePath
		sboxManifestPath WritablePath
		missingDeps []string
		}

		@@ -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)

android/rule_builder_test.go

+31 −39

Original line number	Diff line number	Diff line
		@@ -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)
		}

cc/gen.go

+2 −4

Original line number	Diff line number	Diff line
		@@ -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")

cc/gen_test.go

+1 −4

Original line number	Diff line number	Diff line
		@@ -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)
		}