From da6401b770fd126dcfb0fabf013b5699a4429c49 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 21 Apr 2021 11:32:19 -0700 Subject: [PATCH 1/2] Fix bug in RuleBuilderCommand.OrderOnlys Order only dependencies that were added to a RuleBuilderCommand were only propagated to the rule that was created for a missing dependencies error. Set them on the main rule instead. Test: TestRuleBuilder_build Change-Id: I049d1bb921778d18d678439d236f69081927dc70 --- android/rule_builder.go | 2 +- android/rule_builder_test.go | 57 +++++++++++++++++++++++------------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/android/rule_builder.go b/android/rule_builder.go index 06e82c8e9..1438fcac3 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -460,7 +460,6 @@ func (r *RuleBuilder) Build(name string, desc string) { r.ctx.Build(pctx, BuildParams{ Rule: ErrorRule, Outputs: r.Outputs(), - OrderOnly: r.OrderOnlys(), Description: desc, Args: map[string]string{ "error": "missing dependencies: " + strings.Join(r.missingDeps, ", "), @@ -707,6 +706,7 @@ func (r *RuleBuilder) Build(name string, desc string) { }), Inputs: rspFileInputs, Implicits: inputs, + OrderOnly: r.OrderOnlys(), Output: output, ImplicitOutputs: implicitOutputs, SymlinkOutputs: r.SymlinkOutputs(), diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index d2a7d8d2a..24c8976a3 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -15,6 +15,8 @@ package android import ( + "crypto/sha256" + "encoding/hex" "fmt" "path/filepath" "regexp" @@ -497,6 +499,8 @@ type testRuleBuilderModule struct { func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { in := PathsForSource(ctx, t.properties.Srcs) + implicit := PathForSource(ctx, "implicit") + orderOnly := PathForSource(ctx, "orderonly") out := PathForModuleOut(ctx, "gen", ctx.ModuleName()) outDep := PathForModuleOut(ctx, "gen", ctx.ModuleName()+".d") outDir := PathForModuleOut(ctx, "gen") @@ -506,9 +510,9 @@ func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForModuleOut(ctx, "sbox.textproto") - testRuleBuilder_Build(ctx, in, out, outDep, outDir, manifestPath, t.properties.Restat, - t.properties.Sbox, t.properties.Sbox_inputs, rspFile, rspFileContents, - rspFile2, rspFileContents2) + testRuleBuilder_Build(ctx, in, implicit, orderOnly, out, outDep, outDir, + manifestPath, t.properties.Restat, t.properties.Sbox, t.properties.Sbox_inputs, + rspFile, rspFileContents, rspFile2, rspFileContents2) } type testRuleBuilderSingleton struct{} @@ -518,7 +522,9 @@ func testRuleBuilderSingletonFactory() Singleton { } func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { - in := PathForSource(ctx, "bar") + in := PathsForSource(ctx, []string{"in"}) + implicit := PathForSource(ctx, "implicit") + orderOnly := PathForSource(ctx, "orderonly") out := PathForOutput(ctx, "singleton/gen/baz") outDep := PathForOutput(ctx, "singleton/gen/baz.d") outDir := PathForOutput(ctx, "singleton/gen") @@ -527,11 +533,14 @@ func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { rspFileContents := PathsForSource(ctx, []string{"rsp_in"}) rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForOutput(ctx, "singleton/sbox.textproto") - testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, manifestPath, true, false, false, + + testRuleBuilder_Build(ctx, in, implicit, orderOnly, out, outDep, outDir, + manifestPath, true, false, false, rspFile, rspFileContents, rspFile2, rspFileContents2) } -func testRuleBuilder_Build(ctx BuilderContext, in Paths, out, outDep, outDir, manifestPath WritablePath, +func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly Path, + out, outDep, outDir, manifestPath WritablePath, restat, sbox, sboxInputs bool, rspFile WritablePath, rspFileContents Paths, rspFile2 WritablePath, rspFileContents2 Paths) { @@ -547,6 +556,8 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, out, outDep, outDir, ma rule.Command(). Tool(PathForSource(ctx, "cp")). Inputs(in). + Implicit(implicit). + OrderOnly(orderOnly). Output(out). ImplicitDepFile(outDep). FlagWithRspFileInputList("@", rspFile, rspFileContents). @@ -566,24 +577,24 @@ var prepareForRuleBuilderTest = FixtureRegisterWithContext(func(ctx Registration func TestRuleBuilder_Build(t *testing.T) { fs := MockFS{ - "bar": nil, - "cp": nil, + "in": nil, + "cp": nil, } bp := ` rule_builder_test { name: "foo", - srcs: ["bar"], + srcs: ["in"], restat: true, } rule_builder_test { name: "foo_sbox", - srcs: ["bar"], + srcs: ["in"], sbox: true, } rule_builder_test { name: "foo_sbox_inputs", - srcs: ["bar"], + srcs: ["in"], sbox: true, sbox_inputs: true, } @@ -614,11 +625,14 @@ func TestRuleBuilder_Build(t *testing.T) { wantInputs := []string{"rsp_in"} AssertArrayString(t, "Inputs", wantInputs, params.Inputs.Strings()) - wantImplicits := append([]string{"bar"}, extraImplicits...) + wantImplicits := append([]string{"implicit", "in"}, extraImplicits...) // The second rsp file and the files listed in it should be in implicits wantImplicits = append(wantImplicits, "rsp_in2", wantRspFile2) AssertPathsRelativeToTopEquals(t, "Implicits", wantImplicits, params.Implicits) + wantOrderOnlys := []string{"orderonly"} + AssertPathsRelativeToTopEquals(t, "OrderOnly", wantOrderOnlys, params.OrderOnly) + wantRspFileContent := "$in" AssertStringEquals(t, "RspfileContent", wantRspFileContent, params.RuleParams.RspfileContent) @@ -646,7 +660,7 @@ func TestRuleBuilder_Build(t *testing.T) { rspFile2 := "out/soong/.intermediates/foo/rsp2" module := result.ModuleForTests("foo", "") check(t, module.Rule("rule"), module.Output(rspFile2), - "cp bar "+outFile+" @"+rspFile+" @"+rspFile2, + "cp in "+outFile+" @"+rspFile+" @"+rspFile2, outFile, outFile+".d", rspFile, rspFile2, true, nil, nil) }) t.Run("sbox", func(t *testing.T) { @@ -688,7 +702,7 @@ func TestRuleBuilder_Build(t *testing.T) { rspFile2 := filepath.Join("out/soong/singleton/rsp2") singleton := result.SingletonForTests("rule_builder_test") check(t, singleton.Rule("rule"), singleton.Output(rspFile2), - "cp bar "+outFile+" @"+rspFile+" @"+rspFile2, + "cp in "+outFile+" @"+rspFile+" @"+rspFile2, outFile, outFile+".d", rspFile, rspFile2, true, nil, nil) }) } @@ -702,6 +716,11 @@ func TestRuleBuilderHashInputs(t *testing.T) { // the list of inputs changes because the command line or a dependency // changes. + hashOf := func(s string) string { + sum := sha256.Sum256([]byte(s)) + return hex.EncodeToString(sum[:]) + } + bp := ` rule_builder_test { name: "hash0", @@ -727,14 +746,12 @@ func TestRuleBuilderHashInputs(t *testing.T) { expectedHash string }{ { - name: "hash0", - // sha256 value obtained from: echo -en 'in1.txt\nin2.txt' | sha256sum - expectedHash: "18da75b9b1cc74b09e365b4ca2e321b5d618f438cc632b387ad9dc2ab4b20e9d", + name: "hash0", + expectedHash: hashOf("implicit\nin1.txt\nin2.txt"), }, { - name: "hash1", - // sha256 value obtained from: echo -en 'in1.txt\nin2.txt\nin3.txt' | sha256sum - expectedHash: "a38d432a4b19df93140e1f1fe26c97ff0387dae01fe506412b47208f0595fb45", + name: "hash1", + expectedHash: hashOf("implicit\nin1.txt\nin2.txt\nin3.txt"), }, } From ae89abe381be5f91b57b02fd57ab787ba506ac65 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 21 Apr 2021 11:45:23 -0700 Subject: [PATCH 2/2] Add validations to RuleBuilderCommand Add RuleBuilderCommand.Validation and RuleBuilderCommand.Validations that propagate to BuildParams.Validations. Test: TestRuleBuilder_build, TestRuleBuilder Change-Id: I9a08bbc073a432fd0b53b87d8948ac0f6afcf637 --- android/rule_builder.go | 38 ++++++++++++++++++++++++++++++++++++ android/rule_builder_test.go | 19 +++++++++++++++--- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/android/rule_builder.go b/android/rule_builder.go index 1438fcac3..2507c4c83 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -283,6 +283,28 @@ func (r *RuleBuilder) OrderOnlys() Paths { return orderOnlyList } +// Validations returns the list of paths that were passed to RuleBuilderCommand.Validation or +// RuleBuilderCommand.Validations. The list is sorted and duplicates removed. +func (r *RuleBuilder) Validations() Paths { + validations := make(map[string]Path) + for _, c := range r.commands { + for _, validation := range c.validations { + validations[validation.String()] = validation + } + } + + var validationList Paths + for _, validation := range validations { + validationList = append(validationList, validation) + } + + sort.Slice(validationList, func(i, j int) bool { + return validationList[i].String() < validationList[j].String() + }) + + return validationList +} + func (r *RuleBuilder) outputSet() map[string]WritablePath { outputs := make(map[string]WritablePath) for _, c := range r.commands { @@ -707,6 +729,7 @@ func (r *RuleBuilder) Build(name string, desc string) { Inputs: rspFileInputs, Implicits: inputs, OrderOnly: r.OrderOnlys(), + Validations: r.Validations(), Output: output, ImplicitOutputs: implicitOutputs, SymlinkOutputs: r.SymlinkOutputs(), @@ -727,6 +750,7 @@ type RuleBuilderCommand struct { inputs Paths implicits Paths orderOnlys Paths + validations Paths outputs WritablePaths symlinkOutputs WritablePaths depFiles WritablePaths @@ -1061,6 +1085,20 @@ func (c *RuleBuilderCommand) OrderOnlys(paths Paths) *RuleBuilderCommand { return c } +// Validation adds the specified input path to the validation dependencies by +// RuleBuilder.Validations without modifying the command line. +func (c *RuleBuilderCommand) Validation(path Path) *RuleBuilderCommand { + c.validations = append(c.validations, path) + return c +} + +// Validations adds the specified input paths to the validation dependencies by +// RuleBuilder.Validations without modifying the command line. +func (c *RuleBuilderCommand) Validations(paths Paths) *RuleBuilderCommand { + c.validations = append(c.validations, paths...) + return c +} + // Output adds the specified output path to the command line. The path will also be added to the outputs returned by // RuleBuilder.Outputs. func (c *RuleBuilderCommand) Output(path WritablePath) *RuleBuilderCommand { diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index 24c8976a3..feee90f29 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -322,6 +322,7 @@ func TestRuleBuilder(t *testing.T) { Input(PathForSource(ctx, "Input")). Output(PathForOutput(ctx, "module/Output")). OrderOnly(PathForSource(ctx, "OrderOnly")). + Validation(PathForSource(ctx, "Validation")). SymlinkOutput(PathForOutput(ctx, "module/SymlinkOutput")). ImplicitSymlinkOutput(PathForOutput(ctx, "module/ImplicitSymlinkOutput")). Text("Text"). @@ -333,6 +334,7 @@ func TestRuleBuilder(t *testing.T) { Input(PathForSource(ctx, "input2")). Output(PathForOutput(ctx, "module/output2")). OrderOnlys(PathsForSource(ctx, []string{"OrderOnlys"})). + Validations(PathsForSource(ctx, []string{"Validations"})). Tool(PathForSource(ctx, "tool2")) // Test updates to the first command after the second command has been started @@ -360,6 +362,7 @@ func TestRuleBuilder(t *testing.T) { "module/DepFile", "module/depfile", "module/ImplicitDepFile", "module/depfile2"}) wantTools := PathsForSource(ctx, []string{"Tool", "tool2"}) wantOrderOnlys := PathsForSource(ctx, []string{"OrderOnly", "OrderOnlys"}) + wantValidations := PathsForSource(ctx, []string{"Validation", "Validations"}) wantSymlinkOutputs := PathsForOutput(ctx, []string{ "module/ImplicitSymlinkOutput", "module/SymlinkOutput"}) @@ -387,6 +390,7 @@ func TestRuleBuilder(t *testing.T) { AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles()) AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools()) AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys()) + AssertDeepEquals(t, "rule.Validations()", wantValidations, rule.Validations()) AssertSame(t, "rule.depFileMergerCmd()", wantDepMergerCommand, rule.depFileMergerCmd(rule.DepFiles()).String()) }) @@ -416,6 +420,7 @@ func TestRuleBuilder(t *testing.T) { AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles()) AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools()) AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys()) + AssertDeepEquals(t, "rule.Validations()", wantValidations, rule.Validations()) AssertSame(t, "rule.depFileMergerCmd()", wantDepMergerCommand, rule.depFileMergerCmd(rule.DepFiles()).String()) }) @@ -445,6 +450,7 @@ func TestRuleBuilder(t *testing.T) { AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles()) AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools()) AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys()) + AssertDeepEquals(t, "rule.Validations()", wantValidations, rule.Validations()) AssertSame(t, "rule.depFileMergerCmd()", wantDepMergerCommand, rule.depFileMergerCmd(rule.DepFiles()).String()) }) @@ -474,6 +480,7 @@ func TestRuleBuilder(t *testing.T) { AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles()) AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools()) AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys()) + AssertDeepEquals(t, "rule.Validations()", wantValidations, rule.Validations()) AssertSame(t, "rule.depFileMergerCmd()", wantDepMergerCommand, rule.depFileMergerCmd(rule.DepFiles()).String()) }) @@ -501,6 +508,7 @@ func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { in := PathsForSource(ctx, t.properties.Srcs) implicit := PathForSource(ctx, "implicit") orderOnly := PathForSource(ctx, "orderonly") + validation := PathForSource(ctx, "validation") out := PathForModuleOut(ctx, "gen", ctx.ModuleName()) outDep := PathForModuleOut(ctx, "gen", ctx.ModuleName()+".d") outDir := PathForModuleOut(ctx, "gen") @@ -510,7 +518,7 @@ func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForModuleOut(ctx, "sbox.textproto") - testRuleBuilder_Build(ctx, in, implicit, orderOnly, out, outDep, outDir, + testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, out, outDep, outDir, manifestPath, t.properties.Restat, t.properties.Sbox, t.properties.Sbox_inputs, rspFile, rspFileContents, rspFile2, rspFileContents2) } @@ -525,6 +533,7 @@ func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { in := PathsForSource(ctx, []string{"in"}) implicit := PathForSource(ctx, "implicit") orderOnly := PathForSource(ctx, "orderonly") + validation := PathForSource(ctx, "validation") out := PathForOutput(ctx, "singleton/gen/baz") outDep := PathForOutput(ctx, "singleton/gen/baz.d") outDir := PathForOutput(ctx, "singleton/gen") @@ -534,12 +543,12 @@ func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForOutput(ctx, "singleton/sbox.textproto") - testRuleBuilder_Build(ctx, in, implicit, orderOnly, out, outDep, outDir, + testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, out, outDep, outDir, manifestPath, true, false, false, rspFile, rspFileContents, rspFile2, rspFileContents2) } -func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly Path, +func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, validation Path, out, outDep, outDir, manifestPath WritablePath, restat, sbox, sboxInputs bool, rspFile WritablePath, rspFileContents Paths, rspFile2 WritablePath, rspFileContents2 Paths) { @@ -558,6 +567,7 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly Pat Inputs(in). Implicit(implicit). OrderOnly(orderOnly). + Validation(validation). Output(out). ImplicitDepFile(outDep). FlagWithRspFileInputList("@", rspFile, rspFileContents). @@ -633,6 +643,9 @@ func TestRuleBuilder_Build(t *testing.T) { wantOrderOnlys := []string{"orderonly"} AssertPathsRelativeToTopEquals(t, "OrderOnly", wantOrderOnlys, params.OrderOnly) + wantValidations := []string{"validation"} + AssertPathsRelativeToTopEquals(t, "Validations", wantValidations, params.Validations) + wantRspFileContent := "$in" AssertStringEquals(t, "RspfileContent", wantRspFileContent, params.RuleParams.RspfileContent)