Merge "Move genrule on top of RuleBuilder" am: d4a63eaeb7 am: 9fffe4cd19 am: e2732d4c9d

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/1498170

Change-Id: I8f9cb532ad95f36c8ec9489ef701580400c39563
diff --git a/android/rule_builder.go b/android/rule_builder.go
index 8dc9d6a..86418b2 100644
--- a/android/rule_builder.go
+++ b/android/rule_builder.go
@@ -15,7 +15,9 @@
 package android
 
 import (
+	"crypto/sha256"
 	"fmt"
+	"path/filepath"
 	"sort"
 	"strings"
 
@@ -25,6 +27,8 @@
 	"android/soong/shared"
 )
 
+const sboxOutDir = "__SBOX_OUT_DIR__"
+
 // RuleBuilder provides an alternative to ModuleContext.Rule and ModuleContext.Build to add a command line to the build
 // graph.
 type RuleBuilder struct {
@@ -133,8 +137,8 @@
 // race with any call to Build.
 func (r *RuleBuilder) Command() *RuleBuilderCommand {
 	command := &RuleBuilderCommand{
-		sbox:       r.sbox,
-		sboxOutDir: r.sboxOutDir,
+		sbox:   r.sbox,
+		outDir: r.sboxOutDir,
 	}
 	r.commands = append(r.commands, command)
 	return command
@@ -163,7 +167,7 @@
 }
 
 // Inputs returns the list of paths that were passed to the RuleBuilderCommand methods that take
-// input paths, such as RuleBuilderCommand.Input, RuleBuilderComand.Implicit, or
+// input paths, such as RuleBuilderCommand.Input, RuleBuilderCommand.Implicit, or
 // RuleBuilderCommand.FlagWithInput.  Inputs to a command that are also outputs of another command
 // in the same RuleBuilder are filtered out.  The list is sorted and duplicates removed.
 func (r *RuleBuilder) Inputs() Paths {
@@ -362,7 +366,7 @@
 	return commands
 }
 
-// NinjaEscapedCommands returns a slice containin the built command line after ninja escaping for each call to
+// NinjaEscapedCommands returns a slice containing the built command line after ninja escaping for each call to
 // RuleBuilder.Command.
 func (r *RuleBuilder) NinjaEscapedCommands() []string {
 	var commands []string
@@ -427,6 +431,7 @@
 	tools := r.Tools()
 	commands := r.NinjaEscapedCommands()
 	outputs := r.Outputs()
+	inputs := r.Inputs()
 
 	if len(commands) == 0 {
 		return
@@ -440,7 +445,7 @@
 	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())
+			sboxOutputs[i] = filepath.Join(sboxOutDir, Rel(ctx, r.sboxOutDir.String(), output.String()))
 		}
 
 		commandString = proptools.ShellEscape(commandString)
@@ -458,10 +463,19 @@
 			sboxCmd.Flag("--depfile-out").Text(depFile.String())
 		}
 
+		// Add a hash of the list of input files to the xbox command line so that ninja reruns
+		// it when the list of input files changes.
+		sboxCmd.FlagWithArg("--input-hash ", hashSrcFiles(inputs))
+
 		sboxCmd.Flags(sboxOutputs)
 
 		commandString = sboxCmd.buf.String()
 		tools = append(tools, sboxCmd.tools...)
+	} 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
+		// reruns the rule when the list of input files changes.
+		commandString += " # hash of input list: " + hashSrcFiles(inputs)
 	}
 
 	// Ninja doesn't like multiple outputs when depfiles are enabled, move all but the first output to
@@ -499,7 +513,7 @@
 			Pool:           pool,
 		}),
 		Inputs:          rspFileInputs,
-		Implicits:       r.Inputs(),
+		Implicits:       inputs,
 		Output:          output,
 		ImplicitOutputs: implicitOutputs,
 		SymlinkOutputs:  r.SymlinkOutputs(),
@@ -527,14 +541,16 @@
 	// spans [start,end) of the command that should not be ninja escaped
 	unescapedSpans [][2]int
 
-	sbox       bool
-	sboxOutDir WritablePath
+	sbox bool
+	// outDir is the directory that will contain the output files of the rules.  sbox will copy
+	// the output files from the sandbox directory to this directory when it finishes.
+	outDir 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
+		if rel, isRel, _ := maybeRelErr(c.outDir.String(), path.String()); isRel {
+			return filepath.Join(sboxOutDir, rel)
 		}
 	}
 	c.inputs = append(c.inputs, path)
