Support multiple rsp files in RuleBuilder

The lint rule is manually creating a second rsp file because Ninja
only supports on per rule.  Move the support into RuleBuilder so
that it can apply the same rewrites that it does to the primary
one.

Test: TestRuleBuilder_Build
Change-Id: Iec250a2d60e74ccf1b4ad085a960fec6867ea559
This commit is contained in:
Colin Cross
2021-03-19 16:22:12 -07:00
parent e55bd423df
commit ce3a51dc96
3 changed files with 127 additions and 51 deletions

View File

@@ -409,30 +409,21 @@ func (r *RuleBuilder) Tools() Paths {
func (r *RuleBuilder) RspFileInputs() Paths { func (r *RuleBuilder) RspFileInputs() Paths {
var rspFileInputs Paths var rspFileInputs Paths
for _, c := range r.commands { for _, c := range r.commands {
if c.rspFileInputs != nil { for _, rspFile := range c.rspFiles {
if rspFileInputs != nil { rspFileInputs = append(rspFileInputs, rspFile.paths...)
panic("Multiple commands in a rule may not have rsp file inputs")
}
rspFileInputs = c.rspFileInputs
} }
} }
return rspFileInputs return rspFileInputs
} }
// RspFile returns the path to the rspfile that was passed to the RuleBuilderCommand.FlagWithRspFileInputList method. func (r *RuleBuilder) rspFiles() []rspFileAndPaths {
func (r *RuleBuilder) RspFile() WritablePath { var rspFiles []rspFileAndPaths
var rspFile WritablePath
for _, c := range r.commands { for _, c := range r.commands {
if c.rspFile != nil { rspFiles = append(rspFiles, c.rspFiles...)
if rspFile != nil {
panic("Multiple commands in a rule may not have rsp file inputs")
}
rspFile = c.rspFile
}
} }
return rspFile return rspFiles
} }
// Commands returns a slice containing the built command line for each call to RuleBuilder.Command. // Commands returns a slice containing the built command line for each call to RuleBuilder.Command.
@@ -501,8 +492,7 @@ func (r *RuleBuilder) Build(name string, desc string) {
commands := r.Commands() commands := r.Commands()
outputs := r.Outputs() outputs := r.Outputs()
inputs := r.Inputs() inputs := r.Inputs()
rspFileInputs := r.RspFileInputs() rspFiles := r.rspFiles()
rspFilePath := r.RspFile()
if len(commands) == 0 { if len(commands) == 0 {
return return
@@ -556,10 +546,11 @@ func (r *RuleBuilder) Build(name string, desc string) {
}) })
} }
// If using an rsp file copy it into the sbox directory. // If using rsp files copy them and their contents into the sbox directory with
if rspFilePath != nil { // the appropriate path mappings.
for _, rspFile := range rspFiles {
command.RspFiles = append(command.RspFiles, &sbox_proto.RspFile{ command.RspFiles = append(command.RspFiles, &sbox_proto.RspFile{
File: proto.String(rspFilePath.String()), File: proto.String(rspFile.file.String()),
// These have to match the logic in sboxPathForInputRel // These have to match the logic in sboxPathForInputRel
PathMappings: []*sbox_proto.PathMapping{ PathMappings: []*sbox_proto.PathMapping{
{ {
@@ -641,9 +632,9 @@ func (r *RuleBuilder) Build(name string, desc string) {
remoteInputs = append(remoteInputs, inputs...) remoteInputs = append(remoteInputs, inputs...)
remoteInputs = append(remoteInputs, tools...) remoteInputs = append(remoteInputs, tools...)
if rspFilePath != nil { for _, rspFile := range rspFiles {
remoteInputs = append(remoteInputs, rspFilePath) remoteInputs = append(remoteInputs, rspFile.file)
remoteRspFiles = append(remoteRspFiles, rspFilePath) remoteRspFiles = append(remoteRspFiles, rspFile.file)
} }
if len(remoteInputs) > 0 { if len(remoteInputs) > 0 {
@@ -673,12 +664,24 @@ func (r *RuleBuilder) Build(name string, desc string) {
implicitOutputs := outputs[1:] implicitOutputs := outputs[1:]
var rspFile, rspFileContent string var rspFile, rspFileContent string
if rspFilePath != nil { var rspFileInputs Paths
rspFile = rspFilePath.String() if len(rspFiles) > 0 {
// The first rsp files uses Ninja's rsp file support for the rule
rspFile = rspFiles[0].file.String()
// Use "$in" for rspFileContent to avoid duplicating the list of files in the dependency // Use "$in" for rspFileContent to avoid duplicating the list of files in the dependency
// list and in the contents of the rsp file. Inputs to the rule that are not in the // list and in the contents of the rsp file. Inputs to the rule that are not in the
// rsp file will be listed in Implicits instead of Inputs so they don't show up in "$in". // rsp file will be listed in Implicits instead of Inputs so they don't show up in "$in".
rspFileContent = "$in" rspFileContent = "$in"
rspFileInputs = append(rspFileInputs, rspFiles[0].paths...)
for _, rspFile := range rspFiles[1:] {
// Any additional rsp files need an extra rule to write the file.
writeRspFileRule(r.ctx, rspFile.file, rspFile.paths)
// The main rule needs to depend on the inputs listed in the extra rsp file.
inputs = append(inputs, rspFile.paths...)
// The main rule needs to depend on the extra rsp file.
inputs = append(inputs, rspFile.file)
}
} }
var pool blueprint.Pool var pool blueprint.Pool
@@ -729,8 +732,12 @@ type RuleBuilderCommand struct {
depFiles WritablePaths depFiles WritablePaths
tools Paths tools Paths
packagedTools []PackagingSpec packagedTools []PackagingSpec
rspFileInputs Paths rspFiles []rspFileAndPaths
rspFile WritablePath }
type rspFileAndPaths struct {
file WritablePath
paths Paths
} }
func (c *RuleBuilderCommand) addInput(path Path) string { func (c *RuleBuilderCommand) addInput(path Path) string {
@@ -1174,22 +1181,19 @@ func (c *RuleBuilderCommand) FlagWithDepFile(flag string, path WritablePath) *Ru
return c.Text(flag + c.PathForOutput(path)) return c.Text(flag + c.PathForOutput(path))
} }
// FlagWithRspFileInputList adds the specified flag and path to an rspfile to the command line, with no separator // FlagWithRspFileInputList adds the specified flag and path to an rspfile to the command line, with
// between them. The paths will be written to the rspfile. If sbox is enabled, the rspfile must // no separator between them. The paths will be written to the rspfile. If sbox is enabled, the
// be outside the sbox directory. // rspfile must be outside the sbox directory. The first use of FlagWithRspFileInputList in any
// RuleBuilderCommand of a RuleBuilder will use Ninja's rsp file support for the rule, additional
// uses will result in an auxiliary rules to write the rspFile contents.
func (c *RuleBuilderCommand) FlagWithRspFileInputList(flag string, rspFile WritablePath, paths Paths) *RuleBuilderCommand { 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")
}
// Use an empty slice if paths is nil, the non-nil slice is used as an indicator that the rsp file must be // Use an empty slice if paths is nil, the non-nil slice is used as an indicator that the rsp file must be
// generated. // generated.
if paths == nil { if paths == nil {
paths = Paths{} paths = Paths{}
} }
c.rspFileInputs = paths c.rspFiles = append(c.rspFiles, rspFileAndPaths{rspFile, paths})
c.rspFile = rspFile
if c.rule.sbox { if c.rule.sbox {
if _, isRel, _ := maybeRelErr(c.rule.outDir.String(), rspFile.String()); isRel { if _, isRel, _ := maybeRelErr(c.rule.outDir.String(), rspFile.String()); isRel {

View File

@@ -489,8 +489,9 @@ type testRuleBuilderModule struct {
properties struct { properties struct {
Srcs []string Srcs []string
Restat bool Restat bool
Sbox bool Sbox bool
Sbox_inputs bool
} }
} }
@@ -499,9 +500,15 @@ func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) {
out := PathForModuleOut(ctx, "gen", ctx.ModuleName()) out := PathForModuleOut(ctx, "gen", ctx.ModuleName())
outDep := PathForModuleOut(ctx, "gen", ctx.ModuleName()+".d") outDep := PathForModuleOut(ctx, "gen", ctx.ModuleName()+".d")
outDir := PathForModuleOut(ctx, "gen") outDir := PathForModuleOut(ctx, "gen")
rspFile := PathForModuleOut(ctx, "rsp")
rspFile2 := PathForModuleOut(ctx, "rsp2")
rspFileContents := PathsForSource(ctx, []string{"rsp_in"})
rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"})
manifestPath := PathForModuleOut(ctx, "sbox.textproto") manifestPath := PathForModuleOut(ctx, "sbox.textproto")
testRuleBuilder_Build(ctx, in, out, outDep, outDir, manifestPath, t.properties.Restat, t.properties.Sbox) testRuleBuilder_Build(ctx, in, out, outDep, outDir, manifestPath, t.properties.Restat,
t.properties.Sbox, t.properties.Sbox_inputs, rspFile, rspFileContents,
rspFile2, rspFileContents2)
} }
type testRuleBuilderSingleton struct{} type testRuleBuilderSingleton struct{}
@@ -515,18 +522,35 @@ func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) {
out := PathForOutput(ctx, "singleton/gen/baz") out := PathForOutput(ctx, "singleton/gen/baz")
outDep := PathForOutput(ctx, "singleton/gen/baz.d") outDep := PathForOutput(ctx, "singleton/gen/baz.d")
outDir := PathForOutput(ctx, "singleton/gen") outDir := PathForOutput(ctx, "singleton/gen")
rspFile := PathForOutput(ctx, "singleton/rsp")
rspFile2 := PathForOutput(ctx, "singleton/rsp2")
rspFileContents := PathsForSource(ctx, []string{"rsp_in"})
rspFileContents2 := PathsForSource(ctx, []string{"rsp_in2"})
manifestPath := PathForOutput(ctx, "singleton/sbox.textproto") manifestPath := PathForOutput(ctx, "singleton/sbox.textproto")
testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, manifestPath, true, false) testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, manifestPath, true, false, false,
rspFile, rspFileContents, rspFile2, rspFileContents2)
} }
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, manifestPath WritablePath,
restat, sbox, sboxInputs bool,
rspFile WritablePath, rspFileContents Paths, rspFile2 WritablePath, rspFileContents2 Paths) {
rule := NewRuleBuilder(pctx, ctx) rule := NewRuleBuilder(pctx, ctx)
if sbox { if sbox {
rule.Sbox(outDir, manifestPath) rule.Sbox(outDir, manifestPath)
if sboxInputs {
rule.SandboxInputs()
}
} }
rule.Command().Tool(PathForSource(ctx, "cp")).Inputs(in).Output(out).ImplicitDepFile(outDep) rule.Command().
Tool(PathForSource(ctx, "cp")).
Inputs(in).
Output(out).
ImplicitDepFile(outDep).
FlagWithRspFileInputList("@", rspFile, rspFileContents).
FlagWithRspFileInputList("@", rspFile2, rspFileContents2)
if restat { if restat {
rule.Restat() rule.Restat()
@@ -557,6 +581,12 @@ func TestRuleBuilder_Build(t *testing.T) {
srcs: ["bar"], srcs: ["bar"],
sbox: true, sbox: true,
} }
rule_builder_test {
name: "foo_sbox_inputs",
srcs: ["bar"],
sbox: true,
sbox_inputs: true,
}
` `
result := GroupFixturePreparers( result := GroupFixturePreparers(
@@ -565,7 +595,10 @@ func TestRuleBuilder_Build(t *testing.T) {
fs.AddToFixture(), fs.AddToFixture(),
).RunTest(t) ).RunTest(t)
check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraImplicits, extraCmdDeps []string) { check := func(t *testing.T, params TestingBuildParams, rspFile2Params TestingBuildParams,
wantCommand, wantOutput, wantDepfile, wantRspFile, wantRspFile2 string,
wantRestat bool, extraImplicits, extraCmdDeps []string) {
t.Helper() t.Helper()
command := params.RuleParams.Command command := params.RuleParams.Command
re := regexp.MustCompile(" # hash of input list: [a-z0-9]*$") re := regexp.MustCompile(" # hash of input list: [a-z0-9]*$")
@@ -578,9 +611,19 @@ func TestRuleBuilder_Build(t *testing.T) {
AssertBoolEquals(t, "RuleParams.Restat", wantRestat, params.RuleParams.Restat) AssertBoolEquals(t, "RuleParams.Restat", wantRestat, params.RuleParams.Restat)
wantInputs := []string{"rsp_in"}
AssertArrayString(t, "Inputs", wantInputs, params.Inputs.Strings())
wantImplicits := append([]string{"bar"}, extraImplicits...) wantImplicits := append([]string{"bar"}, 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) AssertPathsRelativeToTopEquals(t, "Implicits", wantImplicits, params.Implicits)
wantRspFileContent := "$in"
AssertStringEquals(t, "RspfileContent", wantRspFileContent, params.RuleParams.RspfileContent)
AssertStringEquals(t, "Rspfile", wantRspFile, params.RuleParams.Rspfile)
AssertPathRelativeToTopEquals(t, "Output", wantOutput, params.Output) AssertPathRelativeToTopEquals(t, "Output", wantOutput, params.Output)
if len(params.ImplicitOutputs) != 0 { if len(params.ImplicitOutputs) != 0 {
@@ -592,18 +635,42 @@ func TestRuleBuilder_Build(t *testing.T) {
if params.Deps != blueprint.DepsGCC { if params.Deps != blueprint.DepsGCC {
t.Errorf("want Deps = %q, got %q", blueprint.DepsGCC, params.Deps) t.Errorf("want Deps = %q, got %q", blueprint.DepsGCC, params.Deps)
} }
rspFile2Content := ContentFromFileRuleForTests(t, rspFile2Params)
AssertStringEquals(t, "rspFile2 content", "rsp_in2\n", rspFile2Content)
} }
t.Run("module", func(t *testing.T) { t.Run("module", func(t *testing.T) {
outFile := "out/soong/.intermediates/foo/gen/foo" outFile := "out/soong/.intermediates/foo/gen/foo"
check(t, result.ModuleForTests("foo", "").Rule("rule").RelativeToTop(), rspFile := "out/soong/.intermediates/foo/rsp"
"cp bar "+outFile, rspFile2 := "out/soong/.intermediates/foo/rsp2"
outFile, outFile+".d", true, nil, nil) module := result.ModuleForTests("foo", "")
check(t, module.Rule("rule").RelativeToTop(), module.Output(rspFile2).RelativeToTop(),
"cp bar "+outFile+" @"+rspFile+" @"+rspFile2,
outFile, outFile+".d", rspFile, rspFile2, true, nil, nil)
}) })
t.Run("sbox", func(t *testing.T) { t.Run("sbox", func(t *testing.T) {
outDir := "out/soong/.intermediates/foo_sbox" outDir := "out/soong/.intermediates/foo_sbox"
outFile := filepath.Join(outDir, "gen/foo_sbox") outFile := filepath.Join(outDir, "gen/foo_sbox")
depFile := filepath.Join(outDir, "gen/foo_sbox.d") depFile := filepath.Join(outDir, "gen/foo_sbox.d")
rspFile := filepath.Join(outDir, "rsp")
rspFile2 := filepath.Join(outDir, "rsp2")
manifest := filepath.Join(outDir, "sbox.textproto")
sbox := filepath.Join("out", "soong", "host", result.Config.PrebuiltOS(), "bin/sbox")
sandboxPath := shared.TempDirForOutDir("out/soong")
cmd := `rm -rf ` + outDir + `/gen && ` +
sbox + ` --sandbox-path ` + sandboxPath + ` --manifest ` + manifest
module := result.ModuleForTests("foo_sbox", "")
check(t, module.Output("gen/foo_sbox").RelativeToTop(), module.Output(rspFile2).RelativeToTop(),
cmd, outFile, depFile, rspFile, rspFile2, false, []string{manifest}, []string{sbox})
})
t.Run("sbox_inputs", func(t *testing.T) {
outDir := "out/soong/.intermediates/foo_sbox_inputs"
outFile := filepath.Join(outDir, "gen/foo_sbox_inputs")
depFile := filepath.Join(outDir, "gen/foo_sbox_inputs.d")
rspFile := filepath.Join(outDir, "rsp")
rspFile2 := filepath.Join(outDir, "rsp2")
manifest := filepath.Join(outDir, "sbox.textproto") manifest := filepath.Join(outDir, "sbox.textproto")
sbox := filepath.Join("out", "soong", "host", result.Config.PrebuiltOS(), "bin/sbox") sbox := filepath.Join("out", "soong", "host", result.Config.PrebuiltOS(), "bin/sbox")
sandboxPath := shared.TempDirForOutDir("out/soong") sandboxPath := shared.TempDirForOutDir("out/soong")
@@ -611,13 +678,18 @@ func TestRuleBuilder_Build(t *testing.T) {
cmd := `rm -rf ` + outDir + `/gen && ` + cmd := `rm -rf ` + outDir + `/gen && ` +
sbox + ` --sandbox-path ` + sandboxPath + ` --manifest ` + manifest sbox + ` --sandbox-path ` + sandboxPath + ` --manifest ` + manifest
check(t, result.ModuleForTests("foo_sbox", "").Output("gen/foo_sbox").RelativeToTop(), module := result.ModuleForTests("foo_sbox_inputs", "")
cmd, outFile, depFile, false, []string{manifest}, []string{sbox}) check(t, module.Output("gen/foo_sbox_inputs").RelativeToTop(), module.Output(rspFile2).RelativeToTop(),
cmd, outFile, depFile, rspFile, rspFile2, false, []string{manifest}, []string{sbox})
}) })
t.Run("singleton", func(t *testing.T) { t.Run("singleton", func(t *testing.T) {
outFile := filepath.Join("out/soong/singleton/gen/baz") outFile := filepath.Join("out/soong/singleton/gen/baz")
check(t, result.SingletonForTests("rule_builder_test").Rule("rule").RelativeToTop(), rspFile := filepath.Join("out/soong/singleton/rsp")
"cp bar "+outFile, outFile, outFile+".d", true, nil, nil) rspFile2 := filepath.Join("out/soong/singleton/rsp2")
singleton := result.SingletonForTests("rule_builder_test")
check(t, singleton.Rule("rule").RelativeToTop(), singleton.Output(rspFile2).RelativeToTop(),
"cp bar "+outFile+" @"+rspFile+" @"+rspFile2,
outFile, outFile+".d", rspFile, rspFile2, true, nil, nil)
}) })
} }

View File

@@ -628,7 +628,7 @@ prebuilt_stubs_sources {
} }
t.Run("empty/missing directory", func(t *testing.T) { t.Run("empty/missing directory", func(t *testing.T) {
test(t, "empty-directory", []string{}) test(t, "empty-directory", nil)
}) })
t.Run("non-empty set of sources", func(t *testing.T) { t.Run("non-empty set of sources", func(t *testing.T) {