From 285b66a117ffe4edb44ffd7e1a0b10f93de0c277 Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Mon, 25 Sep 2023 12:13:17 +0000 Subject: [PATCH] Revert^4 "allow Ninja variables in RuleBuilder API" 789a7e0883d5e142ff98898ac02797f106d38b9c Change-Id: Iee1156adf0ad6e6d2ae7e22d90a447d4b564e59f --- android/rule_builder.go | 41 +++++++++++++++-- android/rule_builder_test.go | 86 +++++++++++++++++++++++++++++++----- cmd/sbox/sbox.go | 3 ++ 3 files changed, 115 insertions(+), 15 deletions(-) diff --git a/android/rule_builder.go b/android/rule_builder.go index 777c1cfc3..97582db2c 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -474,13 +474,23 @@ func (r *RuleBuilder) depFileMergerCmd(depFiles WritablePaths) *RuleBuilderComma Inputs(depFiles.Paths()) } +// BuildWithNinjaVars adds the built command line to the build graph, with dependencies on Inputs and Tools, and output files for +// Outputs. This function will not escape Ninja variables, so it may be used to write sandbox manifests using Ninja variables. +func (r *RuleBuilder) BuildWithUnescapedNinjaVars(name string, desc string) { + r.build(name, desc, false) +} + // Build adds the built command line to the build graph, with dependencies on Inputs and Tools, and output files for // Outputs. func (r *RuleBuilder) Build(name string, desc string) { + r.build(name, desc, true) +} + +func (r *RuleBuilder) build(name string, desc string, ninjaEscapeCommandString bool) { name = ninjaNameEscape(name) if len(r.missingDeps) > 0 { - r.ctx.Build(pctx, BuildParams{ + r.ctx.Build(r.pctx, BuildParams{ Rule: ErrorRule, Outputs: r.Outputs(), Description: desc, @@ -619,12 +629,35 @@ func (r *RuleBuilder) Build(name string, desc string) { name, r.sboxManifestPath.String(), r.outDir.String()) } - // Create a rule to write the manifest as a the textproto. + // Create a rule to write the manifest as textproto. pbText, err := prototext.Marshal(&manifest) if err != nil { ReportPathErrorf(r.ctx, "sbox manifest failed to marshal: %q", err) } - WriteFileRule(r.ctx, r.sboxManifestPath, string(pbText)) + if ninjaEscapeCommandString { + WriteFileRule(r.ctx, r.sboxManifestPath, string(pbText)) + } else { + // We need to have a rule to write files that is + // defined on the RuleBuilder's pctx in order to + // write Ninja variables in the string. + // The WriteFileRule function above rule can only write + // raw strings because it is defined on the android + // package's pctx, and it can't access variables defined + // in another context. + r.ctx.Build(r.pctx, BuildParams{ + Rule: r.ctx.Rule(r.pctx, "unescapedWriteFile", blueprint.RuleParams{ + Command: `rm -rf ${out} && cat ${out}.rsp > ${out}`, + Rspfile: "${out}.rsp", + RspfileContent: "${content}", + Description: "write file", + }, "content"), + Output: r.sboxManifestPath, + Description: "write sbox manifest " + r.sboxManifestPath.Base(), + Args: map[string]string{ + "content": string(pbText), + }, + }) + } // 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 @@ -724,7 +757,7 @@ func (r *RuleBuilder) Build(name string, desc string) { } r.ctx.Build(r.pctx, BuildParams{ - Rule: r.ctx.Rule(pctx, name, blueprint.RuleParams{ + Rule: r.ctx.Rule(r.pctx, name, blueprint.RuleParams{ Command: proptools.NinjaEscape(commandString), CommandDeps: proptools.NinjaEscapeList(tools.Strings()), Restat: r.restat, diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index 86647eb22..a6b3a27a1 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -28,6 +28,17 @@ import ( "android/soong/shared" ) +var ( + pctx_ruleBuilderTest = NewPackageContext("android/soong/rule_builder") + pctx_ruleBuilderTestSubContext = NewPackageContext("android/soong/rule_builder/config") +) + +func init() { + pctx_ruleBuilderTest.Import("android/soong/rule_builder/config") + pctx_ruleBuilderTest.StaticVariable("cmdFlags", "${config.ConfigFlags}") + pctx_ruleBuilderTestSubContext.StaticVariable("ConfigFlags", "--some-clang-flag") +} + func builderContext() BuilderContext { return BuilderContextForTesting(TestConfig("out", nil, "", map[string][]byte{ "ld": nil, @@ -496,11 +507,13 @@ func testRuleBuilderFactory() Module { type testRuleBuilderModule struct { ModuleBase properties struct { - Srcs []string + Srcs []string + Flags []string - Restat bool - Sbox bool - Sbox_inputs bool + Restat bool + Sbox bool + Sbox_inputs bool + Unescape_ninja_vars bool } } @@ -518,8 +531,9 @@ func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForModuleOut(ctx, "sbox.textproto") - testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, out, outDep, outDir, - manifestPath, t.properties.Restat, t.properties.Sbox, t.properties.Sbox_inputs, + testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, t.properties.Flags, + out, outDep, outDir, + manifestPath, t.properties.Restat, t.properties.Sbox, t.properties.Sbox_inputs, t.properties.Unescape_ninja_vars, rspFile, rspFileContents, rspFile2, rspFileContents2) } @@ -543,17 +557,18 @@ func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"}) manifestPath := PathForOutput(ctx, "singleton/sbox.textproto") - testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, out, outDep, outDir, - manifestPath, true, false, false, + testRuleBuilder_Build(ctx, in, implicit, orderOnly, validation, nil, out, outDep, outDir, + manifestPath, true, false, false, false, rspFile, rspFileContents, rspFile2, rspFileContents2) } func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, validation Path, + flags []string, out, outDep, outDir, manifestPath WritablePath, - restat, sbox, sboxInputs bool, + restat, sbox, sboxInputs, unescapeNinjaVars bool, rspFile WritablePath, rspFileContents Paths, rspFile2 WritablePath, rspFileContents2 Paths) { - rule := NewRuleBuilder(pctx, ctx) + rule := NewRuleBuilder(pctx_ruleBuilderTest, ctx) if sbox { rule.Sbox(outDir, manifestPath) @@ -564,6 +579,7 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, va rule.Command(). Tool(PathForSource(ctx, "cp")). + Flags(flags). Inputs(in). Implicit(implicit). OrderOnly(orderOnly). @@ -577,7 +593,11 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, va rule.Restat() } - rule.Build("rule", "desc") + if unescapeNinjaVars { + rule.BuildWithUnescapedNinjaVars("rule", "desc") + } else { + rule.Build("rule", "desc") + } } var prepareForRuleBuilderTest = FixtureRegisterWithContext(func(ctx RegistrationContext) { @@ -792,3 +812,47 @@ func TestRuleBuilderHashInputs(t *testing.T) { }) } } + +func TestRuleBuilderWithNinjaVarEscaping(t *testing.T) { + bp := ` + rule_builder_test { + name: "foo_sbox_escaped_ninja", + flags: ["${cmdFlags}"], + sbox: true, + sbox_inputs: true, + } + rule_builder_test { + name: "foo_sbox", + flags: ["${cmdFlags}"], + sbox: true, + sbox_inputs: true, + unescape_ninja_vars: true, + } + ` + result := GroupFixturePreparers( + prepareForRuleBuilderTest, + FixtureWithRootAndroidBp(bp), + ).RunTest(t) + + escapedNinjaMod := result.ModuleForTests("foo_sbox_escaped_ninja", "").Rule("writeFile") + AssertStringDoesContain( + t, + "", + escapedNinjaMod.BuildParams.Args["content"], + "$${cmdFlags}", + ) + + unescapedNinjaMod := result.ModuleForTests("foo_sbox", "").Rule("unescapedWriteFile") + AssertStringDoesContain( + t, + "", + unescapedNinjaMod.BuildParams.Args["content"], + "${cmdFlags}", + ) + AssertStringDoesNotContain( + t, + "", + unescapedNinjaMod.BuildParams.Args["content"], + "$${cmdFlags}", + ) +} diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go index fc56dd526..3364f503f 100644 --- a/cmd/sbox/sbox.go +++ b/cmd/sbox/sbox.go @@ -119,6 +119,9 @@ func run() error { } manifest, err := readManifest(manifestFile) + if err != nil { + return err + } if len(manifest.Commands) == 0 { return fmt.Errorf("at least one commands entry is required in %q", manifestFile)