@@ -543,8 +559,8 @@
 
 func (c *RuleBuilderCommand) addImplicit(path Path) string {
 	if c.sbox {
-		if rel, isRel, _ := maybeRelErr(c.sboxOutDir.String(), path.String()); isRel {
-			return "__SBOX_OUT_DIR__/" + rel
+		if rel, isRel, _ := maybeRelErr(c.outDir.String(), path.String()); isRel {
+			return filepath.Join(sboxOutDir, rel)
 		}
 	}
 	c.implicits = append(c.implicits, path)
@@ -555,15 +571,22 @@
 	c.orderOnlys = append(c.orderOnlys, path)
 }
 
-func (c *RuleBuilderCommand) outputStr(path Path) string {
+func (c *RuleBuilderCommand) outputStr(path WritablePath) 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 SboxPathForOutput(path, c.outDir)
 	}
 	return path.String()
 }
 
+// SboxPathForOutput takes an output path and the out directory passed to RuleBuilder.Sbox(),
+// and returns the corresponding path for the output in the sbox sandbox.  This can be used
+// on the RuleBuilder command line to reference the output.
+func SboxPathForOutput(path WritablePath, outDir WritablePath) string {
+	// Errors will be handled in RuleBuilder.Build where we have a context to report them
+	rel, _, _ := maybeRelErr(outDir.String(), path.String())
+	return filepath.Join(sboxOutDir, rel)
+}
+
 // 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 {
@@ -727,7 +750,7 @@
 	if !c.sbox {
 		panic("OutputDir only valid with Sbox")
 	}
-	return c.Text("__SBOX_OUT_DIR__")
+	return c.Text(sboxOutDir)
 }
 
 // DepFile adds the specified depfile path to the paths returned by RuleBuilder.DepFiles and adds it to the command
@@ -906,3 +929,12 @@
 	}
 	return s
 }
+
+// hashSrcFiles returns a hash of the list of source files.  It is used to ensure the command line
+// or the sbox textproto manifest change even if the input files are not listed on the command line.
+func hashSrcFiles(srcFiles Paths) string {
+	h := sha256.New()
+	srcFileList := strings.Join(srcFiles.Strings(), "\n")
+	h.Write([]byte(srcFileList))
+	return fmt.Sprintf("%x", h.Sum(nil))
+}
diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go
index ca6359d..c1d5521 100644
--- a/android/rule_builder_test.go
+++ b/android/rule_builder_test.go
@@ -18,6 +18,7 @@
 	"fmt"
 	"path/filepath"
 	"reflect"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -441,7 +442,7 @@
 type testRuleBuilderModule struct {
 	ModuleBase
 	properties struct {
-		Src string
+		Srcs []string
 
 		Restat bool
 		Sbox   bool
@@ -449,7 +450,7 @@
 }
 
 func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) {
-	in := PathForSource(ctx, t.properties.Src)
+	in := PathsForSource(ctx, t.properties.Srcs)
 	out := PathForModuleOut(ctx, ctx.ModuleName())
 	outDep := PathForModuleOut(ctx, ctx.ModuleName()+".d")
 	outDir := PathForModuleOut(ctx)
@@ -468,17 +469,17 @@
 	out := PathForOutput(ctx, "baz")
 	outDep := PathForOutput(ctx, "baz.d")
 	outDir := PathForOutput(ctx)
-	testRuleBuilder_Build(ctx, in, out, outDep, outDir, true, false)
+	testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, true, false)
 }
 
