Revert "allow Ninja variables in RuleBuilder API" am: 7b02d8159e

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2746975

Change-Id: I3fa604a507c4fe1370ca8706ed6edb1e80953cd6
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

android/rule_builder.go

@@ -474,23 +474,13 @@ 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(r.pctx, BuildParams{
+		r.ctx.Build(pctx, BuildParams{
 			Rule:        ErrorRule,
 			Outputs:     r.Outputs(),
 			Description: desc,
@@ -629,35 +619,12 @@ func (r *RuleBuilder) build(name string, desc string, ninjaEscapeCommandString b
 				name, r.sboxManifestPath.String(), r.outDir.String())
 		}
 
-		// Create a rule to write the manifest as textproto.
+		// Create a rule to write the manifest as a the textproto.
 		pbText, err := prototext.Marshal(&manifest)
 		if err != nil {
 			ReportPathErrorf(r.ctx, "sbox manifest failed to marshal: %q", err)
 		}
-		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
@@ -757,7 +724,7 @@ func (r *RuleBuilder) build(name string, desc string, ninjaEscapeCommandString b
 	}
 
 	r.ctx.Build(r.pctx, BuildParams{
-		Rule: r.ctx.Rule(r.pctx, name, blueprint.RuleParams{
+		Rule: r.ctx.Rule(pctx, name, blueprint.RuleParams{
 			Command:        proptools.NinjaEscape(commandString),
 			CommandDeps:    proptools.NinjaEscapeList(tools.Strings()),
 			Restat:         r.restat,

android/rule_builder_test.go

@@ -28,17 +28,6 @@ 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,
@@ -508,12 +497,10 @@ type testRuleBuilderModule struct {
 	ModuleBase
 	properties struct {
 		Srcs []string
-		Flags []string
 
 		Restat      bool
 		Sbox        bool
 		Sbox_inputs bool
-		Unescape_ninja_vars bool
 	}
 }
 
@@ -531,9 +518,8 @@ func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) {
 	rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"})
 	manifestPath := PathForModuleOut(ctx, "sbox.textproto")
 
-	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,
+	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)
 }
 
@@ -557,18 +543,17 @@ 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, nil, out, outDep, outDir,
-		manifestPath, true, false, false, false,
+	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, validation Path,
-	flags []string,
 	out, outDep, outDir, manifestPath WritablePath,
-	restat, sbox, sboxInputs, unescapeNinjaVars bool,
+	restat, sbox, sboxInputs bool,
 	rspFile WritablePath, rspFileContents Paths, rspFile2 WritablePath, rspFileContents2 Paths) {
 
-	rule := NewRuleBuilder(pctx_ruleBuilderTest, ctx)
+	rule := NewRuleBuilder(pctx, ctx)
 
 	if sbox {
 		rule.Sbox(outDir, manifestPath)
@@ -579,7 +564,6 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, va
 
 	rule.Command().
 		Tool(PathForSource(ctx, "cp")).
-		Flags(flags).
 		Inputs(in).
 		Implicit(implicit).
 		OrderOnly(orderOnly).
@@ -593,12 +577,8 @@ func testRuleBuilder_Build(ctx BuilderContext, in Paths, implicit, orderOnly, va
 		rule.Restat()
 	}
 
-	if unescapeNinjaVars {
-		rule.BuildWithUnescapedNinjaVars("rule", "desc")
-	} else {
 	rule.Build("rule", "desc")
 }
-}
 
 var prepareForRuleBuilderTest = FixtureRegisterWithContext(func(ctx RegistrationContext) {
 	ctx.RegisterModuleType("rule_builder_test", testRuleBuilderFactory)
@@ -812,47 +792,3 @@ 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}",
-	)
-}

cmd/sbox/sbox.go

@@ -119,9 +119,6 @@ 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)