From ef972743e8d4c047bbba7747caf3e90215889eb0 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 12 Mar 2021 17:24:45 -0800 Subject: [PATCH] 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() +}