-func testRuleBuilder_Build(ctx BuilderContext, in Path, out, outDep, outDir WritablePath, restat, sbox bool) {
+func testRuleBuilder_Build(ctx BuilderContext, in Paths, out, outDep, outDir WritablePath, restat, sbox bool) {
 	rule := NewRuleBuilder()
 
 	if sbox {
 		rule.Sbox(outDir)
 	}
 
-	rule.Command().Tool(PathForSource(ctx, "cp")).Input(in).Output(out).ImplicitDepFile(outDep)
+	rule.Command().Tool(PathForSource(ctx, "cp")).Inputs(in).Output(out).ImplicitDepFile(outDep)
 
 	if restat {
 		rule.Restat()
@@ -496,12 +497,12 @@
 	bp := `
 		rule_builder_test {
 			name: "foo",
-			src: "bar",
+			srcs: ["bar"],
 			restat: true,
 		}
 		rule_builder_test {
 			name: "foo_sbox",
-			src: "bar",
+			srcs: ["bar"],
 			sbox: true,
 		}
 	`
@@ -519,7 +520,10 @@
 
 	check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraCmdDeps []string) {
 		t.Helper()
-		if params.RuleParams.Command != wantCommand {
+		command := params.RuleParams.Command
+		re := regexp.MustCompile(" (# hash of input list:|--input-hash) [a-z0-9]*")
+		command = re.ReplaceAllLiteralString(command, "")
+		if command != wantCommand {
 			t.Errorf("\nwant RuleParams.Command = %q\n                      got %q", wantCommand, params.RuleParams.Command)
 		}
 
@@ -651,3 +655,78 @@
 		})
 	}
 }
+
+func TestRuleBuilderHashInputs(t *testing.T) {
+	// The basic idea here is to verify that the command (in the case of a
+	// non-sbox rule) or the sbox textproto manifest contain a hash of the
+	// inputs.
+
+	// By including a hash of the inputs, we cause the rule to re-run if
+	// the list of inputs changes because the command line or a dependency
+	// changes.
+
+	bp := `
+			rule_builder_test {
+				name: "hash0",
+				srcs: ["in1.txt", "in2.txt"],
+			}
+			rule_builder_test {
+				name: "hash0_sbox",
+				srcs: ["in1.txt", "in2.txt"],
+				sbox: true,
+			}
+			rule_builder_test {
+				name: "hash1",
+				srcs: ["in1.txt", "in2.txt", "in3.txt"],
+			}
+			rule_builder_test {
+				name: "hash1_sbox",
+				srcs: ["in1.txt", "in2.txt", "in3.txt"],
+				sbox: true,
+			}
+		`
+	testcases := []struct {
+		name         string
+		expectedHash string
+	}{
+		{
+			name: "hash0",
+			// sha256 value obtained from: echo -en 'in1.txt\nin2.txt' | sha256sum
+			expectedHash: "18da75b9b1cc74b09e365b4ca2e321b5d618f438cc632b387ad9dc2ab4b20e9d",
+		},
+		{
+			name: "hash1",
+			// sha256 value obtained from: echo -en 'in1.txt\nin2.txt\nin3.txt' | sha256sum
+			expectedHash: "a38d432a4b19df93140e1f1fe26c97ff0387dae01fe506412b47208f0595fb45",
+		},
+	}
+
+	config := TestConfig(buildDir, nil, bp, nil)
+	ctx := NewTestContext(config)
+	ctx.RegisterModuleType("rule_builder_test", testRuleBuilderFactory)
+	ctx.Register()
+
+	_, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+	FailIfErrored(t, errs)
+	_, errs = ctx.PrepareBuildActions(config)
+	FailIfErrored(t, errs)
+
+	for _, test := range testcases {
+		t.Run(test.name, func(t *testing.T) {
+			t.Run("sbox", func(t *testing.T) {
+				gen := ctx.ModuleForTests(test.name+"_sbox", "")
+				command := gen.Output(test.name + "_sbox").RuleParams.Command
+				if g, w := command, " --input-hash "+test.expectedHash; !strings.Contains(g, w) {
+					t.Errorf("Expected command line to end with %q, got %q", w, g)
+				}
+			})
+			t.Run("", func(t *testing.T) {
+				gen := ctx.ModuleForTests(test.name+"", "")
+				command := gen.Output(test.name).RuleParams.Command
+				if g, w := command, " # hash of input list: "+test.expectedHash; !strings.HasSuffix(g, w) {
+					t.Errorf("Expected command line to end with %q, got %q", w, g)
+				}
+			})
+		})
+	}
+}
diff --git a/cc/gen.go b/cc/gen.go
index ccc3d0e..134d6d9 100644
--- a/cc/gen.go
+++ b/cc/gen.go
@@ -75,7 +75,11 @@
 	cmd := rule.Command()
 
 	// Fix up #line markers to not use the sbox temporary directory
