diff --git a/android/module.go b/android/module.go index d54032477..429f31192 100644 --- a/android/module.go +++ b/android/module.go @@ -2738,7 +2738,6 @@ func outputFilesForModule(ctx PathContext, module blueprint.Module, tag string) // Modules can implement HostToolProvider and return a valid OptionalPath from HostToolPath() to // specify that they can be used as a tool by a genrule module. type HostToolProvider interface { - Module // HostToolPath returns the path to the host tool for the module if it is one, or an invalid // OptionalPath. HostToolPath() OptionalPath diff --git a/android/rule_builder.go b/android/rule_builder.go index 84501fe9f..e2d81877c 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -32,7 +32,6 @@ import ( const sboxSandboxBaseDir = "__SBOX_SANDBOX_DIR__" const sboxOutSubDir = "out" -const sboxToolsSubDir = "tools" const sboxOutDir = sboxSandboxBaseDir + "/" + sboxOutSubDir // RuleBuilder provides an alternative to ModuleContext.Rule and ModuleContext.Build to add a command line to the build @@ -49,7 +48,6 @@ type RuleBuilder struct { highmem bool remoteable RemoteRuleSupports outDir WritablePath - sboxTools bool sboxManifestPath WritablePath missingDeps []string } @@ -142,19 +140,6 @@ func (r *RuleBuilder) Sbox(outputDir WritablePath, manifestPath WritablePath) *R return r } -// SandboxTools enables tool sandboxing for the rule by copying any referenced tools into the -// sandbox. -func (r *RuleBuilder) SandboxTools() *RuleBuilder { - if !r.sbox { - panic("SandboxTools() must be called after Sbox()") - } - if len(r.commands) > 0 { - panic("SandboxTools() may not be called after Command()") - } - r.sboxTools = true - 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) { @@ -483,29 +468,8 @@ func (r *RuleBuilder) Build(name string, desc string) { manifest.OutputDepfile = proto.String(depFile.String()) } - // If sandboxing tools is enabled, add copy rules to the manifest to copy each tool - // into the sbox directory. - if r.sboxTools { - for _, tool := range tools { - command.CopyBefore = append(command.CopyBefore, &sbox_proto.Copy{ - From: proto.String(tool.String()), - To: proto.String(sboxPathForToolRel(r.ctx, tool)), - }) - } - for _, c := range r.commands { - for _, tool := range c.packagedTools { - command.CopyBefore = append(command.CopyBefore, &sbox_proto.Copy{ - From: proto.String(tool.srcPath.String()), - To: proto.String(sboxPathForPackagedToolRel(tool)), - Executable: proto.Bool(tool.executable), - }) - tools = append(tools, tool.srcPath) - } - } - } - // Add copy rules to the manifest to copy each output file from the sbox directory. - // to the output directory after running the commands. + // to the output directory. sboxOutputs := make([]string, len(outputs)) for i, output := range outputs { rel := Rel(r.ctx, r.outDir.String(), output.String()) @@ -618,7 +582,6 @@ type RuleBuilderCommand struct { symlinkOutputs WritablePaths depFiles WritablePaths tools Paths - packagedTools []PackagingSpec rspFileInputs Paths // spans [start,end) of the command that should not be ninja escaped @@ -662,79 +625,6 @@ func (c *RuleBuilderCommand) PathForOutput(path WritablePath) string { return path.String() } -// SboxPathForTool takes a path to a tool, which may be an output file or a source file, and returns -// the corresponding path for the tool in the sbox sandbox. It assumes that sandboxing and tool -// sandboxing are enabled. -func SboxPathForTool(ctx BuilderContext, path Path) string { - return filepath.Join(sboxSandboxBaseDir, sboxPathForToolRel(ctx, path)) -} - -func sboxPathForToolRel(ctx BuilderContext, path Path) string { - // Errors will be handled in RuleBuilder.Build where we have a context to report them - relOut, isRelOut, _ := maybeRelErr(PathForOutput(ctx, "host", ctx.Config().PrebuiltOS()).String(), path.String()) - if isRelOut { - // The tool is in the output directory, it will be copied to __SBOX_OUT_DIR__/tools/out - return filepath.Join(sboxToolsSubDir, "out", relOut) - } - // The tool is in the source directory, it will be copied to __SBOX_OUT_DIR__/tools/src - return filepath.Join(sboxToolsSubDir, "src", path.String()) -} - -// SboxPathForPackagedTool takes a PackageSpec for a tool and returns the corresponding path for the -// tool after copying it into the sandbox. This can be used on the RuleBuilder command line to -// reference the tool. -func SboxPathForPackagedTool(spec PackagingSpec) string { - return filepath.Join(sboxSandboxBaseDir, sboxPathForPackagedToolRel(spec)) -} - -func sboxPathForPackagedToolRel(spec PackagingSpec) string { - return filepath.Join(sboxToolsSubDir, "out", spec.relPathInPackage) -} - -// PathForTool takes a path to a tool, which may be an output file or a source file, and returns -// the corresponding path for the tool in the sbox sandbox if sbox is enabled, or the original path -// if it is not. This can be used on the RuleBuilder command line to reference the tool. -func (c *RuleBuilderCommand) PathForTool(path Path) string { - if c.rule.sbox && c.rule.sboxTools { - return filepath.Join(sboxSandboxBaseDir, sboxPathForToolRel(c.rule.ctx, path)) - } - return path.String() -} - -// PackagedTool adds the specified tool path to the command line. It can only be used with tool -// sandboxing enabled by SandboxTools(), and will copy the tool into the sandbox. -func (c *RuleBuilderCommand) PackagedTool(spec PackagingSpec) *RuleBuilderCommand { - if !c.rule.sboxTools { - panic("PackagedTool() requires SandboxTools()") - } - - c.packagedTools = append(c.packagedTools, spec) - c.Text(sboxPathForPackagedToolRel(spec)) - return c -} - -// ImplicitPackagedTool copies the specified tool into the sandbox without modifying the command -// line. It can only be used with tool sandboxing enabled by SandboxTools(). -func (c *RuleBuilderCommand) ImplicitPackagedTool(spec PackagingSpec) *RuleBuilderCommand { - if !c.rule.sboxTools { - panic("ImplicitPackagedTool() requires SandboxTools()") - } - - c.packagedTools = append(c.packagedTools, spec) - return c -} - -// ImplicitPackagedTools copies the specified tools into the sandbox without modifying the command -// line. It can only be used with tool sandboxing enabled by SandboxTools(). -func (c *RuleBuilderCommand) ImplicitPackagedTools(specs []PackagingSpec) *RuleBuilderCommand { - if !c.rule.sboxTools { - panic("ImplicitPackagedTools() requires SandboxTools()") - } - - c.packagedTools = append(c.packagedTools, specs...) - return c -} - // Text adds the specified raw text to the command line. The text should not contain input or output paths or the // rule will not have them listed in its dependencies or outputs. func (c *RuleBuilderCommand) Text(text string) *RuleBuilderCommand { @@ -803,19 +693,7 @@ func (c *RuleBuilderCommand) FlagWithList(flag string, list []string, sep string // RuleBuilder.Tools. func (c *RuleBuilderCommand) Tool(path Path) *RuleBuilderCommand { c.tools = append(c.tools, path) - return c.Text(c.PathForTool(path)) -} - -// Tool adds the specified tool path to the dependencies returned by RuleBuilder.Tools. -func (c *RuleBuilderCommand) ImplicitTool(path Path) *RuleBuilderCommand { - c.tools = append(c.tools, path) - return c -} - -// Tool adds the specified tool path to the dependencies returned by RuleBuilder.Tools. -func (c *RuleBuilderCommand) ImplicitTools(paths Paths) *RuleBuilderCommand { - c.tools = append(c.tools, paths...) - return c + return c.Text(path.String()) } // BuiltTool adds the specified tool path that was built using a host Soong module to the command line. The path will diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index 06ea1242d..e676e4a92 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -436,44 +436,6 @@ func TestRuleBuilder(t *testing.T) { t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) } }) - - t.Run("sbox tools", func(t *testing.T) { - rule := NewRuleBuilder(pctx, ctx).Sbox(PathForOutput(ctx, ""), - PathForOutput(ctx, "sbox.textproto")).SandboxTools() - addCommands(rule) - - wantCommands := []string{ - "__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output Input __SBOX_SANDBOX_DIR__/out/Output __SBOX_SANDBOX_DIR__/out/SymlinkOutput Text __SBOX_SANDBOX_DIR__/tools/src/Tool after command2 old cmd", - "command2 __SBOX_SANDBOX_DIR__/out/depfile2 input2 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/tools/src/tool2", - "command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3", - } - - wantDepMergerCommand := "__SBOX_SANDBOX_DIR__/tools/out/bin/dep_fixer __SBOX_SANDBOX_DIR__/out/DepFile __SBOX_SANDBOX_DIR__/out/depfile __SBOX_SANDBOX_DIR__/out/ImplicitDepFile __SBOX_SANDBOX_DIR__/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.OrderOnlys(), wantOrderOnlys; !reflect.DeepEqual(w, g) { - t.Errorf("\nwant rule.OrderOnlys() = %#v\n got %#v", w, g) - } - - if g, w := rule.depFileMergerCmd(rule.DepFiles()).String(), wantDepMergerCommand; g != w { - t.Errorf("\nwant rule.depFileMergerCmd() = %#v\n got %#v", w, g) - } - }) } func testRuleBuilderFactory() Module { diff --git a/genrule/genrule.go b/genrule/genrule.go index 78af97c34..93938c92b 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -139,6 +139,7 @@ type Module struct { // number of shards the input files are sharded into. taskGenerator taskFunc + deps android.Paths rule blueprint.Rule rawCommands []string @@ -243,8 +244,6 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { } } - var tools android.Paths - var packagedTools []android.PackagingSpec if len(g.properties.Tools) > 0 { seenTools := make(map[string]bool) @@ -252,52 +251,37 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { switch tag := ctx.OtherModuleDependencyTag(module).(type) { case hostToolDependencyTag: tool := ctx.OtherModuleName(module) + var path android.OptionalPath - switch t := module.(type) { - case android.HostToolProvider: - // A HostToolProvider provides the path to a tool, which will be copied - // into the sandbox. + if t, ok := module.(android.HostToolProvider); ok { if !t.(android.Module).Enabled() { if ctx.Config().AllowMissingDependencies() { ctx.AddMissingDependencies([]string{tool}) } else { ctx.ModuleErrorf("depends on disabled module %q", tool) } - return + break } - path := t.HostToolPath() - if !path.Valid() { - ctx.ModuleErrorf("host tool %q missing output file", tool) - return - } - if specs := t.TransitivePackagingSpecs(); specs != nil { - // If the HostToolProvider has PackgingSpecs, which are definitions of the - // required relative locations of the tool and its dependencies, use those - // instead. They will be copied to those relative locations in the sbox - // sandbox. - packagedTools = append(packagedTools, specs...) - // Assume that the first PackagingSpec of the module is the tool. - addLocationLabel(tag.label, []string{android.SboxPathForPackagedTool(specs[0])}) - } else { - tools = append(tools, path.Path()) - addLocationLabel(tag.label, []string{android.SboxPathForTool(ctx, path.Path())}) - } - case bootstrap.GoBinaryTool: - // A GoBinaryTool provides the install path to a tool, which will be copied. + path = t.HostToolPath() + } else if t, ok := module.(bootstrap.GoBinaryTool); ok { if s, err := filepath.Rel(android.PathForOutput(ctx).String(), t.InstallPath()); err == nil { - toolPath := android.PathForOutput(ctx, s) - tools = append(tools, toolPath) - addLocationLabel(tag.label, []string{android.SboxPathForTool(ctx, toolPath)}) + path = android.OptionalPathForPath(android.PathForOutput(ctx, s)) } else { ctx.ModuleErrorf("cannot find path for %q: %v", tool, err) - return + break } - default: + } else { ctx.ModuleErrorf("%q is not a host tool provider", tool) - return + break } - seenTools[tag.label] = true + if path.Valid() { + g.deps = append(g.deps, path.Path()) + addLocationLabel(tag.label, []string{path.Path().String()}) + seenTools[tag.label] = true + } else { + ctx.ModuleErrorf("host tool %q missing output file", tool) + } } }) @@ -321,12 +305,8 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { for _, toolFile := range g.properties.Tool_files { paths := android.PathsForModuleSrc(ctx, []string{toolFile}) - tools = append(tools, paths...) - var sandboxPaths []string - for _, path := range paths { - sandboxPaths = append(sandboxPaths, android.SboxPathForTool(ctx, path)) - } - addLocationLabel(toolFile, sandboxPaths) + g.deps = append(g.deps, paths...) + addLocationLabel(toolFile, paths.Strings()) } var srcFiles android.Paths @@ -378,7 +358,7 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { manifestPath := android.PathForModuleOut(ctx, manifestName) // Use a RuleBuilder to create a rule that runs the command inside an sbox sandbox. - rule := android.NewRuleBuilder(pctx, ctx).Sbox(task.genDir, manifestPath).SandboxTools() + rule := android.NewRuleBuilder(pctx, ctx).Sbox(task.genDir, manifestPath) cmd := rule.Command() for _, out := range task.out { @@ -468,9 +448,8 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { cmd.Text(rawCommand) cmd.ImplicitOutputs(task.out) cmd.Implicits(task.in) - cmd.ImplicitTools(tools) - cmd.ImplicitTools(task.extraTools) - cmd.ImplicitPackagedTools(packagedTools) + cmd.Implicits(g.deps) + cmd.Implicits(task.extraTools) if Bool(g.properties.Depfile) { cmd.ImplicitDepFile(task.depFile) } @@ -637,7 +616,7 @@ func NewGenSrcs() *Module { // TODO(ccross): this RuleBuilder is a hack to be able to call // rule.Command().PathForOutput. Replace this with passing the rule into the // generator. - rule := android.NewRuleBuilder(pctx, ctx).Sbox(genDir, nil).SandboxTools() + rule := android.NewRuleBuilder(pctx, ctx).Sbox(genDir, nil) for _, in := range shard { outFile := android.GenPathWithExt(ctx, finalSubDir, in, String(properties.Output_extension)) @@ -690,8 +669,7 @@ func NewGenSrcs() *Module { outputDepfile = android.PathForModuleGen(ctx, genSubDir, "gensrcs.d") depFixerTool := ctx.Config().HostToolPath(ctx, "dep_fixer") fullCommand += fmt.Sprintf(" && %s -o $(depfile) %s", - android.SboxPathForTool(ctx, depFixerTool), - strings.Join(commandDepFiles, " ")) + depFixerTool.String(), strings.Join(commandDepFiles, " ")) extraTools = append(extraTools, depFixerTool) } diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 3cbfaf19a..8d3cfcbba 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -141,7 +141,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool2", @@ -150,7 +150,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool file", @@ -159,7 +159,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool file fg", @@ -168,7 +168,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "empty location tool and tool file", @@ -178,7 +178,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool", @@ -187,7 +187,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location tool) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool2", @@ -196,7 +196,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location :tool) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/out/bin/tool > __SBOX_SANDBOX_DIR__/out/out", + expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool file", @@ -205,7 +205,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location tool_file1) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool file fg", @@ -214,7 +214,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(location :1tool_file) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "tool files", @@ -223,7 +223,7 @@ func TestGenruleCmd(t *testing.T) { out: ["out"], cmd: "$(locations :tool_files) > $(out)", `, - expect: "__SBOX_SANDBOX_DIR__/tools/src/tool_file1 __SBOX_SANDBOX_DIR__/tools/src/tool_file2 > __SBOX_SANDBOX_DIR__/out/out", + expect: "tool_file1 tool_file2 > __SBOX_SANDBOX_DIR__/out/out", }, { name: "in1", @@ -600,7 +600,7 @@ func TestGenSrcs(t *testing.T) { cmd: "$(location) $(in) > $(out)", `, cmds: []string{ - "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", + "bash -c 'out/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", }, deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"}, files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"}, @@ -614,8 +614,8 @@ func TestGenSrcs(t *testing.T) { shard_size: 2, `, cmds: []string{ - "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", - "bash -c '__SBOX_SANDBOX_DIR__/tools/out/bin/tool in3.txt > __SBOX_SANDBOX_DIR__/out/in3.h'", + "bash -c 'out/tool in1.txt > __SBOX_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'", + "bash -c 'out/tool in3.txt > __SBOX_SANDBOX_DIR__/out/in3.h'", }, deps: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"}, files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"}, @@ -757,7 +757,7 @@ func toolFactory() android.Module { } func (t *testTool) GenerateAndroidBuildActions(ctx android.ModuleContext) { - t.outputFile = ctx.InstallFile(android.PathForModuleInstall(ctx, "bin"), ctx.ModuleName(), android.PathForOutput(ctx, ctx.ModuleName())) + t.outputFile = android.PathForTesting("out", ctx.ModuleName()) } func (t *testTool) HostToolPath() android.OptionalPath {