From 0ce5d05f76989c4cc699e0dd6cd1f906a9e0f16d Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Fri, 12 Apr 2019 11:11:38 -0700 Subject: Support RuleBuilder.Sbox to wrap commands in sbox This essentially allows you to declare that everything in a directory will be created by the rule, and we'll ensure that your command actually writes out all of the claimed outputs, and remove any other files that previously existed in that directory. Bug: 144948629 Test: built-in tests Change-Id: I990dce2b3a0d89ebd2736ac1a0cadfb5864c6e73 Merged-In: I990dce2b3a0d89ebd2736ac1a0cadfb5864c6e73 (cherry picked from commit 633c50229544db07f48e75833cab655a294f677d) --- Android.bp | 1 + android/paths.go | 17 +++- android/rule_builder.go | 196 +++++++++++++++++++++++++++-------- android/rule_builder_test.go | 237 ++++++++++++++++++++++++++++++------------- cmd/sbox/sbox.go | 2 +- 5 files changed, 330 insertions(+), 123 deletions(-) diff --git a/Android.bp b/Android.bp index 1d65dff0..2692b1be 100644 --- a/Android.bp +++ b/Android.bp @@ -37,6 +37,7 @@ bootstrap_go_package { "blueprint-bootstrap", "soong", "soong-env", + "soong-shared", ], srcs: [ "android/androidmk.go", diff --git a/android/paths.go b/android/paths.go index 8cc7057a..0f20b844 100644 --- a/android/paths.go +++ b/android/paths.go @@ -1267,16 +1267,23 @@ func Rel(ctx PathContext, basePath string, targetPath string) string { // MaybeRel performs the same function as filepath.Rel, but reports errors to a PathContext, and returns false if // targetPath is not inside basePath. func MaybeRel(ctx PathContext, basePath string, targetPath string) (string, bool) { + rel, isRel, err := maybeRelErr(basePath, targetPath) + if err != nil { + reportPathError(ctx, err) + } + return rel, isRel +} + +func maybeRelErr(basePath string, targetPath string) (string, bool, error) { // filepath.Rel returns an error if one path is absolute and the other is not, handle that case first. if filepath.IsAbs(basePath) != filepath.IsAbs(targetPath) { - return "", false + return "", false, nil } rel, err := filepath.Rel(basePath, targetPath) if err != nil { - reportPathError(ctx, err) - return "", false + return "", false, err } else if rel == ".." || strings.HasPrefix(rel, "../") || strings.HasPrefix(rel, "/") { - return "", false + return "", false, nil } - return rel, true + return rel, true, nil } diff --git a/android/rule_builder.go b/android/rule_builder.go index 2d0fac16..4a3b0223 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -21,6 +21,8 @@ import ( "github.com/google/blueprint" "github.com/google/blueprint/proptools" + + "android/soong/shared" ) // RuleBuilder provides an alternative to ModuleContext.Rule and ModuleContext.Build to add a command line to the build @@ -30,6 +32,8 @@ type RuleBuilder struct { installs RuleBuilderInstalls temporariesSet map[WritablePath]bool restat bool + sbox bool + sboxOutDir WritablePath missingDeps []string } @@ -73,11 +77,36 @@ func (r *RuleBuilder) MissingDeps(missingDeps []string) { } // Restat marks the rule as a restat rule, which will be passed to ModuleContext.Rule in BuildParams.Restat. +// +// Restat is not compatible with Sbox() func (r *RuleBuilder) Restat() *RuleBuilder { + if r.sbox { + panic("Restat() is not compatible with Sbox()") + } r.restat = true return r } +// Sbox marks the rule as needing to be wrapped by sbox. The WritablePath should point to the output +// directory that sbox will wipe. It should not be written to by any other rule. sbox will ensure +// that all outputs have been written, and will discard any output files that were not specified. +// +// Sbox is not compatible with Restat() +func (r *RuleBuilder) Sbox(outputDir WritablePath) *RuleBuilder { + if r.sbox { + panic("Sbox() may not be called more than once") + } + if len(r.commands) > 0 { + panic("Sbox() may not be called after Command()") + } + if r.restat { + panic("Sbox() is not compatible with Restat()") + } + r.sbox = true + r.sboxOutDir = outputDir + return r +} + // Install associates an output of the rule with an install location, which can be retrieved later using // RuleBuilder.Installs. func (r *RuleBuilder) Install(from Path, to string) { @@ -88,7 +117,10 @@ func (r *RuleBuilder) Install(from Path, to string) { // created by this method. That can be mutated through their methods in any order, as long as the mutations do not // race with any call to Build. func (r *RuleBuilder) Command() *RuleBuilderCommand { - command := &RuleBuilderCommand{} + command := &RuleBuilderCommand{ + sbox: r.sbox, + sboxOutDir: r.sboxOutDir, + } r.commands = append(r.commands, command) return command } @@ -120,12 +152,16 @@ func (r *RuleBuilder) DeleteTemporaryFiles() { // that are also outputs of another command in the same RuleBuilder are filtered out. func (r *RuleBuilder) Inputs() Paths { outputs := r.outputSet() + depFiles := r.depFileSet() inputs := make(map[string]Path) for _, c := range r.commands { for _, input := range c.inputs { - if _, isOutput := outputs[input.String()]; !isOutput { - inputs[input.String()] = input + inputStr := input.String() + if _, isOutput := outputs[inputStr]; !isOutput { + if _, isDepFile := depFiles[inputStr]; !isDepFile { + inputs[input.String()] = input + } } } } @@ -171,6 +207,16 @@ func (r *RuleBuilder) Outputs() WritablePaths { return outputList } +func (r *RuleBuilder) depFileSet() map[string]WritablePath { + depFiles := make(map[string]WritablePath) + for _, c := range r.commands { + for _, depFile := range c.depFiles { + depFiles[depFile.String()] = depFile + } + } + return depFiles +} + // 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 { @@ -237,9 +283,9 @@ var _ BuilderContext = ModuleContext(nil) var _ BuilderContext = SingletonContext(nil) func (r *RuleBuilder) depFileMergerCmd(ctx PathContext, depFiles WritablePaths) *RuleBuilderCommand { - return (&RuleBuilderCommand{}). + return r.Command(). Tool(ctx.Config().HostToolPath(ctx, "dep_fixer")). - Flags(depFiles.Strings()) + Inputs(depFiles.Paths()) } // Build adds the built command line to the build graph, with dependencies on Inputs and Tools, and output files for @@ -259,9 +305,6 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string return } - tools := r.Tools() - commands := r.Commands() - var depFile WritablePath var depFormat blueprint.Deps if depFiles := r.DepFiles(); len(depFiles) > 0 { @@ -269,37 +312,75 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string 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...) + r.depFileMergerCmd(ctx, depFiles) + + if r.sbox { + // Check for Rel() errors, as all depfiles should be in the output dir + for _, path := range depFiles[1:] { + Rel(ctx, r.sboxOutDir.String(), path.String()) + } + } } } - // 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:] + tools := r.Tools() + commands := r.Commands() + outputs := r.Outputs() + + if len(commands) == 0 { + return + } + if len(outputs) == 0 { + panic("No outputs specified from any Commands") } - if len(commands) > 0 { - ctx.Build(pctx, BuildParams{ - Rule: ctx.Rule(pctx, name, blueprint.RuleParams{ - Command: strings.Join(proptools.NinjaEscapeList(commands), " && "), - CommandDeps: tools.Strings(), - Restat: r.restat, - }), - Implicits: r.Inputs(), - Output: output, - ImplicitOutputs: implicitOutputs, - Depfile: depFile, - Deps: depFormat, - Description: desc, - }) + commandString := strings.Join(proptools.NinjaEscapeList(commands), " && ") + + if r.sbox { + sboxOutputs := make([]string, len(outputs)) + for i, output := range outputs { + sboxOutputs[i] = "__SBOX_OUT_DIR__/" + Rel(ctx, r.sboxOutDir.String(), output.String()) + } + + if depFile != nil { + sboxOutputs = append(sboxOutputs, "__SBOX_OUT_DIR__/"+Rel(ctx, r.sboxOutDir.String(), depFile.String())) + } + + commandString = proptools.ShellEscape(commandString) + if !strings.HasPrefix(commandString, `'`) { + commandString = `'` + commandString + `'` + } + + sboxCmd := &RuleBuilderCommand{} + sboxCmd.Tool(ctx.Config().HostToolPath(ctx, "sbox")). + Flag("-c").Text(commandString). + Flag("--sandbox-path").Text(shared.TempDirForOutDir(PathForOutput(ctx).String())). + Flag("--output-root").Text(r.sboxOutDir.String()). + Flags(sboxOutputs) + + commandString = string(sboxCmd.buf) + tools = append(tools, sboxCmd.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. + output := outputs[0] + implicitOutputs := outputs[1:] + + ctx.Build(pctx, BuildParams{ + Rule: ctx.Rule(pctx, name, blueprint.RuleParams{ + Command: commandString, + CommandDeps: tools.Strings(), + Restat: r.restat, + }), + Implicits: r.Inputs(), + Output: output, + ImplicitOutputs: implicitOutputs, + Depfile: depFile, + Deps: depFormat, + Description: desc, + }) } // RuleBuilderCommand is a builder for a command in a command line. It can be mutated by its methods to add to the @@ -312,6 +393,28 @@ type RuleBuilderCommand struct { outputs WritablePaths depFiles WritablePaths tools Paths + + sbox bool + sboxOutDir WritablePath +} + +func (c *RuleBuilderCommand) addInput(path Path) string { + if c.sbox { + if rel, isRel, _ := maybeRelErr(c.sboxOutDir.String(), path.String()); isRel { + return "__SBOX_OUT_DIR__/" + rel + } + } + c.inputs = append(c.inputs, path) + return path.String() +} + +func (c *RuleBuilderCommand) outputStr(path Path) string { + if c.sbox { + // Errors will be handled in RuleBuilder.Build where we have a context to report them + rel, _, _ := maybeRelErr(c.sboxOutDir.String(), path.String()) + return "__SBOX_OUT_DIR__/" + rel + } + return path.String() } // Text adds the specified raw text to the command line. The text should not contain input or output paths or the @@ -378,8 +481,7 @@ func (c *RuleBuilderCommand) Tool(path Path) *RuleBuilderCommand { // Input adds the specified input path to the command line. The path will also be added to the dependencies returned by // RuleBuilder.Inputs. func (c *RuleBuilderCommand) Input(path Path) *RuleBuilderCommand { - c.inputs = append(c.inputs, path) - return c.Text(path.String()) + return c.Text(c.addInput(path)) } // Inputs adds the specified input paths to the command line, separated by spaces. The paths will also be added to the @@ -394,14 +496,16 @@ func (c *RuleBuilderCommand) Inputs(paths Paths) *RuleBuilderCommand { // Implicit adds the specified input path to the dependencies returned by RuleBuilder.Inputs without modifying the // command line. func (c *RuleBuilderCommand) Implicit(path Path) *RuleBuilderCommand { - c.inputs = append(c.inputs, path) + c.addInput(path) return c } // Implicits adds the specified input paths to the dependencies returned by RuleBuilder.Inputs without modifying the // command line. func (c *RuleBuilderCommand) Implicits(paths Paths) *RuleBuilderCommand { - c.inputs = append(c.inputs, paths...) + for _, path := range paths { + c.addInput(path) + } return c } @@ -409,7 +513,7 @@ func (c *RuleBuilderCommand) Implicits(paths Paths) *RuleBuilderCommand { // RuleBuilder.Outputs. func (c *RuleBuilderCommand) Output(path WritablePath) *RuleBuilderCommand { c.outputs = append(c.outputs, path) - return c.Text(path.String()) + return c.Text(c.outputStr(path)) } // Outputs adds the specified output paths to the command line, separated by spaces. The paths will also be added to @@ -426,7 +530,7 @@ func (c *RuleBuilderCommand) Outputs(paths WritablePaths) *RuleBuilderCommand { // 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()) + return c.Text(c.outputStr(path)) } // ImplicitOutput adds the specified output path to the dependencies returned by RuleBuilder.Outputs without modifying @@ -455,16 +559,18 @@ func (c *RuleBuilderCommand) ImplicitDepFile(path WritablePath) *RuleBuilderComm // 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 { - c.inputs = append(c.inputs, path) - return c.Text(flag + path.String()) + return c.Text(flag + c.addInput(path)) } // FlagWithInputList adds the specified flag and input paths to the command line, with the inputs joined by sep // and no separator between the flag and inputs. The input paths will also be added to the dependencies returned by // RuleBuilder.Inputs. func (c *RuleBuilderCommand) FlagWithInputList(flag string, paths Paths, sep string) *RuleBuilderCommand { - c.inputs = append(c.inputs, paths...) - return c.FlagWithList(flag, paths.Strings(), sep) + strs := make([]string, len(paths)) + for i, path := range paths { + strs[i] = c.addInput(path) + } + return c.FlagWithList(flag, strs, sep) } // FlagForEachInput adds the specified flag joined with each input path to the command line. The input paths will also @@ -481,14 +587,14 @@ func (c *RuleBuilderCommand) FlagForEachInput(flag string, paths Paths) *RuleBui // will also be added to the outputs returned by RuleBuilder.Outputs. func (c *RuleBuilderCommand) FlagWithOutput(flag string, path WritablePath) *RuleBuilderCommand { c.outputs = append(c.outputs, path) - return c.Text(flag + path.String()) + return c.Text(flag + c.outputStr(path)) } // 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()) + return c.Text(flag + c.outputStr(path)) } // String returns the command line. diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index 7bad0258..df0f2564 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -22,6 +22,10 @@ import ( "reflect" "strings" "testing" + + "github.com/google/blueprint" + + "android/soong/shared" ) func pathContext() PathContext { @@ -234,8 +238,6 @@ func ExampleRuleBuilderCommand_FlagWithList() { } func TestRuleBuilder(t *testing.T) { - rule := NewRuleBuilder() - fs := map[string][]byte{ "dep_fixer": nil, "input": nil, @@ -249,73 +251,114 @@ func TestRuleBuilder(t *testing.T) { 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")). - Text("Text"). - Tool(PathForSource(ctx, "Tool")) - - rule.Command(). - Text("command2"). - DepFile(PathForOutput(ctx, "depfile2")). - Input(PathForSource(ctx, "input2")). - Output(PathForOutput(ctx, "output2")). - Tool(PathForSource(ctx, "tool2")) - - // Test updates to the first command after the second command has been started - cmd.Text("after command2") - // Test updating a command when the previous update did not replace the cmd variable - cmd.Text("old cmd") - - // Test a command that uses the output of a previous command as an input - rule.Command(). - Text("command3"). - Input(PathForSource(ctx, "input3")). - Input(PathForOutput(ctx, "output2")). - Output(PathForOutput(ctx, "output3")) - - wantCommands := []string{ - "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", + addCommands := func(rule *RuleBuilder) { + 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")). + Text("Text"). + Tool(PathForSource(ctx, "Tool")) + + rule.Command(). + Text("command2"). + DepFile(PathForOutput(ctx, "depfile2")). + Input(PathForSource(ctx, "input2")). + Output(PathForOutput(ctx, "output2")). + Tool(PathForSource(ctx, "tool2")) + + // Test updates to the first command after the second command has been started + cmd.Text("after command2") + // Test updating a command when the previous update did not replace the cmd variable + cmd.Text("old cmd") + + // Test a command that uses the output of a previous command as an input + rule.Command(). + Text("command3"). + Input(PathForSource(ctx, "input3")). + Input(PathForOutput(ctx, "output2")). + Output(PathForOutput(ctx, "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 g, w := rule.Commands(), wantCommands; !reflect.DeepEqual(g, w) { - t.Errorf("\nwant rule.Commands() = %#v\n got %#v", w, g) - } + t.Run("normal", func(t *testing.T) { + rule := NewRuleBuilder() + addCommands(rule) - if g, w := rule.depFileMergerCmd(ctx, rule.DepFiles()).String(), wantDepMergerCommand; g != w { - t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) - } + wantCommands := []string{ + "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", + } - if g, w := rule.Inputs(), wantInputs; !reflect.DeepEqual(w, g) { - t.Errorf("\nwant rule.Inputs() = %#v\n got %#v", w, g) - } - 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) - } + wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer out/DepFile out/depfile out/ImplicitDepFile out/depfile2" + + if g, w := rule.Commands(), wantCommands; !reflect.DeepEqual(g, w) { + t.Errorf("\nwant rule.Commands() = %#v\n got %#v", w, g) + } + + if g, w := rule.Inputs(), wantInputs; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Inputs() = %#v\n got %#v", w, g) + } + 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) + } + + if g, w := rule.depFileMergerCmd(ctx, rule.DepFiles()).String(), wantDepMergerCommand; g != w { + t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) + } + }) + + t.Run("sbox", func(t *testing.T) { + rule := NewRuleBuilder().Sbox(PathForOutput(ctx)) + addCommands(rule) + + wantCommands := []string{ + "__SBOX_OUT_DIR__/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_OUT_DIR__/depfile FlagWithInput=input FlagWithOutput=__SBOX_OUT_DIR__/output Input __SBOX_OUT_DIR__/Output Text Tool after command2 old cmd", + "command2 __SBOX_OUT_DIR__/depfile2 input2 __SBOX_OUT_DIR__/output2 tool2", + "command3 input3 __SBOX_OUT_DIR__/output2 __SBOX_OUT_DIR__/output3", + } + + wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer __SBOX_OUT_DIR__/DepFile __SBOX_OUT_DIR__/depfile __SBOX_OUT_DIR__/ImplicitDepFile __SBOX_OUT_DIR__/depfile2" + + if g, w := rule.Commands(), wantCommands; !reflect.DeepEqual(g, w) { + t.Errorf("\nwant rule.Commands() = %#v\n got %#v", w, g) + } + + if g, w := rule.Inputs(), wantInputs; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.Inputs() = %#v\n got %#v", w, g) + } + 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) + } + + if g, w := rule.depFileMergerCmd(ctx, rule.DepFiles()).String(), wantDepMergerCommand; g != w { + t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) + } + }) } func testRuleBuilderFactory() Module { @@ -329,14 +372,19 @@ type testRuleBuilderModule struct { ModuleBase properties struct { Src string + + Restat bool + Sbox bool } } func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) { in := PathForSource(ctx, t.properties.Src) out := PathForModuleOut(ctx, ctx.ModuleName()) + outDep := PathForModuleOut(ctx, ctx.ModuleName()+".d") + outDir := PathForModuleOut(ctx) - testRuleBuilder_Build(ctx, in, out) + testRuleBuilder_Build(ctx, in, out, outDep, outDir, t.properties.Restat, t.properties.Sbox) } type testRuleBuilderSingleton struct{} @@ -348,15 +396,23 @@ func testRuleBuilderSingletonFactory() Singleton { func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) { in := PathForSource(ctx, "bar") out := PathForOutput(ctx, "baz") - testRuleBuilder_Build(ctx, in, out) + outDep := PathForOutput(ctx, "baz.d") + outDir := PathForOutput(ctx) + testRuleBuilder_Build(ctx, in, out, outDep, outDir, true, false) } -func testRuleBuilder_Build(ctx BuilderContext, in Path, out WritablePath) { +func testRuleBuilder_Build(ctx BuilderContext, in Path, out, outDep, outDir WritablePath, restat, sbox bool) { rule := NewRuleBuilder() - rule.Command().Tool(PathForSource(ctx, "cp")).Input(in).Output(out) + if sbox { + rule.Sbox(outDir) + } + + rule.Command().Tool(PathForSource(ctx, "cp")).Input(in).Output(out).ImplicitDepFile(outDep) - rule.Restat() + if restat { + rule.Restat() + } rule.Build(pctx, ctx, "rule", "desc") } @@ -372,6 +428,12 @@ func TestRuleBuilder_Build(t *testing.T) { rule_builder_test { name: "foo", src: "bar", + restat: true, + } + rule_builder_test { + name: "foo_sbox", + src: "bar", + sbox: true, } ` @@ -391,9 +453,18 @@ func TestRuleBuilder_Build(t *testing.T) { _, errs = ctx.PrepareBuildActions(config) FailIfErrored(t, errs) - check := func(t *testing.T, params TestingBuildParams, wantOutput string) { - if len(params.RuleParams.CommandDeps) != 1 || params.RuleParams.CommandDeps[0] != "cp" { - t.Errorf("want RuleParams.CommandDeps = [%q], got %q", "cp", params.RuleParams.CommandDeps) + check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraCmdDeps []string) { + if params.RuleParams.Command != wantCommand { + t.Errorf("\nwant RuleParams.Command = %q\n got %q", wantCommand, params.RuleParams.Command) + } + + wantDeps := append([]string{"cp"}, extraCmdDeps...) + if !reflect.DeepEqual(params.RuleParams.CommandDeps, wantDeps) { + t.Errorf("\nwant RuleParams.CommandDeps = %q\n got %q", wantDeps, params.RuleParams.CommandDeps) + } + + if params.RuleParams.Restat != wantRestat { + t.Errorf("want RuleParams.Restat = %v, got %v", wantRestat, params.RuleParams.Restat) } if len(params.Implicits) != 1 || params.Implicits[0].String() != "bar" { @@ -404,17 +475,39 @@ func TestRuleBuilder_Build(t *testing.T) { t.Errorf("want Output = %q, got %q", wantOutput, params.Output) } - if !params.RuleParams.Restat { - t.Errorf("want RuleParams.Restat = true, got %v", params.RuleParams.Restat) + if len(params.ImplicitOutputs) != 0 { + t.Errorf("want ImplicitOutputs = [], got %q", params.ImplicitOutputs.Strings()) + } + + if params.Depfile.String() != wantDepfile { + t.Errorf("want Depfile = %q, got %q", wantDepfile, params.Depfile) + } + + if params.Deps != blueprint.DepsGCC { + t.Errorf("want Deps = %q, got %q", blueprint.DepsGCC, params.Deps) } } t.Run("module", func(t *testing.T) { + outFile := filepath.Join(buildDir, ".intermediates", "foo", "foo") check(t, ctx.ModuleForTests("foo", "").Rule("rule"), - filepath.Join(buildDir, ".intermediates", "foo", "foo")) + "cp bar "+outFile, + outFile, outFile+".d", true, nil) + }) + t.Run("sbox", func(t *testing.T) { + outDir := filepath.Join(buildDir, ".intermediates", "foo_sbox") + outFile := filepath.Join(outDir, "foo_sbox") + sbox := filepath.Join(buildDir, "host", config.PrebuiltOS(), "bin/sbox") + sandboxPath := shared.TempDirForOutDir(buildDir) + + cmd := sbox + ` -c 'cp bar __SBOX_OUT_DIR__/foo_sbox' --sandbox-path ` + sandboxPath + " --output-root " + outDir + " __SBOX_OUT_DIR__/foo_sbox __SBOX_OUT_DIR__/foo_sbox.d" + + check(t, ctx.ModuleForTests("foo_sbox", "").Rule("rule"), + cmd, outFile, outFile+".d", false, []string{sbox}) }) t.Run("singleton", func(t *testing.T) { + outFile := filepath.Join(buildDir, "baz") check(t, ctx.SingletonForTests("rule_builder_test").Rule("rule"), - filepath.Join(buildDir, "baz")) + "cp bar "+outFile, outFile, outFile+".d", true, nil) }) } diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go index 4167edb3..4ac92953 100644 --- a/cmd/sbox/sbox.go +++ b/cmd/sbox/sbox.go @@ -56,7 +56,7 @@ func usageViolation(violation string) { } fmt.Fprintf(os.Stderr, - "Usage: sbox -c --sandbox-path --output-root --overwrite [--depfile-out depFile] [...]\n"+ + "Usage: sbox -c --sandbox-path --output-root [--depfile-out depFile] [...]\n"+ "\n"+ "Deletes ,"+ "runs ,"+ -- cgit v1.2.3 From db14db3a06d2bfcbc0ab775cdf56630f56a5c957 Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Thu, 29 Aug 2019 14:47:40 -0700 Subject: Rewrite depfile from sbox to stay reproducible sbox will generate a random directory for the output root, and most tools will encode that directory name in the output target of the depfile. So embed the library from dep_fixer into sbox so that it can rewrite the output filename to a static (reproducible) value. Ninja doesn't care what that value is, so it's just "outputfile". Also fix up rule_builder to actually tell sbox about the depfile. Bug: 144948629 Test: mmma system/iorap; check the contents of: out/soong/.intermediates/system/iorap/libiorap-binder/android_arm_armv7-a-neon_core_static/gen/aidl/system/iorap/binder/com/google/android/startop/iorap/IIorap.cpp.d Change-Id: I3640a2e8b0c034f143a35e398a8418a6d621b265 Merged-In: I3640a2e8b0c034f143a35e398a8418a6d621b265 (cherry picked from commit c89b6f19810d368d7d5c128407c3eaaa5e3b2e81) --- android/rule_builder.go | 13 +- android/rule_builder_test.go | 6 +- cmd/dep_fixer/Android.bp | 8 +- cmd/dep_fixer/deps.go | 95 ----------- cmd/dep_fixer/deps_test.go | 389 ------------------------------------------- cmd/dep_fixer/main.go | 6 +- cmd/sbox/Android.bp | 1 + cmd/sbox/sbox.go | 26 ++- makedeps/Android.bp | 21 +++ makedeps/deps.go | 95 +++++++++++ makedeps/deps_test.go | 389 +++++++++++++++++++++++++++++++++++++++++++ 11 files changed, 546 insertions(+), 503 deletions(-) delete mode 100644 cmd/dep_fixer/deps.go delete mode 100644 cmd/dep_fixer/deps_test.go create mode 100644 makedeps/Android.bp create mode 100644 makedeps/deps.go create mode 100644 makedeps/deps_test.go diff --git a/android/rule_builder.go b/android/rule_builder.go index 4a3b0223..b3fccea3 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -342,10 +342,6 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string sboxOutputs[i] = "__SBOX_OUT_DIR__/" + Rel(ctx, r.sboxOutDir.String(), output.String()) } - if depFile != nil { - sboxOutputs = append(sboxOutputs, "__SBOX_OUT_DIR__/"+Rel(ctx, r.sboxOutDir.String(), depFile.String())) - } - commandString = proptools.ShellEscape(commandString) if !strings.HasPrefix(commandString, `'`) { commandString = `'` + commandString + `'` @@ -355,8 +351,13 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string sboxCmd.Tool(ctx.Config().HostToolPath(ctx, "sbox")). Flag("-c").Text(commandString). Flag("--sandbox-path").Text(shared.TempDirForOutDir(PathForOutput(ctx).String())). - Flag("--output-root").Text(r.sboxOutDir.String()). - Flags(sboxOutputs) + Flag("--output-root").Text(r.sboxOutDir.String()) + + if depFile != nil { + sboxCmd.Flag("--depfile-out").Text(depFile.String()) + } + + sboxCmd.Flags(sboxOutputs) commandString = string(sboxCmd.buf) tools = append(tools, sboxCmd.tools...) diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index df0f2564..d122a429 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -454,6 +454,7 @@ func TestRuleBuilder_Build(t *testing.T) { FailIfErrored(t, errs) check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraCmdDeps []string) { + t.Helper() if params.RuleParams.Command != wantCommand { t.Errorf("\nwant RuleParams.Command = %q\n got %q", wantCommand, params.RuleParams.Command) } @@ -497,13 +498,14 @@ func TestRuleBuilder_Build(t *testing.T) { t.Run("sbox", func(t *testing.T) { outDir := filepath.Join(buildDir, ".intermediates", "foo_sbox") outFile := filepath.Join(outDir, "foo_sbox") + depFile := filepath.Join(outDir, "foo_sbox.d") sbox := filepath.Join(buildDir, "host", config.PrebuiltOS(), "bin/sbox") sandboxPath := shared.TempDirForOutDir(buildDir) - cmd := sbox + ` -c 'cp bar __SBOX_OUT_DIR__/foo_sbox' --sandbox-path ` + sandboxPath + " --output-root " + outDir + " __SBOX_OUT_DIR__/foo_sbox __SBOX_OUT_DIR__/foo_sbox.d" + cmd := sbox + ` -c 'cp bar __SBOX_OUT_DIR__/foo_sbox' --sandbox-path ` + sandboxPath + " --output-root " + outDir + " --depfile-out " + depFile + " __SBOX_OUT_DIR__/foo_sbox" check(t, ctx.ModuleForTests("foo_sbox", "").Rule("rule"), - cmd, outFile, outFile+".d", false, []string{sbox}) + cmd, outFile, depFile, false, []string{sbox}) }) t.Run("singleton", func(t *testing.T) { outFile := filepath.Join(buildDir, "baz") diff --git a/cmd/dep_fixer/Android.bp b/cmd/dep_fixer/Android.bp index d2d1113d..97364d58 100644 --- a/cmd/dep_fixer/Android.bp +++ b/cmd/dep_fixer/Android.bp @@ -14,10 +14,6 @@ blueprint_go_binary { name: "dep_fixer", - deps: ["androidmk-parser"], - srcs: [ - "main.go", - "deps.go", - ], - testSrcs: ["deps_test.go"], + deps: ["soong-makedeps"], + srcs: ["main.go"], } diff --git a/cmd/dep_fixer/deps.go b/cmd/dep_fixer/deps.go deleted file mode 100644 index 64c97f52..00000000 --- a/cmd/dep_fixer/deps.go +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright 2018 Google Inc. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package main - -import ( - "bytes" - "fmt" - "io" - "strings" - - "android/soong/androidmk/parser" -) - -type Deps struct { - Output string - Inputs []string -} - -func Parse(filename string, r io.Reader) (*Deps, error) { - p := parser.NewParser(filename, r) - nodes, errs := p.Parse() - - if len(errs) == 1 { - return nil, errs[0] - } else if len(errs) > 1 { - return nil, fmt.Errorf("many errors: %v", errs) - } - - pos := func(node parser.Node) string { - return p.Unpack(node.Pos()).String() + ": " - } - - ret := &Deps{} - - for _, node := range nodes { - switch x := node.(type) { - case *parser.Comment: - // Do nothing - case *parser.Rule: - if x.Recipe != "" { - return nil, fmt.Errorf("%sunexpected recipe in rule: %v", pos(node), x) - } - - if !x.Target.Const() { - return nil, fmt.Errorf("%sunsupported variable expansion: %v", pos(node), x.Target.Dump()) - } - outputs := x.Target.Words() - if len(outputs) == 0 { - return nil, fmt.Errorf("%smissing output: %v", pos(node), x) - } - ret.Output = outputs[0].Value(nil) - - if !x.Prerequisites.Const() { - return nil, fmt.Errorf("%sunsupported variable expansion: %v", pos(node), x.Prerequisites.Dump()) - } - for _, input := range x.Prerequisites.Words() { - ret.Inputs = append(ret.Inputs, input.Value(nil)) - } - default: - return nil, fmt.Errorf("%sunexpected line: %#v", pos(node), node) - } - } - - return ret, nil -} - -func (d *Deps) Print() []byte { - // We don't really have to escape every \, but it's simpler, - // and ninja will handle it. - replacer := strings.NewReplacer(" ", "\\ ", - ":", "\\:", - "#", "\\#", - "$", "$$", - "\\", "\\\\") - - b := &bytes.Buffer{} - fmt.Fprintf(b, "%s:", replacer.Replace(d.Output)) - for _, input := range d.Inputs { - fmt.Fprintf(b, " %s", replacer.Replace(input)) - } - fmt.Fprintln(b) - return b.Bytes() -} diff --git a/cmd/dep_fixer/deps_test.go b/cmd/dep_fixer/deps_test.go deleted file mode 100644 index 0a779b76..00000000 --- a/cmd/dep_fixer/deps_test.go +++ /dev/null @@ -1,389 +0,0 @@ -// Copyright 2018 Google Inc. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package main - -import ( - "bytes" - "io" - "io/ioutil" - "os" - "testing" -) - -func TestParse(t *testing.T) { - testCases := []struct { - name string - input string - output Deps - err error - }{ - // These come from the ninja test suite - { - name: "Basic", - input: "build/ninja.o: ninja.cc ninja.h eval_env.h manifest_parser.h", - output: Deps{ - Output: "build/ninja.o", - Inputs: []string{ - "ninja.cc", - "ninja.h", - "eval_env.h", - "manifest_parser.h", - }, - }, - }, - { - name: "EarlyNewlineAndWhitespace", - input: ` \ - out: in`, - output: Deps{ - Output: "out", - Inputs: []string{"in"}, - }, - }, - { - name: "Continuation", - input: `foo.o: \ - bar.h baz.h -`, - output: Deps{ - Output: "foo.o", - Inputs: []string{"bar.h", "baz.h"}, - }, - }, - { - name: "CarriageReturnContinuation", - input: "foo.o: \\\r\n bar.h baz.h\r\n", - output: Deps{ - Output: "foo.o", - Inputs: []string{"bar.h", "baz.h"}, - }, - }, - { - name: "BackSlashes", - input: `Project\Dir\Build\Release8\Foo\Foo.res : \ - Dir\Library\Foo.rc \ - Dir\Library\Version\Bar.h \ - Dir\Library\Foo.ico \ - Project\Thing\Bar.tlb \ -`, - output: Deps{ - Output: `Project\Dir\Build\Release8\Foo\Foo.res`, - Inputs: []string{ - `Dir\Library\Foo.rc`, - `Dir\Library\Version\Bar.h`, - `Dir\Library\Foo.ico`, - `Project\Thing\Bar.tlb`, - }, - }, - }, - { - name: "Spaces", - input: `a\ bc\ def: a\ b c d`, - output: Deps{ - Output: `a bc def`, - Inputs: []string{"a b", "c", "d"}, - }, - }, - { - name: "Escapes", - input: `\!\@\#$$\%\^\&\\:`, - output: Deps{ - Output: `\!\@#$\%\^\&\`, - }, - }, - { - name: "SpecialChars", - // Ninja includes a number of '=', but our parser can't handle that, - // since it sees the equals and switches over to assuming it's an - // assignment. - // - // We don't have any files in our tree that contain an '=' character, - // and Kati can't handle parsing this either, so for now I'm just - // going to remove all the '=' characters below. - // - // It looks like make will only do this for the first - // dependency, but not later dependencies. - input: `C\:/Program\ Files\ (x86)/Microsoft\ crtdefs.h: \ - en@quot.header~ t+t-x!1 \ - openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif \ - Fu` + "\303\244ball", - output: Deps{ - Output: "C:/Program Files (x86)/Microsoft crtdefs.h", - Inputs: []string{ - "en@quot.header~", - "t+t-x!1", - "openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif", - "Fu\303\244ball", - }, - }, - }, - // Ninja's UnifyMultipleOutputs and RejectMultipleDifferentOutputs tests have been omitted, - // since we don't want the same behavior. - - // Our own tests - { - name: "Multiple outputs", - input: `a b: c -a: d -b: e`, - output: Deps{ - Output: "b", - Inputs: []string{ - "c", - "d", - "e", - }, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - out, err := Parse("test.d", bytes.NewBufferString(tc.input)) - if err != tc.err { - t.Fatalf("Unexpected error: %v (expected %v)", err, tc.err) - } - - if out.Output != tc.output.Output { - t.Errorf("output file doesn't match:\n"+ - " str: %#v\n"+ - "want: %#v\n"+ - " got: %#v", tc.input, tc.output.Output, out.Output) - } - - matches := true - if len(out.Inputs) != len(tc.output.Inputs) { - matches = false - } else { - for i := range out.Inputs { - if out.Inputs[i] != tc.output.Inputs[i] { - matches = false - } - } - } - if !matches { - t.Errorf("input files don't match:\n"+ - " str: %#v\n"+ - "want: %#v\n"+ - " got: %#v", tc.input, tc.output.Inputs, out.Inputs) - } - }) - } -} - -func BenchmarkParsing(b *testing.B) { - // Write it out to a file to most closely match ninja's perftest - tmpfile, err := ioutil.TempFile("", "depfile") - if err != nil { - b.Fatal("Failed to create temp file:", err) - } - defer os.Remove(tmpfile.Name()) - _, err = io.WriteString(tmpfile, `out/soong/.intermediates/external/ninja/ninja/linux_glibc_x86_64/obj/external/ninja/src/ninja.o: \ - external/ninja/src/ninja.cc external/libcxx/include/errno.h \ - external/libcxx/include/__config \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/features.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/predefs.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/cdefs.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/wordsize.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/gnu/stubs.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/gnu/stubs-64.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/errno.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/errno.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/linux/errno.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/asm/errno.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/asm-generic/errno.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/asm-generic/errno-base.h \ - external/libcxx/include/limits.h \ - prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/limits.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/limits.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/posix1_lim.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/local_lim.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/linux/limits.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/posix2_lim.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/xopen_lim.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/stdio_lim.h \ - external/libcxx/include/stdio.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/stdio.h \ - external/libcxx/include/stddef.h \ - prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/stddef.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/types.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/typesizes.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/libio.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/_G_config.h \ - external/libcxx/include/wchar.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/wchar.h \ - prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/stdarg.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/sys_errlist.h \ - external/libcxx/include/stdlib.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/stdlib.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/waitflags.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/waitstatus.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/endian.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/endian.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/byteswap.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/xlocale.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/types.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/time.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/select.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/select.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/sigset.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/time.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/select2.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/sysmacros.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/pthreadtypes.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/alloca.h \ - external/libcxx/include/string.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/string.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/getopt.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/unistd.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/posix_opt.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/environments.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/confname.h \ - external/ninja/src/browse.h external/ninja/src/build.h \ - external/libcxx/include/cstdio external/libcxx/include/map \ - external/libcxx/include/__tree external/libcxx/include/iterator \ - external/libcxx/include/iosfwd \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/wchar.h \ - external/libcxx/include/__functional_base \ - external/libcxx/include/type_traits external/libcxx/include/cstddef \ - prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/__stddef_max_align_t.h \ - external/libcxx/include/__nullptr external/libcxx/include/typeinfo \ - external/libcxx/include/exception external/libcxx/include/cstdlib \ - external/libcxx/include/cstdint external/libcxx/include/stdint.h \ - prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/stdint.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/stdint.h \ - external/libcxx/include/new external/libcxx/include/utility \ - external/libcxx/include/__tuple \ - external/libcxx/include/initializer_list \ - external/libcxx/include/cstring external/libcxx/include/__debug \ - external/libcxx/include/memory external/libcxx/include/limits \ - external/libcxx/include/__undef_macros external/libcxx/include/tuple \ - external/libcxx/include/stdexcept external/libcxx/include/cassert \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/assert.h \ - external/libcxx/include/atomic external/libcxx/include/algorithm \ - external/libcxx/include/functional external/libcxx/include/queue \ - external/libcxx/include/deque external/libcxx/include/__split_buffer \ - external/libcxx/include/vector external/libcxx/include/__bit_reference \ - external/libcxx/include/climits external/libcxx/include/set \ - external/libcxx/include/string external/libcxx/include/string_view \ - external/libcxx/include/__string external/libcxx/include/cwchar \ - external/libcxx/include/cwctype external/libcxx/include/cctype \ - external/libcxx/include/ctype.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/ctype.h \ - external/libcxx/include/wctype.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/wctype.h \ - external/ninja/src/graph.h external/ninja/src/eval_env.h \ - external/ninja/src/string_piece.h external/ninja/src/timestamp.h \ - external/ninja/src/util.h external/ninja/src/exit_status.h \ - external/ninja/src/line_printer.h external/ninja/src/metrics.h \ - external/ninja/src/build_log.h external/ninja/src/hash_map.h \ - external/libcxx/include/unordered_map \ - external/libcxx/include/__hash_table external/libcxx/include/cmath \ - external/libcxx/include/math.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/math.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/huge_val.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/huge_valf.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/huge_vall.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/inf.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/nan.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/mathdef.h \ - prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/mathcalls.h \ - external/ninja/src/deps_log.h external/ninja/src/clean.h \ - external/ninja/src/debug_flags.h external/ninja/src/disk_interface.h \ - external/ninja/src/graphviz.h external/ninja/src/manifest_parser.h \ - external/ninja/src/lexer.h external/ninja/src/state.h \ - external/ninja/src/version.h`) - tmpfile.Close() - if err != nil { - b.Fatal("Failed to write dep file:", err) - } - b.ResetTimer() - - for n := 0; n < b.N; n++ { - depfile, err := ioutil.ReadFile(tmpfile.Name()) - if err != nil { - b.Fatal("Failed to read dep file:", err) - } - - _, err = Parse(tmpfile.Name(), bytes.NewBuffer(depfile)) - if err != nil { - b.Fatal("Failed to parse:", err) - } - } -} - -func TestDepPrint(t *testing.T) { - testCases := []struct { - name string - input Deps - output string - }{ - { - name: "Empty", - input: Deps{ - Output: "a", - }, - output: "a:", - }, - { - name: "Basic", - input: Deps{ - Output: "a", - Inputs: []string{"b", "c"}, - }, - output: "a: b c", - }, - { - name: "Escapes", - input: Deps{ - Output: `\!\@#$\%\^\&\`, - }, - output: `\\!\\@\#$$\\%\\^\\&\\:`, - }, - { - name: "Spaces", - input: Deps{ - Output: "a b", - Inputs: []string{"c d", "e f "}, - }, - output: `a\ b: c\ d e\ f\ `, - }, - { - name: "SpecialChars", - input: Deps{ - Output: "C:/Program Files (x86)/Microsoft crtdefs.h", - Inputs: []string{ - "en@quot.header~", - "t+t-x!1", - "openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif", - "Fu\303\244ball", - }, - }, - output: `C\:/Program\ Files\ (x86)/Microsoft\ crtdefs.h: en@quot.header~ t+t-x!1 openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif Fu` + "\303\244ball", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - out := tc.input.Print() - outStr := string(out) - want := tc.output + "\n" - - if outStr != want { - t.Errorf("output doesn't match:\nwant:%q\n got:%q", want, outStr) - } - }) - } -} diff --git a/cmd/dep_fixer/main.go b/cmd/dep_fixer/main.go index f94cf2fd..d1bd1391 100644 --- a/cmd/dep_fixer/main.go +++ b/cmd/dep_fixer/main.go @@ -25,6 +25,8 @@ import ( "io/ioutil" "log" "os" + + "android/soong/makedeps" ) func main() { @@ -39,7 +41,7 @@ func main() { log.Fatal("Expected at least one input file as an argument") } - var mergedDeps *Deps + var mergedDeps *makedeps.Deps var firstInput []byte for i, arg := range flag.Args() { @@ -48,7 +50,7 @@ func main() { log.Fatalf("Error opening %q: %v", arg, err) } - deps, err := Parse(arg, bytes.NewBuffer(append([]byte(nil), input...))) + deps, err := makedeps.Parse(arg, bytes.NewBuffer(append([]byte(nil), input...))) if err != nil { log.Fatalf("Failed to parse: %v", err) } diff --git a/cmd/sbox/Android.bp b/cmd/sbox/Android.bp index fe4c7bbc..a706810d 100644 --- a/cmd/sbox/Android.bp +++ b/cmd/sbox/Android.bp @@ -14,6 +14,7 @@ blueprint_go_binary { name: "sbox", + deps: ["soong-makedeps"], srcs: [ "sbox.go", ], diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go index 4ac92953..7057b33f 100644 --- a/cmd/sbox/sbox.go +++ b/cmd/sbox/sbox.go @@ -15,6 +15,7 @@ package main import ( + "bytes" "errors" "flag" "fmt" @@ -25,6 +26,8 @@ import ( "path/filepath" "strings" "time" + + "android/soong/makedeps" ) var ( @@ -152,9 +155,6 @@ func run() error { return err } allOutputs = append(allOutputs, sandboxedDepfile) - if !strings.Contains(rawCommand, "__SBOX_DEPFILE__") { - return fmt.Errorf("the --depfile-out argument only makes sense if the command contains the text __SBOX_DEPFILE__") - } rawCommand = strings.Replace(rawCommand, "__SBOX_DEPFILE__", filepath.Join(tempDir, sandboxedDepfile), -1) } @@ -281,6 +281,26 @@ func run() error { } } + // Rewrite the depfile so that it doesn't include the (randomized) sandbox directory + if depfileOut != "" { + in, err := ioutil.ReadFile(depfileOut) + if err != nil { + return err + } + + deps, err := makedeps.Parse(depfileOut, bytes.NewBuffer(in)) + if err != nil { + return err + } + + deps.Output = "outputfile" + + err = ioutil.WriteFile(depfileOut, deps.Print(), 0666) + if err != nil { + return err + } + } + // TODO(jeffrygaston) if a process creates more output files than it declares, should there be a warning? return nil } diff --git a/makedeps/Android.bp b/makedeps/Android.bp new file mode 100644 index 00000000..b77b08f0 --- /dev/null +++ b/makedeps/Android.bp @@ -0,0 +1,21 @@ +// Copyright 2019 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +bootstrap_go_package { + name: "soong-makedeps", + pkgPath: "android/soong/makedeps", + deps: ["androidmk-parser"], + srcs: ["deps.go"], + testSrcs: ["deps_test.go"], +} diff --git a/makedeps/deps.go b/makedeps/deps.go new file mode 100644 index 00000000..e64e6f78 --- /dev/null +++ b/makedeps/deps.go @@ -0,0 +1,95 @@ +// Copyright 2018 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package makedeps + +import ( + "bytes" + "fmt" + "io" + "strings" + + "android/soong/androidmk/parser" +) + +type Deps struct { + Output string + Inputs []string +} + +func Parse(filename string, r io.Reader) (*Deps, error) { + p := parser.NewParser(filename, r) + nodes, errs := p.Parse() + + if len(errs) == 1 { + return nil, errs[0] + } else if len(errs) > 1 { + return nil, fmt.Errorf("many errors: %v", errs) + } + + pos := func(node parser.Node) string { + return p.Unpack(node.Pos()).String() + ": " + } + + ret := &Deps{} + + for _, node := range nodes { + switch x := node.(type) { + case *parser.Comment: + // Do nothing + case *parser.Rule: + if x.Recipe != "" { + return nil, fmt.Errorf("%sunexpected recipe in rule: %v", pos(node), x) + } + + if !x.Target.Const() { + return nil, fmt.Errorf("%sunsupported variable expansion: %v", pos(node), x.Target.Dump()) + } + outputs := x.Target.Words() + if len(outputs) == 0 { + return nil, fmt.Errorf("%smissing output: %v", pos(node), x) + } + ret.Output = outputs[0].Value(nil) + + if !x.Prerequisites.Const() { + return nil, fmt.Errorf("%sunsupported variable expansion: %v", pos(node), x.Prerequisites.Dump()) + } + for _, input := range x.Prerequisites.Words() { + ret.Inputs = append(ret.Inputs, input.Value(nil)) + } + default: + return nil, fmt.Errorf("%sunexpected line: %#v", pos(node), node) + } + } + + return ret, nil +} + +func (d *Deps) Print() []byte { + // We don't really have to escape every \, but it's simpler, + // and ninja will handle it. + replacer := strings.NewReplacer(" ", "\\ ", + ":", "\\:", + "#", "\\#", + "$", "$$", + "\\", "\\\\") + + b := &bytes.Buffer{} + fmt.Fprintf(b, "%s:", replacer.Replace(d.Output)) + for _, input := range d.Inputs { + fmt.Fprintf(b, " %s", replacer.Replace(input)) + } + fmt.Fprintln(b) + return b.Bytes() +} diff --git a/makedeps/deps_test.go b/makedeps/deps_test.go new file mode 100644 index 00000000..a32df650 --- /dev/null +++ b/makedeps/deps_test.go @@ -0,0 +1,389 @@ +// Copyright 2018 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package makedeps + +import ( + "bytes" + "io" + "io/ioutil" + "os" + "testing" +) + +func TestParse(t *testing.T) { + testCases := []struct { + name string + input string + output Deps + err error + }{ + // These come from the ninja test suite + { + name: "Basic", + input: "build/ninja.o: ninja.cc ninja.h eval_env.h manifest_parser.h", + output: Deps{ + Output: "build/ninja.o", + Inputs: []string{ + "ninja.cc", + "ninja.h", + "eval_env.h", + "manifest_parser.h", + }, + }, + }, + { + name: "EarlyNewlineAndWhitespace", + input: ` \ + out: in`, + output: Deps{ + Output: "out", + Inputs: []string{"in"}, + }, + }, + { + name: "Continuation", + input: `foo.o: \ + bar.h baz.h +`, + output: Deps{ + Output: "foo.o", + Inputs: []string{"bar.h", "baz.h"}, + }, + }, + { + name: "CarriageReturnContinuation", + input: "foo.o: \\\r\n bar.h baz.h\r\n", + output: Deps{ + Output: "foo.o", + Inputs: []string{"bar.h", "baz.h"}, + }, + }, + { + name: "BackSlashes", + input: `Project\Dir\Build\Release8\Foo\Foo.res : \ + Dir\Library\Foo.rc \ + Dir\Library\Version\Bar.h \ + Dir\Library\Foo.ico \ + Project\Thing\Bar.tlb \ +`, + output: Deps{ + Output: `Project\Dir\Build\Release8\Foo\Foo.res`, + Inputs: []string{ + `Dir\Library\Foo.rc`, + `Dir\Library\Version\Bar.h`, + `Dir\Library\Foo.ico`, + `Project\Thing\Bar.tlb`, + }, + }, + }, + { + name: "Spaces", + input: `a\ bc\ def: a\ b c d`, + output: Deps{ + Output: `a bc def`, + Inputs: []string{"a b", "c", "d"}, + }, + }, + { + name: "Escapes", + input: `\!\@\#$$\%\^\&\\:`, + output: Deps{ + Output: `\!\@#$\%\^\&\`, + }, + }, + { + name: "SpecialChars", + // Ninja includes a number of '=', but our parser can't handle that, + // since it sees the equals and switches over to assuming it's an + // assignment. + // + // We don't have any files in our tree that contain an '=' character, + // and Kati can't handle parsing this either, so for now I'm just + // going to remove all the '=' characters below. + // + // It looks like make will only do this for the first + // dependency, but not later dependencies. + input: `C\:/Program\ Files\ (x86)/Microsoft\ crtdefs.h: \ + en@quot.header~ t+t-x!1 \ + openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif \ + Fu` + "\303\244ball", + output: Deps{ + Output: "C:/Program Files (x86)/Microsoft crtdefs.h", + Inputs: []string{ + "en@quot.header~", + "t+t-x!1", + "openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif", + "Fu\303\244ball", + }, + }, + }, + // Ninja's UnifyMultipleOutputs and RejectMultipleDifferentOutputs tests have been omitted, + // since we don't want the same behavior. + + // Our own tests + { + name: "Multiple outputs", + input: `a b: c +a: d +b: e`, + output: Deps{ + Output: "b", + Inputs: []string{ + "c", + "d", + "e", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + out, err := Parse("test.d", bytes.NewBufferString(tc.input)) + if err != tc.err { + t.Fatalf("Unexpected error: %v (expected %v)", err, tc.err) + } + + if out.Output != tc.output.Output { + t.Errorf("output file doesn't match:\n"+ + " str: %#v\n"+ + "want: %#v\n"+ + " got: %#v", tc.input, tc.output.Output, out.Output) + } + + matches := true + if len(out.Inputs) != len(tc.output.Inputs) { + matches = false + } else { + for i := range out.Inputs { + if out.Inputs[i] != tc.output.Inputs[i] { + matches = false + } + } + } + if !matches { + t.Errorf("input files don't match:\n"+ + " str: %#v\n"+ + "want: %#v\n"+ + " got: %#v", tc.input, tc.output.Inputs, out.Inputs) + } + }) + } +} + +func BenchmarkParsing(b *testing.B) { + // Write it out to a file to most closely match ninja's perftest + tmpfile, err := ioutil.TempFile("", "depfile") + if err != nil { + b.Fatal("Failed to create temp file:", err) + } + defer os.Remove(tmpfile.Name()) + _, err = io.WriteString(tmpfile, `out/soong/.intermediates/external/ninja/ninja/linux_glibc_x86_64/obj/external/ninja/src/ninja.o: \ + external/ninja/src/ninja.cc external/libcxx/include/errno.h \ + external/libcxx/include/__config \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/features.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/predefs.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/cdefs.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/wordsize.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/gnu/stubs.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/gnu/stubs-64.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/errno.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/errno.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/linux/errno.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/asm/errno.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/asm-generic/errno.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/asm-generic/errno-base.h \ + external/libcxx/include/limits.h \ + prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/limits.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/limits.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/posix1_lim.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/local_lim.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/linux/limits.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/posix2_lim.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/xopen_lim.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/stdio_lim.h \ + external/libcxx/include/stdio.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/stdio.h \ + external/libcxx/include/stddef.h \ + prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/stddef.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/types.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/typesizes.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/libio.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/_G_config.h \ + external/libcxx/include/wchar.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/wchar.h \ + prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/stdarg.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/sys_errlist.h \ + external/libcxx/include/stdlib.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/stdlib.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/waitflags.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/waitstatus.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/endian.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/endian.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/byteswap.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/xlocale.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/types.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/time.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/select.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/select.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/sigset.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/time.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/select2.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/sys/sysmacros.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/pthreadtypes.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/alloca.h \ + external/libcxx/include/string.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/string.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/getopt.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/unistd.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/posix_opt.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/environments.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/confname.h \ + external/ninja/src/browse.h external/ninja/src/build.h \ + external/libcxx/include/cstdio external/libcxx/include/map \ + external/libcxx/include/__tree external/libcxx/include/iterator \ + external/libcxx/include/iosfwd \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/wchar.h \ + external/libcxx/include/__functional_base \ + external/libcxx/include/type_traits external/libcxx/include/cstddef \ + prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/__stddef_max_align_t.h \ + external/libcxx/include/__nullptr external/libcxx/include/typeinfo \ + external/libcxx/include/exception external/libcxx/include/cstdlib \ + external/libcxx/include/cstdint external/libcxx/include/stdint.h \ + prebuilts/clang/host/linux-x86/clang-4639204/lib64/clang/6.0.1/include/stdint.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/stdint.h \ + external/libcxx/include/new external/libcxx/include/utility \ + external/libcxx/include/__tuple \ + external/libcxx/include/initializer_list \ + external/libcxx/include/cstring external/libcxx/include/__debug \ + external/libcxx/include/memory external/libcxx/include/limits \ + external/libcxx/include/__undef_macros external/libcxx/include/tuple \ + external/libcxx/include/stdexcept external/libcxx/include/cassert \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/assert.h \ + external/libcxx/include/atomic external/libcxx/include/algorithm \ + external/libcxx/include/functional external/libcxx/include/queue \ + external/libcxx/include/deque external/libcxx/include/__split_buffer \ + external/libcxx/include/vector external/libcxx/include/__bit_reference \ + external/libcxx/include/climits external/libcxx/include/set \ + external/libcxx/include/string external/libcxx/include/string_view \ + external/libcxx/include/__string external/libcxx/include/cwchar \ + external/libcxx/include/cwctype external/libcxx/include/cctype \ + external/libcxx/include/ctype.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/ctype.h \ + external/libcxx/include/wctype.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/wctype.h \ + external/ninja/src/graph.h external/ninja/src/eval_env.h \ + external/ninja/src/string_piece.h external/ninja/src/timestamp.h \ + external/ninja/src/util.h external/ninja/src/exit_status.h \ + external/ninja/src/line_printer.h external/ninja/src/metrics.h \ + external/ninja/src/build_log.h external/ninja/src/hash_map.h \ + external/libcxx/include/unordered_map \ + external/libcxx/include/__hash_table external/libcxx/include/cmath \ + external/libcxx/include/math.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/math.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/huge_val.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/huge_valf.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/huge_vall.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/inf.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/nan.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/mathdef.h \ + prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.15-4.8/sysroot/usr/include/x86_64-linux-gnu/bits/mathcalls.h \ + external/ninja/src/deps_log.h external/ninja/src/clean.h \ + external/ninja/src/debug_flags.h external/ninja/src/disk_interface.h \ + external/ninja/src/graphviz.h external/ninja/src/manifest_parser.h \ + external/ninja/src/lexer.h external/ninja/src/state.h \ + external/ninja/src/version.h`) + tmpfile.Close() + if err != nil { + b.Fatal("Failed to write dep file:", err) + } + b.ResetTimer() + + for n := 0; n < b.N; n++ { + depfile, err := ioutil.ReadFile(tmpfile.Name()) + if err != nil { + b.Fatal("Failed to read dep file:", err) + } + + _, err = Parse(tmpfile.Name(), bytes.NewBuffer(depfile)) + if err != nil { + b.Fatal("Failed to parse:", err) + } + } +} + +func TestDepPrint(t *testing.T) { + testCases := []struct { + name string + input Deps + output string + }{ + { + name: "Empty", + input: Deps{ + Output: "a", + }, + output: "a:", + }, + { + name: "Basic", + input: Deps{ + Output: "a", + Inputs: []string{"b", "c"}, + }, + output: "a: b c", + }, + { + name: "Escapes", + input: Deps{ + Output: `\!\@#$\%\^\&\`, + }, + output: `\\!\\@\#$$\\%\\^\\&\\:`, + }, + { + name: "Spaces", + input: Deps{ + Output: "a b", + Inputs: []string{"c d", "e f "}, + }, + output: `a\ b: c\ d e\ f\ `, + }, + { + name: "SpecialChars", + input: Deps{ + Output: "C:/Program Files (x86)/Microsoft crtdefs.h", + Inputs: []string{ + "en@quot.header~", + "t+t-x!1", + "openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif", + "Fu\303\244ball", + }, + }, + output: `C\:/Program\ Files\ (x86)/Microsoft\ crtdefs.h: en@quot.header~ t+t-x!1 openldap/slapd.d/cnconfig/cnschema/cn{0}core.ldif Fu` + "\303\244ball", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + out := tc.input.Print() + outStr := string(out) + want := tc.output + "\n" + + if outStr != want { + t.Errorf("output doesn't match:\nwant:%q\n got:%q", want, outStr) + } + }) + } +} -- cgit v1.2.3 From a95304ec10b3b518fd47cd7677c4d46b124fdde9 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 23 Sep 2019 14:33:09 -0700 Subject: Move sharding functions for reuse Move shardPaths and shardTests to android.ShardPaths and android.ShardStrings for reuse in other packages. Bug: 144948629 Test: m checkbuild Change-Id: I868802872c73616b80f56cbf11f959c01a8b793a Merged-In: I868802872c73616b80f56cbf11f959c01a8b793a (cherry picked from commit 0a2f719bcaad397f27bcf52cd6604df6b5cec03b) --- android/util.go | 33 ++++++++++++++++++ android/util_test.go | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++ java/aapt2.go | 2 +- java/java.go | 14 +------- 4 files changed, 134 insertions(+), 14 deletions(-) diff --git a/android/util.go b/android/util.go index f9dce6fe..b17422df 100644 --- a/android/util.go +++ b/android/util.go @@ -243,3 +243,36 @@ func matchPattern(pat, str string) bool { } return strings.HasPrefix(str, pat[:i]) && strings.HasSuffix(str, pat[i+1:]) } + +// ShardPaths takes a Paths, and returns a slice of Paths where each one has at most shardSize paths. +func ShardPaths(paths Paths, shardSize int) []Paths { + if len(paths) == 0 { + return nil + } + ret := make([]Paths, 0, (len(paths)+shardSize-1)/shardSize) + for len(paths) > shardSize { + ret = append(ret, paths[0:shardSize]) + paths = paths[shardSize:] + } + if len(paths) > 0 { + ret = append(ret, paths) + } + return ret +} + +// ShardStrings takes a slice of strings, and returns a slice of slices of strings where each one has at most shardSize +// elements. +func ShardStrings(s []string, shardSize int) [][]string { + if len(s) == 0 { + return nil + } + ret := make([][]string, 0, (len(s)+shardSize-1)/shardSize) + for len(s) > shardSize { + ret = append(ret, s[0:shardSize]) + s = s[shardSize:] + } + if len(s) > 0 { + ret = append(ret, s) + } + return ret +} diff --git a/android/util_test.go b/android/util_test.go index 2e5eb07e..7f0d331c 100644 --- a/android/util_test.go +++ b/android/util_test.go @@ -404,3 +404,102 @@ func ExampleCopyOf_append() { // b = ["foo" "bar"] // c = ["foo" "baz"] } + +func Test_Shard(t *testing.T) { + type args struct { + strings []string + shardSize int + } + tests := []struct { + name string + args args + want [][]string + }{ + { + name: "empty", + args: args{ + strings: nil, + shardSize: 1, + }, + want: [][]string(nil), + }, + { + name: "single shard", + args: args{ + strings: []string{"a", "b"}, + shardSize: 2, + }, + want: [][]string{{"a", "b"}}, + }, + { + name: "single short shard", + args: args{ + strings: []string{"a", "b"}, + shardSize: 3, + }, + want: [][]string{{"a", "b"}}, + }, + { + name: "shard per input", + args: args{ + strings: []string{"a", "b", "c"}, + shardSize: 1, + }, + want: [][]string{{"a"}, {"b"}, {"c"}}, + }, + { + name: "balanced shards", + args: args{ + strings: []string{"a", "b", "c", "d"}, + shardSize: 2, + }, + want: [][]string{{"a", "b"}, {"c", "d"}}, + }, + { + name: "unbalanced shards", + args: args{ + strings: []string{"a", "b", "c"}, + shardSize: 2, + }, + want: [][]string{{"a", "b"}, {"c"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run("strings", func(t *testing.T) { + if got := ShardStrings(tt.args.strings, tt.args.shardSize); !reflect.DeepEqual(got, tt.want) { + t.Errorf("ShardStrings(%v, %v) = %v, want %v", + tt.args.strings, tt.args.shardSize, got, tt.want) + } + }) + + t.Run("paths", func(t *testing.T) { + stringsToPaths := func(strings []string) Paths { + if strings == nil { + return nil + } + paths := make(Paths, len(strings)) + for i, s := range strings { + paths[i] = PathForTesting(s) + } + return paths + } + + paths := stringsToPaths(tt.args.strings) + + var want []Paths + if sWant := tt.want; sWant != nil { + want = make([]Paths, len(sWant)) + for i, w := range sWant { + want[i] = stringsToPaths(w) + } + } + + if got := ShardPaths(paths, tt.args.shardSize); !reflect.DeepEqual(got, want) { + t.Errorf("ShardPaths(%v, %v) = %v, want %v", + paths, tt.args.shardSize, got, want) + } + }) + }) + } +} diff --git a/java/aapt2.go b/java/aapt2.go index bcc8e976..f21408f9 100644 --- a/java/aapt2.go +++ b/java/aapt2.go @@ -61,7 +61,7 @@ var aapt2CompileRule = pctx.AndroidStaticRule("aapt2Compile", "outDir", "cFlags") func aapt2Compile(ctx android.ModuleContext, dir android.Path, paths android.Paths) android.WritablePaths { - shards := shardPaths(paths, AAPT2_SHARD_SIZE) + shards := android.ShardPaths(paths, AAPT2_SHARD_SIZE) ret := make(android.WritablePaths, 0, len(paths)) diff --git a/java/java.go b/java/java.go index bf738c4d..9ac38c92 100644 --- a/java/java.go +++ b/java/java.go @@ -530,18 +530,6 @@ func hasSrcExt(srcs []string, ext string) bool { return false } -func shardPaths(paths android.Paths, shardSize int) []android.Paths { - ret := make([]android.Paths, 0, (len(paths)+shardSize-1)/shardSize) - for len(paths) > shardSize { - ret = append(ret, paths[0:shardSize]) - paths = paths[shardSize:] - } - if len(paths) > 0 { - ret = append(ret, paths) - } - return ret -} - func (j *Module) hasSrcExt(ext string) bool { return hasSrcExt(j.properties.Srcs, ext) } @@ -1088,7 +1076,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars ...android.Path shardSize := int(*(j.properties.Javac_shard_size)) var shardSrcs []android.Paths if len(uniqueSrcFiles) > 0 { - shardSrcs = shardPaths(uniqueSrcFiles, shardSize) + shardSrcs = android.ShardPaths(uniqueSrcFiles, shardSize) for idx, shardSrc := range shardSrcs { classes := android.PathForModuleOut(ctx, "javac", jarName+strconv.Itoa(idx)) TransformJavaToClasses(ctx, classes, idx, shardSrc, nil, flags, extraJarDeps) -- cgit v1.2.3 From d9c1c8fbcb242d2aacebbd9dc28702b402fe44f4 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 23 Sep 2019 15:55:30 -0700 Subject: Shard gensrcs modules into multiple commands gensrcs modules use a single command to convert all of the sources, which can lead to command line length limits, long critical path times, and excessive rebuilds. Shard them into multiple rules, defaulting to 100 commands in each. Test: TestGenSrcs Test: m Fixes: 70221552 Fixes: 138438756 Fixes: 138829091 Fixes: 144948629 Change-Id: I8409e43011a4754e095ad1b368570a2ba8d75e50 Merged-In: I8409e43011a4754e095ad1b368570a2ba8d75e50 (cherry picked from commit 1a527688d67b9349f4ea5735916c35bdb7f1847f) --- genrule/genrule.go | 378 +++++++++++++++++++++++++++++------------------- genrule/genrule_test.go | 101 ++++++++++++- 2 files changed, 331 insertions(+), 148 deletions(-) diff --git a/genrule/genrule.go b/genrule/genrule.go index 87e6747e..55c7e628 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -17,6 +17,7 @@ package genrule import ( "fmt" "io" + "strconv" "strings" "github.com/google/blueprint" @@ -37,10 +38,20 @@ func init() { var ( pctx = android.NewPackageContext("android/soong/genrule") + + gensrcsMerge = pctx.AndroidStaticRule("gensrcsMerge", blueprint.RuleParams{ + Command: "${soongZip} -o ${tmpZip} @${tmpZip}.rsp && ${zipSync} -d ${genDir} ${tmpZip}", + CommandDeps: []string{"${soongZip}", "${zipSync}"}, + Rspfile: "${tmpZip}.rsp", + RspfileContent: "${zipArgs}", + }, "tmpZip", "genDir", "zipArgs") ) func init() { pctx.HostBinToolVariable("sboxCmd", "sbox") + + pctx.HostBinToolVariable("soongZip", "soong_zip") + pctx.HostBinToolVariable("zipSync", "zipsync") } type SourceFileGenerator interface { @@ -110,9 +121,9 @@ type Module struct { taskGenerator taskFunc - deps android.Paths - rule blueprint.Rule - rawCommand string + deps android.Paths + rule blueprint.Rule + rawCommands []string exportedIncludeDirs android.Paths @@ -120,15 +131,20 @@ type Module struct { outputDeps android.Paths subName string + subDir string } -type taskFunc func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) generateTask +type taskFunc func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask type generateTask struct { in android.Paths out android.WritablePaths + copyTo android.WritablePaths + genDir android.WritablePath sandboxOuts []string cmd string + shard int + shards int } func (g *Module) GeneratedSourceFiles() android.Paths { @@ -167,10 +183,10 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { if len(g.properties.Export_include_dirs) > 0 { for _, dir := range g.properties.Export_include_dirs { g.exportedIncludeDirs = append(g.exportedIncludeDirs, - android.PathForModuleGen(ctx, ctx.ModuleDir(), dir)) + android.PathForModuleGen(ctx, g.subDir, ctx.ModuleDir(), dir)) } } else { - g.exportedIncludeDirs = append(g.exportedIncludeDirs, android.PathForModuleGen(ctx, "")) + g.exportedIncludeDirs = append(g.exportedIncludeDirs, android.PathForModuleGen(ctx, g.subDir)) } locationLabels := map[string][]string{} @@ -275,120 +291,154 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { } } - task := g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles) - - for _, out := range task.out { - addLocationLabel(out.Rel(), []string{filepath.Join("__SBOX_OUT_DIR__", out.Rel())}) - } - - referencedDepfile := false + var copyFrom android.Paths + var outputFiles android.WritablePaths + var zipArgs strings.Builder - rawCommand, err := android.Expand(task.cmd, func(name string) (string, error) { - // report the error directly without returning an error to android.Expand to catch multiple errors in a - // single run - reportError := func(fmt string, args ...interface{}) (string, error) { - ctx.PropertyErrorf("cmd", fmt, args...) - return "SOONG_ERROR", nil + for _, task := range g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles) { + for _, out := range task.out { + addLocationLabel(out.Rel(), []string{filepath.Join("__SBOX_OUT_DIR__", out.Rel())}) } - switch name { - case "location": - if len(g.properties.Tools) == 0 && len(g.properties.Tool_files) == 0 { - return reportError("at least one `tools` or `tool_files` is required if $(location) is used") - } - paths := locationLabels[firstLabel] - if len(paths) == 0 { - return reportError("default label %q has no files", firstLabel) - } else if len(paths) > 1 { - return reportError("default label %q has multiple files, use $(locations %s) to reference it", - firstLabel, firstLabel) - } - return locationLabels[firstLabel][0], nil - case "in": - return "${in}", nil - case "out": - return "__SBOX_OUT_FILES__", nil - case "depfile": - referencedDepfile = true - if !Bool(g.properties.Depfile) { - return reportError("$(depfile) used without depfile property") + referencedDepfile := false + + rawCommand, err := android.Expand(task.cmd, func(name string) (string, error) { + // report the error directly without returning an error to android.Expand to catch multiple errors in a + // single run + reportError := func(fmt string, args ...interface{}) (string, error) { + ctx.PropertyErrorf("cmd", fmt, args...) + return "SOONG_ERROR", nil } - return "__SBOX_DEPFILE__", nil - case "genDir": - return "__SBOX_OUT_DIR__", nil - default: - if strings.HasPrefix(name, "location ") { - label := strings.TrimSpace(strings.TrimPrefix(name, "location ")) - if paths, ok := locationLabels[label]; ok { - if len(paths) == 0 { - return reportError("label %q has no files", label) - } else if len(paths) > 1 { - return reportError("label %q has multiple files, use $(locations %s) to reference it", - label, label) - } - return paths[0], nil - } else { - return reportError("unknown location label %q", label) + + switch name { + case "location": + if len(g.properties.Tools) == 0 && len(g.properties.Tool_files) == 0 { + return reportError("at least one `tools` or `tool_files` is required if $(location) is used") + } + paths := locationLabels[firstLabel] + if len(paths) == 0 { + return reportError("default label %q has no files", firstLabel) + } else if len(paths) > 1 { + return reportError("default label %q has multiple files, use $(locations %s) to reference it", + firstLabel, firstLabel) + } + return locationLabels[firstLabel][0], nil + case "in": + return "${in}", nil + case "out": + return "__SBOX_OUT_FILES__", nil + case "depfile": + referencedDepfile = true + if !Bool(g.properties.Depfile) { + return reportError("$(depfile) used without depfile property") } - } else if strings.HasPrefix(name, "locations ") { - label := strings.TrimSpace(strings.TrimPrefix(name, "locations ")) - if paths, ok := locationLabels[label]; ok { - if len(paths) == 0 { - return reportError("label %q has no files", label) + return "__SBOX_DEPFILE__", nil + case "genDir": + return "__SBOX_OUT_DIR__", nil + default: + if strings.HasPrefix(name, "location ") { + label := strings.TrimSpace(strings.TrimPrefix(name, "location ")) + if paths, ok := locationLabels[label]; ok { + if len(paths) == 0 { + return reportError("label %q has no files", label) + } else if len(paths) > 1 { + return reportError("label %q has multiple files, use $(locations %s) to reference it", + label, label) + } + return paths[0], nil + } else { + return reportError("unknown location label %q", label) + } + } else if strings.HasPrefix(name, "locations ") { + label := strings.TrimSpace(strings.TrimPrefix(name, "locations ")) + if paths, ok := locationLabels[label]; ok { + if len(paths) == 0 { + return reportError("label %q has no files", label) + } + return strings.Join(paths, " "), nil + } else { + return reportError("unknown locations label %q", label) } - return strings.Join(paths, " "), nil } else { - return reportError("unknown locations label %q", label) + return reportError("unknown variable '$(%s)'", name) } - } else { - return reportError("unknown variable '$(%s)'", name) } + }) + + if err != nil { + ctx.PropertyErrorf("cmd", "%s", err.Error()) + return } - }) - if err != nil { - ctx.PropertyErrorf("cmd", "%s", err.Error()) - return - } + if Bool(g.properties.Depfile) && !referencedDepfile { + ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd") + return + } - if Bool(g.properties.Depfile) && !referencedDepfile { - ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd") - } + // tell the sbox command which directory to use as its sandbox root + buildDir := android.PathForOutput(ctx).String() + sandboxPath := shared.TempDirForOutDir(buildDir) - // tell the sbox command which directory to use as its sandbox root - buildDir := android.PathForOutput(ctx).String() - sandboxPath := shared.TempDirForOutDir(buildDir) + // recall that Sprintf replaces percent sign expressions, whereas dollar signs expressions remain as written, + // to be replaced later by ninja_strings.go + depfilePlaceholder := "" + if Bool(g.properties.Depfile) { + depfilePlaceholder = "$depfileArgs" + } - // recall that Sprintf replaces percent sign expressions, whereas dollar signs expressions remain as written, - // to be replaced later by ninja_strings.go - depfilePlaceholder := "" - if Bool(g.properties.Depfile) { - depfilePlaceholder = "$depfileArgs" - } + // Escape the command for the shell + rawCommand = "'" + strings.Replace(rawCommand, "'", `'\''`, -1) + "'" + g.rawCommands = append(g.rawCommands, rawCommand) + sandboxCommand := fmt.Sprintf("rm -rf %s && $sboxCmd --sandbox-path %s --output-root %s -c %s %s $allouts", + task.genDir, sandboxPath, task.genDir, rawCommand, depfilePlaceholder) - genDir := android.PathForModuleGen(ctx) - // Escape the command for the shell - rawCommand = "'" + strings.Replace(rawCommand, "'", `'\''`, -1) + "'" - g.rawCommand = rawCommand - sandboxCommand := fmt.Sprintf("$sboxCmd --sandbox-path %s --output-root %s -c %s %s $allouts", - sandboxPath, genDir, rawCommand, depfilePlaceholder) + ruleParams := blueprint.RuleParams{ + Command: sandboxCommand, + CommandDeps: []string{"$sboxCmd"}, + } + args := []string{"allouts"} + if Bool(g.properties.Depfile) { + ruleParams.Deps = blueprint.DepsGCC + args = append(args, "depfileArgs") + } + name := "generator" + if task.shards > 1 { + name += strconv.Itoa(task.shard) + } + rule := ctx.Rule(pctx, name, ruleParams, args...) - ruleParams := blueprint.RuleParams{ - Command: sandboxCommand, - CommandDeps: []string{"$sboxCmd"}, - } - args := []string{"allouts"} - if Bool(g.properties.Depfile) { - ruleParams.Deps = blueprint.DepsGCC - args = append(args, "depfileArgs") + g.generateSourceFile(ctx, task, rule) + + if len(task.copyTo) > 0 { + outputFiles = append(outputFiles, task.copyTo...) + copyFrom = append(copyFrom, task.out.Paths()...) + zipArgs.WriteString(" -C " + task.genDir.String()) + zipArgs.WriteString(android.JoinWithPrefix(task.out.Strings(), " -f ")) + } else { + outputFiles = append(outputFiles, task.out...) + } } - g.rule = ctx.Rule(pctx, "generator", ruleParams, args...) - g.generateSourceFile(ctx, task) + if len(copyFrom) > 0 { + ctx.Build(pctx, android.BuildParams{ + Rule: gensrcsMerge, + Implicits: copyFrom, + Outputs: outputFiles, + Args: map[string]string{ + "zipArgs": zipArgs.String(), + "tmpZip": android.PathForModuleGen(ctx, g.subDir+".zip").String(), + "genDir": android.PathForModuleGen(ctx, g.subDir).String(), + }, + }) + } + g.outputFiles = outputFiles.Paths() + if len(g.outputFiles) > 0 { + g.outputDeps = append(g.outputDeps, g.outputFiles[0]) + } } -func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask) { +func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask, rule blueprint.Rule) { desc := "generate" if len(task.out) == 0 { ctx.ModuleErrorf("must have at least one output file") @@ -403,9 +453,13 @@ func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask depFile = android.PathForModuleGen(ctx, task.out[0].Rel()+".d") } + if task.shards > 1 { + desc += " " + strconv.Itoa(task.shard) + } + params := android.BuildParams{ - Rule: g.rule, - Description: "generate", + Rule: rule, + Description: desc, Output: task.out[0], ImplicitOutputs: task.out[1:], Inputs: task.in, @@ -420,11 +474,6 @@ func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask } ctx.Build(pctx, params) - - for _, outputFile := range task.out { - g.outputFiles = append(g.outputFiles, outputFile) - } - g.outputDeps = append(g.outputDeps, task.out[0]) } // Collect information for opening IDE project files in java/jdeps.go. @@ -446,7 +495,7 @@ func (g *Module) AndroidMk() android.AndroidMkData { SubName: g.subName, Extra: []android.AndroidMkExtraFunc{ func(w io.Writer, outputFile android.Path) { - fmt.Fprintln(w, "LOCAL_ADDITIONAL_DEPENDENCIES :=", strings.Join(g.outputFiles.Strings(), " ")) + fmt.Fprintln(w, "LOCAL_ADDITIONAL_DEPENDENCIES :=", strings.Join(g.outputDeps.Strings(), " ")) }, }, Custom: func(w io.Writer, name, prefix, moduleDir string, data android.AndroidMkData) { @@ -483,47 +532,80 @@ func pathToSandboxOut(path android.Path, genDir android.Path) string { func NewGenSrcs() *Module { properties := &genSrcsProperties{} - taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) generateTask { - commands := []string{} - outFiles := android.WritablePaths{} - genDir := android.PathForModuleGen(ctx) - sandboxOuts := []string{} - for _, in := range srcFiles { - outFile := android.GenPathWithExt(ctx, "", in, String(properties.Output_extension)) - outFiles = append(outFiles, outFile) - - sandboxOutfile := pathToSandboxOut(outFile, genDir) - sandboxOuts = append(sandboxOuts, sandboxOutfile) - - command, err := android.Expand(rawCommand, func(name string) (string, error) { - switch name { - case "in": - return in.String(), nil - case "out": - return sandboxOutfile, nil - default: - return "$(" + name + ")", nil - } - }) - if err != nil { - ctx.PropertyErrorf("cmd", err.Error()) + taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask { + genDir := android.PathForModuleGen(ctx, "gensrcs") + shardSize := defaultShardSize + if s := properties.Shard_size; s != nil { + shardSize = int(*s) + } + + shards := android.ShardPaths(srcFiles, shardSize) + var generateTasks []generateTask + + for i, shard := range shards { + var commands []string + var outFiles android.WritablePaths + var copyTo android.WritablePaths + var shardDir android.WritablePath + var sandboxOuts []string + + if len(shards) > 1 { + shardDir = android.PathForModuleGen(ctx, strconv.Itoa(i)) + } else { + shardDir = genDir } - // escape the command in case for example it contains '#', an odd number of '"', etc - command = fmt.Sprintf("bash -c %v", proptools.ShellEscape(command)) - commands = append(commands, command) - } - fullCommand := strings.Join(commands, " && ") + for _, in := range shard { + outFile := android.GenPathWithExt(ctx, "gensrcs", in, String(properties.Output_extension)) + sandboxOutfile := pathToSandboxOut(outFile, genDir) - return generateTask{ - in: srcFiles, - out: outFiles, - sandboxOuts: sandboxOuts, - cmd: fullCommand, + if len(shards) > 1 { + shardFile := android.GenPathWithExt(ctx, strconv.Itoa(i), in, String(properties.Output_extension)) + copyTo = append(copyTo, outFile) + outFile = shardFile + } + + outFiles = append(outFiles, outFile) + sandboxOuts = append(sandboxOuts, sandboxOutfile) + + command, err := android.Expand(rawCommand, func(name string) (string, error) { + switch name { + case "in": + return in.String(), nil + case "out": + return sandboxOutfile, nil + default: + return "$(" + name + ")", nil + } + }) + if err != nil { + ctx.PropertyErrorf("cmd", err.Error()) + } + + // escape the command in case for example it contains '#', an odd number of '"', etc + command = fmt.Sprintf("bash -c %v", proptools.ShellEscape(command)) + commands = append(commands, command) + } + fullCommand := strings.Join(commands, " && ") + + generateTasks = append(generateTasks, generateTask{ + in: shard, + out: outFiles, + copyTo: copyTo, + genDir: shardDir, + sandboxOuts: sandboxOuts, + cmd: fullCommand, + shard: i, + shards: len(shards), + }) } + + return generateTasks } - return generatorFactory(taskGenerator, properties) + g := generatorFactory(taskGenerator, properties) + g.subDir = "gensrcs" + return g } func GenSrcsFactory() android.Module { @@ -535,12 +617,17 @@ func GenSrcsFactory() android.Module { type genSrcsProperties struct { // extension that will be substituted for each output file Output_extension *string + + // maximum number of files that will be passed on a single command line. + Shard_size *int64 } +const defaultShardSize = 100 + func NewGenRule() *Module { properties := &genRuleProperties{} - taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) generateTask { + taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask { outs := make(android.WritablePaths, len(properties.Out)) sandboxOuts := make([]string, len(properties.Out)) genDir := android.PathForModuleGen(ctx) @@ -548,12 +635,13 @@ func NewGenRule() *Module { outs[i] = android.PathForModuleGen(ctx, out) sandboxOuts[i] = pathToSandboxOut(outs[i], genDir) } - return generateTask{ + return []generateTask{{ in: srcFiles, out: outs, + genDir: android.PathForModuleGen(ctx), sandboxOuts: sandboxOuts, cmd: rawCommand, - } + }} } return generatorFactory(taskGenerator, properties) diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 0b6952f3..e8dc3b55 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -57,6 +57,7 @@ func testContext(config android.Config, bp string, ctx := android.NewTestArchContext() ctx.RegisterModuleType("filegroup", android.ModuleFactoryAdaptor(android.FileGroupFactory)) ctx.RegisterModuleType("genrule", android.ModuleFactoryAdaptor(GenRuleFactory)) + ctx.RegisterModuleType("gensrcs", android.ModuleFactoryAdaptor(GenSrcsFactory)) ctx.RegisterModuleType("genrule_defaults", android.ModuleFactoryAdaptor(defaultsFactory)) ctx.RegisterModuleType("tool", android.ModuleFactoryAdaptor(toolFactory)) ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators) @@ -109,6 +110,9 @@ func testContext(config android.Config, bp string, "tool_file2": nil, "in1": nil, "in2": nil, + "in1.txt": nil, + "in2.txt": nil, + "in3.txt": nil, } for k, v := range fs { @@ -491,11 +495,102 @@ func TestGenruleCmd(t *testing.T) { } gen := ctx.ModuleForTests("gen", "").Module().(*Module) - if g, w := gen.rawCommand, "'"+test.expect+"'"; w != g { + if g, w := gen.rawCommands[0], "'"+test.expect+"'"; w != g { t.Errorf("want %q, got %q", w, g) } }) } +} + +func TestGenSrcs(t *testing.T) { + testcases := []struct { + name string + prop string + + allowMissingDependencies bool + + err string + cmds []string + deps []string + files []string + }{ + { + name: "gensrcs", + prop: ` + tools: ["tool"], + srcs: ["in1.txt", "in2.txt"], + cmd: "$(location) $(in) > $(out)", + `, + cmds: []string{ + "'bash -c '\\''out/tool in1.txt > __SBOX_OUT_DIR__/in1.h'\\'' && bash -c '\\''out/tool in2.txt > __SBOX_OUT_DIR__/in2.h'\\'''", + }, + deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h"}, + files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"}, + }, + { + name: "shards", + prop: ` + tools: ["tool"], + srcs: ["in1.txt", "in2.txt", "in3.txt"], + cmd: "$(location) $(in) > $(out)", + shard_size: 2, + `, + cmds: []string{ + "'bash -c '\\''out/tool in1.txt > __SBOX_OUT_DIR__/in1.h'\\'' && bash -c '\\''out/tool in2.txt > __SBOX_OUT_DIR__/in2.h'\\'''", + "'bash -c '\\''out/tool in3.txt > __SBOX_OUT_DIR__/in3.h'\\'''", + }, + deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h"}, + files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"}, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + config := android.TestArchConfig(buildDir, nil) + bp := "gensrcs {\n" + bp += `name: "gen",` + "\n" + bp += `output_extension: "h",` + "\n" + bp += test.prop + bp += "}\n" + + ctx := testContext(config, bp, nil) + + _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) + if errs == nil { + _, errs = ctx.PrepareBuildActions(config) + } + if errs == nil && test.err != "" { + t.Fatalf("want error %q, got no error", test.err) + } else if errs != nil && test.err == "" { + android.FailIfErrored(t, errs) + } else if test.err != "" { + if len(errs) != 1 { + t.Errorf("want 1 error, got %d errors:", len(errs)) + for _, err := range errs { + t.Errorf(" %s", err.Error()) + } + t.FailNow() + } + if !strings.Contains(errs[0].Error(), test.err) { + t.Fatalf("want %q, got %q", test.err, errs[0].Error()) + } + return + } + + gen := ctx.ModuleForTests("gen", "").Module().(*Module) + if g, w := gen.rawCommands, test.cmds; !reflect.DeepEqual(w, g) { + t.Errorf("want %q, got %q", w, g) + } + + if g, w := gen.outputDeps.Strings(), test.deps; !reflect.DeepEqual(w, g) { + t.Errorf("want deps %q, got %q", w, g) + } + + if g, w := gen.outputFiles.Strings(), test.files; !reflect.DeepEqual(w, g) { + t.Errorf("want files %q, got %q", w, g) + } + }) + } } @@ -529,8 +624,8 @@ func TestGenruleDefaults(t *testing.T) { gen := ctx.ModuleForTests("gen", "").Module().(*Module) expectedCmd := "'cp ${in} __SBOX_OUT_FILES__'" - if gen.rawCommand != expectedCmd { - t.Errorf("Expected cmd: %q, actual: %q", expectedCmd, gen.rawCommand) + if gen.rawCommands[0] != expectedCmd { + t.Errorf("Expected cmd: %q, actual: %q", expectedCmd, gen.rawCommands[0]) } expectedSrcs := []string{"in1"} -- cgit v1.2.3