-	sedCmd := "sed -i.bak 's#__SBOX_OUT_DIR__#" + outDir.String() + "#'"
+	// android.SboxPathForOutput(outDir, outDir) returns the sbox placeholder for the out
+	// directory itself, without any filename appended.
+	// TODO(ccross): make this cmd.PathForOutput(outDir) instead.
+	sboxOutDir := android.SboxPathForOutput(outDir, outDir)
+	sedCmd := "sed -i.bak 's#" + sboxOutDir + "#" + outDir.String() + "#'"
 	rule.Command().Text(sedCmd).Input(outFile)
 	rule.Command().Text(sedCmd).Input(headerFile)
 
diff --git a/cc/genrule_test.go b/cc/genrule_test.go
index 0c7952b..fa0c6f2 100644
--- a/cc/genrule_test.go
+++ b/cc/genrule_test.go
@@ -66,14 +66,14 @@
 
 	gen := ctx.ModuleForTests("gen", "android_arm_armv7-a-neon").Output("out_arm")
 	expected := []string{"foo"}
-	if !reflect.DeepEqual(expected, gen.Inputs.Strings()) {
-		t.Errorf(`want arm inputs %v, got %v`, expected, gen.Inputs.Strings())
+	if !reflect.DeepEqual(expected, gen.Implicits.Strings()[:len(expected)]) {
+		t.Errorf(`want arm inputs %v, got %v`, expected, gen.Implicits.Strings())
 	}
 
 	gen = ctx.ModuleForTests("gen", "android_arm64_armv8-a").Output("out_arm64")
 	expected = []string{"bar"}
-	if !reflect.DeepEqual(expected, gen.Inputs.Strings()) {
-		t.Errorf(`want arm64 inputs %v, got %v`, expected, gen.Inputs.Strings())
+	if !reflect.DeepEqual(expected, gen.Implicits.Strings()[:len(expected)]) {
+		t.Errorf(`want arm64 inputs %v, got %v`, expected, gen.Implicits.Strings())
 	}
 }
 
@@ -108,10 +108,10 @@
 	gen := ctx.ModuleForTests("gen", "android_arm_armv7-a-neon").Output("out")
 	expected := []string{"libboth.so", "libshared.so", "libstatic.a"}
 	var got []string
-	for _, input := range gen.Inputs {
+	for _, input := range gen.Implicits {
 		got = append(got, input.Base())
 	}
-	if !reflect.DeepEqual(expected, got) {
+	if !reflect.DeepEqual(expected, got[:len(expected)]) {
 		t.Errorf(`want inputs %v, got %v`, expected, got)
 	}
 }
diff --git a/genrule/genrule.go b/genrule/genrule.go
index 53b9dbe..f85146c 100644
--- a/genrule/genrule.go
+++ b/genrule/genrule.go
@@ -17,6 +17,7 @@
 import (
 	"fmt"
 	"io"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -25,9 +26,6 @@
 	"github.com/google/blueprint/proptools"
 
 	"android/soong/android"
-	"android/soong/shared"
-	"crypto/sha256"
-	"path/filepath"
 )
 
 func init() {
@@ -156,14 +154,14 @@
 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
+	in      android.Paths
+	out     android.WritablePaths
+	depFile android.WritablePath
+	copyTo  android.WritablePaths
+	genDir  android.WritablePath
+	cmd     string
+	shard   int
+	shards  int
 }
 
 func (g *Module) GeneratedSourceFiles() android.Paths {
@@ -330,19 +328,23 @@
 	var zipArgs strings.Builder
 
 	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())})
+		if len(task.out) == 0 {
+			ctx.ModuleErrorf("must have at least one output file")
+			return
 		}
 
-		referencedIn := false
+		for _, out := range task.out {
+			addLocationLabel(out.Rel(), []string{android.SboxPathForOutput(out, task.genDir)})
+		}
+
 		referencedDepfile := false
 
