From 1d2cf0494af3027f57d8d0d7495dc1a46d20886e Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 29 Mar 2019 15:33:06 -0700 Subject: [PATCH] Add depfile support to RuleBuilder Allow rules built with RuleBuilder to use depfiles. Ninja only supports a single depfile with single output. If there are multiple outputs in a rule, move all but the first to implicit outputs. If multiple depfiles are specified, use new support in dep_fixer to combine additional depfiles into the first depfile. Test: rule_builder_test.go Change-Id: I7dd4ba7fdf9feaf89b3dd2b7abb0e79006e06018 --- android/rule_builder.go | 117 +++++++++++++++++++++++++++++--- android/rule_builder_test.go | 55 +++++++++------ android/testing.go | 2 + cmd/dep_fixer/main.go | 38 +++++++---- java/dexpreopt_bootjars_test.go | 2 +- 5 files changed, 171 insertions(+), 43 deletions(-) diff --git a/android/rule_builder.go b/android/rule_builder.go index ee61b9499..2d0fac168 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -171,6 +171,20 @@ func (r *RuleBuilder) Outputs() WritablePaths { return outputList } +// DepFiles returns the list of paths that were passed to the RuleBuilderCommand methods that take depfile paths, such +// as RuleBuilderCommand.DepFile or RuleBuilderCommand.FlagWithDepFile. +func (r *RuleBuilder) DepFiles() WritablePaths { + var depFiles WritablePaths + + for _, c := range r.commands { + for _, depFile := range c.depFiles { + depFiles = append(depFiles, depFile) + } + } + + return depFiles +} + // Installs returns the list of tuples passed to Install. func (r *RuleBuilder) Installs() RuleBuilderInstalls { return append(RuleBuilderInstalls(nil), r.installs...) @@ -222,9 +236,17 @@ type BuilderContext interface { var _ BuilderContext = ModuleContext(nil) var _ BuilderContext = SingletonContext(nil) +func (r *RuleBuilder) depFileMergerCmd(ctx PathContext, depFiles WritablePaths) *RuleBuilderCommand { + return (&RuleBuilderCommand{}). + Tool(ctx.Config().HostToolPath(ctx, "dep_fixer")). + Flags(depFiles.Strings()) +} + // 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(pctx PackageContext, ctx BuilderContext, name string, desc string) { + name = ninjaNameEscape(name) + if len(r.missingDeps) > 0 { ctx.Build(pctx, BuildParams{ Rule: ErrorRule, @@ -237,16 +259,45 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string return } - if len(r.Commands()) > 0 { + tools := r.Tools() + commands := r.Commands() + + var depFile WritablePath + var depFormat blueprint.Deps + if depFiles := r.DepFiles(); len(depFiles) > 0 { + depFile = depFiles[0] + depFormat = blueprint.DepsGCC + if len(depFiles) > 1 { + // Add a command locally that merges all depfiles together into the first depfile. + cmd := r.depFileMergerCmd(ctx, depFiles) + commands = append(commands, string(cmd.buf)) + tools = append(tools, cmd.tools...) + } + } + + // Ninja doesn't like multiple outputs when depfiles are enabled, move all but the first output to + // ImplicitOutputs. RuleBuilder never uses "$out", so the distinction between Outputs and ImplicitOutputs + // doesn't matter. + var output WritablePath + var implicitOutputs WritablePaths + if outputs := r.Outputs(); len(outputs) > 0 { + output = outputs[0] + implicitOutputs = outputs[1:] + } + + if len(commands) > 0 { ctx.Build(pctx, BuildParams{ Rule: ctx.Rule(pctx, name, blueprint.RuleParams{ - Command: strings.Join(proptools.NinjaEscapeList(r.Commands()), " && "), - CommandDeps: r.Tools().Strings(), + Command: strings.Join(proptools.NinjaEscapeList(commands), " && "), + CommandDeps: tools.Strings(), Restat: r.restat, }), - Implicits: r.Inputs(), - Outputs: r.Outputs(), - Description: desc, + Implicits: r.Inputs(), + Output: output, + ImplicitOutputs: implicitOutputs, + Depfile: depFile, + Deps: depFormat, + Description: desc, }) } } @@ -256,10 +307,11 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string // RuleBuilderCommand, so they can be used chained or unchained. All methods that add text implicitly add a single // space as a separator from the previous method. type RuleBuilderCommand struct { - buf []byte - inputs Paths - outputs WritablePaths - tools Paths + buf []byte + inputs Paths + outputs WritablePaths + depFiles WritablePaths + tools Paths } // Text adds the specified raw text to the command line. The text should not contain input or output paths or the @@ -369,6 +421,14 @@ func (c *RuleBuilderCommand) Outputs(paths WritablePaths) *RuleBuilderCommand { return c } +// DepFile adds the specified depfile path to the paths returned by RuleBuilder.DepFiles and adds it to the command +// line, and causes RuleBuilder.Build file to set the depfile flag for ninja. If multiple depfiles are added to +// commands in a single RuleBuilder then RuleBuilder.Build will add an extra command to merge the depfiles together. +func (c *RuleBuilderCommand) DepFile(path WritablePath) *RuleBuilderCommand { + c.depFiles = append(c.depFiles, path) + return c.Text(path.String()) +} + // ImplicitOutput adds the specified output path to the dependencies returned by RuleBuilder.Outputs without modifying // the command line. func (c *RuleBuilderCommand) ImplicitOutput(path WritablePath) *RuleBuilderCommand { @@ -383,6 +443,15 @@ func (c *RuleBuilderCommand) ImplicitOutputs(paths WritablePaths) *RuleBuilderCo return c } +// ImplicitDepFile adds the specified depfile path to the paths returned by RuleBuilder.DepFiles without modifying +// the command line, and causes RuleBuilder.Build file to set the depfile flag for ninja. If multiple depfiles +// are added to commands in a single RuleBuilder then RuleBuilder.Build will add an extra command to merge the +// depfiles together. +func (c *RuleBuilderCommand) ImplicitDepFile(path WritablePath) *RuleBuilderCommand { + c.depFiles = append(c.depFiles, path) + return c +} + // FlagWithInput adds the specified flag and input path to the command line, with no separator between them. The path // will also be added to the dependencies returned by RuleBuilder.Inputs. func (c *RuleBuilderCommand) FlagWithInput(flag string, path Path) *RuleBuilderCommand { @@ -415,7 +484,35 @@ func (c *RuleBuilderCommand) FlagWithOutput(flag string, path WritablePath) *Rul return c.Text(flag + path.String()) } +// FlagWithDepFile adds the specified flag and depfile path to the command line, with no separator between them. The path +// will also be added to the outputs returned by RuleBuilder.Outputs. +func (c *RuleBuilderCommand) FlagWithDepFile(flag string, path WritablePath) *RuleBuilderCommand { + c.depFiles = append(c.depFiles, path) + return c.Text(flag + path.String()) +} + // String returns the command line. func (c *RuleBuilderCommand) String() string { return string(c.buf) } + +func ninjaNameEscape(s string) string { + b := []byte(s) + escaped := false + for i, c := range b { + valid := (c >= 'a' && c <= 'z') || + (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || + (c == '_') || + (c == '-') || + (c == '.') + if !valid { + b[i] = '_' + escaped = true + } + } + if escaped { + s = string(b) + } + return s +} diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index 101276808..7bad02586 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -237,23 +237,27 @@ func TestRuleBuilder(t *testing.T) { rule := NewRuleBuilder() fs := map[string][]byte{ - "input": nil, - "Implicit": nil, - "Input": nil, - "Tool": nil, - "input2": nil, - "tool2": nil, - "input3": nil, + "dep_fixer": nil, + "input": nil, + "Implicit": nil, + "Input": nil, + "Tool": nil, + "input2": nil, + "tool2": nil, + "input3": nil, } ctx := PathContextForTesting(TestConfig("out", nil), fs) cmd := rule.Command(). + DepFile(PathForOutput(ctx, "DepFile")). Flag("Flag"). FlagWithArg("FlagWithArg=", "arg"). + FlagWithDepFile("FlagWithDepFile=", PathForOutput(ctx, "depfile")). FlagWithInput("FlagWithInput=", PathForSource(ctx, "input")). FlagWithOutput("FlagWithOutput=", PathForOutput(ctx, "output")). Implicit(PathForSource(ctx, "Implicit")). + ImplicitDepFile(PathForOutput(ctx, "ImplicitDepFile")). ImplicitOutput(PathForOutput(ctx, "ImplicitOutput")). Input(PathForSource(ctx, "Input")). Output(PathForOutput(ctx, "Output")). @@ -262,6 +266,7 @@ func TestRuleBuilder(t *testing.T) { rule.Command(). Text("command2"). + DepFile(PathForOutput(ctx, "depfile2")). Input(PathForSource(ctx, "input2")). Output(PathForOutput(ctx, "output2")). Tool(PathForSource(ctx, "tool2")) @@ -279,25 +284,37 @@ func TestRuleBuilder(t *testing.T) { Output(PathForOutput(ctx, "output3")) wantCommands := []string{ - "Flag FlagWithArg=arg FlagWithInput=input FlagWithOutput=out/output Input out/Output Text Tool after command2 old cmd", - "command2 input2 out/output2 tool2", + "out/DepFile Flag FlagWithArg=arg FlagWithDepFile=out/depfile FlagWithInput=input FlagWithOutput=out/output Input out/Output Text Tool after command2 old cmd", + "command2 out/depfile2 input2 out/output2 tool2", "command3 input3 out/output2 out/output3", } + + wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer out/DepFile out/depfile out/ImplicitDepFile out/depfile2" + wantInputs := PathsForSource(ctx, []string{"Implicit", "Input", "input", "input2", "input3"}) wantOutputs := PathsForOutput(ctx, []string{"ImplicitOutput", "Output", "output", "output2", "output3"}) + wantDepFiles := PathsForOutput(ctx, []string{"DepFile", "depfile", "ImplicitDepFile", "depfile2"}) wantTools := PathsForSource(ctx, []string{"Tool", "tool2"}) - if !reflect.DeepEqual(rule.Commands(), wantCommands) { - t.Errorf("\nwant rule.Commands() = %#v\n got %#v", wantCommands, rule.Commands()) + if g, w := rule.Commands(), wantCommands; !reflect.DeepEqual(g, w) { + t.Errorf("\nwant rule.Commands() = %#v\n got %#v", w, g) } - if !reflect.DeepEqual(rule.Inputs(), wantInputs) { - t.Errorf("\nwant rule.Inputs() = %#v\n got %#v", wantInputs, rule.Inputs()) + + if g, w := rule.depFileMergerCmd(ctx, rule.DepFiles()).String(), wantDepMergerCommand; g != w { + t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) } - if !reflect.DeepEqual(rule.Outputs(), wantOutputs) { - t.Errorf("\nwant rule.Outputs() = %#v\n got %#v", wantOutputs, rule.Outputs()) + + if g, w := rule.Inputs(), wantInputs; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Inputs() = %#v\n got %#v", w, g) } - if !reflect.DeepEqual(rule.Tools(), wantTools) { - t.Errorf("\nwant rule.Tools() = %#v\n got %#v", wantTools, rule.Tools()) + if g, w := rule.Outputs(), wantOutputs; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Outputs() = %#v\n got %#v", w, g) + } + if g, w := rule.DepFiles(), wantDepFiles; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.DepFiles() = %#v\n got %#v", w, g) + } + if g, w := rule.Tools(), wantTools; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Tools() = %#v\n got %#v", w, g) } } @@ -383,8 +400,8 @@ func TestRuleBuilder_Build(t *testing.T) { t.Errorf("want Implicits = [%q], got %q", "bar", params.Implicits.Strings()) } - if len(params.Outputs) != 1 || params.Outputs[0].String() != wantOutput { - t.Errorf("want Outputs = [%q], got %q", wantOutput, params.Outputs.Strings()) + if params.Output.String() != wantOutput { + t.Errorf("want Output = %q, got %q", wantOutput, params.Output) } if !params.RuleParams.Restat { diff --git a/android/testing.go b/android/testing.go index 7f443a395..0ec5af58a 100644 --- a/android/testing.go +++ b/android/testing.go @@ -196,6 +196,7 @@ func maybeBuildParamsFromOutput(provider testBuildProvider, file string) (Testin var searchedOutputs []string for _, p := range provider.BuildParamsForTests() { outputs := append(WritablePaths(nil), p.Outputs...) + outputs = append(outputs, p.ImplicitOutputs...) if p.Output != nil { outputs = append(outputs, p.Output) } @@ -222,6 +223,7 @@ func allOutputs(provider testBuildProvider) []string { var outputFullPaths []string for _, p := range provider.BuildParamsForTests() { outputs := append(WritablePaths(nil), p.Outputs...) + outputs = append(outputs, p.ImplicitOutputs...) if p.Output != nil { outputs = append(outputs, p.Output) } diff --git a/cmd/dep_fixer/main.go b/cmd/dep_fixer/main.go index 0647fb293..f94cf2fd4 100644 --- a/cmd/dep_fixer/main.go +++ b/cmd/dep_fixer/main.go @@ -29,30 +29,42 @@ import ( func main() { flag.Usage = func() { - fmt.Fprintf(os.Stderr, "Usage: %s ", os.Args[0]) + fmt.Fprintf(os.Stderr, "Usage: %s [-o ] [...]", os.Args[0]) flag.PrintDefaults() } output := flag.String("o", "", "Optional output file (defaults to rewriting source if necessary)") flag.Parse() - if flag.NArg() != 1 { - log.Fatal("Expected a single file as an argument") + if flag.NArg() < 1 { + log.Fatal("Expected at least one input file as an argument") } - old, err := ioutil.ReadFile(flag.Arg(0)) - if err != nil { - log.Fatalf("Error opening %q: %v", flag.Arg(0), err) + var mergedDeps *Deps + var firstInput []byte + + for i, arg := range flag.Args() { + input, err := ioutil.ReadFile(arg) + if err != nil { + log.Fatalf("Error opening %q: %v", arg, err) + } + + deps, err := Parse(arg, bytes.NewBuffer(append([]byte(nil), input...))) + if err != nil { + log.Fatalf("Failed to parse: %v", err) + } + + if i == 0 { + mergedDeps = deps + firstInput = input + } else { + mergedDeps.Inputs = append(mergedDeps.Inputs, deps.Inputs...) + } } - deps, err := Parse(flag.Arg(0), bytes.NewBuffer(append([]byte(nil), old...))) - if err != nil { - log.Fatalf("Failed to parse: %v", err) - } - - new := deps.Print() + new := mergedDeps.Print() if *output == "" || *output == flag.Arg(0) { - if !bytes.Equal(old, new) { + if !bytes.Equal(firstInput, new) { err := ioutil.WriteFile(flag.Arg(0), new, 0666) if err != nil { log.Fatalf("Failed to write: %v", err) diff --git a/java/dexpreopt_bootjars_test.go b/java/dexpreopt_bootjars_test.go index 141f7ba47..cbb52f15c 100644 --- a/java/dexpreopt_bootjars_test.go +++ b/java/dexpreopt_bootjars_test.go @@ -103,7 +103,7 @@ func TestDexpreoptBootJars(t *testing.T) { expectedOutputs[i] = filepath.Join(buildDir, "test_device", expectedOutputs[i]) } - outputs := bootArt.Outputs.Strings() + outputs := append(android.WritablePaths{bootArt.Output}, bootArt.ImplicitOutputs...).Strings() sort.Strings(outputs) sort.Strings(expectedOutputs)