Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 619b9ab2 authored by Colin Cross's avatar Colin Cross
Browse files

Revert "Rewrite sbox to use a textproto manifest" 

This reverts commit 151b9ff0.

Reason for revert: broke builds

Change-Id: I69b3b8795d5a36b4fa0debb1af2d433be3c15d6c
parent 151b9ff0

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)

		}