-		rawCommand, err := android.ExpandNinjaEscaped(task.cmd, func(name string) (string, bool, error) {
+		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, bool, error) {
+			reportError := func(fmt string, args ...interface{}) (string, error) {
 				ctx.PropertyErrorf("cmd", fmt, args...)
-				return "SOONG_ERROR", false, nil
+				return "SOONG_ERROR", nil
 			}
 
 			switch name {
@@ -357,20 +359,23 @@
 					return reportError("default label %q has multiple files, use $(locations %s) to reference it",
 						firstLabel, firstLabel)
 				}
-				return locationLabels[firstLabel][0], false, nil
+				return locationLabels[firstLabel][0], nil
 			case "in":
-				referencedIn = true
-				return "${in}", true, nil
+				return strings.Join(srcFiles.Strings(), " "), nil
 			case "out":
-				return "__SBOX_OUT_FILES__", false, nil
+				var sandboxOuts []string
+				for _, out := range task.out {
+					sandboxOuts = append(sandboxOuts, android.SboxPathForOutput(out, task.genDir))
+				}
+				return strings.Join(sandboxOuts, " "), nil
 			case "depfile":
 				referencedDepfile = true
 				if !Bool(g.properties.Depfile) {
 					return reportError("$(depfile) used without depfile property")
 				}
-				return "__SBOX_DEPFILE__", false, nil
+				return "__SBOX_DEPFILE__", nil
 			case "genDir":
-				return "__SBOX_OUT_DIR__", false, nil
+				return android.SboxPathForOutput(task.genDir, task.genDir), nil
 			default:
 				if strings.HasPrefix(name, "location ") {
 					label := strings.TrimSpace(strings.TrimPrefix(name, "location "))
@@ -381,7 +386,7 @@
 							return reportError("label %q has multiple files, use $(locations %s) to reference it",
 								label, label)
 						}
-						return paths[0], false, nil
+						return paths[0], nil
 					} else {
 						return reportError("unknown location label %q", label)
 					}
@@ -391,7 +396,7 @@
 						if len(paths) == 0 {
 							return reportError("label %q has no files", label)
 						}
-						return strings.Join(paths, " "), false, nil
+						return strings.Join(paths, " "), nil
 					} else {
 						return reportError("unknown locations label %q", label)
 					}
@@ -410,50 +415,39 @@
 			ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd")
 			return
 		}
-
-		// 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"
-		}
-
-		// 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",
-			task.genDir, sandboxPath, task.genDir)
-
-		if !referencedIn {
-			sandboxCommand = sandboxCommand + hashSrcFiles(srcFiles)
-		}
-
-		sandboxCommand = sandboxCommand + fmt.Sprintf(" -c %s %s $allouts",
-			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")
-		}
+		// Pick a unique rule name and the user-visible description.
+		desc := "generate"
 		name := "generator"
