From ab020a73331e444a62e64b24c2c9facc8572293a Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 12 Mar 2021 17:52:23 -0800 Subject: [PATCH 1/4] Support sandboxing inputs in RuleBuilder When RuleBuilder.SandboxInputs() is called configure sbox to copy all the input files into the sandbox directory and then change the working directory there when running the command. Copying input files into the sandbox directory gets tricky when the input file is the output file from another rule, and could be at an arbitrary, possibly absolute path based on the value of OUT_DIR. They will need to be copied to a directory in the sandbox using the path relative to OUT_DIR. RSP files need special handling, they need to both be copied into the sandbox as an input, rewritten to contain paths as seen in the sandbox, and references to them on the command line need to use sandbox paths. Bug: 182612695 Test: rule_builder_test.go Change-Id: Ic0db961961b186e4ed9b76246881e3f04971825c --- android/rule_builder.go | 138 +++++++++++++++++++++++++++++++---- android/rule_builder_test.go | 93 ++++++++++++++++------- 2 files changed, 189 insertions(+), 42 deletions(-) diff --git a/android/rule_builder.go b/android/rule_builder.go index 75f1b5d9c..cc6c9b49e 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -50,6 +50,7 @@ type RuleBuilder struct { remoteable RemoteRuleSupports outDir WritablePath sboxTools bool + sboxInputs bool sboxManifestPath WritablePath missingDeps []string } @@ -155,6 +156,25 @@ func (r *RuleBuilder) SandboxTools() *RuleBuilder { return r } +// SandboxInputs enables input sandboxing for the rule by copying any referenced inputs into the +// sandbox. It also implies SandboxTools(). +// +// Sandboxing inputs requires RuleBuilder to be aware of all references to input paths. Paths +// that are passed to RuleBuilder outside of the methods that expect inputs, for example +// FlagWithArg, must use RuleBuilderCommand.PathForInput to translate the path to one that matches +// the sandbox layout. +func (r *RuleBuilder) SandboxInputs() *RuleBuilder { + if !r.sbox { + panic("SandboxInputs() must be called after Sbox()") + } + if len(r.commands) > 0 { + panic("SandboxInputs() may not be called after Command()") + } + r.sboxTools = true + r.sboxInputs = 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) { @@ -425,6 +445,26 @@ func (r *RuleBuilder) depFileMergerCmd(depFiles WritablePaths) *RuleBuilderComma Inputs(depFiles.Paths()) } +// composeRspFileContent returns a string that will serve as the contents of the rsp file to pass +// the listed input files to the command running in the sandbox. +func (r *RuleBuilder) composeRspFileContent(rspFileInputs Paths) string { + if r.sboxInputs { + if len(rspFileInputs) > 0 { + // When SandboxInputs is used the paths need to be rewritten to be relative to the sandbox + // directory so that they are valid after sbox chdirs into the sandbox directory. + return proptools.NinjaEscape(strings.Join(r.sboxPathsForInputsRel(rspFileInputs), " ")) + } else { + // If the list of inputs is empty fall back to "$in" so that the rspfilecontent Ninja + // variable is set to something non-empty, otherwise ninja will complain. The inputs + // will be empty (all the non-rspfile inputs are implicits), so $in will evaluate to + // an empty string. + return "$in" + } + } else { + return "$in" + } +} + // Build adds the built command line to the build graph, with dependencies on Inputs and Tools, and output files for // Outputs. func (r *RuleBuilder) Build(name string, desc string) { @@ -511,6 +551,27 @@ func (r *RuleBuilder) Build(name string, desc string) { } } + // If sandboxing inputs is enabled, add copy rules to the manifest to copy each input + // into the sbox directory. + if r.sboxInputs { + for _, input := range inputs { + command.CopyBefore = append(command.CopyBefore, &sbox_proto.Copy{ + From: proto.String(input.String()), + To: proto.String(r.sboxPathForInputRel(input)), + }) + } + + // If using an rsp file copy it into the sbox directory. + if rspFilePath != nil { + command.CopyBefore = append(command.CopyBefore, &sbox_proto.Copy{ + From: proto.String(rspFilePath.String()), + To: proto.String(r.sboxPathForInputRel(rspFilePath)), + }) + } + + command.Chdir = proto.Bool(true) + } + // Add copy rules to the manifest to copy each output file from the sbox directory. // to the output directory after running the commands. sboxOutputs := make([]string, len(outputs)) @@ -580,7 +641,7 @@ func (r *RuleBuilder) Build(name string, desc string) { var rspFile, rspFileContent string if rspFilePath != nil { rspFile = rspFilePath.String() - rspFileContent = "$in" + rspFileContent = r.composeRspFileContent(rspFileInputs) } var pool blueprint.Pool @@ -636,29 +697,45 @@ type RuleBuilderCommand struct { } func (c *RuleBuilderCommand) addInput(path Path) string { - if c.rule.sbox { - if rel, isRel, _ := maybeRelErr(c.rule.outDir.String(), path.String()); isRel { - return filepath.Join(sboxOutDir, rel) - } - } c.inputs = append(c.inputs, path) - return path.String() + return c.PathForInput(path) } -func (c *RuleBuilderCommand) addImplicit(path Path) string { - if c.rule.sbox { - if rel, isRel, _ := maybeRelErr(c.rule.outDir.String(), path.String()); isRel { - return filepath.Join(sboxOutDir, rel) - } - } +func (c *RuleBuilderCommand) addImplicit(path Path) { c.implicits = append(c.implicits, path) - return path.String() } func (c *RuleBuilderCommand) addOrderOnly(path Path) { c.orderOnlys = append(c.orderOnlys, path) } +// PathForInput takes an input path and returns the appropriate path to use on the command line. If +// sbox was enabled via a call to RuleBuilder.Sbox() and the path was an output path it returns a +// path with the placeholder prefix used for outputs in sbox. If sbox is not enabled it returns the +// original path. +func (c *RuleBuilderCommand) PathForInput(path Path) string { + if c.rule.sbox { + rel, inSandbox := c.rule._sboxPathForInputRel(path) + if inSandbox { + rel = filepath.Join(sboxSandboxBaseDir, rel) + } + return rel + } + return path.String() +} + +// PathsForInputs takes a list of input paths and returns the appropriate paths to use on the +// command line. If sbox was enabled via a call to RuleBuilder.Sbox() a path was an output path, it +// returns the path with the placeholder prefix used for outputs in sbox. If sbox is not enabled it +// returns the original paths. +func (c *RuleBuilderCommand) PathsForInputs(paths Paths) []string { + ret := make([]string, len(paths)) + for i, path := range paths { + ret[i] = c.PathForInput(path) + } + return ret +} + // PathForOutput takes an output path and returns the appropriate path to use on the command // line. If sbox was enabled via a call to RuleBuilder.Sbox(), it returns a path with the // placeholder prefix used for outputs in sbox. If sbox is not enabled it returns the @@ -690,6 +767,37 @@ func sboxPathForToolRel(ctx BuilderContext, path Path) string { return filepath.Join(sboxToolsSubDir, "src", path.String()) } +func (r *RuleBuilder) _sboxPathForInputRel(path Path) (rel string, inSandbox bool) { + // Errors will be handled in RuleBuilder.Build where we have a context to report them + rel, isRelSboxOut, _ := maybeRelErr(r.outDir.String(), path.String()) + if isRelSboxOut { + return filepath.Join(sboxOutSubDir, rel), true + } + if r.sboxInputs { + // When sandboxing inputs all inputs have to be copied into the sandbox. Input files that + // are outputs of other rules could be an arbitrary absolute path if OUT_DIR is set, so they + // will be copied to relative paths under __SBOX_OUT_DIR__/out. + rel, isRelOut, _ := maybeRelErr(PathForOutput(r.ctx).String(), path.String()) + if isRelOut { + return filepath.Join(sboxOutSubDir, rel), true + } + } + return path.String(), false +} + +func (r *RuleBuilder) sboxPathForInputRel(path Path) string { + rel, _ := r._sboxPathForInputRel(path) + return rel +} + +func (r *RuleBuilder) sboxPathsForInputsRel(paths Paths) []string { + ret := make([]string, len(paths)) + for i, path := range paths { + ret[i] = r.sboxPathForInputRel(path) + } + return ret +} + // 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. @@ -1053,7 +1161,7 @@ func (c *RuleBuilderCommand) FlagWithRspFileInputList(flag string, rspFile Writa } } - c.FlagWithArg(flag, rspFile.String()) + c.FlagWithArg(flag, c.PathForInput(rspFile)) return c } diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index bd3582070..3415aed1f 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -296,35 +296,40 @@ func TestRuleBuilder(t *testing.T) { "input3": nil, } - pathCtx := PathContextForTesting(TestConfig("out", nil, "", fs)) + pathCtx := PathContextForTesting(TestConfig("out_local", nil, "", fs)) ctx := builderContextForTests{ PathContext: pathCtx, } addCommands := func(rule *RuleBuilder) { cmd := rule.Command(). - DepFile(PathForOutput(ctx, "DepFile")). + DepFile(PathForOutput(ctx, "module/DepFile")). Flag("Flag"). FlagWithArg("FlagWithArg=", "arg"). - FlagWithDepFile("FlagWithDepFile=", PathForOutput(ctx, "depfile")). + FlagWithDepFile("FlagWithDepFile=", PathForOutput(ctx, "module/depfile")). FlagWithInput("FlagWithInput=", PathForSource(ctx, "input")). - FlagWithOutput("FlagWithOutput=", PathForOutput(ctx, "output")). + FlagWithOutput("FlagWithOutput=", PathForOutput(ctx, "module/output")). + FlagWithRspFileInputList("FlagWithRspFileInputList=", PathForOutput(ctx, "rsp"), + Paths{ + PathForSource(ctx, "RspInput"), + PathForOutput(ctx, "other/RspOutput2"), + }). Implicit(PathForSource(ctx, "Implicit")). - ImplicitDepFile(PathForOutput(ctx, "ImplicitDepFile")). - ImplicitOutput(PathForOutput(ctx, "ImplicitOutput")). + ImplicitDepFile(PathForOutput(ctx, "module/ImplicitDepFile")). + ImplicitOutput(PathForOutput(ctx, "module/ImplicitOutput")). Input(PathForSource(ctx, "Input")). - Output(PathForOutput(ctx, "Output")). + Output(PathForOutput(ctx, "module/Output")). OrderOnly(PathForSource(ctx, "OrderOnly")). - SymlinkOutput(PathForOutput(ctx, "SymlinkOutput")). - ImplicitSymlinkOutput(PathForOutput(ctx, "ImplicitSymlinkOutput")). + SymlinkOutput(PathForOutput(ctx, "module/SymlinkOutput")). + ImplicitSymlinkOutput(PathForOutput(ctx, "module/ImplicitSymlinkOutput")). Text("Text"). Tool(PathForSource(ctx, "Tool")) rule.Command(). Text("command2"). - DepFile(PathForOutput(ctx, "depfile2")). + DepFile(PathForOutput(ctx, "module/depfile2")). Input(PathForSource(ctx, "input2")). - Output(PathForOutput(ctx, "output2")). + Output(PathForOutput(ctx, "module/output2")). OrderOnlys(PathsForSource(ctx, []string{"OrderOnlys"})). Tool(PathForSource(ctx, "tool2")) @@ -337,32 +342,46 @@ func TestRuleBuilder(t *testing.T) { rule.Command(). Text("command3"). Input(PathForSource(ctx, "input3")). - Input(PathForOutput(ctx, "output2")). - Output(PathForOutput(ctx, "output3")) + Input(PathForOutput(ctx, "module/output2")). + Output(PathForOutput(ctx, "module/output3")). + Text(cmd.PathForInput(PathForSource(ctx, "input3"))). + Text(cmd.PathForOutput(PathForOutput(ctx, "module/output2"))) } wantInputs := PathsForSource(ctx, []string{"Implicit", "Input", "input", "input2", "input3"}) - wantOutputs := PathsForOutput(ctx, []string{"ImplicitOutput", "ImplicitSymlinkOutput", "Output", "SymlinkOutput", "output", "output2", "output3"}) - wantDepFiles := PathsForOutput(ctx, []string{"DepFile", "depfile", "ImplicitDepFile", "depfile2"}) + wantRspFileInputs := Paths{PathForSource(ctx, "RspInput"), + PathForOutput(ctx, "other/RspOutput2")} + wantOutputs := PathsForOutput(ctx, []string{ + "module/ImplicitOutput", "module/ImplicitSymlinkOutput", "module/Output", "module/SymlinkOutput", + "module/output", "module/output2", "module/output3"}) + wantDepFiles := PathsForOutput(ctx, []string{ + "module/DepFile", "module/depfile", "module/ImplicitDepFile", "module/depfile2"}) wantTools := PathsForSource(ctx, []string{"Tool", "tool2"}) wantOrderOnlys := PathsForSource(ctx, []string{"OrderOnly", "OrderOnlys"}) - wantSymlinkOutputs := PathsForOutput(ctx, []string{"ImplicitSymlinkOutput", "SymlinkOutput"}) + wantSymlinkOutputs := PathsForOutput(ctx, []string{ + "module/ImplicitSymlinkOutput", "module/SymlinkOutput"}) t.Run("normal", func(t *testing.T) { rule := NewRuleBuilder(pctx, ctx) addCommands(rule) wantCommands := []string{ - "out/DepFile Flag FlagWithArg=arg FlagWithDepFile=out/depfile FlagWithInput=input FlagWithOutput=out/output Input out/Output out/SymlinkOutput Text Tool after command2 old cmd", - "command2 out/depfile2 input2 out/output2 tool2", - "command3 input3 out/output2 out/output3", + "out_local/module/DepFile Flag FlagWithArg=arg FlagWithDepFile=out_local/module/depfile " + + "FlagWithInput=input FlagWithOutput=out_local/module/output FlagWithRspFileInputList=out_local/rsp " + + "Input out_local/module/Output out_local/module/SymlinkOutput Text Tool after command2 old cmd", + "command2 out_local/module/depfile2 input2 out_local/module/output2 tool2", + "command3 input3 out_local/module/output2 out_local/module/output3 input3 out_local/module/output2", } - wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer out/DepFile out/depfile out/ImplicitDepFile out/depfile2" + wantDepMergerCommand := "out_local/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer " + + "out_local/module/DepFile out_local/module/depfile out_local/module/ImplicitDepFile out_local/module/depfile2" + + wantRspFileContent := "$in" AssertDeepEquals(t, "rule.Commands()", wantCommands, rule.Commands()) AssertDeepEquals(t, "rule.Inputs()", wantInputs, rule.Inputs()) + AssertDeepEquals(t, "rule.RspfileInputs()", wantRspFileInputs, rule.RspFileInputs()) AssertDeepEquals(t, "rule.Outputs()", wantOutputs, rule.Outputs()) AssertDeepEquals(t, "rule.SymlinkOutputs()", wantSymlinkOutputs, rule.SymlinkOutputs()) AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles()) @@ -370,54 +389,74 @@ func TestRuleBuilder(t *testing.T) { AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys()) AssertSame(t, "rule.depFileMergerCmd()", wantDepMergerCommand, rule.depFileMergerCmd(rule.DepFiles()).String()) + + AssertSame(t, "rule.composeRspFileContent()", wantRspFileContent, rule.composeRspFileContent(rule.RspFileInputs())) }) t.Run("sbox", func(t *testing.T) { - rule := NewRuleBuilder(pctx, ctx).Sbox(PathForOutput(ctx, ""), + rule := NewRuleBuilder(pctx, ctx).Sbox(PathForOutput(ctx, "module"), PathForOutput(ctx, "sbox.textproto")) 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 Tool after command2 old cmd", + "__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile " + + "FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output " + + "FlagWithRspFileInputList=out_local/rsp Input __SBOX_SANDBOX_DIR__/out/Output " + + "__SBOX_SANDBOX_DIR__/out/SymlinkOutput Text Tool after command2 old cmd", "command2 __SBOX_SANDBOX_DIR__/out/depfile2 input2 __SBOX_SANDBOX_DIR__/out/output2 tool2", - "command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3", + "command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3 input3 __SBOX_SANDBOX_DIR__/out/output2", } - wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer __SBOX_SANDBOX_DIR__/out/DepFile __SBOX_SANDBOX_DIR__/out/depfile __SBOX_SANDBOX_DIR__/out/ImplicitDepFile __SBOX_SANDBOX_DIR__/out/depfile2" + wantDepMergerCommand := "out_local/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer __SBOX_SANDBOX_DIR__/out/DepFile __SBOX_SANDBOX_DIR__/out/depfile __SBOX_SANDBOX_DIR__/out/ImplicitDepFile __SBOX_SANDBOX_DIR__/out/depfile2" + + wantRspFileContent := "$in" AssertDeepEquals(t, "rule.Commands()", wantCommands, rule.Commands()) AssertDeepEquals(t, "rule.Inputs()", wantInputs, rule.Inputs()) + AssertDeepEquals(t, "rule.RspfileInputs()", wantRspFileInputs, rule.RspFileInputs()) AssertDeepEquals(t, "rule.Outputs()", wantOutputs, rule.Outputs()) + AssertDeepEquals(t, "rule.SymlinkOutputs()", wantSymlinkOutputs, rule.SymlinkOutputs()) AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles()) AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools()) AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys()) AssertSame(t, "rule.depFileMergerCmd()", wantDepMergerCommand, rule.depFileMergerCmd(rule.DepFiles()).String()) + + AssertSame(t, "rule.composeRspFileContent()", wantRspFileContent, rule.composeRspFileContent(rule.RspFileInputs())) }) t.Run("sbox tools", func(t *testing.T) { - rule := NewRuleBuilder(pctx, ctx).Sbox(PathForOutput(ctx, ""), + rule := NewRuleBuilder(pctx, ctx).Sbox(PathForOutput(ctx, "module"), 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", + "__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile " + + "FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output " + + "FlagWithRspFileInputList=out_local/rsp 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", + "command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3 input3 __SBOX_SANDBOX_DIR__/out/output2", } 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" + wantRspFileContent := "$in" + AssertDeepEquals(t, "rule.Commands()", wantCommands, rule.Commands()) AssertDeepEquals(t, "rule.Inputs()", wantInputs, rule.Inputs()) + AssertDeepEquals(t, "rule.RspfileInputs()", wantRspFileInputs, rule.RspFileInputs()) AssertDeepEquals(t, "rule.Outputs()", wantOutputs, rule.Outputs()) + AssertDeepEquals(t, "rule.SymlinkOutputs()", wantSymlinkOutputs, rule.SymlinkOutputs()) AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles()) AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools()) AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys()) AssertSame(t, "rule.depFileMergerCmd()", wantDepMergerCommand, rule.depFileMergerCmd(rule.DepFiles()).String()) + + AssertSame(t, "rule.composeRspFileContent()", wantRspFileContent, rule.composeRspFileContent(rule.RspFileInputs())) }) } From 77cdcfdeafd383ef1f1214226c47eb20c902a28f Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 12 Mar 2021 11:28:25 -0800 Subject: [PATCH 2/4] Move android package on top of remotexec Remove the references to the android package in remotexec so that the android package can reference the remoteexec package. This will allow RuleBuilder to integrate directly with remoteexec. Bug: 182612695 Test: m checkbuild Change-Id: I15be5ef126d8aacbd605518638f341daf6f31bb3 --- android/Android.bp | 1 + android/config.go | 5 +++ android/defs.go | 4 +++ android/package_ctx.go | 39 ++++++++++++++++++++++ cc/builder.go | 11 +++---- cc/config/global.go | 14 ++++---- java/app_builder.go | 2 +- java/builder.go | 9 +++--- java/config/config.go | 16 ++++----- java/dex.go | 4 +-- java/droiddoc.go | 2 +- java/lint.go | 2 +- remoteexec/Android.bp | 4 --- remoteexec/remoteexec.go | 61 ++--------------------------------- remoteexec/remoteexec_test.go | 10 +++--- 15 files changed, 85 insertions(+), 99 deletions(-) diff --git a/android/Android.bp b/android/Android.bp index 9f6ae02af..240632173 100644 --- a/android/Android.bp +++ b/android/Android.bp @@ -13,6 +13,7 @@ bootstrap_go_package { "soong-android-soongconfig", "soong-bazel", "soong-cquery", + "soong-remoteexec", "soong-shared", "soong-ui-metrics_proto", ], diff --git a/android/config.go b/android/config.go index e09173155..181141f7b 100644 --- a/android/config.go +++ b/android/config.go @@ -35,6 +35,7 @@ import ( "github.com/google/blueprint/proptools" "android/soong/android/soongconfig" + "android/soong/remoteexec" ) // Bool re-exports proptools.Bool for the android package. @@ -1754,3 +1755,7 @@ func (c *config) NonUpdatableBootJars() ConfiguredJarList { func (c *config) UpdatableBootJars() ConfiguredJarList { return c.productVariables.UpdatableBootJars } + +func (c *config) RBEWrapper() string { + return c.GetenvWithDefault("RBE_WRAPPER", remoteexec.DefaultWrapperPath) +} diff --git a/android/defs.go b/android/defs.go index 1a7c459ec..b3ff376d1 100644 --- a/android/defs.go +++ b/android/defs.go @@ -124,6 +124,10 @@ var ( func init() { pctx.Import("github.com/google/blueprint/bootstrap") + + pctx.VariableFunc("RBEWrapper", func(ctx PackageVarContext) string { + return ctx.Config().RBEWrapper() + }) } var ( diff --git a/android/package_ctx.go b/android/package_ctx.go index 6d0fcb33f..c19debbb4 100644 --- a/android/package_ctx.go +++ b/android/package_ctx.go @@ -19,6 +19,8 @@ import ( "strings" "github.com/google/blueprint" + + "android/soong/remoteexec" ) // PackageContext is a wrapper for blueprint.PackageContext that adds @@ -260,3 +262,40 @@ func (p PackageContext) AndroidRemoteStaticRule(name string, supports RemoteRule return params, nil }, argNames...) } + +// RemoteStaticRules returns a pair of rules based on the given RuleParams, where the first rule is a +// locally executable rule and the second rule is a remotely executable rule. commonArgs are args +// used for both the local and remotely executable rules. reArgs are used only for remote +// execution. +func (p PackageContext) RemoteStaticRules(name string, ruleParams blueprint.RuleParams, reParams *remoteexec.REParams, commonArgs []string, reArgs []string) (blueprint.Rule, blueprint.Rule) { + ruleParamsRE := ruleParams + ruleParams.Command = strings.ReplaceAll(ruleParams.Command, "$reTemplate", "") + ruleParamsRE.Command = strings.ReplaceAll(ruleParamsRE.Command, "$reTemplate", reParams.Template()) + + return p.AndroidStaticRule(name, ruleParams, commonArgs...), + p.AndroidRemoteStaticRule(name+"RE", RemoteRuleSupports{RBE: true}, ruleParamsRE, append(commonArgs, reArgs...)...) +} + +// MultiCommandStaticRules returns a pair of rules based on the given RuleParams, where the first +// rule is a locally executable rule and the second rule is a remotely executable rule. This +// function supports multiple remote execution wrappers placed in the template when commands are +// chained together with &&. commonArgs are args used for both the local and remotely executable +// rules. reArgs are args used only for remote execution. +func (p PackageContext) MultiCommandRemoteStaticRules(name string, ruleParams blueprint.RuleParams, reParams map[string]*remoteexec.REParams, commonArgs []string, reArgs []string) (blueprint.Rule, blueprint.Rule) { + ruleParamsRE := ruleParams + for k, v := range reParams { + ruleParams.Command = strings.ReplaceAll(ruleParams.Command, k, "") + ruleParamsRE.Command = strings.ReplaceAll(ruleParamsRE.Command, k, v.Template()) + } + + return p.AndroidStaticRule(name, ruleParams, commonArgs...), + p.AndroidRemoteStaticRule(name+"RE", RemoteRuleSupports{RBE: true}, ruleParamsRE, append(commonArgs, reArgs...)...) +} + +// StaticVariableWithEnvOverride creates a static variable that evaluates to the value of the given +// environment variable if set, otherwise the given default. +func (p PackageContext) StaticVariableWithEnvOverride(name, envVar, defaultVal string) blueprint.Variable { + return p.VariableFunc(name, func(ctx PackageVarContext) string { + return ctx.Config().GetenvWithDefault(envVar, defaultVal) + }) +} diff --git a/cc/builder.go b/cc/builder.go index 9cd78d59a..273cdd3f4 100644 --- a/cc/builder.go +++ b/cc/builder.go @@ -59,7 +59,7 @@ var ( // Rules to invoke ld to link binaries. Uses a .rsp file to list dependencies, as there may // be many. - ld, ldRE = remoteexec.StaticRules(pctx, "ld", + ld, ldRE = pctx.RemoteStaticRules("ld", blueprint.RuleParams{ Command: "$reTemplate$ldCmd ${crtBegin} @${out}.rsp " + "${libFlags} ${crtEnd} -o ${out} ${ldFlags} ${extraLibFlags}", @@ -80,7 +80,7 @@ var ( }, []string{"ldCmd", "crtBegin", "libFlags", "crtEnd", "ldFlags", "extraLibFlags"}, []string{"implicitInputs", "implicitOutputs"}) // Rules for .o files to combine to other .o files, using ld partial linking. - partialLd, partialLdRE = remoteexec.StaticRules(pctx, "partialLd", + partialLd, partialLdRE = pctx.RemoteStaticRules("partialLd", blueprint.RuleParams{ // Without -no-pie, clang 7.0 adds -pie to link Android files, // but -r and -pie cannot be used together. @@ -189,7 +189,7 @@ var ( "crossCompile", "format") // Rule for invoking clang-tidy (a clang-based linter). - clangTidy, clangTidyRE = remoteexec.StaticRules(pctx, "clangTidy", + clangTidy, clangTidyRE = pctx.RemoteStaticRules("clangTidy", blueprint.RuleParams{ Command: "rm -f $out && $reTemplate${config.ClangBin}/clang-tidy $tidyFlags $in -- $cFlags && touch $out", CommandDeps: []string{"${config.ClangBin}/clang-tidy"}, @@ -228,7 +228,7 @@ var ( _ = pctx.SourcePathVariable("sAbiDumper", "prebuilts/clang-tools/${config.HostPrebuiltTag}/bin/header-abi-dumper") // -w has been added since header-abi-dumper does not need to produce any sort of diagnostic information. - sAbiDump, sAbiDumpRE = remoteexec.StaticRules(pctx, "sAbiDump", + sAbiDump, sAbiDumpRE = pctx.RemoteStaticRules("sAbiDump", blueprint.RuleParams{ Command: "rm -f $out && $reTemplate$sAbiDumper -o ${out} $in $exportDirs -- $cFlags -w -isystem prebuilts/clang-tools/${config.HostPrebuiltTag}/clang-headers", CommandDeps: []string{"$sAbiDumper"}, @@ -246,7 +246,7 @@ var ( // Rule to combine .dump sAbi dump files from multiple source files into a single .ldump // sAbi dump file. - sAbiLink, sAbiLinkRE = remoteexec.StaticRules(pctx, "sAbiLink", + sAbiLink, sAbiLinkRE = pctx.RemoteStaticRules("sAbiLink", blueprint.RuleParams{ Command: "$reTemplate$sAbiLinker -o ${out} $symbolFilter -arch $arch $exportedHeaderFlags @${out}.rsp ", CommandDeps: []string{"$sAbiLinker"}, @@ -331,7 +331,6 @@ func init() { pctx.StaticVariable("relPwd", PwdPrefix()) pctx.HostBinToolVariable("SoongZipCmd", "soong_zip") - pctx.Import("android/soong/remoteexec") } // builderFlags contains various types of command line flags (and settings) for use in building diff --git a/cc/config/global.go b/cc/config/global.go index e60bb3d51..cb7d17da4 100644 --- a/cc/config/global.go +++ b/cc/config/global.go @@ -270,13 +270,13 @@ func init() { return "" }) - pctx.VariableFunc("RECXXPool", remoteexec.EnvOverrideFunc("RBE_CXX_POOL", remoteexec.DefaultPool)) - pctx.VariableFunc("RECXXLinksPool", remoteexec.EnvOverrideFunc("RBE_CXX_LINKS_POOL", remoteexec.DefaultPool)) - pctx.VariableFunc("REClangTidyPool", remoteexec.EnvOverrideFunc("RBE_CLANG_TIDY_POOL", remoteexec.DefaultPool)) - pctx.VariableFunc("RECXXLinksExecStrategy", remoteexec.EnvOverrideFunc("RBE_CXX_LINKS_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) - pctx.VariableFunc("REClangTidyExecStrategy", remoteexec.EnvOverrideFunc("RBE_CLANG_TIDY_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) - pctx.VariableFunc("REAbiDumperExecStrategy", remoteexec.EnvOverrideFunc("RBE_ABI_DUMPER_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) - pctx.VariableFunc("REAbiLinkerExecStrategy", remoteexec.EnvOverrideFunc("RBE_ABI_LINKER_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) + pctx.StaticVariableWithEnvOverride("RECXXPool", "RBE_CXX_POOL", remoteexec.DefaultPool) + pctx.StaticVariableWithEnvOverride("RECXXLinksPool", "RBE_CXX_LINKS_POOL", remoteexec.DefaultPool) + pctx.StaticVariableWithEnvOverride("REClangTidyPool", "RBE_CLANG_TIDY_POOL", remoteexec.DefaultPool) + pctx.StaticVariableWithEnvOverride("RECXXLinksExecStrategy", "RBE_CXX_LINKS_EXEC_STRATEGY", remoteexec.LocalExecStrategy) + pctx.StaticVariableWithEnvOverride("REClangTidyExecStrategy", "RBE_CLANG_TIDY_EXEC_STRATEGY", remoteexec.LocalExecStrategy) + pctx.StaticVariableWithEnvOverride("REAbiDumperExecStrategy", "RBE_ABI_DUMPER_EXEC_STRATEGY", remoteexec.LocalExecStrategy) + pctx.StaticVariableWithEnvOverride("REAbiLinkerExecStrategy", "RBE_ABI_LINKER_EXEC_STRATEGY", remoteexec.LocalExecStrategy) } var HostPrebuiltTag = pctx.VariableConfigMethod("HostPrebuiltTag", android.Config.PrebuiltOS) diff --git a/java/app_builder.go b/java/app_builder.go index b53c15ab4..4a18dcada 100644 --- a/java/app_builder.go +++ b/java/app_builder.go @@ -30,7 +30,7 @@ import ( ) var ( - Signapk, SignapkRE = remoteexec.StaticRules(pctx, "signapk", + Signapk, SignapkRE = pctx.RemoteStaticRules("signapk", blueprint.RuleParams{ Command: `rm -f $out && $reTemplate${config.JavaCmd} ${config.JavaVmFlags} -Djava.library.path=$$(dirname ${config.SignapkJniLibrary}) ` + `-jar ${config.SignapkCmd} $flags $certificates $in $out`, diff --git a/java/builder.go b/java/builder.go index 33206ceeb..fc740a831 100644 --- a/java/builder.go +++ b/java/builder.go @@ -40,7 +40,7 @@ var ( // (if the rule produces .class files) or a .srcjar file (if the rule produces .java files). // .srcjar files are unzipped into a temporary directory when compiled with javac. // TODO(b/143658984): goma can't handle the --system argument to javac. - javac, javacRE = remoteexec.MultiCommandStaticRules(pctx, "javac", + javac, javacRE = pctx.MultiCommandRemoteStaticRules("javac", blueprint.RuleParams{ Command: `rm -rf "$outDir" "$annoDir" "$srcJarDir" "$out" && mkdir -p "$outDir" "$annoDir" "$srcJarDir" && ` + `${config.ZipSyncCmd} -d $srcJarDir -l $srcJarDir/list -f "*.java" $srcJars && ` + @@ -129,7 +129,7 @@ var ( }, "abis", "allow-prereleased", "screen-densities", "sdk-version", "stem", "apkcerts", "partition") - turbine, turbineRE = remoteexec.StaticRules(pctx, "turbine", + turbine, turbineRE = pctx.RemoteStaticRules("turbine", blueprint.RuleParams{ Command: `rm -rf "$outDir" && mkdir -p "$outDir" && ` + `$reTemplate${config.JavaCmd} ${config.JavaVmFlags} -jar ${config.TurbineJar} --output $out.tmp ` + @@ -157,7 +157,7 @@ var ( Platform: map[string]string{remoteexec.PoolKey: "${config.REJavaPool}"}, }, []string{"javacFlags", "bootClasspath", "classpath", "srcJars", "outDir", "javaVersion"}, []string{"implicits"}) - jar, jarRE = remoteexec.StaticRules(pctx, "jar", + jar, jarRE = pctx.RemoteStaticRules("jar", blueprint.RuleParams{ Command: `$reTemplate${config.SoongZipCmd} -jar -o $out @$out.rsp`, CommandDeps: []string{"${config.SoongZipCmd}"}, @@ -172,7 +172,7 @@ var ( Platform: map[string]string{remoteexec.PoolKey: "${config.REJavaPool}"}, }, []string{"jarArgs"}, nil) - zip, zipRE = remoteexec.StaticRules(pctx, "zip", + zip, zipRE = pctx.RemoteStaticRules("zip", blueprint.RuleParams{ Command: `${config.SoongZipCmd} -o $out @$out.rsp`, CommandDeps: []string{"${config.SoongZipCmd}"}, @@ -244,7 +244,6 @@ var ( func init() { pctx.Import("android/soong/android") pctx.Import("android/soong/java/config") - pctx.Import("android/soong/remoteexec") } type javaBuilderFlags struct { diff --git a/java/config/config.go b/java/config/config.go index 31e2b0ffe..30c6f91aa 100644 --- a/java/config/config.go +++ b/java/config/config.go @@ -149,14 +149,14 @@ func init() { pctx.HostBinToolVariable("SoongJavacWrapper", "soong_javac_wrapper") pctx.HostBinToolVariable("DexpreoptGen", "dexpreopt_gen") - pctx.VariableFunc("REJavaPool", remoteexec.EnvOverrideFunc("RBE_JAVA_POOL", "java16")) - pctx.VariableFunc("REJavacExecStrategy", remoteexec.EnvOverrideFunc("RBE_JAVAC_EXEC_STRATEGY", remoteexec.RemoteLocalFallbackExecStrategy)) - pctx.VariableFunc("RED8ExecStrategy", remoteexec.EnvOverrideFunc("RBE_D8_EXEC_STRATEGY", remoteexec.RemoteLocalFallbackExecStrategy)) - pctx.VariableFunc("RER8ExecStrategy", remoteexec.EnvOverrideFunc("RBE_R8_EXEC_STRATEGY", remoteexec.RemoteLocalFallbackExecStrategy)) - pctx.VariableFunc("RETurbineExecStrategy", remoteexec.EnvOverrideFunc("RBE_TURBINE_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) - pctx.VariableFunc("RESignApkExecStrategy", remoteexec.EnvOverrideFunc("RBE_SIGNAPK_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) - pctx.VariableFunc("REJarExecStrategy", remoteexec.EnvOverrideFunc("RBE_JAR_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) - pctx.VariableFunc("REZipExecStrategy", remoteexec.EnvOverrideFunc("RBE_ZIP_EXEC_STRATEGY", remoteexec.LocalExecStrategy)) + pctx.StaticVariableWithEnvOverride("REJavaPool", "RBE_JAVA_POOL", "java16") + pctx.StaticVariableWithEnvOverride("REJavacExecStrategy", "RBE_JAVAC_EXEC_STRATEGY", remoteexec.RemoteLocalFallbackExecStrategy) + pctx.StaticVariableWithEnvOverride("RED8ExecStrategy", "RBE_D8_EXEC_STRATEGY", remoteexec.RemoteLocalFallbackExecStrategy) + pctx.StaticVariableWithEnvOverride("RER8ExecStrategy", "RBE_R8_EXEC_STRATEGY", remoteexec.RemoteLocalFallbackExecStrategy) + pctx.StaticVariableWithEnvOverride("RETurbineExecStrategy", "RBE_TURBINE_EXEC_STRATEGY", remoteexec.LocalExecStrategy) + pctx.StaticVariableWithEnvOverride("RESignApkExecStrategy", "RBE_SIGNAPK_EXEC_STRATEGY", remoteexec.LocalExecStrategy) + pctx.StaticVariableWithEnvOverride("REJarExecStrategy", "RBE_JAR_EXEC_STRATEGY", remoteexec.LocalExecStrategy) + pctx.StaticVariableWithEnvOverride("REZipExecStrategy", "RBE_ZIP_EXEC_STRATEGY", remoteexec.LocalExecStrategy) pctx.HostJavaToolVariable("JacocoCLIJar", "jacoco-cli.jar") diff --git a/java/dex.go b/java/dex.go index b2a998f78..b042f133f 100644 --- a/java/dex.go +++ b/java/dex.go @@ -83,7 +83,7 @@ func (d *dexer) effectiveOptimizeEnabled() bool { return BoolDefault(d.dexProperties.Optimize.Enabled, d.dexProperties.Optimize.EnabledByDefault) } -var d8, d8RE = remoteexec.MultiCommandStaticRules(pctx, "d8", +var d8, d8RE = pctx.MultiCommandRemoteStaticRules("d8", blueprint.RuleParams{ Command: `rm -rf "$outDir" && mkdir -p "$outDir" && ` + `$d8Template${config.D8Cmd} ${config.DexFlags} --output $outDir $d8Flags $in && ` + @@ -111,7 +111,7 @@ var d8, d8RE = remoteexec.MultiCommandStaticRules(pctx, "d8", }, }, []string{"outDir", "d8Flags", "zipFlags"}, nil) -var r8, r8RE = remoteexec.MultiCommandStaticRules(pctx, "r8", +var r8, r8RE = pctx.MultiCommandRemoteStaticRules("r8", blueprint.RuleParams{ Command: `rm -rf "$outDir" && mkdir -p "$outDir" && ` + `rm -f "$outDict" && rm -rf "${outUsageDir}" && ` + diff --git a/java/droiddoc.go b/java/droiddoc.go index da13c621b..706111ffd 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -1235,7 +1235,7 @@ func metalavaCmd(ctx android.ModuleContext, rule *android.RuleBuilder, javaVersi ToolchainInputs: []string{config.JavaCmd(ctx).String()}, Platform: map[string]string{remoteexec.PoolKey: pool}, EnvironmentVariables: []string{"ANDROID_SDK_HOME"}, - }).NoVarTemplate(ctx.Config())) + }).NoVarTemplate(ctx.Config().RBEWrapper())) } cmd.BuiltTool("metalava"). diff --git a/java/lint.go b/java/lint.go index fccd1a552..9743c7a4e 100644 --- a/java/lint.go +++ b/java/lint.go @@ -430,7 +430,7 @@ func (l *linter) lint(ctx android.ModuleContext) { "LANG", }, Platform: map[string]string{remoteexec.PoolKey: pool}, - }).NoVarTemplate(ctx.Config())) + }).NoVarTemplate(ctx.Config().RBEWrapper())) } cmd.BuiltTool("lint"). diff --git a/remoteexec/Android.bp b/remoteexec/Android.bp index 9f75df5a3..0d5516852 100644 --- a/remoteexec/Android.bp +++ b/remoteexec/Android.bp @@ -5,10 +5,6 @@ package { bootstrap_go_package { name: "soong-remoteexec", pkgPath: "android/soong/remoteexec", - deps: [ - "blueprint", - "soong-android", - ], srcs: [ "remoteexec.go", ], diff --git a/remoteexec/remoteexec.go b/remoteexec/remoteexec.go index 5f0426a7f..166f68c54 100644 --- a/remoteexec/remoteexec.go +++ b/remoteexec/remoteexec.go @@ -17,10 +17,6 @@ package remoteexec import ( "sort" "strings" - - "android/soong/android" - - "github.com/google/blueprint" ) const ( @@ -56,7 +52,6 @@ const ( var ( defaultLabels = map[string]string{"type": "tool"} defaultExecStrategy = LocalExecStrategy - pctx = android.NewPackageContext("android/soong/remoteexec") ) // REParams holds information pertinent to the remote execution of a rule. @@ -87,28 +82,18 @@ type REParams struct { } func init() { - pctx.VariableFunc("Wrapper", func(ctx android.PackageVarContext) string { - return wrapper(ctx.Config()) - }) -} - -func wrapper(cfg android.Config) string { - if override := cfg.Getenv("RBE_WRAPPER"); override != "" { - return override - } - return DefaultWrapperPath } // Template generates the remote execution wrapper template to be added as a prefix to the rule's // command. func (r *REParams) Template() string { - return "${remoteexec.Wrapper}" + r.wrapperArgs() + return "${android.RBEWrapper}" + r.wrapperArgs() } // NoVarTemplate generates the remote execution wrapper template without variables, to be used in // RuleBuilder. -func (r *REParams) NoVarTemplate(cfg android.Config) string { - return wrapper(cfg) + r.wrapperArgs() +func (r *REParams) NoVarTemplate(wrapper string) string { + return wrapper + r.wrapperArgs() } func (r *REParams) wrapperArgs() string { @@ -171,43 +156,3 @@ func (r *REParams) wrapperArgs() string { return args + " -- " } - -// StaticRules returns a pair of rules based on the given RuleParams, where the first rule is a -// locally executable rule and the second rule is a remotely executable rule. commonArgs are args -// used for both the local and remotely executable rules. reArgs are used only for remote -// execution. -func StaticRules(ctx android.PackageContext, name string, ruleParams blueprint.RuleParams, reParams *REParams, commonArgs []string, reArgs []string) (blueprint.Rule, blueprint.Rule) { - ruleParamsRE := ruleParams - ruleParams.Command = strings.ReplaceAll(ruleParams.Command, "$reTemplate", "") - ruleParamsRE.Command = strings.ReplaceAll(ruleParamsRE.Command, "$reTemplate", reParams.Template()) - - return ctx.AndroidStaticRule(name, ruleParams, commonArgs...), - ctx.AndroidRemoteStaticRule(name+"RE", android.RemoteRuleSupports{RBE: true}, ruleParamsRE, append(commonArgs, reArgs...)...) -} - -// MultiCommandStaticRules returns a pair of rules based on the given RuleParams, where the first -// rule is a locally executable rule and the second rule is a remotely executable rule. This -// function supports multiple remote execution wrappers placed in the template when commands are -// chained together with &&. commonArgs are args used for both the local and remotely executable -// rules. reArgs are args used only for remote execution. -func MultiCommandStaticRules(ctx android.PackageContext, name string, ruleParams blueprint.RuleParams, reParams map[string]*REParams, commonArgs []string, reArgs []string) (blueprint.Rule, blueprint.Rule) { - ruleParamsRE := ruleParams - for k, v := range reParams { - ruleParams.Command = strings.ReplaceAll(ruleParams.Command, k, "") - ruleParamsRE.Command = strings.ReplaceAll(ruleParamsRE.Command, k, v.Template()) - } - - return ctx.AndroidStaticRule(name, ruleParams, commonArgs...), - ctx.AndroidRemoteStaticRule(name+"RE", android.RemoteRuleSupports{RBE: true}, ruleParamsRE, append(commonArgs, reArgs...)...) -} - -// EnvOverrideFunc retrieves a variable func that evaluates to the value of the given environment -// variable if set, otherwise the given default. -func EnvOverrideFunc(envVar, defaultVal string) func(ctx android.PackageVarContext) string { - return func(ctx android.PackageVarContext) string { - if override := ctx.Config().Getenv(envVar); override != "" { - return override - } - return defaultVal - } -} diff --git a/remoteexec/remoteexec_test.go b/remoteexec/remoteexec_test.go index 56985d356..875aa6ae7 100644 --- a/remoteexec/remoteexec_test.go +++ b/remoteexec/remoteexec_test.go @@ -17,8 +17,6 @@ package remoteexec import ( "fmt" "testing" - - "android/soong/android" ) func TestTemplate(t *testing.T) { @@ -38,7 +36,7 @@ func TestTemplate(t *testing.T) { PoolKey: "default", }, }, - want: fmt.Sprintf("${remoteexec.Wrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage), + want: fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage), }, { name: "all params", @@ -54,7 +52,7 @@ func TestTemplate(t *testing.T) { PoolKey: "default", }, }, - want: fmt.Sprintf("${remoteexec.Wrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=remote --inputs=$in --input_list_paths=$out.rsp --output_files=$out --toolchain_inputs=clang++ -- ", DefaultImage), + want: fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=remote --inputs=$in --input_list_paths=$out.rsp --output_files=$out --toolchain_inputs=clang++ -- ", DefaultImage), }, } for _, test := range tests { @@ -77,7 +75,7 @@ func TestNoVarTemplate(t *testing.T) { }, } want := fmt.Sprintf("prebuilts/remoteexecution-client/live/rewrapper --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage) - if got := params.NoVarTemplate(android.NullConfig("")); got != want { + if got := params.NoVarTemplate(DefaultWrapperPath); got != want { t.Errorf("NoVarTemplate() returned\n%s\nwant\n%s", got, want) } } @@ -92,7 +90,7 @@ func TestTemplateDeterminism(t *testing.T) { PoolKey: "default", }, } - want := fmt.Sprintf("${remoteexec.Wrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage) + want := fmt.Sprintf("${android.RBEWrapper} --labels=compiler=clang,lang=cpp,type=compile --platform=\"Pool=default,container-image=%s\" --exec_strategy=local --inputs=$in --output_files=$out -- ", DefaultImage) for i := 0; i < 1000; i++ { if got := r.Template(); got != want { t.Fatalf("Template() returned\n%s\nwant\n%s", got, want) From ef972743e8d4c047bbba7747caf3e90215889eb0 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 12 Mar 2021 17:24:45 -0800 Subject: [PATCH 3/4] Support sbox-in-RBE Allow passing a remoteexec.REParams to RuleBuilder to configure it to run the rule remotely through RBE. Requires the rule to use SandboxInputs, which ensures that RuleBuilder is aware of all of the inputs and outputs of the rule. Running sbox in RBE initially seems unnecessary, as RBE is already a good sandbox, but reproxy can execute RBE actions locally when configured for local execution, local fallback or racing. Using sbox in RBE ensures that these local actions are also sandboxed, giving consistent results between directly executed actions, local RBE actions, and remote RBE actions. Bug: 182612695 Test: manual Change-Id: Icf2f24dde8dee833eb680ba22566a8e1c0143b15 --- android/rule_builder.go | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/android/rule_builder.go b/android/rule_builder.go index cc6c9b49e..0d8e2b7c9 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -27,6 +27,7 @@ import ( "github.com/google/blueprint/proptools" "android/soong/cmd/sbox/sbox_proto" + "android/soong/remoteexec" "android/soong/shared" ) @@ -48,6 +49,7 @@ type RuleBuilder struct { sbox bool highmem bool remoteable RemoteRuleSupports + rbeParams *remoteexec.REParams outDir WritablePath sboxTools bool sboxInputs bool @@ -120,6 +122,18 @@ func (r *RuleBuilder) Remoteable(supports RemoteRuleSupports) *RuleBuilder { return r } +// Rewrapper marks the rule as running inside rewrapper using the given params in order to support +// running on RBE. During RuleBuilder.Build the params will be combined with the inputs, outputs +// and tools known to RuleBuilder to prepend an appropriate rewrapper command line to the rule's +// command line. +func (r *RuleBuilder) Rewrapper(params *remoteexec.REParams) *RuleBuilder { + if !r.sboxInputs { + panic(fmt.Errorf("RuleBuilder.Rewrapper must be called after RuleBuilder.SandboxInputs")) + } + r.rbeParams = params + return r +} + // Sbox marks the rule as needing to be wrapped by sbox. The outputDir should point to the output // directory that sbox will wipe. It should not be written to by any other rule. manifestPath should // point to a location where sbox's manifest will be written and must be outside outputDir. sbox @@ -625,6 +639,25 @@ func (r *RuleBuilder) Build(name string, desc string) { commandString = sboxCmd.buf.String() tools = append(tools, sboxCmd.tools...) inputs = append(inputs, sboxCmd.inputs...) + + if r.rbeParams != nil { + var remoteInputs []string + remoteInputs = append(remoteInputs, inputs.Strings()...) + remoteInputs = append(remoteInputs, tools.Strings()...) + remoteInputs = append(remoteInputs, rspFileInputs.Strings()...) + if rspFilePath != nil { + remoteInputs = append(remoteInputs, rspFilePath.String()) + } + inputsListFile := r.sboxManifestPath.ReplaceExtension(r.ctx, "rbe_inputs.list") + inputsListContents := rspFileForInputs(remoteInputs) + WriteFileRule(r.ctx, inputsListFile, inputsListContents) + inputs = append(inputs, inputsListFile) + + r.rbeParams.OutputFiles = outputs.Strings() + r.rbeParams.RSPFile = inputsListFile.String() + rewrapperCommand := r.rbeParams.NoVarTemplate(r.ctx.Config().RBEWrapper()) + commandString = rewrapperCommand + " bash -c '" + strings.ReplaceAll(commandString, `'`, `'\''`) + "'" + } } else { // If not using sbox the rule will run the command directly, put the hash of the // list of input files in a comment at the end of the command line to ensure ninja @@ -1230,3 +1263,14 @@ func (builderContextForTests) Rule(PackageContext, string, blueprint.RuleParams, return nil } func (builderContextForTests) Build(PackageContext, BuildParams) {} + +func rspFileForInputs(paths []string) string { + s := strings.Builder{} + for i, path := range paths { + if i != 0 { + s.WriteByte(' ') + } + s.WriteString(proptools.ShellEscape(path)) + } + return s.String() +} From 1661aff8bea44396b3587e19c8c79fd574bf043d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 12 Mar 2021 17:56:51 -0800 Subject: [PATCH 4/4] Run lint actions in sbox Run lint actions in sbox with RuleBuilder.SandboxInputs. This copies all input files into the sandbox, which prevents the lint tool from finding nearby source files that were not presented to it. Using SandboxInputs requires use of PathForInput or PathForOutput anywhere a path is used outside of the RuleBuilderCommand methods that take paths so that they can be translated to the paths that will be used in the sandbox. Bug: 181681346 Test: lint_test.go Test: m lint-check dist Test: m USE_RBE=true RBE_LINT=true lint-check dist Test: m USE_RBE=true RBE_LINT=true RBE_LINT_EXEC_STRATEGY=remote lint-check dist Change-Id: Iab4e09d961891ef182643583d4d456e413bc5e39 --- java/droiddoc.go | 11 +++--- java/java_test.go | 14 ++++---- java/lint.go | 92 +++++++++++++++++++---------------------------- 3 files changed, 50 insertions(+), 67 deletions(-) diff --git a/java/droiddoc.go b/java/droiddoc.go index 706111ffd..a892b363e 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -1678,14 +1678,17 @@ func StubsDefaultsFactory() android.Module { func zipSyncCmd(ctx android.ModuleContext, rule *android.RuleBuilder, srcJarDir android.ModuleOutPath, srcJars android.Paths) android.OutputPath { - rule.Command().Text("rm -rf").Text(srcJarDir.String()) - rule.Command().Text("mkdir -p").Text(srcJarDir.String()) + cmd := rule.Command() + cmd.Text("rm -rf").Text(cmd.PathForOutput(srcJarDir)) + cmd = rule.Command() + cmd.Text("mkdir -p").Text(cmd.PathForOutput(srcJarDir)) srcJarList := srcJarDir.Join(ctx, "list") rule.Temporary(srcJarList) - rule.Command().BuiltTool("zipsync"). - FlagWithArg("-d ", srcJarDir.String()). + cmd = rule.Command() + cmd.BuiltTool("zipsync"). + FlagWithArg("-d ", cmd.PathForOutput(srcJarDir)). FlagWithOutput("-l ", srcJarList). FlagWithArg("-f ", `"*.java"`). Inputs(srcJars) diff --git a/java/java_test.go b/java/java_test.go index b68945fd1..75813e03c 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -25,12 +25,12 @@ import ( "strings" "testing" - "android/soong/genrule" "github.com/google/blueprint/proptools" "android/soong/android" "android/soong/cc" "android/soong/dexpreopt" + "android/soong/genrule" "android/soong/python" ) @@ -1296,9 +1296,9 @@ func TestJavaLint(t *testing.T) { }) foo := ctx.ModuleForTests("foo", "android_common") - rule := foo.Rule("lint") - if !strings.Contains(rule.RuleParams.Command, "--baseline lint-baseline.xml") { + sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml") { t.Error("did not pass --baseline flag") } } @@ -1318,9 +1318,9 @@ func TestJavaLintWithoutBaseline(t *testing.T) { `, map[string][]byte{}) foo := ctx.ModuleForTests("foo", "android_common") - rule := foo.Rule("lint") - if strings.Contains(rule.RuleParams.Command, "--baseline") { + sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) + if strings.Contains(*sboxProto.Commands[0].Command, "--baseline") { t.Error("passed --baseline flag for non existent file") } } @@ -1376,9 +1376,9 @@ func TestJavaLintUsesCorrectBpConfig(t *testing.T) { }) foo := ctx.ModuleForTests("foo", "android_common") - rule := foo.Rule("lint") - if !strings.Contains(rule.RuleParams.Command, "--baseline mybaseline.xml") { + sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline mybaseline.xml") { t.Error("did not use the correct file for baseline") } } diff --git a/java/lint.go b/java/lint.go index 9743c7a4e..938e2b0d7 100644 --- a/java/lint.go +++ b/java/lint.go @@ -218,7 +218,7 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru // The list of resources may be too long to put on the command line, but // we can't use the rsp file because it is already being used for srcs. // Insert a second rule to write out the list of resources to a file. - resourcesList = android.PathForModuleOut(ctx, "lint", "resources.list") + resourcesList = android.PathForModuleOut(ctx, "resources.list") resListRule := android.NewRuleBuilder(pctx, ctx) resListRule.Command().Text("cp"). FlagWithRspFileInputList("", resourcesList.ReplaceExtension(ctx, "rsp"), l.resources). @@ -233,7 +233,7 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cacheDir := android.PathForModuleOut(ctx, "lint", "cache") homeDir := android.PathForModuleOut(ctx, "lint", "home") - srcJarDir := android.PathForModuleOut(ctx, "lint-srcjars") + srcJarDir := android.PathForModuleOut(ctx, "lint", "srcjars") srcJarList := zipSyncCmd(ctx, rule, srcJarDir, l.srcJars) // TODO(ccross): this is a little fishy. The files extracted from the srcjars are referenced // by the project.xml and used by the later lint rule, but the lint rule depends on the srcjars, @@ -248,6 +248,7 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru FlagWithRspFileInputList("", srcsListRsp, l.srcs). Output(srcsList) trackRSPDependency(l.srcs, srcsList) + rule.Temporary(srcsList) cmd := rule.Command(). BuiltTool("lint-project-xml"). @@ -262,11 +263,11 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cmd.Flag("--test") } if l.manifest != nil { - cmd.FlagWithArg("--manifest ", l.manifest.String()) + cmd.FlagWithArg("--manifest ", cmd.PathForInput(l.manifest)) trackInputDependency(l.manifest) } if l.mergedManifest != nil { - cmd.FlagWithArg("--merged_manifest ", l.mergedManifest.String()) + cmd.FlagWithArg("--merged_manifest ", cmd.PathForInput(l.mergedManifest)) trackInputDependency(l.mergedManifest) } @@ -279,23 +280,17 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru } if l.classes != nil { - cmd.FlagWithArg("--classes ", l.classes.String()) + cmd.FlagWithArg("--classes ", cmd.PathForInput(l.classes)) trackInputDependency(l.classes) } - cmd.FlagForEachArg("--classpath ", l.classpath.Strings()) + cmd.FlagForEachArg("--classpath ", cmd.PathsForInputs(l.classpath)) trackInputDependency(l.classpath...) - cmd.FlagForEachArg("--extra_checks_jar ", l.extraLintCheckJars.Strings()) + cmd.FlagForEachArg("--extra_checks_jar ", cmd.PathsForInputs(l.extraLintCheckJars)) trackInputDependency(l.extraLintCheckJars...) - if ctx.Config().UseRBE() && ctx.Config().IsEnvTrue("RBE_LINT") && - lintRBEExecStrategy(ctx) != remoteexec.LocalExecStrategy { - // TODO(b/181912787): remove these and use "." instead. - cmd.FlagWithArg("--root_dir ", "/b/f/w") - } else { - cmd.FlagWithArg("--root_dir ", "$PWD") - } + cmd.FlagWithArg("--root_dir ", "$PWD") // The cache tag in project.xml is relative to the root dir, or the project.xml file if // the root dir is not set. @@ -325,7 +320,7 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru // generateManifest adds a command to the rule to write a simple manifest that contains the // minSdkVersion and targetSdkVersion for modules (like java_library) that don't have a manifest. -func (l *linter) generateManifest(ctx android.ModuleContext, rule *android.RuleBuilder) android.Path { +func (l *linter) generateManifest(ctx android.ModuleContext, rule *android.RuleBuilder) android.WritablePath { manifestPath := android.PathForModuleOut(ctx, "lint", "AndroidManifest.xml") rule.Command().Text("("). @@ -356,18 +351,36 @@ func (l *linter) lint(ctx android.ModuleContext) { } } - rule := android.NewRuleBuilder(pctx, ctx) + rule := android.NewRuleBuilder(pctx, ctx). + Sbox(android.PathForModuleOut(ctx, "lint"), + android.PathForModuleOut(ctx, "lint.sbox.textproto")). + SandboxInputs() + + if ctx.Config().UseRBE() && ctx.Config().IsEnvTrue("RBE_LINT") { + pool := ctx.Config().GetenvWithDefault("RBE_LINT_POOL", "java16") + rule.Remoteable(android.RemoteRuleSupports{RBE: true}) + rule.Rewrapper(&remoteexec.REParams{ + Labels: map[string]string{"type": "tool", "name": "lint"}, + ExecStrategy: lintRBEExecStrategy(ctx), + ToolchainInputs: []string{config.JavaCmd(ctx).String()}, + EnvironmentVariables: []string{ + "LANG", + }, + Platform: map[string]string{remoteexec.PoolKey: pool}, + }) + } if l.manifest == nil { manifest := l.generateManifest(ctx, rule) l.manifest = manifest + rule.Temporary(manifest) } lintPaths := l.writeLintProjectXML(ctx, rule) - html := android.PathForModuleOut(ctx, "lint-report.html") - text := android.PathForModuleOut(ctx, "lint-report.txt") - xml := android.PathForModuleOut(ctx, "lint-report.xml") + html := android.PathForModuleOut(ctx, "lint", "lint-report.html") + text := android.PathForModuleOut(ctx, "lint", "lint-report.txt") + xml := android.PathForModuleOut(ctx, "lint", "lint-report.xml") depSetsBuilder := NewLintDepSetBuilder().Direct(html, text, xml) @@ -397,43 +410,7 @@ func (l *linter) lint(ctx android.ModuleContext) { FlagWithInput("SDK_ANNOTATIONS=", annotationsZipPath). FlagWithInput("LINT_OPTS=-DLINT_API_DATABASE=", apiVersionsXMLPath) - if ctx.Config().UseRBE() && ctx.Config().IsEnvTrue("RBE_LINT") { - pool := ctx.Config().GetenvWithDefault("RBE_LINT_POOL", "java16") - // TODO(b/181912787): this should be local fallback once the hack that passes /b/f/w in project.xml - // is removed. - execStrategy := lintRBEExecStrategy(ctx) - labels := map[string]string{"type": "tool", "name": "lint"} - rule.Remoteable(android.RemoteRuleSupports{RBE: true}) - remoteInputs := lintPaths.remoteInputs - remoteInputs = append(remoteInputs, - lintPaths.projectXML, - lintPaths.configXML, - lintPaths.homeDir, - lintPaths.cacheDir, - ctx.Config().HostJavaToolPath(ctx, "lint.jar"), - annotationsZipPath, - apiVersionsXMLPath, - ) - - cmd.Text((&remoteexec.REParams{ - Labels: labels, - ExecStrategy: execStrategy, - ToolchainInputs: []string{config.JavaCmd(ctx).String()}, - Inputs: remoteInputs.Strings(), - OutputFiles: android.Paths{html, text, xml}.Strings(), - RSPFile: strings.Join(lintPaths.remoteRSPInputs.Strings(), ","), - EnvironmentVariables: []string{ - "JAVA_OPTS", - "ANDROID_SDK_HOME", - "SDK_ANNOTATIONS", - "LINT_OPTS", - "LANG", - }, - Platform: map[string]string{remoteexec.PoolKey: pool}, - }).NoVarTemplate(ctx.Config().RBEWrapper())) - } - - cmd.BuiltTool("lint"). + cmd.BuiltTool("lint").ImplicitTool(ctx.Config().HostJavaToolPath(ctx, "lint.jar")). Flag("--quiet"). FlagWithInput("--project ", lintPaths.projectXML). FlagWithInput("--config ", lintPaths.configXML). @@ -450,6 +427,9 @@ func (l *linter) lint(ctx android.ModuleContext) { Implicit(apiVersionsXMLPath). Implicits(lintPaths.deps) + rule.Temporary(lintPaths.projectXML) + rule.Temporary(lintPaths.configXML) + if checkOnly := ctx.Config().Getenv("ANDROID_LINT_CHECK"); checkOnly != "" { cmd.FlagWithArg("--check ", checkOnly) }