From 70c4741215a72d66209b2421157a04bf8b2c76d3 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 12 Mar 2021 17:48:14 -0800 Subject: [PATCH] Add explicit rspfile argument to RuleBuilderCommand.FlagWithRspFileInputList Using $out.rsp as the rsp file adds extra complexity around keeping the $ unescaped. Make callers to FlagWithRspFileInputList provide an explicit path for the rsp file instead. Bug: 182612695 Test: rule_builder_test.go Change-Id: I3f531d80c1efa8a9d09aac0a63790c5b11a9f0c6 --- android/rule_builder.go | 42 ++++++++++++++++++++++++++++-------- android/rule_builder_test.go | 6 +++--- android/test_suites.go | 2 +- cc/fuzz.go | 3 ++- cc/vendor_snapshot.go | 3 ++- cc/vndk.go | 3 ++- genrule/genrule_test.go | 8 +++++++ java/droiddoc.go | 10 +++++---- java/kotlin.go | 4 +++- java/lint.go | 11 +++++++--- 10 files changed, 68 insertions(+), 24 deletions(-) diff --git a/android/rule_builder.go b/android/rule_builder.go index 22ae2b24a..cc0bfa6af 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -385,6 +385,21 @@ func (r *RuleBuilder) RspFileInputs() Paths { return rspFileInputs } +// RspFile returns the path to the rspfile that was passed to the RuleBuilderCommand.FlagWithRspFileInputList method. +func (r *RuleBuilder) RspFile() WritablePath { + var rspFile WritablePath + for _, c := range r.commands { + if c.rspFile != nil { + if rspFile != nil { + panic("Multiple commands in a rule may not have rsp file inputs") + } + rspFile = c.rspFile + } + } + + return rspFile +} + // Commands returns a slice containing the built command line for each call to RuleBuilder.Command. func (r *RuleBuilder) Commands() []string { var commands []string @@ -461,6 +476,8 @@ func (r *RuleBuilder) Build(name string, desc string) { commands := r.NinjaEscapedCommands() outputs := r.Outputs() inputs := r.Inputs() + rspFileInputs := r.RspFileInputs() + rspFilePath := r.RspFile() if len(commands) == 0 { return @@ -559,15 +576,14 @@ func (r *RuleBuilder) Build(name string, desc string) { } // Ninja doesn't like multiple outputs when depfiles are enabled, move all but the first output to - // ImplicitOutputs. RuleBuilder only uses "$out" for the rsp file location, so the distinction between Outputs and + // ImplicitOutputs. RuleBuilder doesn't use "$out", so the distinction between Outputs and // ImplicitOutputs doesn't matter. output := outputs[0] implicitOutputs := outputs[1:] var rspFile, rspFileContent string - rspFileInputs := r.RspFileInputs() - if rspFileInputs != nil { - rspFile = "$out.rsp" + if rspFilePath != nil { + rspFile = rspFilePath.String() rspFileContent = "$in" } @@ -620,6 +636,7 @@ type RuleBuilderCommand struct { tools Paths packagedTools []PackagingSpec rspFileInputs Paths + rspFile WritablePath // spans [start,end) of the command that should not be ninja escaped unescapedSpans [][2]int @@ -1020,8 +1037,9 @@ func (c *RuleBuilderCommand) FlagWithDepFile(flag string, path WritablePath) *Ru } // FlagWithRspFileInputList adds the specified flag and path to an rspfile to the command line, with no separator -// between them. The paths will be written to the rspfile. -func (c *RuleBuilderCommand) FlagWithRspFileInputList(flag string, paths Paths) *RuleBuilderCommand { +// between them. The paths will be written to the rspfile. If sbox is enabled, the rspfile must +// be outside the sbox directory. +func (c *RuleBuilderCommand) FlagWithRspFileInputList(flag string, rspFile WritablePath, paths Paths) *RuleBuilderCommand { if c.rspFileInputs != nil { panic("FlagWithRspFileInputList cannot be called if rsp file inputs have already been provided") } @@ -1033,10 +1051,16 @@ func (c *RuleBuilderCommand) FlagWithRspFileInputList(flag string, paths Paths) } c.rspFileInputs = paths + c.rspFile = rspFile - rspFile := "$out.rsp" - c.FlagWithArg(flag, rspFile) - c.unescapedSpans = append(c.unescapedSpans, [2]int{c.buf.Len() - len(rspFile), c.buf.Len()}) + if c.rule.sbox { + if _, isRel, _ := maybeRelErr(c.rule.outDir.String(), rspFile.String()); isRel { + panic(fmt.Errorf("FlagWithRspFileInputList rspfile %q must not be inside out dir %q", + rspFile.String(), c.rule.outDir.String())) + } + } + + c.FlagWithArg(flag, rspFile.String()) return c } diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index 06ea1242d..fbf624ee3 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -267,10 +267,10 @@ func ExampleRuleBuilderCommand_FlagWithRspFileInputList() { ctx := builderContext() fmt.Println(NewRuleBuilder(pctx, ctx).Command(). Tool(PathForSource(ctx, "javac")). - FlagWithRspFileInputList("@", PathsForTesting("a.java", "b.java")). - NinjaEscapedString()) + FlagWithRspFileInputList("@", PathForOutput(ctx, "foo.rsp"), PathsForTesting("a.java", "b.java")). + String()) // Output: - // javac @$out.rsp + // javac @out/foo.rsp } func ExampleRuleBuilderCommand_String() { diff --git a/android/test_suites.go b/android/test_suites.go index 7ecb8d27b..6b7b909fc 100644 --- a/android/test_suites.go +++ b/android/test_suites.go @@ -68,7 +68,7 @@ func robolectricTestSuite(ctx SingletonContext, files map[string]InstallPaths) W FlagWithOutput("-o ", outputFile). FlagWithArg("-P ", "host/testcases"). FlagWithArg("-C ", testCasesDir.String()). - FlagWithRspFileInputList("-r ", installedPaths.Paths()) + FlagWithRspFileInputList("-r ", outputFile.ReplaceExtension(ctx, "rsp"), installedPaths.Paths()) rule.Build("robolectric_tests_zip", "robolectric-tests.zip") return outputFile diff --git a/cc/fuzz.go b/cc/fuzz.go index 9fe5b17f5..5219ebc04 100644 --- a/cc/fuzz.go +++ b/cc/fuzz.go @@ -440,7 +440,8 @@ func (s *fuzzPackager) GenerateBuildActions(ctx android.SingletonContext) { command := builder.Command().BuiltTool("soong_zip"). Flag("-j"). FlagWithOutput("-o ", corpusZip) - command.FlagWithRspFileInputList("-r ", fuzzModule.corpus) + rspFile := corpusZip.ReplaceExtension(ctx, "rsp") + command.FlagWithRspFileInputList("-r ", rspFile, fuzzModule.corpus) files = append(files, fileToZip{corpusZip, ""}) } diff --git a/cc/vendor_snapshot.go b/cc/vendor_snapshot.go index fdd1fec20..4014fe0c4 100644 --- a/cc/vendor_snapshot.go +++ b/cc/vendor_snapshot.go @@ -528,10 +528,11 @@ func (c *snapshotSingleton) GenerateBuildActions(ctx android.SingletonContext) { ctx, snapshotDir, c.name+"-"+ctx.Config().DeviceName()+"_list") + rspFile := snapshotOutputList.ReplaceExtension(ctx, "rsp") zipRule.Command(). Text("tr"). FlagWithArg("-d ", "\\'"). - FlagWithRspFileInputList("< ", snapshotOutputs). + FlagWithRspFileInputList("< ", rspFile, snapshotOutputs). FlagWithOutput("> ", snapshotOutputList) zipRule.Temporary(snapshotOutputList) diff --git a/cc/vndk.go b/cc/vndk.go index 85028d0aa..b7047e92e 100644 --- a/cc/vndk.go +++ b/cc/vndk.go @@ -827,10 +827,11 @@ func (c *vndkSnapshotSingleton) GenerateBuildActions(ctx android.SingletonContex // filenames in rspfile from FlagWithRspFileInputList might be single-quoted. Remove it with tr snapshotOutputList := android.PathForOutput(ctx, snapshotDir, "android-vndk-"+ctx.DeviceConfig().DeviceArch()+"_list") + rspFile := snapshotOutputList.ReplaceExtension(ctx, "rsp") zipRule.Command(). Text("tr"). FlagWithArg("-d ", "\\'"). - FlagWithRspFileInputList("< ", snapshotOutputs). + FlagWithRspFileInputList("< ", rspFile, snapshotOutputs). FlagWithOutput("> ", snapshotOutputList) zipRule.Temporary(snapshotOutputList) diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index d131e94f9..199a7df7b 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -298,6 +298,14 @@ func TestGenruleCmd(t *testing.T) { `, expect: "echo foo > __SBOX_SANDBOX_DIR__/out/foo && cp __SBOX_SANDBOX_DIR__/out/foo __SBOX_SANDBOX_DIR__/out/out", }, + { + name: "$", + prop: ` + out: ["out"], + cmd: "echo $$ > $(out)", + `, + expect: "echo $ > __SBOX_SANDBOX_DIR__/out/out", + }, { name: "error empty location", diff --git a/java/droiddoc.go b/java/droiddoc.go index f0decec74..da13c621b 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -812,7 +812,7 @@ func javadocCmd(ctx android.ModuleContext, rule *android.RuleBuilder, srcs andro BuiltTool("soong_javac_wrapper").Tool(config.JavadocCmd(ctx)). Flag(config.JavacVmFlags). FlagWithArg("-encoding ", "UTF-8"). - FlagWithRspFileInputList("@", srcs). + FlagWithRspFileInputList("@", android.PathForModuleOut(ctx, "javadoc.rsp"), srcs). FlagWithInput("@", srcJarList) // TODO(ccross): Remove this if- statement once we finish migration for all Doclava @@ -1243,7 +1243,7 @@ func metalavaCmd(ctx android.ModuleContext, rule *android.RuleBuilder, javaVersi Flag("-J--add-opens=java.base/java.util=ALL-UNNAMED"). FlagWithArg("-encoding ", "UTF-8"). FlagWithArg("-source ", javaVersion.String()). - FlagWithRspFileInputList("@", srcs). + FlagWithRspFileInputList("@", android.PathForModuleOut(ctx, "metalava.rsp"), srcs). FlagWithInput("@", srcJarList) if javaHome := ctx.Config().Getenv("ANDROID_JAVA_HOME"); javaHome != "" { @@ -1446,7 +1446,9 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { // add a large number of inputs to a file without exceeding bash command length limits (which // would happen if we use the WriteFile rule). The cp is needed because RuleBuilder sets the // rsp file to be ${output}.rsp. - impCmd.Text("cp").FlagWithRspFileInputList("", cmd.GetImplicits()).Output(implicitsRsp) + impCmd.Text("cp"). + FlagWithRspFileInputList("", android.PathForModuleOut(ctx, "metalava-implicits.rsp"), cmd.GetImplicits()). + Output(implicitsRsp) impRule.Build("implicitsGen", "implicits generation") cmd.Implicit(implicitsRsp) @@ -1750,7 +1752,7 @@ func (p *PrebuiltStubsSources) GenerateAndroidBuildActions(ctx android.ModuleCon Flag("-jar"). FlagWithOutput("-o ", p.stubsSrcJar). FlagWithArg("-C ", srcDir.String()). - FlagWithRspFileInputList("-r ", srcPaths) + FlagWithRspFileInputList("-r ", p.stubsSrcJar.ReplaceExtension(ctx, "rsp"), srcPaths) rule.Restat() diff --git a/java/kotlin.go b/java/kotlin.go index 8067ad521..2960f819d 100644 --- a/java/kotlin.go +++ b/java/kotlin.go @@ -64,7 +64,9 @@ func kotlinCommonSrcsList(ctx android.ModuleContext, commonSrcFiles android.Path // Insert a second rule to write out the list of resources to a file. commonSrcsList := android.PathForModuleOut(ctx, "kotlinc_common_srcs.list") rule := android.NewRuleBuilder(pctx, ctx) - rule.Command().Text("cp").FlagWithRspFileInputList("", commonSrcFiles).Output(commonSrcsList) + rule.Command().Text("cp"). + FlagWithRspFileInputList("", commonSrcsList.ReplaceExtension(ctx, "rsp"), commonSrcFiles). + Output(commonSrcsList) rule.Build("kotlin_common_srcs_list", "kotlin common_srcs list") return android.OptionalPathForPath(commonSrcsList) } diff --git a/java/lint.go b/java/lint.go index 9f677db3a..fccd1a552 100644 --- a/java/lint.go +++ b/java/lint.go @@ -220,7 +220,9 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru // Insert a second rule to write out the list of resources to a file. resourcesList = android.PathForModuleOut(ctx, "lint", "resources.list") resListRule := android.NewRuleBuilder(pctx, ctx) - resListRule.Command().Text("cp").FlagWithRspFileInputList("", l.resources).Output(resourcesList) + resListRule.Command().Text("cp"). + FlagWithRspFileInputList("", resourcesList.ReplaceExtension(ctx, "rsp"), l.resources). + Output(resourcesList) resListRule.Build("lint_resources_list", "lint resources list") trackRSPDependency(l.resources, resourcesList) } @@ -241,7 +243,10 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru // TODO(ccross): some of the files in l.srcs are generated sources and should be passed to // lint separately. srcsList := android.PathForModuleOut(ctx, "lint", "srcs.list") - rule.Command().Text("cp").FlagWithRspFileInputList("", l.srcs).Output(srcsList) + srcsListRsp := android.PathForModuleOut(ctx, "lint-srcs.list.rsp") + rule.Command().Text("cp"). + FlagWithRspFileInputList("", srcsListRsp, l.srcs). + Output(srcsList) trackRSPDependency(l.srcs, srcsList) cmd := rule.Command(). @@ -635,7 +640,7 @@ func lintZip(ctx android.BuilderContext, paths android.Paths, outputPath android rule.Command().BuiltTool("soong_zip"). FlagWithOutput("-o ", outputPath). FlagWithArg("-C ", android.PathForIntermediates(ctx).String()). - FlagWithRspFileInputList("-r ", paths) + FlagWithRspFileInputList("-r ", outputPath.ReplaceExtension(ctx, "rsp"), paths) rule.Build(outputPath.Base(), outputPath.Base()) }