-		if task.shards > 1 {
+		if task.shards > 0 {
+			desc += " " + strconv.Itoa(task.shard)
 			name += strconv.Itoa(task.shard)
+		} else if len(task.out) == 1 {
+			desc += " " + task.out[0].Base()
 		}
-		rule := ctx.Rule(pctx, name, ruleParams, args...)
 
-		g.generateSourceFile(ctx, task, rule)
+		// Use a RuleBuilder to create a rule that runs the command inside an sbox sandbox.
+		rule := android.NewRuleBuilder().Sbox(task.genDir)
+		cmd := rule.Command()
+		cmd.Text(rawCommand)
+		cmd.ImplicitOutputs(task.out)
+		cmd.Implicits(task.in)
+		cmd.Implicits(g.deps)
+		if Bool(g.properties.Depfile) {
+			cmd.ImplicitDepFile(task.depFile)
+		}
+
+		// Create the rule to run the genrule command inside sbox.
+		rule.Build(pctx, ctx, name, desc)
 
 		if len(task.copyTo) > 0 {
+			// If copyTo is set, multiple shards need to be copied into a single directory.
+			// task.out contains the per-shard paths, and copyTo contains the corresponding
+			// final path.  The files need to be copied into the final directory by a
+			// single rule so it can remove the directory before it starts to ensure no
+			// old files remain.  zipsync already does this, so build up zipArgs that
+			// zip all the per-shard directories into a single zip.
 			outputFiles = append(outputFiles, task.copyTo...)
 			copyFrom = append(copyFrom, task.out.Paths()...)
 			zipArgs.WriteString(" -C " + task.genDir.String())
@@ -464,6 +458,8 @@
 	}
 
 	if len(copyFrom) > 0 {
+		// Create a rule that zips all the per-shard directories into a single zip and then
+		// uses zipsync to unzip it into the final directory.
 		ctx.Build(pctx, android.BuildParams{
 			Rule:      gensrcsMerge,
 			Implicits: copyFrom,
@@ -501,51 +497,6 @@
 		}
 	}
 }
-func hashSrcFiles(srcFiles android.Paths) string {
-	h := sha256.New()
-	for _, src := range srcFiles {
-		h.Write([]byte(src.String()))
-	}
-	return fmt.Sprintf(" --input-hash %x", h.Sum(nil))
-}
-
-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")
-		return
-	}
-	if len(task.out) == 1 {
-		desc += " " + task.out[0].Base()
-	}
-
-	var depFile android.ModuleGenPath
-	if Bool(g.properties.Depfile) {
-		depFile = android.PathForModuleGen(ctx, task.out[0].Rel()+".d")
-	}
-
-	if task.shards > 1 {
-		desc += " " + strconv.Itoa(task.shard)
-	}
-
-	params := android.BuildParams{
-		Rule:            rule,
-		Description:     desc,
-		Output:          task.out[0],
-		ImplicitOutputs: task.out[1:],
-		Inputs:          task.in,
-		Implicits:       g.deps,
-		Args: map[string]string{
-			"allouts": strings.Join(task.sandboxOuts, " "),
-		},
-	}
-	if Bool(g.properties.Depfile) {
-		params.Depfile = android.PathForModuleGen(ctx, task.out[0].Rel()+".d")
-		params.Args["depfileArgs"] = "--depfile-out " + depFile.String()
-	}
-
-	ctx.Build(pctx, params)
-}
 
 // Collect information for opening IDE project files in java/jdeps.go.
 func (g *Module) IDEInfo(dpInfo *android.IdeInfo) {
@@ -610,16 +561,6 @@
 func (x noopImageInterface) SetImageVariation(ctx android.BaseModuleContext, variation string, module android.Module) {
 }
 
-// replace "out" with "__SBOX_OUT_DIR__/<the value of ${out}>"
-func pathToSandboxOut(path android.Path, genDir android.Path) string {
-	relOut, err := filepath.Rel(genDir.String(), path.String())
-	if err != nil {
-		panic(fmt.Sprintf("Could not make ${out} relative: %v", err))
-	}
-	return filepath.Join("__SBOX_OUT_DIR__", relOut)
-
-}
-
 func NewGenSrcs() *Module {
 	properties := &genSrcsProperties{}
 
@@ -638,7 +579,7 @@
 			var outFiles android.WritablePaths
 			var copyTo android.WritablePaths
 			var shardDir android.WritablePath
-			var sandboxOuts []string
+			var depFile android.WritablePath
 
 			if len(shards) > 1 {
 				shardDir = android.PathForModuleGen(ctx, strconv.Itoa(i))
@@ -646,9 +587,11 @@
 				shardDir = genDir
 			}
 
-			for _, in := range shard {
+			for j, in := range shard {
 				outFile := android.GenPathWithExt(ctx, "gensrcs", in, String(properties.Output_extension))
-				sandboxOutfile := pathToSandboxOut(outFile, genDir)
+				if j == 0 {
+					depFile = outFile.ReplaceExtension(ctx, "d")
+				}
 
 				if len(shards) > 1 {
 					shardFile := android.GenPathWithExt(ctx, strconv.Itoa(i), in, String(properties.Output_extension))
@@ -657,14 +600,13 @@
 				}
 
 				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
+						return android.SboxPathForOutput(outFile, shardDir), nil
 					default:
 						return "$(" + name + ")", nil
 					}
@@ -680,14 +622,14 @@
 			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),
+				in:      shard,
+				out:     outFiles,
+				depFile: depFile,
+				copyTo:  copyTo,
+				genDir:  shardDir,
+				cmd:     fullCommand,
+				shard:   i,
+				shards:  len(shards),
 			})
 		}
 
@@ -720,18 +662,20 @@
 
 	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)
+		var depFile android.WritablePath
 		for i, out := range properties.Out {
-			outs[i] = android.PathForModuleGen(ctx, out)
-			sandboxOuts[i] = pathToSandboxOut(outs[i], genDir)
+			outPath := android.PathForModuleGen(ctx, out)
+			if i == 0 {
+				depFile = outPath.ReplaceExtension(ctx, "d")
+			}
+			outs[i] = outPath
 		}
 		return []generateTask{{
-			in:          srcFiles,
-			out:         outs,
-			genDir:      android.PathForModuleGen(ctx),
-			sandboxOuts: sandboxOuts,
-			cmd:         rawCommand,
+			in:      srcFiles,
+			out:     outs,
+			depFile: depFile,
+			genDir:  android.PathForModuleGen(ctx),
+			cmd:     rawCommand,
 		}}
 	}
 
diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go
index c19078f..6779c73 100644
--- a/genrule/genrule_test.go
+++ b/genrule/genrule_test.go
@@ -141,7 +141,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_FILES__",
+			expect: "out/tool > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "empty location tool2",
@@ -150,7 +150,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_FILES__",
+			expect: "out/tool > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "empty location tool file",
@@ -159,7 +159,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_FILES__",
+			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "empty location tool file fg",
@@ -168,7 +168,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_FILES__",
+			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "empty location tool and tool file",
@@ -178,7 +178,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_FILES__",
+			expect: "out/tool > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "tool",
@@ -187,7 +187,7 @@
 				out: ["out"],
 				cmd: "$(location tool) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_FILES__",
+			expect: "out/tool > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "tool2",
@@ -196,7 +196,7 @@
 				out: ["out"],
 				cmd: "$(location :tool) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_FILES__",
+			expect: "out/tool > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "tool file",
@@ -205,7 +205,7 @@
 				out: ["out"],
 				cmd: "$(location tool_file1) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_FILES__",
+			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "tool file fg",
@@ -214,7 +214,7 @@
 				out: ["out"],
 				cmd: "$(location :1tool_file) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_FILES__",
+			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "tool files",
@@ -223,7 +223,7 @@
 				out: ["out"],
 				cmd: "$(locations :tool_files) > $(out)",
 			`,
-			expect: "tool_file1 tool_file2 > __SBOX_OUT_FILES__",
+			expect: "tool_file1 tool_file2 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "in1",
@@ -232,7 +232,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat ${in} > __SBOX_OUT_FILES__",
+			expect: "cat in1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "in1 fg",
@@ -241,7 +241,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat ${in} > __SBOX_OUT_FILES__",
+			expect: "cat in1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "ins",
@@ -250,7 +250,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat ${in} > __SBOX_OUT_FILES__",
+			expect: "cat in1 in2 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "ins fg",
@@ -259,7 +259,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat ${in} > __SBOX_OUT_FILES__",
+			expect: "cat in1 in2 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "location in1",
@@ -268,7 +268,7 @@
 				out: ["out"],
 				cmd: "cat $(location in1) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_FILES__",
+			expect: "cat in1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "location in1 fg",
@@ -277,7 +277,7 @@
 				out: ["out"],
 				cmd: "cat $(location :1in) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_FILES__",
+			expect: "cat in1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "location ins",
@@ -286,7 +286,7 @@
 				out: ["out"],
 				cmd: "cat $(location in1) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_FILES__",
+			expect: "cat in1 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "location ins fg",
@@ -295,7 +295,7 @@
 				out: ["out"],
 				cmd: "cat $(locations :ins) > $(out)",
 			`,
-			expect: "cat in1 in2 > __SBOX_OUT_FILES__",
+			expect: "cat in1 in2 > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "outs",
@@ -303,7 +303,7 @@
 				out: ["out", "out2"],
 				cmd: "echo foo > $(out)",
 			`,
-			expect: "echo foo > __SBOX_OUT_FILES__",
+			expect: "echo foo > __SBOX_OUT_DIR__/out __SBOX_OUT_DIR__/out2",
 		},
 		{
 			name: "location out",
@@ -320,7 +320,7 @@
 				depfile: true,
 				cmd: "echo foo > $(out) && touch $(depfile)",
 			`,
-			expect: "echo foo > __SBOX_OUT_FILES__ && touch __SBOX_DEPFILE__",
+			expect: "echo foo > __SBOX_OUT_DIR__/out && touch __SBOX_DEPFILE__",
 		},
 		{
 			name: "gendir",
@@ -328,7 +328,7 @@
 				out: ["out"],
 				cmd: "echo foo > $(genDir)/foo && cp $(genDir)/foo $(out)",
 			`,
-			expect: "echo foo > __SBOX_OUT_DIR__/foo && cp __SBOX_OUT_DIR__/foo __SBOX_OUT_FILES__",
+			expect: "echo foo > __SBOX_OUT_DIR__/foo && cp __SBOX_OUT_DIR__/foo __SBOX_OUT_DIR__/out",
 		},
 
 		{
@@ -443,7 +443,7 @@
 
 			allowMissingDependencies: true,
 
-			expect: "cat ***missing srcs :missing*** > __SBOX_OUT_FILES__",
+			expect: "cat ***missing srcs :missing*** > __SBOX_OUT_DIR__/out",
 		},
 		{
 			name: "tool allow missing dependencies",
@@ -455,7 +455,7 @@
 
 			allowMissingDependencies: true,
 
-			expect: "***missing tool :missing*** > __SBOX_OUT_FILES__",
+			expect: "***missing tool :missing*** > __SBOX_OUT_DIR__/out",
 		},
 	}
 
@@ -495,7 +495,7 @@
 			}
 
 			gen := ctx.ModuleForTests("gen", "").Module().(*Module)
-			if g, w := gen.rawCommands[0], "'"+test.expect+"'"; w != g {
+			if g, w := gen.rawCommands[0], test.expect; w != g {
 				t.Errorf("want %q, got %q", w, g)
 			}
 		})
@@ -542,18 +542,18 @@
 	}{
 		{
 			name: "hash0",
-			// sha256 value obtained from: echo -n 'in1.txtin2.txt' | sha256sum
-			expectedHash: "031097e11e0a8c822c960eb9742474f46336360a515744000d086d94335a9cb9",
+			// sha256 value obtained from: echo -en 'in1.txt\nin2.txt' | sha256sum
+			expectedHash: "18da75b9b1cc74b09e365b4ca2e321b5d618f438cc632b387ad9dc2ab4b20e9d",
 		},
 		{
 			name: "hash1",
-			// sha256 value obtained from: echo -n 'in1.txtin2.txtin3.txt' | sha256sum
-			expectedHash: "de5d22a4a7ab50d250cc59fcdf7a7e0775790d270bfca3a7a9e1f18a70dd996c",
+			// sha256 value obtained from: echo -en 'in1.txt\nin2.txt\nin3.txt' | sha256sum
+			expectedHash: "a38d432a4b19df93140e1f1fe26c97ff0387dae01fe506412b47208f0595fb45",
 		},
 		{
 			name: "hash2",
-			// $(in) is present, option should not appear
-			expectedHash: "",
+			// sha256 value obtained from: echo -en 'in1.txt\nin2.txt\nin3.txt' | sha256sum
+			expectedHash: "a38d432a4b19df93140e1f1fe26c97ff0387dae01fe506412b47208f0595fb45",
 		},
 	}
 
@@ -575,7 +575,7 @@
 			if len(test.expectedHash) > 0 {
 				// We add spaces before and after to make sure that
 				// this option doesn't abutt another sbox option.
-				expectedInputHashOption := " --input-hash " + test.expectedHash + " "
+				expectedInputHashOption := " --input-hash " + test.expectedHash
 
 				if !strings.Contains(command, expectedInputHashOption) {
 					t.Errorf("Expected command \"%s\" to contain \"%s\"", command, expectedInputHashOption)
@@ -609,7 +609,7 @@
 				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'\\'''",
+				"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", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"},
 			files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"},
@@ -623,8 +623,8 @@
 				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'\\'''",
+				"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", 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"},
@@ -710,7 +710,7 @@
 	}
 	gen := ctx.ModuleForTests("gen", "").Module().(*Module)
 
-	expectedCmd := "'cp ${in} __SBOX_OUT_FILES__'"
+	expectedCmd := "cp in1 __SBOX_OUT_DIR__/out"
 	if gen.rawCommands[0] != expectedCmd {
 		t.Errorf("Expected cmd: %q, actual: %q", expectedCmd, gen.rawCommands[0])
 	}
diff --git a/java/java_test.go b/java/java_test.go
index 87d6ebb..efca039 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -1379,8 +1379,8 @@
 	baz := ctx.ModuleForTests("baz", "android_common").Output("javac/baz.jar")
 	barCombined := ctx.ModuleForTests("bar", "android_common").Output("combined/bar.jar")
 
-	if len(jargen.Inputs) != 1 || jargen.Inputs[0].String() != foo.Output.String() {
-		t.Errorf("expected jargen inputs [%q], got %q", foo.Output.String(), jargen.Inputs.Strings())
+	if g, w := jargen.Implicits.Strings(), foo.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected jargen inputs [%q], got %q", w, g)
 	}
 
 	if !strings.Contains(bar.Args["classpath"], jargen.Output.String()) {