Merge "Comment android/config.go"
diff --git a/android/Android.bp b/android/Android.bp
index 66d361e..4bd272d 100644
--- a/android/Android.bp
+++ b/android/Android.bp
@@ -4,8 +4,10 @@
     deps: [
         "blueprint",
         "blueprint-bootstrap",
+        "sbox_proto",
         "soong",
         "soong-android-soongconfig",
+        "soong-bazel",
         "soong-env",
         "soong-shared",
         "soong-ui-metrics_proto",
@@ -22,6 +24,7 @@
         "defaults.go",
         "defs.go",
         "depset.go",
+        "deptag.go",
         "expand.go",
         "filegroup.go",
         "hooks.go",
@@ -71,6 +74,7 @@
         "config_test.go",
         "csuite_config_test.go",
         "depset_test.go",
+        "deptag_test.go",
         "expand_test.go",
         "module_test.go",
         "mutator_test.go",
diff --git a/android/androidmk.go b/android/androidmk.go
index cfd7c91..4adbb22 100644
--- a/android/androidmk.go
+++ b/android/androidmk.go
@@ -417,7 +417,7 @@
 type androidMkSingleton struct{}
 
 func (c *androidMkSingleton) GenerateBuildActions(ctx SingletonContext) {
-	if !ctx.Config().EmbeddedInMake() {
+	if !ctx.Config().KatiEnabled() {
 		return
 	}
 
diff --git a/android/androidmk_test.go b/android/androidmk_test.go
index 10527b9..bb03378 100644
--- a/android/androidmk_test.go
+++ b/android/androidmk_test.go
@@ -78,7 +78,7 @@
 	`
 
 	config := TestConfig(buildDir, nil, bp, nil)
-	config.inMake = true // Enable androidmk Singleton
+	config.katiEnabled = true // Enable androidmk Singleton
 
 	ctx := NewTestContext(config)
 	ctx.RegisterSingletonType("androidmk", AndroidMkSingleton)
@@ -250,7 +250,7 @@
 
 	for _, testCase := range testCases {
 		config := TestConfig(buildDir, nil, testCase.bp, nil)
-		config.inMake = true // Enable androidmk Singleton
+		config.katiEnabled = true // Enable androidmk Singleton
 
 		ctx := NewTestContext(config)
 		ctx.RegisterSingletonType("androidmk", AndroidMkSingleton)
diff --git a/android/arch.go b/android/arch.go
index eb651e6..df407d4 100644
--- a/android/arch.go
+++ b/android/arch.go
@@ -271,7 +271,7 @@
 	if _, found := commonTargetMap[name]; found {
 		panic(fmt.Errorf("Found Os type duplicate during OsType registration: %q", name))
 	} else {
-		commonTargetMap[name] = Target{Os: os, Arch: Arch{ArchType: Common}}
+		commonTargetMap[name] = Target{Os: os, Arch: CommonArch}
 	}
 	osArchTypeMap[os] = archTypes
 
@@ -341,6 +341,10 @@
 	// CommonOS is a pseudo OSType for a common OS variant, which is OsType agnostic and which
 	// has dependencies on all the OS variants.
 	CommonOS = newOsType("common_os", Generic, false)
+
+	// CommonArch is the Arch for all modules that are os-specific but not arch specific,
+	// for example most Java modules.
+	CommonArch = Arch{ArchType: Common}
 )
 
 // Target specifies the OS and architecture that a module is being compiled for.
@@ -511,9 +515,6 @@
 // Identifies the dependency from CommonOS variant to the os specific variants.
 var commonOsToOsSpecificVariantTag = archDepTag{name: "common os to os specific"}
 
-// Identifies the dependency from arch variant to the common variant for a "common_first" multilib.
-var firstArchToCommonArchDepTag = archDepTag{name: "first arch to common arch"}
-
 // Get the OsType specific variants for the current CommonOS variant.
 //
 // The returned list will only contain enabled OsType specific variants of the
@@ -667,12 +668,6 @@
 		addTargetProperties(m, targets[i], multiTargets, i == 0)
 		m.base().setArchProperties(mctx)
 	}
-
-	if multilib == "common_first" && len(modules) >= 2 {
-		for i := range modules[1:] {
-			mctx.AddInterVariantDependency(firstArchToCommonArchDepTag, modules[i+1], modules[0])
-		}
-	}
 }
 
 // addTargetProperties annotates a variant with the Target is is being compiled for, the list
diff --git a/android/config.go b/android/config.go
index 5ec621c..04c9129 100644
--- a/android/config.go
+++ b/android/config.go
@@ -137,7 +137,9 @@
 	envDeps   map[string]string
 	envFrozen bool
 
-	inMake bool
+	// Changes behavior based on whether Kati runs after soong_build, or if soong_build
+	// runs standalone.
+	katiEnabled bool
 
 	captureBuild      bool // true for tests, saves build parameters for each module
 	ignoreEnvironment bool // true for tests, returns empty from all Getenv calls
@@ -256,7 +258,8 @@
 		envCopy[k] = v
 	}
 
-	// Copy the real PATH value to the test environment, it's needed by HostSystemTool() used in x86_darwin_host.go
+	// Copy the real PATH value to the test environment, it's needed by
+	// NonHermeticHostSystemTool() used in x86_darwin_host.go
 	envCopy["PATH"] = originalEnv["PATH"]
 
 	config := &config{
@@ -420,9 +423,9 @@
 		return Config{}, err
 	}
 
-	inMakeFile := filepath.Join(buildDir, ".soong.in_make")
-	if _, err := os.Stat(absolutePath(inMakeFile)); err == nil {
-		config.inMake = true
+	KatiEnabledMarkerFile := filepath.Join(buildDir, ".soong.kati_enabled")
+	if _, err := os.Stat(absolutePath(KatiEnabledMarkerFile)); err == nil {
+		config.katiEnabled = true
 	}
 
 	// Sets up the map of target OSes to the finer grained compilation targets
@@ -551,9 +554,12 @@
 	return PathForOutput(ctx, "host", c.PrebuiltOS(), "framework", path)
 }
 
-// HostSystemTool looks for non-hermetic tools from the system we're running on.
-// Generally shouldn't be used, but useful to find the XCode SDK, etc.
-func (c *config) HostSystemTool(name string) string {
+// NonHermeticHostSystemTool looks for non-hermetic tools from the system we're
+// running on. These tools are not checked-in to AOSP, and therefore could lead
+// to reproducibility problems. Should not be used for other than finding the
+// XCode SDK (xcrun, sw_vers), etc. See ui/build/paths/config.go for the
+// allowlist of host system tools.
+func (c *config) NonHermeticHostSystemTool(name string) string {
 	for _, dir := range filepath.SplitList(c.Getenv("PATH")) {
 		path := filepath.Join(dir, name)
 		if s, err := os.Stat(path); err != nil {
@@ -562,7 +568,10 @@
 			return path
 		}
 	}
-	return name
+	panic(fmt.Errorf(
+		"Unable to use '%s' as a host system tool for build system "+
+			"hermeticity reasons. See build/soong/ui/build/paths/config.go "+
+			"for the full list of allowed host tools on your system.", name))
 }
 
 // PrebuiltOS returns the name of the host OS used in prebuilts directories.
@@ -646,8 +655,8 @@
 	return c.envDeps
 }
 
-func (c *config) EmbeddedInMake() bool {
-	return c.inMake
+func (c *config) KatiEnabled() bool {
+	return c.katiEnabled
 }
 
 func (c *config) BuildId() string {
diff --git a/android/deptag.go b/android/deptag.go
new file mode 100644
index 0000000..be5c35c
--- /dev/null
+++ b/android/deptag.go
@@ -0,0 +1,45 @@
+// Copyright 2020 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package android
+
+import "github.com/google/blueprint"
+
+// Dependency tags can implement this interface and return true from InstallDepNeeded to annotate
+// that the installed files of the parent should depend on the installed files of the child.
+type InstallNeededDependencyTag interface {
+	// If InstallDepNeeded returns true then the installed files of the parent will depend on the
+	// installed files of the child.
+	InstallDepNeeded() bool
+}
+
+// Dependency tags can embed this struct to annotate that the installed files of the parent should
+// depend on the installed files of the child.
+type InstallAlwaysNeededDependencyTag struct{}
+
+func (i InstallAlwaysNeededDependencyTag) InstallDepNeeded() bool {
+	return true
+}
+
+var _ InstallNeededDependencyTag = InstallAlwaysNeededDependencyTag{}
+
+// IsInstallDepNeeded returns true if the dependency tag implements the InstallNeededDependencyTag
+// interface and the InstallDepNeeded returns true, meaning that the installed files of the parent
+// should depend on the installed files of the child.
+func IsInstallDepNeeded(tag blueprint.DependencyTag) bool {
+	if i, ok := tag.(InstallNeededDependencyTag); ok {
+		return i.InstallDepNeeded()
+	}
+	return false
+}
diff --git a/android/deptag_test.go b/android/deptag_test.go
new file mode 100644
index 0000000..bdd449e
--- /dev/null
+++ b/android/deptag_test.go
@@ -0,0 +1,135 @@
+// Copyright 2020 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package android
+
+import (
+	"testing"
+
+	"github.com/google/blueprint"
+)
+
+type testInstallDependencyTagModule struct {
+	ModuleBase
+	Properties struct {
+		Install_deps []string
+		Deps         []string
+	}
+}
+
+func (t *testInstallDependencyTagModule) GenerateAndroidBuildActions(ctx ModuleContext) {
+	outputFile := PathForModuleOut(ctx, "out")
+	ctx.Build(pctx, BuildParams{
+		Rule:   Touch,
+		Output: outputFile,
+	})
+	ctx.InstallFile(PathForModuleInstall(ctx), ctx.ModuleName(), outputFile)
+}
+
+var testInstallDependencyTagAlwaysDepTag = struct {
+	blueprint.DependencyTag
+	InstallAlwaysNeededDependencyTag
+}{}
+
+var testInstallDependencyTagNeverDepTag = struct {
+	blueprint.DependencyTag
+}{}
+
+func (t *testInstallDependencyTagModule) DepsMutator(ctx BottomUpMutatorContext) {
+	ctx.AddVariationDependencies(nil, testInstallDependencyTagAlwaysDepTag, t.Properties.Install_deps...)
+	ctx.AddVariationDependencies(nil, testInstallDependencyTagNeverDepTag, t.Properties.Deps...)
+}
+
+func testInstallDependencyTagModuleFactory() Module {
+	module := &testInstallDependencyTagModule{}
+	InitAndroidArchModule(module, HostAndDeviceDefault, MultilibCommon)
+	module.AddProperties(&module.Properties)
+	return module
+}
+
+func TestInstallDependencyTag(t *testing.T) {
+	bp := `
+		test_module {
+			name: "foo",
+			deps: ["dep"],
+			install_deps: ["install_dep"],
+		}
+
+		test_module {
+			name: "install_dep",
+			install_deps: ["transitive"],
+		}
+
+		test_module {
+			name: "transitive",
+		}
+
+		test_module {
+			name: "dep",
+		}
+	`
+
+	config := TestArchConfig(buildDir, nil, bp, nil)
+	ctx := NewTestArchContext(config)
+
+	ctx.RegisterModuleType("test_module", testInstallDependencyTagModuleFactory)
+
+	ctx.Register()
+	_, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+	FailIfErrored(t, errs)
+	_, errs = ctx.PrepareBuildActions(config)
+	FailIfErrored(t, errs)
+
+	hostFoo := ctx.ModuleForTests("foo", config.BuildOSCommonTarget.String()).Description("install")
+	hostInstallDep := ctx.ModuleForTests("install_dep", config.BuildOSCommonTarget.String()).Description("install")
+	hostTransitive := ctx.ModuleForTests("transitive", config.BuildOSCommonTarget.String()).Description("install")
+	hostDep := ctx.ModuleForTests("dep", config.BuildOSCommonTarget.String()).Description("install")
+
+	if g, w := hostFoo.Implicits.Strings(), hostInstallDep.Output.String(); !InList(w, g) {
+		t.Errorf("expected host dependency %q, got %q", w, g)
+	}
+
+	if g, w := hostFoo.Implicits.Strings(), hostTransitive.Output.String(); !InList(w, g) {
+		t.Errorf("expected host dependency %q, got %q", w, g)
+	}
+
+	if g, w := hostInstallDep.Implicits.Strings(), hostTransitive.Output.String(); !InList(w, g) {
+		t.Errorf("expected host dependency %q, got %q", w, g)
+	}
+
+	if g, w := hostFoo.Implicits.Strings(), hostDep.Output.String(); InList(w, g) {
+		t.Errorf("expected no host dependency %q, got %q", w, g)
+	}
+
+	deviceFoo := ctx.ModuleForTests("foo", "android_common").Description("install")
+	deviceInstallDep := ctx.ModuleForTests("install_dep", "android_common").Description("install")
+	deviceTransitive := ctx.ModuleForTests("transitive", "android_common").Description("install")
+	deviceDep := ctx.ModuleForTests("dep", "android_common").Description("install")
+
+	if g, w := deviceFoo.OrderOnly.Strings(), deviceInstallDep.Output.String(); !InList(w, g) {
+		t.Errorf("expected device dependency %q, got %q", w, g)
+	}
+
+	if g, w := deviceFoo.OrderOnly.Strings(), deviceTransitive.Output.String(); !InList(w, g) {
+		t.Errorf("expected device dependency %q, got %q", w, g)
+	}
+
+	if g, w := deviceInstallDep.OrderOnly.Strings(), deviceTransitive.Output.String(); !InList(w, g) {
+		t.Errorf("expected device dependency %q, got %q", w, g)
+	}
+
+	if g, w := deviceFoo.OrderOnly.Strings(), deviceDep.Output.String(); InList(w, g) {
+		t.Errorf("expected no device dependency %q, got %q", w, g)
+	}
+}
diff --git a/android/filegroup.go b/android/filegroup.go
index 68311e3..9425616 100644
--- a/android/filegroup.go
+++ b/android/filegroup.go
@@ -15,6 +15,7 @@
 package android
 
 import (
+	"android/soong/bazel"
 	"strings"
 )
 
@@ -37,6 +38,9 @@
 	// Create a make variable with the specified name that contains the list of files in the
 	// filegroup, relative to the root of the source tree.
 	Export_to_make_var *string
+
+	// Properties for Bazel migration purposes.
+	bazel.Properties
 }
 
 type fileGroup struct {
diff --git a/android/makevars.go b/android/makevars.go
index f784395..5101436 100644
--- a/android/makevars.go
+++ b/android/makevars.go
@@ -213,7 +213,7 @@
 }
 
 func (s *makeVarsSingleton) GenerateBuildActions(ctx SingletonContext) {
-	if !ctx.Config().EmbeddedInMake() {
+	if !ctx.Config().KatiEnabled() {
 		return
 	}
 
diff --git a/android/module.go b/android/module.go
index 8be00b2..bd60829 100644
--- a/android/module.go
+++ b/android/module.go
@@ -1286,14 +1286,18 @@
 	return m.commonProperties.NamespaceExportedToMake
 }
 
+// computeInstallDeps finds the installed paths of all dependencies that have a dependency
+// tag that is annotated as needing installation via the IsInstallDepNeeded method.
 func (m *ModuleBase) computeInstallDeps(ctx blueprint.ModuleContext) InstallPaths {
-
 	var result InstallPaths
-	// TODO(ccross): we need to use WalkDeps and have some way to know which dependencies require installation
-	ctx.VisitDepsDepthFirst(func(m blueprint.Module) {
-		if a, ok := m.(Module); ok {
-			result = append(result, a.FilesToInstall()...)
+	ctx.WalkDeps(func(child, parent blueprint.Module) bool {
+		if a, ok := child.(Module); ok {
+			if IsInstallDepNeeded(ctx.OtherModuleDependencyTag(child)) {
+				result = append(result, a.FilesToInstall()...)
+				return true
+			}
 		}
+		return false
 	})
 
 	return result
@@ -1440,7 +1444,7 @@
 
 	if len(deps) > 0 {
 		suffix := ""
-		if ctx.Config().EmbeddedInMake() {
+		if ctx.Config().KatiEnabled() {
 			suffix = "-soong"
 		}
 
@@ -2320,7 +2324,7 @@
 	}
 
 	if m.Device() {
-		if m.Config().EmbeddedInMake() && !m.InstallBypassMake() {
+		if m.Config().KatiEnabled() && !m.InstallBypassMake() {
 			return true
 		}
 
@@ -2373,7 +2377,7 @@
 			Input:       srcPath,
 			Implicits:   implicitDeps,
 			OrderOnly:   orderOnlyDeps,
-			Default:     !m.Config().EmbeddedInMake(),
+			Default:     !m.Config().KatiEnabled(),
 		})
 
 		m.installFiles = append(m.installFiles, fullInstallPath)
@@ -2405,7 +2409,7 @@
 			Description: "install symlink " + fullInstallPath.Base(),
 			Output:      fullInstallPath,
 			Input:       srcPath,
-			Default:     !m.Config().EmbeddedInMake(),
+			Default:     !m.Config().KatiEnabled(),
 			Args: map[string]string{
 				"fromPath": relPath,
 			},
@@ -2436,7 +2440,7 @@
 			Rule:        Symlink,
 			Description: "install symlink " + fullInstallPath.Base() + " -> " + absPath,
 			Output:      fullInstallPath,
-			Default:     !m.Config().EmbeddedInMake(),
+			Default:     !m.Config().KatiEnabled(),
 			Args: map[string]string{
 				"fromPath": absPath,
 			},
@@ -2674,7 +2678,7 @@
 	})
 
 	suffix := ""
-	if ctx.Config().EmbeddedInMake() {
+	if ctx.Config().KatiEnabled() {
 		suffix = "-soong"
 	}
 
@@ -2682,7 +2686,7 @@
 	ctx.Phony("checkbuild"+suffix, checkbuildDeps...)
 
 	// Make will generate the MODULES-IN-* targets
-	if ctx.Config().EmbeddedInMake() {
+	if ctx.Config().KatiEnabled() {
 		return
 	}
 
diff --git a/android/paths.go b/android/paths.go
index b13979d..a62c9e3 100644
--- a/android/paths.go
+++ b/android/paths.go
@@ -1312,7 +1312,7 @@
 
 	ret := pathForInstall(ctx, os, arch, partition, ctx.Debug(), pathComponents...)
 
-	if ctx.InstallBypassMake() && ctx.Config().EmbeddedInMake() {
+	if ctx.InstallBypassMake() && ctx.Config().KatiEnabled() {
 		ret = ret.ToMakePath()
 	}
 
diff --git a/android/phony.go b/android/phony.go
index f8e5a44..0adbb55 100644
--- a/android/phony.go
+++ b/android/phony.go
@@ -53,7 +53,7 @@
 		p.phonyMap[phony] = SortedUniquePaths(p.phonyMap[phony])
 	}
 
-	if !ctx.Config().EmbeddedInMake() {
+	if !ctx.Config().KatiEnabled() {
 		for _, phony := range p.phonyList {
 			ctx.Build(pctx, BuildParams{
 				Rule:      blueprint.Phony,
diff --git a/android/rule_builder.go b/android/rule_builder.go
index 86418b2..3efe9f8 100644
--- a/android/rule_builder.go
+++ b/android/rule_builder.go
@@ -20,27 +20,33 @@
 	"path/filepath"
 	"sort"
 	"strings"
+	"testing"
 
+	"github.com/golang/protobuf/proto"
 	"github.com/google/blueprint"
 	"github.com/google/blueprint/proptools"
 
+	"android/soong/cmd/sbox/sbox_proto"
 	"android/soong/shared"
 )
 
-const sboxOutDir = "__SBOX_OUT_DIR__"
+const sboxSandboxBaseDir = "__SBOX_SANDBOX_DIR__"
+const sboxOutSubDir = "out"
+const sboxOutDir = sboxSandboxBaseDir + "/" + sboxOutSubDir
 
 // RuleBuilder provides an alternative to ModuleContext.Rule and ModuleContext.Build to add a command line to the build
 // graph.
 type RuleBuilder struct {
-	commands       []*RuleBuilderCommand
-	installs       RuleBuilderInstalls
-	temporariesSet map[WritablePath]bool
-	restat         bool
-	sbox           bool
-	highmem        bool
-	remoteable     RemoteRuleSupports
-	sboxOutDir     WritablePath
-	missingDeps    []string
+	commands         []*RuleBuilderCommand
+	installs         RuleBuilderInstalls
+	temporariesSet   map[WritablePath]bool
+	restat           bool
+	sbox             bool
+	highmem          bool
+	remoteable       RemoteRuleSupports
+	sboxOutDir       WritablePath
+	sboxManifestPath WritablePath
+	missingDeps      []string
 }
 
 // NewRuleBuilder returns a newly created RuleBuilder.
@@ -106,12 +112,14 @@
 	return r
 }
 
-// Sbox marks the rule as needing to be wrapped by sbox. The WritablePath should point to the output
-// directory that sbox will wipe. It should not be written to by any other rule. sbox will ensure
-// that all outputs have been written, and will discard any output files that were not specified.
+// Sbox 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
+// will ensure that all outputs have been written, and will discard any output files that were not
+// specified.
 //
 // Sbox is not compatible with Restat()
-func (r *RuleBuilder) Sbox(outputDir WritablePath) *RuleBuilder {
+func (r *RuleBuilder) Sbox(outputDir WritablePath, manifestPath WritablePath) *RuleBuilder {
 	if r.sbox {
 		panic("Sbox() may not be called more than once")
 	}
@@ -123,6 +131,7 @@
 	}
 	r.sbox = true
 	r.sboxOutDir = outputDir
+	r.sboxManifestPath = manifestPath
 	return r
 }
 
@@ -420,7 +429,8 @@
 			r.depFileMergerCmd(ctx, depFiles)
 
 			if r.sbox {
-				// Check for Rel() errors, as all depfiles should be in the output dir
+				// Check for Rel() errors, as all depfiles should be in the output dir.  Errors
+				// will be reported to the ctx.
 				for _, path := range depFiles[1:] {
 					Rel(ctx, r.sboxOutDir.String(), path.String())
 				}
@@ -443,34 +453,60 @@
 	commandString := strings.Join(commands, " && ")
 
 	if r.sbox {
-		sboxOutputs := make([]string, len(outputs))
-		for i, output := range outputs {
-			sboxOutputs[i] = filepath.Join(sboxOutDir, Rel(ctx, r.sboxOutDir.String(), output.String()))
-		}
-
-		commandString = proptools.ShellEscape(commandString)
-		if !strings.HasPrefix(commandString, `'`) {
-			commandString = `'` + commandString + `'`
-		}
-
-		sboxCmd := &RuleBuilderCommand{}
-		sboxCmd.BuiltTool(ctx, "sbox").
-			Flag("-c").Text(commandString).
-			Flag("--sandbox-path").Text(shared.TempDirForOutDir(PathForOutput(ctx).String())).
-			Flag("--output-root").Text(r.sboxOutDir.String())
+		// If running the command inside sbox, write the rule data out to an sbox
+		// manifest.textproto.
+		manifest := sbox_proto.Manifest{}
+		command := sbox_proto.Command{}
+		manifest.Commands = append(manifest.Commands, &command)
+		command.Command = proto.String(commandString)
 
 		if depFile != nil {
-			sboxCmd.Flag("--depfile-out").Text(depFile.String())
+			manifest.OutputDepfile = proto.String(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))
+		// Add copy rules to the manifest to copy each output file from the sbox directory.
+		// to the output directory.
+		sboxOutputs := make([]string, len(outputs))
+		for i, output := range outputs {
+			rel := Rel(ctx, r.sboxOutDir.String(), output.String())
+			sboxOutputs[i] = filepath.Join(sboxOutDir, rel)
+			command.CopyAfter = append(command.CopyAfter, &sbox_proto.Copy{
+				From: proto.String(filepath.Join(sboxOutSubDir, rel)),
+				To:   proto.String(output.String()),
+			})
+		}
 
-		sboxCmd.Flags(sboxOutputs)
+		// Add a hash of the list of input files to the manifest so that the textproto file
+		// changes when the list of input files changes and causes the sbox rule that
+		// depends on it to rerun.
+		command.InputHash = proto.String(hashSrcFiles(inputs))
 
+		// Verify that the manifest textproto is not inside the sbox output directory, otherwise
+		// it will get deleted when the sbox rule clears its output directory.
+		_, manifestInOutDir := MaybeRel(ctx, r.sboxOutDir.String(), r.sboxManifestPath.String())
+		if manifestInOutDir {
+			ReportPathErrorf(ctx, "sbox rule %q manifestPath %q must not be in outputDir %q",
+				name, r.sboxManifestPath.String(), r.sboxOutDir.String())
+		}
+
+		// Create a rule to write the manifest as a the textproto.
+		WriteFileRule(ctx, r.sboxManifestPath, proto.MarshalTextString(&manifest))
+
+		// Generate a new string to use as the command line of the sbox rule.  This uses
+		// a RuleBuilderCommand as a convenience method of building the command line, then
+		// converts it to a string to replace commandString.
+		sboxCmd := &RuleBuilderCommand{}
+		sboxCmd.Text("rm -rf").Output(r.sboxOutDir)
+		sboxCmd.Text("&&")
+		sboxCmd.BuiltTool(ctx, "sbox").
+			Flag("--sandbox-path").Text(shared.TempDirForOutDir(PathForOutput(ctx).String())).
+			Flag("--manifest").Input(r.sboxManifestPath)
+
+		// Replace the command string, and add the sbox tool and manifest textproto to the
+		// dependencies of the final sbox rule.
 		commandString = sboxCmd.buf.String()
 		tools = append(tools, sboxCmd.tools...)
+		inputs = append(inputs, sboxCmd.inputs...)
 	} 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
@@ -890,6 +926,19 @@
 	return ninjaEscapeExceptForSpans(c.String(), c.unescapedSpans)
 }
 
+// RuleBuilderSboxProtoForTests takes the BuildParams for the manifest passed to RuleBuilder.Sbox()
+// and returns sbox testproto generated by the RuleBuilder.
+func RuleBuilderSboxProtoForTests(t *testing.T, params TestingBuildParams) *sbox_proto.Manifest {
+	t.Helper()
+	content := ContentFromFileRuleForTests(t, params)
+	manifest := sbox_proto.Manifest{}
+	err := proto.UnmarshalText(content, &manifest)
+	if err != nil {
+		t.Fatalf("failed to unmarshal manifest: %s", err.Error())
+	}
+	return &manifest
+}
+
 func ninjaEscapeExceptForSpans(s string, spans [][2]int) string {
 	if len(spans) == 0 {
 		return proptools.NinjaEscape(s)
diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go
index c1d5521..dc360c3 100644
--- a/android/rule_builder_test.go
+++ b/android/rule_builder_test.go
@@ -395,16 +395,17 @@
 	})
 
 	t.Run("sbox", func(t *testing.T) {
-		rule := NewRuleBuilder().Sbox(PathForOutput(ctx))
+		rule := NewRuleBuilder().Sbox(PathForOutput(ctx, ""),
+			PathForOutput(ctx, "sbox.textproto"))
 		addCommands(rule)
 
 		wantCommands := []string{
-			"__SBOX_OUT_DIR__/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_OUT_DIR__/depfile FlagWithInput=input FlagWithOutput=__SBOX_OUT_DIR__/output Input __SBOX_OUT_DIR__/Output __SBOX_OUT_DIR__/SymlinkOutput Text Tool after command2 old cmd",
-			"command2 __SBOX_OUT_DIR__/depfile2 input2 __SBOX_OUT_DIR__/output2 tool2",
-			"command3 input3 __SBOX_OUT_DIR__/output2 __SBOX_OUT_DIR__/output3",
+			"__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",
+			"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",
 		}
 
-		wantDepMergerCommand := "out/host/" + ctx.Config().PrebuiltOS() + "/bin/dep_fixer __SBOX_OUT_DIR__/DepFile __SBOX_OUT_DIR__/depfile __SBOX_OUT_DIR__/ImplicitDepFile __SBOX_OUT_DIR__/depfile2"
+		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"
 
 		if g, w := rule.Commands(), wantCommands; !reflect.DeepEqual(g, w) {
 			t.Errorf("\nwant rule.Commands() = %#v\n                   got %#v", w, g)
@@ -451,11 +452,12 @@
 
 func (t *testRuleBuilderModule) GenerateAndroidBuildActions(ctx ModuleContext) {
 	in := PathsForSource(ctx, t.properties.Srcs)
-	out := PathForModuleOut(ctx, ctx.ModuleName())
-	outDep := PathForModuleOut(ctx, ctx.ModuleName()+".d")
-	outDir := PathForModuleOut(ctx)
+	out := PathForModuleOut(ctx, "gen", ctx.ModuleName())
+	outDep := PathForModuleOut(ctx, "gen", ctx.ModuleName()+".d")
+	outDir := PathForModuleOut(ctx, "gen")
+	manifestPath := PathForModuleOut(ctx, "sbox.textproto")
 
-	testRuleBuilder_Build(ctx, in, out, outDep, outDir, t.properties.Restat, t.properties.Sbox)
+	testRuleBuilder_Build(ctx, in, out, outDep, outDir, manifestPath, t.properties.Restat, t.properties.Sbox)
 }
 
 type testRuleBuilderSingleton struct{}
@@ -466,17 +468,18 @@
 
 func (t *testRuleBuilderSingleton) GenerateBuildActions(ctx SingletonContext) {
 	in := PathForSource(ctx, "bar")
-	out := PathForOutput(ctx, "baz")
-	outDep := PathForOutput(ctx, "baz.d")
-	outDir := PathForOutput(ctx)
-	testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, true, false)
+	out := PathForOutput(ctx, "singleton/gen/baz")
+	outDep := PathForOutput(ctx, "singleton/gen/baz.d")
+	outDir := PathForOutput(ctx, "singleton/gen")
+	manifestPath := PathForOutput(ctx, "singleton/sbox.textproto")
+	testRuleBuilder_Build(ctx, Paths{in}, out, outDep, outDir, manifestPath, true, false)
 }
 
-func testRuleBuilder_Build(ctx BuilderContext, in Paths, out, outDep, outDir WritablePath, restat, sbox bool) {
+func testRuleBuilder_Build(ctx BuilderContext, in Paths, out, outDep, outDir, manifestPath WritablePath, restat, sbox bool) {
 	rule := NewRuleBuilder()
 
 	if sbox {
-		rule.Sbox(outDir)
+		rule.Sbox(outDir, manifestPath)
 	}
 
 	rule.Command().Tool(PathForSource(ctx, "cp")).Inputs(in).Output(out).ImplicitDepFile(outDep)
@@ -518,10 +521,10 @@
 	_, errs = ctx.PrepareBuildActions(config)
 	FailIfErrored(t, errs)
 
-	check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraCmdDeps []string) {
+	check := func(t *testing.T, params TestingBuildParams, wantCommand, wantOutput, wantDepfile string, wantRestat bool, extraImplicits, extraCmdDeps []string) {
 		t.Helper()
 		command := params.RuleParams.Command
-		re := regexp.MustCompile(" (# hash of input list:|--input-hash) [a-z0-9]*")
+		re := regexp.MustCompile(" # hash of input list: [a-z0-9]*$")
 		command = re.ReplaceAllLiteralString(command, "")
 		if command != wantCommand {
 			t.Errorf("\nwant RuleParams.Command = %q\n                      got %q", wantCommand, params.RuleParams.Command)
@@ -536,7 +539,8 @@
 			t.Errorf("want RuleParams.Restat = %v, got %v", wantRestat, params.RuleParams.Restat)
 		}
 
-		if len(params.Implicits) != 1 || params.Implicits[0].String() != "bar" {
+		wantImplicits := append([]string{"bar"}, extraImplicits...)
+		if !reflect.DeepEqual(params.Implicits.Strings(), wantImplicits) {
 			t.Errorf("want Implicits = [%q], got %q", "bar", params.Implicits.Strings())
 		}
 
@@ -558,27 +562,29 @@
 	}
 
 	t.Run("module", func(t *testing.T) {
-		outFile := filepath.Join(buildDir, ".intermediates", "foo", "foo")
+		outFile := filepath.Join(buildDir, ".intermediates", "foo", "gen", "foo")
 		check(t, ctx.ModuleForTests("foo", "").Rule("rule"),
 			"cp bar "+outFile,
-			outFile, outFile+".d", true, nil)
+			outFile, outFile+".d", true, nil, nil)
 	})
 	t.Run("sbox", func(t *testing.T) {
 		outDir := filepath.Join(buildDir, ".intermediates", "foo_sbox")
-		outFile := filepath.Join(outDir, "foo_sbox")
-		depFile := filepath.Join(outDir, "foo_sbox.d")
+		outFile := filepath.Join(outDir, "gen/foo_sbox")
+		depFile := filepath.Join(outDir, "gen/foo_sbox.d")
+		manifest := filepath.Join(outDir, "sbox.textproto")
 		sbox := filepath.Join(buildDir, "host", config.PrebuiltOS(), "bin/sbox")
 		sandboxPath := shared.TempDirForOutDir(buildDir)
 
-		cmd := sbox + ` -c 'cp bar __SBOX_OUT_DIR__/foo_sbox' --sandbox-path ` + sandboxPath + " --output-root " + outDir + " --depfile-out " + depFile + " __SBOX_OUT_DIR__/foo_sbox"
+		cmd := `rm -rf ` + outDir + `/gen && ` +
+			sbox + ` --sandbox-path ` + sandboxPath + ` --manifest ` + manifest
 
-		check(t, ctx.ModuleForTests("foo_sbox", "").Rule("rule"),
-			cmd, outFile, depFile, false, []string{sbox})
+		check(t, ctx.ModuleForTests("foo_sbox", "").Output("gen/foo_sbox"),
+			cmd, outFile, depFile, false, []string{manifest}, []string{sbox})
 	})
 	t.Run("singleton", func(t *testing.T) {
-		outFile := filepath.Join(buildDir, "baz")
+		outFile := filepath.Join(buildDir, "singleton/gen/baz")
 		check(t, ctx.SingletonForTests("rule_builder_test").Rule("rule"),
-			"cp bar "+outFile, outFile, outFile+".d", true, nil)
+			"cp bar "+outFile, outFile, outFile+".d", true, nil, nil)
 	})
 }
 
@@ -715,14 +721,16 @@
 		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)
+				manifest := RuleBuilderSboxProtoForTests(t, gen.Output("sbox.textproto"))
+				hash := manifest.Commands[0].GetInputHash()
+
+				if g, w := hash, test.expectedHash; g != w {
+					t.Errorf("Expected has %q, got %q", w, g)
 				}
 			})
 			t.Run("", func(t *testing.T) {
 				gen := ctx.ModuleForTests(test.name+"", "")
-				command := gen.Output(test.name).RuleParams.Command
+				command := gen.Output("gen/" + 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/android/testing.go b/android/testing.go
index 1e2ae13..6539063 100644
--- a/android/testing.go
+++ b/android/testing.go
@@ -424,8 +424,8 @@
 
 }
 
-func SetInMakeForTests(config Config) {
-	config.inMake = true
+func SetKatiEnabledForTests(config Config) {
+	config.katiEnabled = true
 }
 
 func AndroidMkEntriesForTest(t *testing.T, config Config, bpPath string, mod blueprint.Module) []AndroidMkEntries {
diff --git a/apex/androidmk.go b/apex/androidmk.go
index 993260c..fe89b73 100644
--- a/apex/androidmk.go
+++ b/apex/androidmk.go
@@ -36,6 +36,33 @@
 	return a.androidMkForType()
 }
 
+// nameInMake converts apexFileClass into the corresponding class name in Make.
+func (class apexFileClass) nameInMake() string {
+	switch class {
+	case etc:
+		return "ETC"
+	case nativeSharedLib:
+		return "SHARED_LIBRARIES"
+	case nativeExecutable, shBinary, pyBinary, goBinary:
+		return "EXECUTABLES"
+	case javaSharedLib:
+		return "JAVA_LIBRARIES"
+	case nativeTest:
+		return "NATIVE_TESTS"
+	case app, appSet:
+		// b/142537672 Why isn't this APP? We want to have full control over
+		// the paths and file names of the apk file under the flattend APEX.
+		// If this is set to APP, then the paths and file names are modified
+		// by the Make build system. For example, it is installed to
+		// /system/apex/<apexname>/app/<Appname>/<apexname>.<Appname>/ instead of
+		// /system/apex/<apexname>/app/<Appname> because the build system automatically
+		// appends module name (which is <apexname>.<Appname> to the path.
+		return "ETC"
+	default:
+		panic(fmt.Errorf("unknown class %d", class))
+	}
+}
+
 func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, moduleDir string,
 	apexAndroidMkData android.AndroidMkData) []string {
 
@@ -68,10 +95,10 @@
 
 	var postInstallCommands []string
 	for _, fi := range a.filesInfo {
-		if a.linkToSystemLib && fi.transitiveDep && fi.AvailableToPlatform() {
+		if a.linkToSystemLib && fi.transitiveDep && fi.availableToPlatform() {
 			// TODO(jiyong): pathOnDevice should come from fi.module, not being calculated here
-			linkTarget := filepath.Join("/system", fi.Path())
-			linkPath := filepath.Join(a.installDir.ToMakePath().String(), apexBundleName, fi.Path())
+			linkTarget := filepath.Join("/system", fi.path())
+			linkPath := filepath.Join(a.installDir.ToMakePath().String(), apexBundleName, fi.path())
 			mkdirCmd := "mkdir -p " + filepath.Dir(linkPath)
 			linkCmd := "ln -sfn " + linkTarget + " " + linkPath
 			postInstallCommands = append(postInstallCommands, mkdirCmd, linkCmd)
@@ -85,7 +112,7 @@
 			continue
 		}
 
-		linkToSystemLib := a.linkToSystemLib && fi.transitiveDep && fi.AvailableToPlatform()
+		linkToSystemLib := a.linkToSystemLib && fi.transitiveDep && fi.availableToPlatform()
 
 		var moduleName string
 		if linkToSystemLib {
@@ -151,7 +178,7 @@
 			fmt.Fprintln(w, "LOCAL_NO_NOTICE_FILE := true")
 		}
 		fmt.Fprintln(w, "LOCAL_PREBUILT_MODULE_FILE :=", fi.builtFile.String())
-		fmt.Fprintln(w, "LOCAL_MODULE_CLASS :=", fi.class.NameInMake())
+		fmt.Fprintln(w, "LOCAL_MODULE_CLASS :=", fi.class.nameInMake())
 		if fi.module != nil {
 			archStr := fi.module.Target().Arch.ArchType.String()
 			host := false
@@ -189,7 +216,7 @@
 			// soong_java_prebuilt.mk sets LOCAL_MODULE_SUFFIX := .jar  Therefore
 			// we need to remove the suffix from LOCAL_MODULE_STEM, otherwise
 			// we will have foo.jar.jar
-			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", strings.TrimSuffix(fi.Stem(), ".jar"))
+			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", strings.TrimSuffix(fi.stem(), ".jar"))
 			if javaModule, ok := fi.module.(java.ApexDependency); ok {
 				fmt.Fprintln(w, "LOCAL_SOONG_CLASSES_JAR :=", javaModule.ImplementationAndResourcesJars()[0].String())
 				fmt.Fprintln(w, "LOCAL_SOONG_HEADER_JAR :=", javaModule.HeaderJars()[0].String())
@@ -205,7 +232,7 @@
 			// soong_app_prebuilt.mk sets LOCAL_MODULE_SUFFIX := .apk  Therefore
 			// we need to remove the suffix from LOCAL_MODULE_STEM, otherwise
 			// we will have foo.apk.apk
-			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", strings.TrimSuffix(fi.Stem(), ".apk"))
+			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", strings.TrimSuffix(fi.stem(), ".apk"))
 			if app, ok := fi.module.(*java.AndroidApp); ok {
 				if jniCoverageOutputs := app.JniCoverageOutputs(); len(jniCoverageOutputs) > 0 {
 					fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE :=", strings.Join(jniCoverageOutputs.Strings(), " "))
@@ -224,7 +251,7 @@
 			fmt.Fprintln(w, "LOCAL_APKCERTS_FILE :=", as.APKCertsFile().String())
 			fmt.Fprintln(w, "include $(BUILD_SYSTEM)/soong_android_app_set.mk")
 		case nativeSharedLib, nativeExecutable, nativeTest:
-			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", fi.Stem())
+			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", fi.stem())
 			if ccMod, ok := fi.module.(*cc.Module); ok {
 				if ccMod.UnstrippedOutputFile() != nil {
 					fmt.Fprintln(w, "LOCAL_SOONG_UNSTRIPPED_BINARY :=", ccMod.UnstrippedOutputFile().String())
@@ -236,7 +263,7 @@
 			}
 			fmt.Fprintln(w, "include $(BUILD_SYSTEM)/soong_cc_prebuilt.mk")
 		default:
-			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", fi.Stem())
+			fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", fi.stem())
 			if fi.builtFile == a.manifestPbOut && apexType == flattenedApex {
 				if a.primaryApexType {
 					// To install companion files (init_rc, vintf_fragments)
diff --git a/apex/apex.go b/apex/apex.go
index dff0855..a645b06 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// package apex implements build rules for creating the APEX files which are container for
+// lower-level system components. See https://source.android.com/devices/tech/ota/apex
 package apex
 
 import (
@@ -63,57 +65,99 @@
 }
 
 type apexBundleProperties struct {
-	// Json manifest file describing meta info of this APEX bundle. Default:
-	// "apex_manifest.json"
+	// Json manifest file describing meta info of this APEX bundle. Refer to
+	// system/apex/proto/apex_manifest.proto for the schema. Default: "apex_manifest.json"
 	Manifest *string `android:"path"`
 
-	// AndroidManifest.xml file used for the zip container of this APEX bundle.
-	// If unspecified, a default one is automatically generated.
+	// AndroidManifest.xml file used for the zip container of this APEX bundle. If unspecified,
+	// a default one is automatically generated.
 	AndroidManifest *string `android:"path"`
 
-	// Canonical name of the APEX bundle. Used to determine the path to the activated APEX on
-	// device (/apex/<apex_name>).
-	// If unspecified, defaults to the value of name.
+	// Canonical name of this APEX bundle. Used to determine the path to the activated APEX on
+	// device (/apex/<apex_name>). If unspecified, follows the name property.
 	Apex_name *string
 
-	// Determines the file contexts file for setting security context to each file in this APEX bundle.
-	// For platform APEXes, this should points to a file under /system/sepolicy
-	// Default: /system/sepolicy/apex/<module_name>_file_contexts.
+	// Determines the file contexts file for setting the security contexts to files in this APEX
+	// bundle. For platform APEXes, this should points to a file under /system/sepolicy Default:
+	// /system/sepolicy/apex/<module_name>_file_contexts.
 	File_contexts *string `android:"path"`
 
 	ApexNativeDependencies
 
-	// List of java libraries that are embedded inside this APEX bundle
+	Multilib apexMultilibProperties
+
+	// List of java libraries that are embedded inside this APEX bundle.
 	Java_libs []string
 
-	// List of prebuilt files that are embedded inside this APEX bundle
+	// List of prebuilt files that are embedded inside this APEX bundle.
 	Prebuilts []string
 
-	// List of BPF programs inside APEX
+	// List of BPF programs inside this APEX bundle.
 	Bpfs []string
 
-	// Name of the apex_key module that provides the private key to sign APEX
+	// Name of the apex_key module that provides the private key to sign this APEX bundle.
 	Key *string
 
-	// The type of APEX to build. Controls what the APEX payload is. Either
-	// 'image', 'zip' or 'both'. Default: 'image'.
-	Payload_type *string
-
-	// The name of a certificate in the default certificate directory, blank to use the default product certificate,
-	// or an android_app_certificate module name in the form ":module".
+	// Specifies the certificate and the private key to sign the zip container of this APEX. If
+	// this is "foo", foo.x509.pem and foo.pk8 under PRODUCT_DEFAULT_DEV_CERTIFICATE are used
+	// as the certificate and the private key, respectively. If this is ":module", then the
+	// certificate and the private key are provided from the android_app_certificate module
+	// named "module".
 	Certificate *string
 
-	// Whether this APEX is installable to one of the partitions. Default: true.
+	// The minimum SDK version that this APEX must support at minimum. This is usually set to
+	// the SDK version that the APEX was first introduced.
+	Min_sdk_version *string
+
+	// Whether this APEX is considered updatable or not. When set to true, this will enforce
+	// additional rules for making sure that the APEX is truly updatable. To be updatable,
+	// min_sdk_version should be set as well. This will also disable the size optimizations like
+	// symlinking to the system libs. Default is false.
+	Updatable *bool
+
+	// Whether this APEX is installable to one of the partitions like system, vendor, etc.
+	// Default: true.
 	Installable *bool
 
-	// For native libraries and binaries, use the vendor variant instead of the core (platform) variant.
-	// Default is false.
+	// For native libraries and binaries, use the vendor variant instead of the core (platform)
+	// variant. Default is false. DO NOT use this for APEXes that are installed to the system or
+	// system_ext partition.
 	Use_vendor *bool
 
-	// For telling the apex to ignore special handling for system libraries such as bionic. Default is false.
+	// If set true, VNDK libs are considered as stable libs and are not included in this APEX.
+	// Should be only used in non-system apexes (e.g. vendor: true). Default is false.
+	Use_vndk_as_stable *bool
+
+	// List of SDKs that are used to build this APEX. A reference to an SDK should be either
+	// `name#version` or `name` which is an alias for `name#current`. If left empty,
+	// `platform#current` is implied. This value affects all modules included in this APEX. In
+	// other words, they are also built with the SDKs specified here.
+	Uses_sdks []string
+
+	// The type of APEX to build. Controls what the APEX payload is. Either 'image', 'zip' or
+	// 'both'. When set to image, contents are stored in a filesystem image inside a zip
+	// container. When set to zip, contents are stored in a zip container directly. This type is
+	// mostly for host-side debugging. When set to both, the two types are both built. Default
+	// is 'image'.
+	Payload_type *string
+
+	// The type of filesystem to use when the payload_type is 'image'. Either 'ext4' or 'f2fs'.
+	// Default 'ext4'.
+	Payload_fs_type *string
+
+	// For telling the APEX to ignore special handling for system libraries such as bionic.
+	// Default is false.
 	Ignore_system_library_special_case *bool
 
-	Multilib apexMultilibProperties
+	// Whenever apex_payload.img of the APEX should include dm-verity hashtree. Should be only
+	// used in tests.
+	Test_only_no_hashtree *bool
+
+	// Whenever apex_payload.img of the APEX should not be dm-verity signed. Should be only
+	// used in tests.
+	Test_only_unsigned_payload *bool
+
+	IsCoverageVariant bool `blueprint:"mutated"`
 
 	// List of sanitizer names that this APEX is enabled for
 	SanitizerNames []string `blueprint:"mutated"`
@@ -122,57 +166,23 @@
 
 	HideFromMake bool `blueprint:"mutated"`
 
-	// package format of this apex variant; could be non-flattened, flattened, or zip.
-	// imageApex, zipApex or flattened
+	// Internal package method for this APEX. When payload_type is image, this can be either
+	// imageApex or flattenedApex depending on Config.FlattenApex(). When payload_type is zip,
+	// this becomes zipApex.
 	ApexType apexPackaging `blueprint:"mutated"`
-
-	// List of SDKs that are used to build this APEX. A reference to an SDK should be either
-	// `name#version` or `name` which is an alias for `name#current`. If left empty, `platform#current`
-	// is implied. This value affects all modules included in this APEX. In other words, they are
-	// also built with the SDKs specified here.
-	Uses_sdks []string
-
-	// Whenever apex_payload.img of the APEX should include dm-verity hashtree.
-	// Should be only used in tests#.
-	Test_only_no_hashtree *bool
-
-	// Whenever apex_payload.img of the APEX should not be dm-verity signed.
-	// Should be only used in tests#.
-	Test_only_unsigned_payload *bool
-
-	IsCoverageVariant bool `blueprint:"mutated"`
-
-	// Whether this APEX is considered updatable or not. When set to true, this will enforce additional
-	// rules for making sure that the APEX is truly updatable.
-	// - To be updatable, min_sdk_version should be set as well
-	// This will also disable the size optimizations like symlinking to the system libs.
-	// Default is false.
-	Updatable *bool
-
-	// The minimum SDK version that this apex must be compatibile with.
-	Min_sdk_version *string
-
-	// If set true, VNDK libs are considered as stable libs and are not included in this apex.
-	// Should be only used in non-system apexes (e.g. vendor: true).
-	// Default is false.
-	Use_vndk_as_stable *bool
-
-	// The type of filesystem to use for an image apex. Either 'ext4' or 'f2fs'.
-	// Default 'ext4'.
-	Payload_fs_type *string
 }
 
 type ApexNativeDependencies struct {
-	// List of native libraries
+	// List of native libraries that are embedded inside this APEX.
 	Native_shared_libs []string
 
-	// List of JNI libraries
+	// List of JNI libraries that are embedded inside this APEX.
 	Jni_libs []string
 
-	// List of native executables
+	// List of native executables that are embedded inside this APEX.
 	Binaries []string
 
-	// List of native tests
+	// List of native tests that are embedded inside this APEX.
 	Tests []string
 }
 
@@ -217,25 +227,26 @@
 	}
 }
 
+// These properties can be used in override_apex to override the corresponding properties in the
+// base apex.
 type overridableProperties struct {
-	// List of APKs to package inside APEX
+	// List of APKs that are embedded inside this APEX.
 	Apps []string
 
-	// List of runtime resource overlays (RROs) inside APEX
+	// List of runtime resource overlays (RROs) that are embedded inside this APEX.
 	Rros []string
 
-	// Names of modules to be overridden. Listed modules can only be other binaries
-	// (in Make or Soong).
-	// This does not completely prevent installation of the overridden binaries, but if both
-	// binaries would be installed by default (in PRODUCT_PACKAGES) the other binary will be removed
-	// from PRODUCT_PACKAGES.
+	// Names of modules to be overridden. Listed modules can only be other binaries (in Make or
+	// Soong). This does not completely prevent installation of the overridden binaries, but if
+	// both binaries would be installed by default (in PRODUCT_PACKAGES) the other binary will
+	// be removed from PRODUCT_PACKAGES.
 	Overrides []string
 
-	// Logging Parent value
+	// Logging parent value.
 	Logging_parent string
 
-	// Apex Container Package Name.
-	// Override value for attribute package:name in AndroidManifest.xml
+	// Apex Container package name. Override value for attribute package:name in
+	// AndroidManifest.xml
 	Package_name string
 
 	// A txt file containing list of files that are allowed to be included in this APEX.
@@ -243,187 +254,202 @@
 }
 
 type apexBundle struct {
+	// Inherited structs
 	android.ModuleBase
 	android.DefaultableModuleBase
 	android.OverridableModuleBase
 	android.SdkBase
 
+	// Properties
 	properties            apexBundleProperties
 	targetProperties      apexTargetBundleProperties
 	overridableProperties overridableProperties
+	vndkProperties        apexVndkProperties // only for apex_vndk modules
 
-	// specific to apex_vndk modules
-	vndkProperties apexVndkProperties
+	///////////////////////////////////////////////////////////////////////////////////////////
+	// Inputs
 
-	bundleModuleFile android.WritablePath
-	outputFile       android.WritablePath
-	installDir       android.InstallPath
-
-	prebuiltFileToDelete string
-
+	// Keys for apex_paylaod.img
 	public_key_file  android.Path
 	private_key_file android.Path
 
+	// Cert/priv-key for the zip container
 	container_certificate_file android.Path
 	container_private_key_file android.Path
 
-	fileContexts android.WritablePath
+	// Flags for special variants of APEX
+	testApex bool
+	vndkApex bool
+	artApex  bool
 
-	// list of files to be included in this apex
-	filesInfo []apexFile
-
-	// list of module names that should be installed along with this APEX
-	requiredDeps []string
-
-	// list of module names that this APEX is including (to be shown via *-deps-info target)
-	android.ApexBundleDepsInfo
-
-	testApex        bool
-	vndkApex        bool
-	artApex         bool
+	// Tells whether this variant of the APEX bundle is the primary one or not. Only the primary
+	// one gets installed to the device.
 	primaryApexType bool
 
-	manifestJsonOut android.WritablePath
-	manifestPbOut   android.WritablePath
-
-	// list of commands to create symlinks for backward compatibility.
-	// these commands will be attached as LOCAL_POST_INSTALL_CMD to
-	// apex package itself(for unflattened build) or apex_manifest(for flattened build)
-	// so that compat symlinks are always installed regardless of TARGET_FLATTEN_APEX setting.
-	compatSymlinks []string
-
-	// Suffix of module name in Android.mk
-	// ".flattened", ".apex", ".zipapex", or ""
+	// Suffix of module name in Android.mk ".flattened", ".apex", ".zipapex", or ""
 	suffix string
 
-	installedFilesFile android.WritablePath
+	// File system type of apex_payload.img
+	payloadFsType fsType
 
-	// Whether to create symlink to the system file instead of having a file
-	// inside the apex or not
+	// Whether to create symlink to the system file instead of having a file inside the apex or
+	// not
 	linkToSystemLib bool
 
+	// List of files to be included in this APEX. This is filled in the first part of
+	// GenerateAndroidBuildActions.
+	filesInfo []apexFile
+
+	// List of other module names that should be installed when this APEX gets installed.
+	requiredDeps []string
+
+	///////////////////////////////////////////////////////////////////////////////////////////
+	// Outputs (final and intermediates)
+
+	// Processed apex manifest in JSONson format (for Q)
+	manifestJsonOut android.WritablePath
+
+	// Processed apex manifest in PB format (for R+)
+	manifestPbOut android.WritablePath
+
+	// Processed file_contexts files
+	fileContexts android.WritablePath
+
 	// Struct holding the merged notice file paths in different formats
 	mergedNotices android.NoticeOutputs
 
+	// The built APEX file. This is the main product.
+	outputFile android.WritablePath
+
+	// The built APEX file in app bundle format. This file is not directly installed to the
+	// device. For an APEX, multiple app bundles are created each of which is for a specific ABI
+	// like arm, arm64, x86, etc. Then they are processed again (outside of the Android build
+	// system) to be merged into a single app bundle file that Play accepts. See
+	// vendor/google/build/build_unbundled_mainline_module.sh for more detail.
+	bundleModuleFile android.WritablePath
+
+	// Target path to install this APEX. Usually out/target/product/<device>/<partition>/apex.
+	installDir android.InstallPath
+
+	// List of commands to create symlinks for backward compatibility. These commands will be
+	// attached as LOCAL_POST_INSTALL_CMD to apex package itself (for unflattened build) or
+	// apex_manifest (for flattened build) so that compat symlinks are always installed
+	// regardless of TARGET_FLATTEN_APEX setting.
+	compatSymlinks []string
+
+	// Text file having the list of individual files that are included in this APEX. Used for
+	// debugging purpose.
+	installedFilesFile android.WritablePath
+
+	// List of module names that this APEX is including (to be shown via *-deps-info target).
+	// Used for debugging purpose.
+	android.ApexBundleDepsInfo
+
 	// Optional list of lint report zip files for apexes that contain java or app modules
 	lintReports android.Paths
 
-	payloadFsType fsType
+	prebuiltFileToDelete string
 
 	distFiles android.TaggedDistFiles
 }
 
+// apexFileClass represents a type of file that can be included in APEX.
 type apexFileClass int
 
 const (
-	etc apexFileClass = iota
-	nativeSharedLib
-	nativeExecutable
-	shBinary
-	pyBinary
+	app apexFileClass = iota
+	appSet
+	etc
 	goBinary
 	javaSharedLib
+	nativeExecutable
+	nativeSharedLib
 	nativeTest
-	app
-	appSet
+	pyBinary
+	shBinary
 )
 
-func (class apexFileClass) NameInMake() string {
-	switch class {
-	case etc:
-		return "ETC"
-	case nativeSharedLib:
-		return "SHARED_LIBRARIES"
-	case nativeExecutable, shBinary, pyBinary, goBinary:
-		return "EXECUTABLES"
-	case javaSharedLib:
-		return "JAVA_LIBRARIES"
-	case nativeTest:
-		return "NATIVE_TESTS"
-	case app, appSet:
-		// b/142537672 Why isn't this APP? We want to have full control over
-		// the paths and file names of the apk file under the flattend APEX.
-		// If this is set to APP, then the paths and file names are modified
-		// by the Make build system. For example, it is installed to
-		// /system/apex/<apexname>/app/<Appname>/<apexname>.<Appname>/ instead of
-		// /system/apex/<apexname>/app/<Appname> because the build system automatically
-		// appends module name (which is <apexname>.<Appname> to the path.
-		return "ETC"
-	default:
-		panic(fmt.Errorf("unknown class %d", class))
-	}
-}
-
-// apexFile represents a file in an APEX bundle
+// apexFile represents a file in an APEX bundle. This is created during the first half of
+// GenerateAndroidBuildActions by traversing the dependencies of the APEX. Then in the second half
+// of the function, this is used to create commands that copies the files into a staging directory,
+// where they are packaged into the APEX file. This struct is also used for creating Make modules
+// for each of the files in case when the APEX is flattened.
 type apexFile struct {
-	builtFile android.Path
-	stem      string
-	// Module name of `module` in AndroidMk. Note the generated AndroidMk module for
-	// apexFile is named something like <AndroidMk module name>.<apex name>[<apex suffix>]
-	androidMkModuleName string
-	installDir          string
-	class               apexFileClass
-	module              android.Module
-	// list of symlinks that will be created in installDir that point to this apexFile
-	symlinks      []string
-	dataPaths     []android.DataPath
-	transitiveDep bool
-	moduleDir     string
+	// buildFile is put in the installDir inside the APEX.
+	builtFile   android.Path
+	noticeFiles android.Paths
+	installDir  string
+	customStem  string
+	symlinks    []string // additional symlinks
 
-	requiredModuleNames       []string
-	targetRequiredModuleNames []string
-	hostRequiredModuleNames   []string
+	// Info for Android.mk Module name of `module` in AndroidMk. Note the generated AndroidMk
+	// module for apexFile is named something like <AndroidMk module name>.<apex name>[<apex
+	// suffix>]
+	androidMkModuleName       string             // becomes LOCAL_MODULE
+	class                     apexFileClass      // becomes LOCAL_MODULE_CLASS
+	moduleDir                 string             // becomes LOCAL_PATH
+	requiredModuleNames       []string           // becomes LOCAL_REQUIRED_MODULES
+	targetRequiredModuleNames []string           // becomes LOCAL_TARGET_REQUIRED_MODULES
+	hostRequiredModuleNames   []string           // becomes LOCAL_HOST_REQUIRED_MODULES
+	dataPaths                 []android.DataPath // becomes LOCAL_TEST_DATA
 
 	jacocoReportClassesFile android.Path     // only for javalibs and apps
 	lintDepSets             java.LintDepSets // only for javalibs and apps
 	certificate             java.Certificate // only for apps
 	overriddenPackageName   string           // only for apps
 
-	isJniLib bool
+	transitiveDep bool
+	isJniLib      bool
 
-	noticeFiles android.Paths
+	// TODO(jiyong): remove this
+	module android.Module
 }
 
+// TODO(jiyong): shorten the arglist using an option struct
 func newApexFile(ctx android.BaseModuleContext, builtFile android.Path, androidMkModuleName string, installDir string, class apexFileClass, module android.Module) apexFile {
 	ret := apexFile{
 		builtFile:           builtFile,
-		androidMkModuleName: androidMkModuleName,
 		installDir:          installDir,
+		androidMkModuleName: androidMkModuleName,
 		class:               class,
 		module:              module,
 	}
 	if module != nil {
+		ret.noticeFiles = module.NoticeFiles()
 		ret.moduleDir = ctx.OtherModuleDir(module)
 		ret.requiredModuleNames = module.RequiredModuleNames()
 		ret.targetRequiredModuleNames = module.TargetRequiredModuleNames()
 		ret.hostRequiredModuleNames = module.HostRequiredModuleNames()
-		ret.noticeFiles = module.NoticeFiles()
 	}
 	return ret
 }
 
-func (af *apexFile) Ok() bool {
+func (af *apexFile) ok() bool {
 	return af.builtFile != nil && af.builtFile.String() != ""
 }
 
+// apexRelativePath returns the relative path of the given path from the install directory of this
+// apexFile.
+// TODO(jiyong): rename this
 func (af *apexFile) apexRelativePath(path string) string {
 	return filepath.Join(af.installDir, path)
 }
 
-// Path() returns path of this apex file relative to the APEX root
-func (af *apexFile) Path() string {
-	return af.apexRelativePath(af.Stem())
+// path returns path of this apex file relative to the APEX root
+func (af *apexFile) path() string {
+	return af.apexRelativePath(af.stem())
 }
 
-func (af *apexFile) Stem() string {
-	if af.stem != "" {
-		return af.stem
+// stem returns the base filename of this apex file
+func (af *apexFile) stem() string {
+	if af.customStem != "" {
+		return af.customStem
 	}
 	return af.builtFile.Base()
 }
 
-// SymlinkPaths() returns paths of the symlinks (if any) relative to the APEX root
-func (af *apexFile) SymlinkPaths() []string {
+// symlinkPaths returns paths of the symlinks (if any) relative to the APEX root
+func (af *apexFile) symlinkPaths() []string {
 	var ret []string
 	for _, symlink := range af.symlinks {
 		ret = append(ret, af.apexRelativePath(symlink))
@@ -431,7 +457,9 @@
 	return ret
 }
 
-func (af *apexFile) AvailableToPlatform() bool {
+// availableToPlatform tests whether this apexFile is from a module that can be installed to the
+// platform.
+func (af *apexFile) availableToPlatform() bool {
 	if af.module == nil {
 		return false
 	}
@@ -441,37 +469,70 @@
 	return false
 }
 
-func addDependenciesForNativeModules(ctx android.BottomUpMutatorContext,
-	nativeModules ApexNativeDependencies,
-	target android.Target, imageVariation string) {
-	// Use *FarVariation* to be able to depend on modules having
-	// conflicting variations with this module. This is required since
-	// arch variant of an APEX bundle is 'common' but it is 'arm' or 'arm64'
-	// for native shared libs.
+////////////////////////////////////////////////////////////////////////////////////////////////////
+// Mutators
+//
+// Brief description about mutators for APEX. The following three mutators are the most important
+// ones.
+//
+// 1) DepsMutator: from the properties like native_shared_libs, java_libs, etc., modules are added
+// to the (direct) dependencies of this APEX bundle.
+//
+// 2) apexDepsMutator: this is a post-deps mutator, so runs after DepsMutator. Its goal is to
+// collect modules that are direct and transitive dependencies of each APEX bundle. The collected
+// modules are marked as being included in the APEX via BuildForApex().
+//
+// 3) apexMutator: this is a post-deps mutator that runs after apexDepsMutator. For each module that
+// are marked by the apexDepsMutator, apex variations are created using CreateApexVariations().
 
+type dependencyTag struct {
+	blueprint.BaseDependencyTag
+	name string
+
+	// Determines if the dependent will be part of the APEX payload. Can be false for the
+	// dependencies to the signing key module, etc.
+	payload bool
+}
+
+var (
+	androidAppTag  = dependencyTag{name: "androidApp", payload: true}
+	bpfTag         = dependencyTag{name: "bpf", payload: true}
+	certificateTag = dependencyTag{name: "certificate"}
+	executableTag  = dependencyTag{name: "executable", payload: true}
+	javaLibTag     = dependencyTag{name: "javaLib", payload: true}
+	jniLibTag      = dependencyTag{name: "jniLib", payload: true}
+	keyTag         = dependencyTag{name: "key"}
+	prebuiltTag    = dependencyTag{name: "prebuilt", payload: true}
+	rroTag         = dependencyTag{name: "rro", payload: true}
+	sharedLibTag   = dependencyTag{name: "sharedLib", payload: true}
+	testForTag     = dependencyTag{name: "test for"}
+	testTag        = dependencyTag{name: "test", payload: true}
+)
+
+// TODO(jiyong): shorten this function signature
+func addDependenciesForNativeModules(ctx android.BottomUpMutatorContext, nativeModules ApexNativeDependencies, target android.Target, imageVariation string) {
 	binVariations := target.Variations()
-	libVariations := append(target.Variations(),
-		blueprint.Variation{Mutator: "link", Variation: "shared"})
+	libVariations := append(target.Variations(), blueprint.Variation{Mutator: "link", Variation: "shared"})
 
 	if ctx.Device() {
-		binVariations = append(binVariations,
-			blueprint.Variation{Mutator: "image", Variation: imageVariation})
+		binVariations = append(binVariations, blueprint.Variation{Mutator: "image", Variation: imageVariation})
 		libVariations = append(libVariations,
 			blueprint.Variation{Mutator: "image", Variation: imageVariation},
-			blueprint.Variation{Mutator: "version", Variation: ""}) // "" is the non-stub variant
+			blueprint.Variation{Mutator: "version", Variation: ""}, // "" is the non-stub variant
+		)
 	}
 
-	ctx.AddFarVariationDependencies(libVariations, sharedLibTag, nativeModules.Native_shared_libs...)
-
-	ctx.AddFarVariationDependencies(libVariations, jniLibTag, nativeModules.Jni_libs...)
-
+	// Use *FarVariation* to be able to depend on modules having conflicting variations with
+	// this module. This is required since arch variant of an APEX bundle is 'common' but it is
+	// 'arm' or 'arm64' for native shared libs.
 	ctx.AddFarVariationDependencies(binVariations, executableTag, nativeModules.Binaries...)
-
 	ctx.AddFarVariationDependencies(binVariations, testTag, nativeModules.Tests...)
+	ctx.AddFarVariationDependencies(libVariations, jniLibTag, nativeModules.Jni_libs...)
+	ctx.AddFarVariationDependencies(libVariations, sharedLibTag, nativeModules.Native_shared_libs...)
 }
 
 func (a *apexBundle) combineProperties(ctx android.BottomUpMutatorContext) {
-	if ctx.Os().Class == android.Device {
+	if ctx.Device() {
 		proptools.AppendProperties(&a.properties.Multilib, &a.targetProperties.Target.Android.Multilib, nil)
 	} else {
 		proptools.AppendProperties(&a.properties.Multilib, &a.targetProperties.Target.Host.Multilib, nil)
@@ -483,34 +544,46 @@
 	}
 }
 
-type dependencyTag struct {
-	blueprint.BaseDependencyTag
-	name string
-
-	// determines if the dependent will be part of the APEX payload
-	payload bool
-}
-
-var (
-	sharedLibTag   = dependencyTag{name: "sharedLib", payload: true}
-	jniLibTag      = dependencyTag{name: "jniLib", payload: true}
-	executableTag  = dependencyTag{name: "executable", payload: true}
-	javaLibTag     = dependencyTag{name: "javaLib", payload: true}
-	prebuiltTag    = dependencyTag{name: "prebuilt", payload: true}
-	testTag        = dependencyTag{name: "test", payload: true}
-	keyTag         = dependencyTag{name: "key"}
-	certificateTag = dependencyTag{name: "certificate"}
-	androidAppTag  = dependencyTag{name: "androidApp", payload: true}
-	rroTag         = dependencyTag{name: "rro", payload: true}
-	bpfTag         = dependencyTag{name: "bpf", payload: true}
-	testForTag     = dependencyTag{name: "test for"}
-)
-
-func (a *apexBundle) DepsMutator(ctx android.BottomUpMutatorContext) {
-	if proptools.Bool(a.properties.Use_vendor) && !android.InList(a.Name(), useVendorAllowList(ctx.Config())) {
-		ctx.PropertyErrorf("use_vendor", "not allowed to set use_vendor: true")
+// getImageVariation returns the image variant name for this apexBundle. In most cases, it's simply
+// android.CoreVariation, but gets complicated for the vendor APEXes and the VNDK APEX.
+func (a *apexBundle) getImageVariation(ctx android.BottomUpMutatorContext) string {
+	deviceConfig := ctx.DeviceConfig()
+	if a.vndkApex {
+		return cc.VendorVariationPrefix + a.vndkVersion(deviceConfig)
 	}
 
+	var prefix string
+	var vndkVersion string
+	if deviceConfig.VndkVersion() != "" {
+		if proptools.Bool(a.properties.Use_vendor) {
+			prefix = cc.VendorVariationPrefix
+			vndkVersion = deviceConfig.PlatformVndkVersion()
+		} else if a.SocSpecific() || a.DeviceSpecific() {
+			prefix = cc.VendorVariationPrefix
+			vndkVersion = deviceConfig.VndkVersion()
+		} else if a.ProductSpecific() {
+			prefix = cc.ProductVariationPrefix
+			vndkVersion = deviceConfig.ProductVndkVersion()
+		}
+	}
+	if vndkVersion == "current" {
+		vndkVersion = deviceConfig.PlatformVndkVersion()
+	}
+	if vndkVersion != "" {
+		return prefix + vndkVersion
+	}
+
+	return android.CoreVariation // The usual case
+}
+
+func (a *apexBundle) DepsMutator(ctx android.BottomUpMutatorContext) {
+	// TODO(jiyong): move this kind of checks to GenerateAndroidBuildActions?
+	checkUseVendorProperty(ctx, a)
+
+	// apexBundle is a multi-arch targets module. Arch variant of apexBundle is set to 'common'.
+	// arch-specific targets are enabled by the compile_multilib setting of the apex bundle. For
+	// each target os/architectures, appropriate dependencies are selected by their
+	// target.<os>.multilib.<type> groups and are added as (direct) dependencies.
 	targets := ctx.MultiTargets()
 	config := ctx.DeviceConfig()
 	imageVariation := a.getImageVariation(ctx)
@@ -524,80 +597,56 @@
 		}
 	}
 	for i, target := range targets {
+		// Don't include artifacts for the host cross targets because there is no way for us
+		// to run those artifacts natively on host
 		if target.HostCross {
-			// Don't include artifats for the host cross targets because there is no way
-			// for us to run those artifacts natively on host
 			continue
 		}
 
-		// When multilib.* is omitted for native_shared_libs/jni_libs/tests, it implies
-		// multilib.both
-		addDependenciesForNativeModules(ctx,
-			ApexNativeDependencies{
-				Native_shared_libs: a.properties.Native_shared_libs,
-				Tests:              a.properties.Tests,
-				Jni_libs:           a.properties.Jni_libs,
-				Binaries:           nil,
-			},
-			target, imageVariation)
+		var depsList []ApexNativeDependencies
 
-		// Add native modules targetting both ABIs
-		addDependenciesForNativeModules(ctx,
-			a.properties.Multilib.Both,
-			target,
-			imageVariation)
+		// Add native modules targeting both ABIs. When multilib.* is omitted for
+		// native_shared_libs/jni_libs/tests, it implies multilib.both
+		depsList = append(depsList, a.properties.Multilib.Both)
+		depsList = append(depsList, ApexNativeDependencies{
+			Native_shared_libs: a.properties.Native_shared_libs,
+			Tests:              a.properties.Tests,
+			Jni_libs:           a.properties.Jni_libs,
+			Binaries:           nil,
+		})
 
+		// Add native modules targeting the first ABI When multilib.* is omitted for
+		// binaries, it implies multilib.first
 		isPrimaryAbi := i == 0
 		if isPrimaryAbi {
-			// When multilib.* is omitted for binaries, it implies
-			// multilib.first
-			addDependenciesForNativeModules(ctx,
-				ApexNativeDependencies{
-					Native_shared_libs: nil,
-					Tests:              nil,
-					Jni_libs:           nil,
-					Binaries:           a.properties.Binaries,
-				},
-				target, imageVariation)
-
-			// Add native modules targetting the first ABI
-			addDependenciesForNativeModules(ctx,
-				a.properties.Multilib.First,
-				target,
-				imageVariation)
+			depsList = append(depsList, a.properties.Multilib.First)
+			depsList = append(depsList, ApexNativeDependencies{
+				Native_shared_libs: nil,
+				Tests:              nil,
+				Jni_libs:           nil,
+				Binaries:           a.properties.Binaries,
+			})
 		}
 
+		// Add native modules targeting either 32-bit or 64-bit ABI
 		switch target.Arch.ArchType.Multilib {
 		case "lib32":
-			// Add native modules targetting 32-bit ABI
-			addDependenciesForNativeModules(ctx,
-				a.properties.Multilib.Lib32,
-				target,
-				imageVariation)
-
-			addDependenciesForNativeModules(ctx,
-				a.properties.Multilib.Prefer32,
-				target,
-				imageVariation)
+			depsList = append(depsList, a.properties.Multilib.Lib32)
+			depsList = append(depsList, a.properties.Multilib.Prefer32)
 		case "lib64":
-			// Add native modules targetting 64-bit ABI
-			addDependenciesForNativeModules(ctx,
-				a.properties.Multilib.Lib64,
-				target,
-				imageVariation)
-
+			depsList = append(depsList, a.properties.Multilib.Lib64)
 			if !has32BitTarget {
-				addDependenciesForNativeModules(ctx,
-					a.properties.Multilib.Prefer32,
-					target,
-					imageVariation)
+				depsList = append(depsList, a.properties.Multilib.Prefer32)
 			}
 		}
+
+		for _, d := range depsList {
+			addDependenciesForNativeModules(ctx, d, target, imageVariation)
+		}
 	}
 
-	// For prebuilt_etc, use the first variant (64 on 64/32bit device,
-	// 32 on 32bit device) regardless of the TARGET_PREFER_* setting.
-	// b/144532908
+	// For prebuilt_etc, use the first variant (64 on 64/32bit device, 32 on 32bit device)
+	// regardless of the TARGET_PREFER_* setting. See b/144532908
 	archForPrebuiltEtc := config.Arches()[0]
 	for _, arch := range config.Arches() {
 		// Prefer 64-bit arch if there is any
@@ -611,20 +660,19 @@
 		{Mutator: "arch", Variation: archForPrebuiltEtc.String()},
 	}, prebuiltTag, a.properties.Prebuilts...)
 
-	ctx.AddFarVariationDependencies(ctx.Config().AndroidCommonTarget.Variations(),
-		javaLibTag, a.properties.Java_libs...)
-
-	ctx.AddFarVariationDependencies(ctx.Config().AndroidCommonTarget.Variations(),
-		bpfTag, a.properties.Bpfs...)
+	// Common-arch dependencies come next
+	commonVariation := ctx.Config().AndroidCommonTarget.Variations()
+	ctx.AddFarVariationDependencies(commonVariation, javaLibTag, a.properties.Java_libs...)
+	ctx.AddFarVariationDependencies(commonVariation, bpfTag, a.properties.Bpfs...)
 
 	// With EMMA_INSTRUMENT_FRAMEWORK=true the ART boot image includes jacoco library.
 	if a.artApex && ctx.Config().IsEnvTrue("EMMA_INSTRUMENT_FRAMEWORK") {
-		ctx.AddFarVariationDependencies(ctx.Config().AndroidCommonTarget.Variations(),
-			javaLibTag, "jacocoagent")
+		ctx.AddFarVariationDependencies(commonVariation, javaLibTag, "jacocoagent")
 	}
 
+	// Dependencies for signing
 	if String(a.properties.Key) == "" {
-		ctx.ModuleErrorf("key is missing")
+		ctx.PropertyErrorf("key", "missing")
 		return
 	}
 	ctx.AddDependency(ctx.Module(), keyTag, String(a.properties.Key))
@@ -632,8 +680,13 @@
 	cert := android.SrcIsModule(a.getCertString(ctx))
 	if cert != "" {
 		ctx.AddDependency(ctx.Module(), certificateTag, cert)
+		// empty cert is not an error. Cert and private keys will be directly found under
+		// PRODUCT_DEFAULT_DEV_CERTIFICATE
 	}
 
+	// Marks that this APEX (in fact all the modules in it) has to be built with the given SDKs.
+	// This field currently isn't used.
+	// TODO(jiyong): consider dropping this feature
 	// TODO(jiyong): ensure that all apexes are with non-empty uses_sdks
 	if len(a.properties.Uses_sdks) > 0 {
 		sdkRefs := []android.SdkRef{}
@@ -645,14 +698,15 @@
 	}
 }
 
+// DepsMutator for the overridden properties.
 func (a *apexBundle) OverridablePropertiesDepsMutator(ctx android.BottomUpMutatorContext) {
 	if a.overridableProperties.Allowed_files != nil {
 		android.ExtractSourceDeps(ctx, a.overridableProperties.Allowed_files)
 	}
-	ctx.AddFarVariationDependencies(ctx.Config().AndroidCommonTarget.Variations(),
-		androidAppTag, a.overridableProperties.Apps...)
-	ctx.AddFarVariationDependencies(ctx.Config().AndroidCommonTarget.Variations(),
-		rroTag, a.overridableProperties.Rros...)
+
+	commonVariation := ctx.Config().AndroidCommonTarget.Variations()
+	ctx.AddFarVariationDependencies(commonVariation, androidAppTag, a.overridableProperties.Apps...)
+	ctx.AddFarVariationDependencies(commonVariation, rroTag, a.overridableProperties.Rros...)
 }
 
 type ApexBundleInfo struct {
@@ -661,17 +715,34 @@
 
 var ApexBundleInfoProvider = blueprint.NewMutatorProvider(ApexBundleInfo{}, "apex_deps")
 
-// Mark the direct and transitive dependencies of apex bundles so that they
-// can be built for the apex bundles.
+// apexDepsMutator is responsible for collecting modules that need to have apex variants. They are
+// identified by doing a graph walk starting from an apexBundle. Basically, all the (direct and
+// indirect) dependencies are collected. But a few types of modules that shouldn't be included in
+// the apexBundle (e.g. stub libraries) are not collected. Note that a single module can be depended
+// on by multiple apexBundles. In that case, the module is collected for all of the apexBundles.
 func apexDepsMutator(mctx android.TopDownMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
 	}
+
 	a, ok := mctx.Module().(*apexBundle)
-	if !ok || a.vndkApex {
+	if !ok {
 		return
 	}
 
+	// The VNDK APEX is special. For the APEX, the membership is described in a very different
+	// way. There is no dependency from the VNDK APEX to the VNDK libraries. Instead, VNDK
+	// libraries are self-identified by their vndk.enabled properties. There is no need to run
+	// this mutator for the APEX as nothing will be collected. So, let's return fast.
+	if a.vndkApex {
+		return
+	}
+
+	// Special casing for APEXes on non-system (e.g., vendor, odm, etc.) partitions. They are
+	// provided with a property named use_vndk_as_stable, which when set to true doesn't collect
+	// VNDK libraries as transitive dependencies. This option is useful for reducing the size of
+	// the non-system APEXes because the VNDK libraries won't be included (and duped) in the
+	// APEX, but shared across APEXes via the VNDK APEX.
 	useVndk := a.SocSpecific() || a.DeviceSpecific() || (a.ProductSpecific() && mctx.Config().EnforceProductPartitionInterface())
 	excludeVndkLibs := useVndk && proptools.Bool(a.properties.Use_vndk_as_stable)
 	if !useVndk && proptools.Bool(a.properties.Use_vndk_as_stable) {
@@ -679,8 +750,6 @@
 		return
 	}
 
-	contents := make(map[string]android.ApexMembership)
-
 	continueApexDepsWalk := func(child, parent android.Module) bool {
 		am, ok := child.(android.ApexModule)
 		if !ok || !am.CanHaveApexVariants() {
@@ -694,26 +763,33 @@
 				return false
 			}
 		}
+		// By default, all the transitive dependencies are collected, unless filtered out
+		// above.
 		return true
 	}
 
+	// Records whether a certain module is included in this apexBundle via direct dependency or
+	// inndirect dependency.
+	contents := make(map[string]android.ApexMembership)
 	mctx.WalkDeps(func(child, parent android.Module) bool {
 		if !continueApexDepsWalk(child, parent) {
 			return false
 		}
-
-		depName := mctx.OtherModuleName(child)
 		// If the parent is apexBundle, this child is directly depended.
 		_, directDep := parent.(*apexBundle)
+		depName := mctx.OtherModuleName(child)
 		contents[depName] = contents[depName].Add(directDep)
 		return true
 	})
 
+	// The membership information is saved for later access
 	apexContents := android.NewApexContents(contents)
 	mctx.SetProvider(ApexBundleInfoProvider, ApexBundleInfo{
 		Contents: apexContents,
 	})
 
+	// This is the main part of this mutator. Mark the collected dependencies that they need to
+	// be built for this apexBundle.
 	apexInfo := android.ApexInfo{
 		ApexVariationName: mctx.ModuleName(),
 		MinSdkVersionStr:  a.minSdkVersion(mctx).String(),
@@ -722,34 +798,34 @@
 		InApexes:          []string{mctx.ModuleName()},
 		ApexContents:      []*android.ApexContents{apexContents},
 	}
-
 	mctx.WalkDeps(func(child, parent android.Module) bool {
 		if !continueApexDepsWalk(child, parent) {
 			return false
 		}
-
-		child.(android.ApexModule).BuildForApex(apexInfo)
+		child.(android.ApexModule).BuildForApex(apexInfo) // leave a mark!
 		return true
 	})
 }
 
+// apexUniqueVariationsMutator checks if any dependencies use unique apex variations. If so, use
+// unique apex variations for this module. See android/apex.go for more about unique apex variant.
+// TODO(jiyong): move this to android/apex.go?
 func apexUniqueVariationsMutator(mctx android.BottomUpMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
 	}
 	if am, ok := mctx.Module().(android.ApexModule); ok {
-		// Check if any dependencies use unique apex variations.  If so, use unique apex variations
-		// for this module.
 		android.UpdateUniqueApexVariationsForDeps(mctx, am)
 	}
 }
 
+// apexTestForDepsMutator checks if this module is a test for an apex. If so, add a dependency on
+// the apex in order to retrieve its contents later.
+// TODO(jiyong): move this to android/apex.go?
 func apexTestForDepsMutator(mctx android.BottomUpMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
 	}
-	// Check if this module is a test for an apex.  If so, add a dependency on the apex
-	// in order to retrieve its contents later.
 	if am, ok := mctx.Module().(android.ApexModule); ok {
 		if testFor := am.TestFor(); len(testFor) > 0 {
 			mctx.AddFarVariationDependencies([]blueprint.Variation{
@@ -760,11 +836,11 @@
 	}
 }
 
+// TODO(jiyong): move this to android/apex.go?
 func apexTestForMutator(mctx android.BottomUpMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
 	}
-
 	if _, ok := mctx.Module().(android.ApexModule); ok {
 		var contents []*android.ApexContents
 		for _, testFor := range mctx.GetDirectDepsWithTag(testForTag) {
@@ -777,69 +853,70 @@
 	}
 }
 
-// mark if a module cannot be available to platform. A module cannot be available
-// to platform if 1) it is explicitly marked as not available (i.e. "//apex_available:platform"
-// is absent) or 2) it depends on another module that isn't (or can't be) available to platform
+// markPlatformAvailability marks whether or not a module can be available to platform. A module
+// cannot be available to platform if 1) it is explicitly marked as not available (i.e.
+// "//apex_available:platform" is absent) or 2) it depends on another module that isn't (or can't
+// be) available to platform
+// TODO(jiyong): move this to android/apex.go?
 func markPlatformAvailability(mctx android.BottomUpMutatorContext) {
 	// Host and recovery are not considered as platform
 	if mctx.Host() || mctx.Module().InstallInRecovery() {
 		return
 	}
 
-	if am, ok := mctx.Module().(android.ApexModule); ok {
-		availableToPlatform := am.AvailableFor(android.AvailableToPlatform)
+	am, ok := mctx.Module().(android.ApexModule)
+	if !ok {
+		return
+	}
 
-		// If any of the dep is not available to platform, this module is also considered
-		// as being not available to platform even if it has "//apex_available:platform"
-		mctx.VisitDirectDeps(func(child android.Module) {
-			if !am.DepIsInSameApex(mctx, child) {
-				// if the dependency crosses apex boundary, don't consider it
-				return
-			}
-			if dep, ok := child.(android.ApexModule); ok && dep.NotAvailableForPlatform() {
-				availableToPlatform = false
-				// TODO(b/154889534) trigger an error when 'am' has "//apex_available:platform"
-			}
-		})
+	availableToPlatform := am.AvailableFor(android.AvailableToPlatform)
 
-		// Exception 1: stub libraries and native bridge libraries are always available to platform
-		if cc, ok := mctx.Module().(*cc.Module); ok &&
-			(cc.IsStubs() || cc.Target().NativeBridge == android.NativeBridgeEnabled) {
-			availableToPlatform = true
+	// If any of the dep is not available to platform, this module is also considered as being
+	// not available to platform even if it has "//apex_available:platform"
+	mctx.VisitDirectDeps(func(child android.Module) {
+		if !am.DepIsInSameApex(mctx, child) {
+			// if the dependency crosses apex boundary, don't consider it
+			return
 		}
-
-		// Exception 2: bootstrap bionic libraries are also always available to platform
-		if cc.InstallToBootstrap(mctx.ModuleName(), mctx.Config()) {
-			availableToPlatform = true
+		if dep, ok := child.(android.ApexModule); ok && dep.NotAvailableForPlatform() {
+			availableToPlatform = false
+			// TODO(b/154889534) trigger an error when 'am' has
+			// "//apex_available:platform"
 		}
+	})
 
-		if !availableToPlatform {
-			am.SetNotAvailableForPlatform()
-		}
+	// Exception 1: stub libraries and native bridge libraries are always available to platform
+	if cc, ok := mctx.Module().(*cc.Module); ok &&
+		(cc.IsStubs() || cc.Target().NativeBridge == android.NativeBridgeEnabled) {
+		availableToPlatform = true
+	}
+
+	// Exception 2: bootstrap bionic libraries are also always available to platform
+	if cc.InstallToBootstrap(mctx.ModuleName(), mctx.Config()) {
+		availableToPlatform = true
+	}
+
+	if !availableToPlatform {
+		am.SetNotAvailableForPlatform()
 	}
 }
 
-// If a module in an APEX depends on a module from an SDK then it needs an APEX
-// specific variant created for it. Refer to sdk.sdkDepsReplaceMutator.
-func inAnySdk(module android.Module) bool {
-	if sa, ok := module.(android.SdkAware); ok {
-		return sa.IsInAnySdk()
-	}
-
-	return false
-}
-
-// Create apex variations if a module is included in APEX(s).
+// apexMutator visits each module and creates apex variations if the module was marked in the
+// previous run of apexDepsMutator.
 func apexMutator(mctx android.BottomUpMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
 	}
 
+	// This is the usual path.
 	if am, ok := mctx.Module().(android.ApexModule); ok && am.CanHaveApexVariants() {
 		android.CreateApexVariations(mctx, am)
-	} else if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex {
-		// apex bundle itself is mutated so that it and its modules have same
-		// apex variant.
+		return
+	}
+
+	// apexBundle itself is mutated so that it and its dependencies have the same apex variant.
+	// TODO(jiyong): document the reason why the VNDK APEX is an exception here.
+	if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex {
 		apexBundleName := mctx.ModuleName()
 		mctx.CreateVariations(apexBundleName)
 	} else if o, ok := mctx.Module().(*OverrideApex); ok {
@@ -850,9 +927,10 @@
 		}
 		mctx.CreateVariations(apexBundleName)
 	}
-
 }
 
+// See android.UpdateDirectlyInAnyApex
+// TODO(jiyong): move this to android/apex.go?
 func apexDirectlyInAnyMutator(mctx android.BottomUpMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
@@ -862,19 +940,32 @@
 	}
 }
 
+// apexPackaging represents a specific packaging method for an APEX.
 type apexPackaging int
 
 const (
+	// imageApex is a packaging method where contents are included in a filesystem image which
+	// is then included in a zip container. This is the most typical way of packaging.
 	imageApex apexPackaging = iota
+
+	// zipApex is a packaging method where contents are directly included in the zip container.
+	// This is used for host-side testing - because the contents are easily accessible by
+	// unzipping the container.
 	zipApex
+
+	// flattendApex is a packaging method where contents are not included in the APEX file, but
+	// installed to /apex/<apexname> directory on the device. This packaging method is used for
+	// old devices where the filesystem-based APEX file can't be supported.
 	flattenedApex
 )
 
 const (
+	// File extensions of an APEX for different packaging methods
 	imageApexSuffix = ".apex"
 	zipApexSuffix   = ".zipapex"
 	flattenedSuffix = ".flattened"
 
+	// variant names each of which is for a packaging method
 	imageApexType     = "image"
 	zipApexType       = "zip"
 	flattenedApexType = "flattened"
@@ -906,6 +997,8 @@
 	}
 }
 
+// apexFlattenedMutator creates one or more variations each of which is for a packaging method.
+// TODO(jiyong): give a better name to this mutator
 func apexFlattenedMutator(mctx android.BottomUpMutatorContext) {
 	if !mctx.Module().Enabled() {
 		return
@@ -914,13 +1007,21 @@
 		var variants []string
 		switch proptools.StringDefault(ab.properties.Payload_type, "image") {
 		case "image":
+			// This is the normal case. Note that both image and flattend APEXes are
+			// created. The image type is installed to the system partition, while the
+			// flattened APEX is (optionally) installed to the system_ext partition.
+			// This is mostly for GSI which has to support wide range of devices. If GSI
+			// is installed on a newer (APEX-capable) device, the image APEX in the
+			// system will be used. However, if the same GSI is installed on an old
+			// device which can't support image APEX, the flattened APEX in the
+			// system_ext partion (which still is part of GSI) is used instead.
 			variants = append(variants, imageApexType, flattenedApexType)
 		case "zip":
 			variants = append(variants, zipApexType)
 		case "both":
 			variants = append(variants, imageApexType, zipApexType, flattenedApexType)
 		default:
-			mctx.PropertyErrorf("type", "%q is not one of \"image\", \"zip\", or \"both\".", *ab.properties.Payload_type)
+			mctx.PropertyErrorf("payload_type", "%q is not one of \"image\", \"zip\", or \"both\".", *ab.properties.Payload_type)
 			return
 		}
 
@@ -934,26 +1035,35 @@
 				modules[i].(*apexBundle).properties.ApexType = zipApex
 			case flattenedApexType:
 				modules[i].(*apexBundle).properties.ApexType = flattenedApex
+				// See the comment above for why system_ext.
 				if !mctx.Config().FlattenApex() && ab.Platform() {
 					modules[i].(*apexBundle).MakeAsSystemExt()
 				}
 			}
 		}
 	} else if _, ok := mctx.Module().(*OverrideApex); ok {
+		// payload_type is forcibly overridden to "image"
+		// TODO(jiyong): is this the right decision?
 		mctx.CreateVariations(imageApexType, flattenedApexType)
 	}
 }
 
+// checkUseVendorProperty checks if the use of `use_vendor` property is allowed for the given APEX.
+// When use_vendor is used, native modules are built with __ANDROID_VNDK__ and __ANDROID_APEX__,
+// which may cause compatibility issues. (e.g. libbinder) Even though libbinder restricts its
+// availability via 'apex_available' property and relies on yet another macro
+// __ANDROID_APEX_<NAME>__, we restrict usage of "use_vendor:" from other APEX modules to avoid
+// similar problems.
+func checkUseVendorProperty(ctx android.BottomUpMutatorContext, a *apexBundle) {
+	if proptools.Bool(a.properties.Use_vendor) && !android.InList(a.Name(), useVendorAllowList(ctx.Config())) {
+		ctx.PropertyErrorf("use_vendor", "not allowed to set use_vendor: true")
+	}
+}
+
 var (
 	useVendorAllowListKey = android.NewOnceKey("useVendorAllowList")
 )
 
-// useVendorAllowList returns the list of APEXes which are allowed to use_vendor.
-// When use_vendor is used, native modules are built with __ANDROID_VNDK__ and __ANDROID_APEX__,
-// which may cause compatibility issues. (e.g. libbinder)
-// Even though libbinder restricts its availability via 'apex_available' property and relies on
-// yet another macro __ANDROID_APEX_<NAME>__, we restrict usage of "use_vendor:" from other APEX modules
-// to avoid similar problems.
 func useVendorAllowList(config android.Config) []string {
 	return config.Once(useVendorAllowListKey, func() interface{} {
 		return []string{
@@ -964,41 +1074,74 @@
 	}).([]string)
 }
 
-// setUseVendorAllowListForTest overrides useVendorAllowList and must be
-// called before the first call to useVendorAllowList()
+// setUseVendorAllowListForTest overrides useVendorAllowList and must be called before the first
+// call to useVendorAllowList()
 func setUseVendorAllowListForTest(config android.Config, allowList []string) {
 	config.Once(useVendorAllowListKey, func() interface{} {
 		return allowList
 	})
 }
 
-type fsType int
+var _ android.DepIsInSameApex = (*apexBundle)(nil)
 
-const (
-	ext4 fsType = iota
-	f2fs
-)
-
-func (f fsType) string() string {
-	switch f {
-	case ext4:
-		return ext4FsType
-	case f2fs:
-		return f2fsFsType
-	default:
-		panic(fmt.Errorf("unknown APEX payload type %d", f))
-	}
-}
-
+// Implements android.DepInInSameApex
 func (a *apexBundle) DepIsInSameApex(ctx android.BaseModuleContext, dep android.Module) bool {
 	// direct deps of an APEX bundle are all part of the APEX bundle
+	// TODO(jiyong): shouldn't we look into the payload field of the dependencyTag?
 	return true
 }
 
+var _ android.OutputFileProducer = (*apexBundle)(nil)
+
+// Implements android.OutputFileProducer
+func (a *apexBundle) OutputFiles(tag string) (android.Paths, error) {
+	switch tag {
+	case "":
+		return android.Paths{a.outputFile}, nil
+	default:
+		return nil, fmt.Errorf("unsupported module reference tag %q", tag)
+	}
+}
+
+var _ cc.Coverage = (*apexBundle)(nil)
+
+// Implements cc.Coverage
+func (a *apexBundle) IsNativeCoverageNeeded(ctx android.BaseModuleContext) bool {
+	return ctx.Device() && ctx.DeviceConfig().NativeCoverageEnabled()
+}
+
+// Implements cc.Coverage
+func (a *apexBundle) PreventInstall() {
+	a.properties.PreventInstall = true
+}
+
+// Implements cc.Coverage
+func (a *apexBundle) HideFromMake() {
+	a.properties.HideFromMake = true
+}
+
+// Implements cc.Coverage
+func (a *apexBundle) MarkAsCoverageVariant(coverage bool) {
+	a.properties.IsCoverageVariant = coverage
+}
+
+// Implements cc.Coverage
+func (a *apexBundle) EnableCoverageIfNeeded() {}
+
+var _ android.ApexBundleDepsInfoIntf = (*apexBundle)(nil)
+
+// Implements android.ApexBudleDepsInfoIntf
+func (a *apexBundle) Updatable() bool {
+	return proptools.Bool(a.properties.Updatable)
+}
+
+// getCertString returns the name of the cert that should be used to sign this APEX. This is
+// basically from the "certificate" property, but could be overridden by the device config.
 func (a *apexBundle) getCertString(ctx android.BaseModuleContext) string {
 	moduleName := ctx.ModuleName()
-	// VNDK APEXes share the same certificate. To avoid adding a new VNDK version to the OVERRIDE_* list,
-	// we check with the pseudo module name to see if its certificate is overridden.
+	// VNDK APEXes share the same certificate. To avoid adding a new VNDK version to the
+	// OVERRIDE_* list, we check with the pseudo module name to see if its certificate is
+	// overridden.
 	if a.vndkApex {
 		moduleName = vndkApexName
 	}
@@ -1009,55 +1152,24 @@
 	return String(a.properties.Certificate)
 }
 
-func (a *apexBundle) OutputFiles(tag string) (android.Paths, error) {
-	switch tag {
-	case "":
-		return android.Paths{a.outputFile}, nil
-	default:
-		return nil, fmt.Errorf("unsupported module reference tag %q", tag)
-	}
-}
-
+// See the installable property
 func (a *apexBundle) installable() bool {
 	return !a.properties.PreventInstall && (a.properties.Installable == nil || proptools.Bool(a.properties.Installable))
 }
 
+// See the test_only_no_hashtree property
 func (a *apexBundle) testOnlyShouldSkipHashtreeGeneration() bool {
 	return proptools.Bool(a.properties.Test_only_no_hashtree)
 }
 
+// See the test_only_unsigned_payload property
 func (a *apexBundle) testOnlyShouldSkipPayloadSign() bool {
 	return proptools.Bool(a.properties.Test_only_unsigned_payload)
 }
 
-func (a *apexBundle) getImageVariation(ctx android.BottomUpMutatorContext) string {
-	deviceConfig := ctx.DeviceConfig()
-	if a.vndkApex {
-		return cc.VendorVariationPrefix + a.vndkVersion(deviceConfig)
-	}
-
-	var prefix string
-	var vndkVersion string
-	if deviceConfig.VndkVersion() != "" {
-		if proptools.Bool(a.properties.Use_vendor) {
-			prefix = cc.VendorVariationPrefix
-			vndkVersion = deviceConfig.PlatformVndkVersion()
-		} else if a.SocSpecific() || a.DeviceSpecific() {
-			prefix = cc.VendorVariationPrefix
-			vndkVersion = deviceConfig.VndkVersion()
-		} else if a.ProductSpecific() {
-			prefix = cc.ProductVariationPrefix
-			vndkVersion = deviceConfig.ProductVndkVersion()
-		}
-	}
-	if vndkVersion == "current" {
-		vndkVersion = deviceConfig.PlatformVndkVersion()
-	}
-	if vndkVersion != "" {
-		return prefix + vndkVersion
-	}
-	return android.CoreVariation
-}
+// These functions are interfacing with cc/sanitizer.go. The entire APEX (along with all of its
+// members) can be sanitized, either forcibly, or by the global configuration. For some of the
+// sanitizers, extra dependencies can be forcibly added as well.
 
 func (a *apexBundle) EnableSanitizer(sanitizerName string) {
 	if !android.InList(sanitizerName, a.properties.SanitizerNames) {
@@ -1084,44 +1196,31 @@
 }
 
 func (a *apexBundle) AddSanitizerDependencies(ctx android.BottomUpMutatorContext, sanitizerName string) {
+	// TODO(jiyong): move this info (the sanitizer name, the lib name, etc.) to cc/sanitize.go
+	// Keep only the mechanism here.
 	if ctx.Device() && sanitizerName == "hwaddress" && strings.HasPrefix(a.Name(), "com.android.runtime") {
+		imageVariation := a.getImageVariation(ctx)
 		for _, target := range ctx.MultiTargets() {
 			if target.Arch.ArchType.Multilib == "lib64" {
-				ctx.AddFarVariationDependencies(append(target.Variations(), []blueprint.Variation{
-					{Mutator: "image", Variation: a.getImageVariation(ctx)},
-					{Mutator: "link", Variation: "shared"},
-					{Mutator: "version", Variation: ""}, // "" is the non-stub variant
-				}...), sharedLibTag, "libclang_rt.hwasan-aarch64-android")
+				addDependenciesForNativeModules(ctx, ApexNativeDependencies{
+					Native_shared_libs: []string{"libclang_rt.hwasan-aarch64-android"},
+					Tests:              nil,
+					Jni_libs:           nil,
+					Binaries:           nil,
+				}, target, imageVariation)
 				break
 			}
 		}
 	}
 }
 
-var _ cc.Coverage = (*apexBundle)(nil)
-
-func (a *apexBundle) IsNativeCoverageNeeded(ctx android.BaseModuleContext) bool {
-	return ctx.Device() && ctx.DeviceConfig().NativeCoverageEnabled()
-}
-
-func (a *apexBundle) PreventInstall() {
-	a.properties.PreventInstall = true
-}
-
-func (a *apexBundle) HideFromMake() {
-	a.properties.HideFromMake = true
-}
-
-func (a *apexBundle) MarkAsCoverageVariant(coverage bool) {
-	a.properties.IsCoverageVariant = coverage
-}
-
-func (a *apexBundle) EnableCoverageIfNeeded() {}
-
-// TODO(jiyong) move apexFileFor* close to the apexFile type definition
+// apexFileFor<Type> functions below create an apexFile struct for a given Soong module. The
+// returned apexFile saves information about the Soong module that will be used for creating the
+// build rules.
 func apexFileForNativeLibrary(ctx android.BaseModuleContext, ccMod *cc.Module, handleSpecialLibs bool) apexFile {
-	// Decide the APEX-local directory by the multilib of the library
-	// In the future, we may query this to the module.
+	// Decide the APEX-local directory by the multilib of the library In the future, we may
+	// query this to the module.
+	// TODO(jiyong): use the new PackagingSpec
 	var dirInApex string
 	switch ccMod.Arch().ArchType.Multilib {
 	case "lib32":
@@ -1134,16 +1233,15 @@
 	}
 	dirInApex = filepath.Join(dirInApex, ccMod.RelativeInstallPath())
 	if handleSpecialLibs && cc.InstallToBootstrap(ccMod.BaseModuleName(), ctx.Config()) {
-		// Special case for Bionic libs and other libs installed with them. This is
-		// to prevent those libs from being included in the search path
-		// /apex/com.android.runtime/${LIB}. This exclusion is required because
-		// those libs in the Runtime APEX are available via the legacy paths in
-		// /system/lib/. By the init process, the libs in the APEX are bind-mounted
-		// to the legacy paths and thus will be loaded into the default linker
-		// namespace (aka "platform" namespace). If the libs are directly in
-		// /apex/com.android.runtime/${LIB} then the same libs will be loaded again
-		// into the runtime linker namespace, which will result in double loading of
-		// them, which isn't supported.
+		// Special case for Bionic libs and other libs installed with them. This is to
+		// prevent those libs from being included in the search path
+		// /apex/com.android.runtime/${LIB}. This exclusion is required because those libs
+		// in the Runtime APEX are available via the legacy paths in /system/lib/. By the
+		// init process, the libs in the APEX are bind-mounted to the legacy paths and thus
+		// will be loaded into the default linker namespace (aka "platform" namespace). If
+		// the libs are directly in /apex/com.android.runtime/${LIB} then the same libs will
+		// be loaded again into the runtime linker namespace, which will result in double
+		// loading of them, which isn't supported.
 		dirInApex = filepath.Join(dirInApex, "bionic")
 	}
 
@@ -1171,6 +1269,7 @@
 	fileToCopy := py.HostToolPath().Path()
 	return newApexFile(ctx, fileToCopy, py.BaseModuleName(), dirInApex, pyBinary, py)
 }
+
 func apexFileForGoBinary(ctx android.BaseModuleContext, depName string, gb bootstrap.GoBinaryTool) apexFile {
 	dirInApex := "bin"
 	s, err := filepath.Rel(android.PathForOutput(ctx).String(), gb.InstallPath())
@@ -1193,31 +1292,6 @@
 	return af
 }
 
-type javaModule interface {
-	android.Module
-	BaseModuleName() string
-	DexJarBuildPath() android.Path
-	JacocoReportClassesFile() android.Path
-	LintDepSets() java.LintDepSets
-
-	Stem() string
-}
-
-var _ javaModule = (*java.Library)(nil)
-var _ javaModule = (*java.SdkLibrary)(nil)
-var _ javaModule = (*java.DexImport)(nil)
-var _ javaModule = (*java.SdkLibraryImport)(nil)
-
-func apexFileForJavaLibrary(ctx android.BaseModuleContext, module javaModule) apexFile {
-	dirInApex := "javalib"
-	fileToCopy := module.DexJarBuildPath()
-	af := newApexFile(ctx, fileToCopy, module.BaseModuleName(), dirInApex, javaSharedLib, module)
-	af.jacocoReportClassesFile = module.JacocoReportClassesFile()
-	af.lintDepSets = module.LintDepSets()
-	af.stem = module.Stem() + ".jar"
-	return af
-}
-
 func apexFileForPrebuiltEtc(ctx android.BaseModuleContext, prebuilt prebuilt_etc.PrebuiltEtcModule, depName string) apexFile {
 	dirInApex := filepath.Join(prebuilt.BaseDir(), prebuilt.SubDir())
 	fileToCopy := prebuilt.OutputFile()
@@ -1230,7 +1304,35 @@
 	return newApexFile(ctx, fileToCopy, depName, dirInApex, etc, config)
 }
 
-func apexFileForAndroidApp(ctx android.BaseModuleContext, aapp interface {
+// javaModule is an interface to handle all Java modules (java_library, dex_import, etc) in the same
+// way.
+type javaModule interface {
+	android.Module
+	BaseModuleName() string
+	DexJarBuildPath() android.Path
+	JacocoReportClassesFile() android.Path
+	LintDepSets() java.LintDepSets
+	Stem() string
+}
+
+var _ javaModule = (*java.Library)(nil)
+var _ javaModule = (*java.SdkLibrary)(nil)
+var _ javaModule = (*java.DexImport)(nil)
+var _ javaModule = (*java.SdkLibraryImport)(nil)
+
+func apexFileForJavaModule(ctx android.BaseModuleContext, module javaModule) apexFile {
+	dirInApex := "javalib"
+	fileToCopy := module.DexJarBuildPath()
+	af := newApexFile(ctx, fileToCopy, module.BaseModuleName(), dirInApex, javaSharedLib, module)
+	af.jacocoReportClassesFile = module.JacocoReportClassesFile()
+	af.lintDepSets = module.LintDepSets()
+	af.customStem = module.Stem() + ".jar"
+	return af
+}
+
+// androidApp is an interface to handle all app modules (android_app, android_app_import, etc.) in
+// the same way.
+type androidApp interface {
 	android.Module
 	Privileged() bool
 	InstallApkName() string
@@ -1238,7 +1340,12 @@
 	JacocoReportClassesFile() android.Path
 	Certificate() java.Certificate
 	BaseModuleName() string
-}) apexFile {
+}
+
+var _ androidApp = (*java.AndroidApp)(nil)
+var _ androidApp = (*java.AndroidAppImport)(nil)
+
+func apexFileForAndroidApp(ctx android.BaseModuleContext, aapp androidApp) apexFile {
 	appDir := "app"
 	if aapp.Privileged() {
 		appDir = "priv-app"
@@ -1277,16 +1384,10 @@
 	return newApexFile(ctx, builtFile, builtFile.Base(), dirInApex, etc, bpfProgram)
 }
 
-// Context "decorator", overriding the InstallBypassMake method to always reply `true`.
-type flattenedApexContext struct {
-	android.ModuleContext
-}
-
-func (c *flattenedApexContext) InstallBypassMake() bool {
-	return true
-}
-
-// Visit dependencies that contributes to the payload of this APEX
+// WalyPayloadDeps visits dependencies that contributes to the payload of this APEX. For each of the
+// visited module, the `do` callback is executed. Returning true in the callback continues the visit
+// to the child modules. Returning false makes the visit to continue in the sibling or the parent
+// modules. This is used in check* functions below.
 func (a *apexBundle) WalkPayloadDeps(ctx android.ModuleContext, do android.PayloadDepsCallback) {
 	ctx.WalkDeps(func(child, parent android.Module) bool {
 		am, ok := child.(android.ApexModule)
@@ -1294,213 +1395,74 @@
 			return false
 		}
 
-		childApexInfo := ctx.OtherModuleProvider(child, android.ApexInfoProvider).(android.ApexInfo)
-
-		dt := ctx.OtherModuleDependencyTag(child)
-
-		if _, ok := dt.(android.ExcludeFromApexContentsTag); ok {
+		// Filter-out unwanted depedendencies
+		depTag := ctx.OtherModuleDependencyTag(child)
+		if _, ok := depTag.(android.ExcludeFromApexContentsTag); ok {
+			return false
+		}
+		if dt, ok := depTag.(dependencyTag); ok && !dt.payload {
 			return false
 		}
 
-		// Check for the direct dependencies that contribute to the payload
-		if adt, ok := dt.(dependencyTag); ok {
-			if adt.payload {
-				return do(ctx, parent, am, false /* externalDep */)
-			}
-			// As soon as the dependency graph crosses the APEX boundary, don't go further.
-			return false
-		}
+		ai := ctx.OtherModuleProvider(child, android.ApexInfoProvider).(android.ApexInfo)
+		externalDep := !android.InList(ctx.ModuleName(), ai.InApexes)
 
-		// Check for the indirect dependencies if it is considered as part of the APEX
-		if android.InList(ctx.ModuleName(), childApexInfo.InApexes) {
-			return do(ctx, parent, am, false /* externalDep */)
-		}
-
-		return do(ctx, parent, am, true /* externalDep */)
+		// Visit actually
+		return do(ctx, parent, am, externalDep)
 	})
 }
 
-func (a *apexBundle) minSdkVersion(ctx android.BaseModuleContext) android.ApiLevel {
-	ver := proptools.String(a.properties.Min_sdk_version)
-	if ver == "" {
-		return android.FutureApiLevel
-	}
-	apiLevel, err := android.ApiLevelFromUser(ctx, ver)
-	if err != nil {
-		ctx.PropertyErrorf("min_sdk_version", "%s", err.Error())
-		return android.NoneApiLevel
-	}
-	if apiLevel.IsPreview() {
-		// All codenames should build against "current".
-		return android.FutureApiLevel
-	}
-	return apiLevel
-}
+// filesystem type of the apex_payload.img inside the APEX. Currently, ext4 and f2fs are supported.
+type fsType int
 
-func (a *apexBundle) Updatable() bool {
-	return proptools.Bool(a.properties.Updatable)
-}
+const (
+	ext4 fsType = iota
+	f2fs
+)
 
-var _ android.ApexBundleDepsInfoIntf = (*apexBundle)(nil)
-
-// Ensures that the dependencies are marked as available for this APEX
-func (a *apexBundle) checkApexAvailability(ctx android.ModuleContext) {
-	// Let's be practical. Availability for test, host, and the VNDK apex isn't important
-	if ctx.Host() || a.testApex || a.vndkApex {
-		return
-	}
-
-	// Because APEXes targeting other than system/system_ext partitions
-	// can't set apex_available, we skip checks for these APEXes
-	if a.SocSpecific() || a.DeviceSpecific() ||
-		(a.ProductSpecific() && ctx.Config().EnforceProductPartitionInterface()) {
-		return
-	}
-
-	// Coverage build adds additional dependencies for the coverage-only runtime libraries.
-	// Requiring them and their transitive depencies with apex_available is not right
-	// because they just add noise.
-	if ctx.Config().IsEnvTrue("EMMA_INSTRUMENT") || a.IsNativeCoverageNeeded(ctx) {
-		return
-	}
-
-	a.WalkPayloadDeps(ctx, func(ctx android.ModuleContext, from blueprint.Module, to android.ApexModule, externalDep bool) bool {
-		if externalDep {
-			// As soon as the dependency graph crosses the APEX boundary, don't go further.
-			return false
-		}
-
-		apexName := ctx.ModuleName()
-		fromName := ctx.OtherModuleName(from)
-		toName := ctx.OtherModuleName(to)
-
-		// If `to` is not actually in the same APEX as `from` then it does not need apex_available and neither
-		// do any of its dependencies.
-		if am, ok := from.(android.DepIsInSameApex); ok && !am.DepIsInSameApex(ctx, to) {
-			// As soon as the dependency graph crosses the APEX boundary, don't go further.
-			return false
-		}
-
-		if to.AvailableFor(apexName) || baselineApexAvailable(apexName, toName) {
-			return true
-		}
-		ctx.ModuleErrorf("%q requires %q that doesn't list the APEX under 'apex_available'. Dependency path:%s", fromName, toName, ctx.GetPathString(true))
-		// Visit this module's dependencies to check and report any issues with their availability.
-		return true
-	})
-}
-
-func (a *apexBundle) checkUpdatable(ctx android.ModuleContext) {
-	if a.Updatable() {
-		if String(a.properties.Min_sdk_version) == "" {
-			ctx.PropertyErrorf("updatable", "updatable APEXes should set min_sdk_version as well")
-		}
-
-		a.checkJavaStableSdkVersion(ctx)
+func (f fsType) string() string {
+	switch f {
+	case ext4:
+		return ext4FsType
+	case f2fs:
+		return f2fsFsType
+	default:
+		panic(fmt.Errorf("unknown APEX payload type %d", f))
 	}
 }
 
-func (a *apexBundle) checkMinSdkVersion(ctx android.ModuleContext) {
-	if a.testApex || a.vndkApex {
-		return
-	}
-	// Meaningless to check min_sdk_version when building use_vendor modules against non-Trebleized targets
-	if proptools.Bool(a.properties.Use_vendor) && ctx.DeviceConfig().VndkVersion() == "" {
-		return
-	}
-	// apexBundle::minSdkVersion reports its own errors.
-	minSdkVersion := a.minSdkVersion(ctx)
-	android.CheckMinSdkVersion(a, ctx, minSdkVersion)
-}
-
-// Ensures that a lib providing stub isn't statically linked
-func (a *apexBundle) checkStaticLinkingToStubLibraries(ctx android.ModuleContext) {
-	// Practically, we only care about regular APEXes on the device.
-	if ctx.Host() || a.testApex || a.vndkApex {
-		return
-	}
-
-	abInfo := ctx.Provider(ApexBundleInfoProvider).(ApexBundleInfo)
-
-	a.WalkPayloadDeps(ctx, func(ctx android.ModuleContext, from blueprint.Module, to android.ApexModule, externalDep bool) bool {
-		if ccm, ok := to.(*cc.Module); ok {
-			apexName := ctx.ModuleName()
-			fromName := ctx.OtherModuleName(from)
-			toName := ctx.OtherModuleName(to)
-
-			// If `to` is not actually in the same APEX as `from` then it does not need apex_available and neither
-			// do any of its dependencies.
-			if am, ok := from.(android.DepIsInSameApex); ok && !am.DepIsInSameApex(ctx, to) {
-				// As soon as the dependency graph crosses the APEX boundary, don't go further.
-				return false
-			}
-
-			// The dynamic linker and crash_dump tool in the runtime APEX is the only exception to this rule.
-			// It can't make the static dependencies dynamic because it can't
-			// do the dynamic linking for itself.
-			if apexName == "com.android.runtime" && (fromName == "linker" || fromName == "crash_dump") {
-				return false
-			}
-
-			isStubLibraryFromOtherApex := ccm.HasStubsVariants() && !abInfo.Contents.DirectlyInApex(toName)
-			if isStubLibraryFromOtherApex && !externalDep {
-				ctx.ModuleErrorf("%q required by %q is a native library providing stub. "+
-					"It shouldn't be included in this APEX via static linking. Dependency path: %s", to.String(), fromName, ctx.GetPathString(false))
-			}
-
-		}
-		return true
-	})
-}
-
+// Creates build rules for an APEX. It consists of the following major steps:
+//
+// 1) do some validity checks such as apex_available, min_sdk_version, etc.
+// 2) traverse the dependency tree to collect apexFile structs from them.
+// 3) some fields in apexBundle struct are configured
+// 4) generate the build rules to create the APEX. This is mostly done in builder.go.
 func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) {
-	buildFlattenedAsDefault := ctx.Config().FlattenApex() && !ctx.Config().UnbundledBuildApps()
-	switch a.properties.ApexType {
-	case imageApex:
-		if buildFlattenedAsDefault {
-			a.suffix = imageApexSuffix
-		} else {
-			a.suffix = ""
-			a.primaryApexType = true
-
-			if ctx.Config().InstallExtraFlattenedApexes() {
-				a.requiredDeps = append(a.requiredDeps, a.Name()+flattenedSuffix)
-			}
-		}
-	case zipApex:
-		if proptools.String(a.properties.Payload_type) == "zip" {
-			a.suffix = ""
-			a.primaryApexType = true
-		} else {
-			a.suffix = zipApexSuffix
-		}
-	case flattenedApex:
-		if buildFlattenedAsDefault {
-			a.suffix = ""
-			a.primaryApexType = true
-		} else {
-			a.suffix = flattenedSuffix
-		}
-	}
-
-	if len(a.properties.Tests) > 0 && !a.testApex {
-		ctx.PropertyErrorf("tests", "property not allowed in apex module type")
-		return
-	}
-
+	////////////////////////////////////////////////////////////////////////////////////////////
+	// 1) do some validity checks such as apex_available, min_sdk_version, etc.
 	a.checkApexAvailability(ctx)
 	a.checkUpdatable(ctx)
 	a.checkMinSdkVersion(ctx)
 	a.checkStaticLinkingToStubLibraries(ctx)
+	if len(a.properties.Tests) > 0 && !a.testApex {
+		ctx.PropertyErrorf("tests", "property allowed only in apex_test module type")
+		return
+	}
 
-	handleSpecialLibs := !android.Bool(a.properties.Ignore_system_library_special_case)
+	////////////////////////////////////////////////////////////////////////////////////////////
+	// 2) traverse the dependency tree to collect apexFile structs from them.
+
+	// all the files that will be included in this APEX
+	var filesInfo []apexFile
 
 	// native lib dependencies
 	var provideNativeLibs []string
 	var requireNativeLibs []string
 
-	var filesInfo []apexFile
-	// TODO(jiyong) do this using WalkPayloadDeps
+	handleSpecialLibs := !android.Bool(a.properties.Ignore_system_library_special_case)
+
+	// TODO(jiyong): do this using WalkPayloadDeps
+	// TODO(jiyong): make this clean!!!
 	ctx.WalkDepsBlueprint(func(child, parent blueprint.Module) bool {
 		depTag := ctx.OtherModuleDependencyTag(child)
 		if _, ok := depTag.(android.ExcludeFromApexContentsTag); ok {
@@ -1519,7 +1481,7 @@
 					// - VNDK libs are only for vendors
 					// - bootstrap bionic libs are treated as provided by system
 					if c.HasStubsVariants() && !a.vndkApex && !cc.InstallToBootstrap(c.BaseModuleName(), ctx.Config()) {
-						provideNativeLibs = append(provideNativeLibs, fi.Stem())
+						provideNativeLibs = append(provideNativeLibs, fi.stem())
 					}
 					return true // track transitive dependencies
 				} else {
@@ -1545,8 +1507,8 @@
 			case javaLibTag:
 				switch child.(type) {
 				case *java.Library, *java.SdkLibrary, *java.DexImport, *java.SdkLibraryImport:
-					af := apexFileForJavaLibrary(ctx, child.(javaModule))
-					if !af.Ok() {
+					af := apexFileForJavaModule(ctx, child.(javaModule))
+					if !af.ok() {
 						ctx.PropertyErrorf("java_libs", "%q is not configured to be compiled into dex", depName)
 						return false
 					}
@@ -1674,7 +1636,7 @@
 									a.requiredDeps = append(a.requiredDeps, name)
 								}
 							}
-							requireNativeLibs = append(requireNativeLibs, af.Stem())
+							requireNativeLibs = append(requireNativeLibs, af.stem())
 							// Don't track further
 							return false
 						}
@@ -1710,10 +1672,14 @@
 		}
 		return false
 	})
+	if a.private_key_file == nil {
+		ctx.PropertyErrorf("key", "private_key for %q could not be found", String(a.properties.Key))
+		return
+	}
 
-	// Specific to the ART apex: dexpreopt artifacts for libcore Java libraries.
-	// Build rules are generated by the dexpreopt singleton, and here we access build artifacts
-	// via the global boot image config.
+	// Specific to the ART apex: dexpreopt artifacts for libcore Java libraries. Build rules are
+	// generated by the dexpreopt singleton, and here we access build artifacts via the global
+	// boot image config.
 	if a.artApex {
 		for arch, files := range java.DexpreoptedArtApexJars(ctx) {
 			dirInApex := filepath.Join("javalib", arch.String())
@@ -1725,12 +1691,7 @@
 		}
 	}
 
-	if a.private_key_file == nil {
-		ctx.PropertyErrorf("key", "private_key for %q could not be found", String(a.properties.Key))
-		return
-	}
-
-	// remove duplicates in filesInfo
+	// Remove duplicates in filesInfo
 	removeDup := func(filesInfo []apexFile) []apexFile {
 		encountered := make(map[string]apexFile)
 		for _, f := range filesInfo {
@@ -1752,14 +1713,46 @@
 	}
 	filesInfo = removeDup(filesInfo)
 
-	// to have consistent build rules
+	// Sort to have consistent build rules
 	sort.Slice(filesInfo, func(i, j int) bool {
 		return filesInfo[i].builtFile.String() < filesInfo[j].builtFile.String()
 	})
 
+	////////////////////////////////////////////////////////////////////////////////////////////
+	// 3) some fields in apexBundle struct are configured
 	a.installDir = android.PathForModuleInstall(ctx, "apex")
 	a.filesInfo = filesInfo
 
+	// Set suffix and primaryApexType depending on the ApexType
+	buildFlattenedAsDefault := ctx.Config().FlattenApex() && !ctx.Config().UnbundledBuildApps()
+	switch a.properties.ApexType {
+	case imageApex:
+		if buildFlattenedAsDefault {
+			a.suffix = imageApexSuffix
+		} else {
+			a.suffix = ""
+			a.primaryApexType = true
+
+			if ctx.Config().InstallExtraFlattenedApexes() {
+				a.requiredDeps = append(a.requiredDeps, a.Name()+flattenedSuffix)
+			}
+		}
+	case zipApex:
+		if proptools.String(a.properties.Payload_type) == "zip" {
+			a.suffix = ""
+			a.primaryApexType = true
+		} else {
+			a.suffix = zipApexSuffix
+		}
+	case flattenedApex:
+		if buildFlattenedAsDefault {
+			a.suffix = ""
+			a.primaryApexType = true
+		} else {
+			a.suffix = flattenedSuffix
+		}
+	}
+
 	switch proptools.StringDefault(a.properties.Payload_fs_type, ext4FsType) {
 	case ext4FsType:
 		a.payloadFsType = ext4
@@ -1774,14 +1767,11 @@
 	// the same library in the system partition, thus effectively sharing the same libraries
 	// across the APEX boundary. For unbundled APEX, all the gathered files are actually placed
 	// in the APEX.
-	a.linkToSystemLib = !ctx.Config().UnbundledBuild() &&
-		a.installable() &&
-		!proptools.Bool(a.properties.Use_vendor)
+	a.linkToSystemLib = !ctx.Config().UnbundledBuild() && a.installable() && !proptools.Bool(a.properties.Use_vendor)
 
 	// APEXes targeting other than system/system_ext partitions use vendor/product variants.
 	// So we can't link them to /system/lib libs which are core variants.
-	if a.SocSpecific() || a.DeviceSpecific() ||
-		(a.ProductSpecific() && ctx.Config().EnforceProductPartitionInterface()) {
+	if a.SocSpecific() || a.DeviceSpecific() || (a.ProductSpecific() && ctx.Config().EnforceProductPartitionInterface()) {
 		a.linkToSystemLib = false
 	}
 
@@ -1796,31 +1786,201 @@
 		a.linkToSystemLib = false
 	}
 
-	// prepare apex_manifest.json
-	a.buildManifest(ctx, provideNativeLibs, requireNativeLibs)
-
-	a.buildFileContexts(ctx)
-
 	a.setCertificateAndPrivateKey(ctx)
+
+	a.compatSymlinks = makeCompatSymlinks(a.BaseModuleName(), ctx)
+
+	////////////////////////////////////////////////////////////////////////////////////////////
+	// 4) generate the build rules to create the APEX. This is done in builder.go.
+	a.buildManifest(ctx, provideNativeLibs, requireNativeLibs)
+	a.buildFileContexts(ctx)
 	if a.properties.ApexType == flattenedApex {
 		a.buildFlattenedApex(ctx)
 	} else {
 		a.buildUnflattenedApex(ctx)
 	}
-
-	a.compatSymlinks = makeCompatSymlinks(a.BaseModuleName(), ctx)
-
 	a.buildApexDependencyInfo(ctx)
-
 	a.buildLintReports(ctx)
-
 	a.distFiles = a.GenerateTaggedDistFiles(ctx)
 }
 
+///////////////////////////////////////////////////////////////////////////////////////////////////
+// Factory functions
+//
+
+func newApexBundle() *apexBundle {
+	module := &apexBundle{}
+
+	module.AddProperties(&module.properties)
+	module.AddProperties(&module.targetProperties)
+	module.AddProperties(&module.overridableProperties)
+
+	android.InitAndroidMultiTargetsArchModule(module, android.HostAndDeviceSupported, android.MultilibCommon)
+	android.InitDefaultableModule(module)
+	android.InitSdkAwareModule(module)
+	android.InitOverridableModule(module, &module.overridableProperties.Overrides)
+	return module
+}
+
+func ApexBundleFactory(testApex bool, artApex bool) android.Module {
+	bundle := newApexBundle()
+	bundle.testApex = testApex
+	bundle.artApex = artApex
+	return bundle
+}
+
+// apex_test is an APEX for testing. The difference from the ordinary apex module type is that
+// certain compatibility checks such as apex_available are not done for apex_test.
+func testApexBundleFactory() android.Module {
+	bundle := newApexBundle()
+	bundle.testApex = true
+	return bundle
+}
+
+// apex packages other modules into an APEX file which is a packaging format for system-level
+// components like binaries, shared libraries, etc.
+func BundleFactory() android.Module {
+	return newApexBundle()
+}
+
+type Defaults struct {
+	android.ModuleBase
+	android.DefaultsModuleBase
+}
+
+// apex_defaults provides defaultable properties to other apex modules.
+func defaultsFactory() android.Module {
+	return DefaultsFactory()
+}
+
+func DefaultsFactory(props ...interface{}) android.Module {
+	module := &Defaults{}
+
+	module.AddProperties(props...)
+	module.AddProperties(
+		&apexBundleProperties{},
+		&apexTargetBundleProperties{},
+		&overridableProperties{},
+	)
+
+	android.InitDefaultsModule(module)
+	return module
+}
+
+type OverrideApex struct {
+	android.ModuleBase
+	android.OverrideModuleBase
+}
+
+func (o *OverrideApex) GenerateAndroidBuildActions(ctx android.ModuleContext) {
+	// All the overrides happen in the base module.
+}
+
+// override_apex is used to create an apex module based on another apex module by overriding some of
+// its properties.
+func overrideApexFactory() android.Module {
+	m := &OverrideApex{}
+
+	m.AddProperties(&overridableProperties{})
+
+	android.InitAndroidMultiTargetsArchModule(m, android.DeviceSupported, android.MultilibCommon)
+	android.InitOverrideModule(m)
+	return m
+}
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+// Vality check routines
+//
+// These are called in at the very beginning of GenerateAndroidBuildActions to flag an error when
+// certain conditions are not met.
+//
+// TODO(jiyong): move these checks to a separate go file.
+
+// Entures that min_sdk_version of the included modules are equal or less than the min_sdk_version
+// of this apexBundle.
+func (a *apexBundle) checkMinSdkVersion(ctx android.ModuleContext) {
+	if a.testApex || a.vndkApex {
+		return
+	}
+	// Meaningless to check min_sdk_version when building use_vendor modules against non-Trebleized targets
+	if proptools.Bool(a.properties.Use_vendor) && ctx.DeviceConfig().VndkVersion() == "" {
+		return
+	}
+	// apexBundle::minSdkVersion reports its own errors.
+	minSdkVersion := a.minSdkVersion(ctx)
+	android.CheckMinSdkVersion(a, ctx, minSdkVersion)
+}
+
+func (a *apexBundle) minSdkVersion(ctx android.BaseModuleContext) android.ApiLevel {
+	ver := proptools.String(a.properties.Min_sdk_version)
+	if ver == "" {
+		return android.FutureApiLevel
+	}
+	apiLevel, err := android.ApiLevelFromUser(ctx, ver)
+	if err != nil {
+		ctx.PropertyErrorf("min_sdk_version", "%s", err.Error())
+		return android.NoneApiLevel
+	}
+	if apiLevel.IsPreview() {
+		// All codenames should build against "current".
+		return android.FutureApiLevel
+	}
+	return apiLevel
+}
+
+// Ensures that a lib providing stub isn't statically linked
+func (a *apexBundle) checkStaticLinkingToStubLibraries(ctx android.ModuleContext) {
+	// Practically, we only care about regular APEXes on the device.
+	if ctx.Host() || a.testApex || a.vndkApex {
+		return
+	}
+
+	abInfo := ctx.Provider(ApexBundleInfoProvider).(ApexBundleInfo)
+
+	a.WalkPayloadDeps(ctx, func(ctx android.ModuleContext, from blueprint.Module, to android.ApexModule, externalDep bool) bool {
+		if ccm, ok := to.(*cc.Module); ok {
+			apexName := ctx.ModuleName()
+			fromName := ctx.OtherModuleName(from)
+			toName := ctx.OtherModuleName(to)
+
+			// If `to` is not actually in the same APEX as `from` then it does not need
+			// apex_available and neither do any of its dependencies.
+			if am, ok := from.(android.DepIsInSameApex); ok && !am.DepIsInSameApex(ctx, to) {
+				// As soon as the dependency graph crosses the APEX boundary, don't go further.
+				return false
+			}
+
+			// The dynamic linker and crash_dump tool in the runtime APEX is the only
+			// exception to this rule. It can't make the static dependencies dynamic
+			// because it can't do the dynamic linking for itself.
+			if apexName == "com.android.runtime" && (fromName == "linker" || fromName == "crash_dump") {
+				return false
+			}
+
+			isStubLibraryFromOtherApex := ccm.HasStubsVariants() && !abInfo.Contents.DirectlyInApex(toName)
+			if isStubLibraryFromOtherApex && !externalDep {
+				ctx.ModuleErrorf("%q required by %q is a native library providing stub. "+
+					"It shouldn't be included in this APEX via static linking. Dependency path: %s", to.String(), fromName, ctx.GetPathString(false))
+			}
+
+		}
+		return true
+	})
+}
+
 // Enforce that Java deps of the apex are using stable SDKs to compile
+func (a *apexBundle) checkUpdatable(ctx android.ModuleContext) {
+	if a.Updatable() {
+		if String(a.properties.Min_sdk_version) == "" {
+			ctx.PropertyErrorf("updatable", "updatable APEXes should set min_sdk_version as well")
+		}
+		a.checkJavaStableSdkVersion(ctx)
+	}
+}
+
 func (a *apexBundle) checkJavaStableSdkVersion(ctx android.ModuleContext) {
-	// Visit direct deps only. As long as we guarantee top-level deps are using
-	// stable SDKs, java's checkLinkType guarantees correct usage for transitive deps
+	// Visit direct deps only. As long as we guarantee top-level deps are using stable SDKs,
+	// java's checkLinkType guarantees correct usage for transitive deps
 	ctx.VisitDirectDepsBlueprint(func(module blueprint.Module) {
 		tag := ctx.OtherModuleDependencyTag(module)
 		switch tag {
@@ -1834,6 +1994,59 @@
 	})
 }
 
+// Ensures that the all the dependencies are marked as available for this APEX
+func (a *apexBundle) checkApexAvailability(ctx android.ModuleContext) {
+	// Let's be practical. Availability for test, host, and the VNDK apex isn't important
+	if ctx.Host() || a.testApex || a.vndkApex {
+		return
+	}
+
+	// Because APEXes targeting other than system/system_ext partitions can't set
+	// apex_available, we skip checks for these APEXes
+	if a.SocSpecific() || a.DeviceSpecific() || (a.ProductSpecific() && ctx.Config().EnforceProductPartitionInterface()) {
+		return
+	}
+
+	// Coverage build adds additional dependencies for the coverage-only runtime libraries.
+	// Requiring them and their transitive depencies with apex_available is not right
+	// because they just add noise.
+	if ctx.Config().IsEnvTrue("EMMA_INSTRUMENT") || a.IsNativeCoverageNeeded(ctx) {
+		return
+	}
+
+	a.WalkPayloadDeps(ctx, func(ctx android.ModuleContext, from blueprint.Module, to android.ApexModule, externalDep bool) bool {
+		// As soon as the dependency graph crosses the APEX boundary, don't go further.
+		if externalDep {
+			return false
+		}
+
+		apexName := ctx.ModuleName()
+		fromName := ctx.OtherModuleName(from)
+		toName := ctx.OtherModuleName(to)
+
+		// If `to` is not actually in the same APEX as `from` then it does not need
+		// apex_available and neither do any of its dependencies.
+		if am, ok := from.(android.DepIsInSameApex); ok && !am.DepIsInSameApex(ctx, to) {
+			// As soon as the dependency graph crosses the APEX boundary, don't go
+			// further.
+			return false
+		}
+
+		if to.AvailableFor(apexName) || baselineApexAvailable(apexName, toName) {
+			return true
+		}
+		ctx.ModuleErrorf("%q requires %q that doesn't list the APEX under 'apex_available'. Dependency path:%s",
+			fromName, toName, ctx.GetPathString(true))
+		// Visit this module's dependencies to check and report any issues with their availability.
+		return true
+	})
+}
+
+var (
+	apexAvailBaseline        = makeApexAvailableBaseline()
+	inverseApexAvailBaseline = invertApexBaseline(apexAvailBaseline)
+)
+
 func baselineApexAvailable(apex, moduleName string) bool {
 	key := apex
 	moduleName = normalizeModuleName(moduleName)
@@ -1866,93 +2079,6 @@
 	return moduleName
 }
 
-func newApexBundle() *apexBundle {
-	module := &apexBundle{}
-	module.AddProperties(&module.properties)
-	module.AddProperties(&module.targetProperties)
-	module.AddProperties(&module.overridableProperties)
-	android.InitAndroidMultiTargetsArchModule(module, android.HostAndDeviceSupported, android.MultilibCommon)
-	android.InitDefaultableModule(module)
-	android.InitSdkAwareModule(module)
-	android.InitOverridableModule(module, &module.overridableProperties.Overrides)
-	return module
-}
-
-func ApexBundleFactory(testApex bool, artApex bool) android.Module {
-	bundle := newApexBundle()
-	bundle.testApex = testApex
-	bundle.artApex = artApex
-	return bundle
-}
-
-// apex_test is an APEX for testing. The difference from the ordinary apex module type is that
-// certain compatibility checks such as apex_available are not done for apex_test.
-func testApexBundleFactory() android.Module {
-	bundle := newApexBundle()
-	bundle.testApex = true
-	return bundle
-}
-
-// apex packages other modules into an APEX file which is a packaging format for system-level
-// components like binaries, shared libraries, etc.
-func BundleFactory() android.Module {
-	return newApexBundle()
-}
-
-//
-// Defaults
-//
-type Defaults struct {
-	android.ModuleBase
-	android.DefaultsModuleBase
-}
-
-func defaultsFactory() android.Module {
-	return DefaultsFactory()
-}
-
-func DefaultsFactory(props ...interface{}) android.Module {
-	module := &Defaults{}
-
-	module.AddProperties(props...)
-	module.AddProperties(
-		&apexBundleProperties{},
-		&apexTargetBundleProperties{},
-		&overridableProperties{},
-	)
-
-	android.InitDefaultsModule(module)
-	return module
-}
-
-//
-// OverrideApex
-//
-type OverrideApex struct {
-	android.ModuleBase
-	android.OverrideModuleBase
-}
-
-func (o *OverrideApex) GenerateAndroidBuildActions(ctx android.ModuleContext) {
-	// All the overrides happen in the base module.
-}
-
-// override_apex is used to create an apex module based on another apex module
-// by overriding some of its properties.
-func overrideApexFactory() android.Module {
-	m := &OverrideApex{}
-	m.AddProperties(&overridableProperties{})
-
-	android.InitAndroidMultiTargetsArchModule(m, android.DeviceSupported, android.MultilibCommon)
-	android.InitOverrideModule(m)
-	return m
-}
-
-var (
-	apexAvailBaseline        = makeApexAvailableBaseline()
-	inverseApexAvailBaseline = invertApexBaseline(apexAvailBaseline)
-)
-
 // Transform the map of apex -> modules to module -> apexes.
 func invertApexBaseline(m map[string][]string) map[string][]string {
 	r := make(map[string][]string)
@@ -1969,9 +2095,8 @@
 	return inverseApexAvailBaseline[normalizeModuleName(moduleName)]
 }
 
-// This is a map from apex to modules, which overrides the
-// apex_available setting for that particular module to make
-// it available for the apex regardless of its setting.
+// This is a map from apex to modules, which overrides the apex_available setting for that
+// particular module to make it available for the apex regardless of its setting.
 // TODO(b/147364041): remove this
 func makeApexAvailableBaseline() map[string][]string {
 	// The "Module separator"s below are employed to minimize merge conflicts.
diff --git a/apex/builder.go b/apex/builder.go
index acfb8c5..6f27dd1 100644
--- a/apex/builder.go
+++ b/apex/builder.go
@@ -195,8 +195,8 @@
 	// collect jniLibs. Notice that a.filesInfo is already sorted
 	var jniLibs []string
 	for _, fi := range a.filesInfo {
-		if fi.isJniLib && !android.InList(fi.Stem(), jniLibs) {
-			jniLibs = append(jniLibs, fi.Stem())
+		if fi.isJniLib && !android.InList(fi.stem(), jniLibs) {
+			jniLibs = append(jniLibs, fi.stem())
 		}
 	}
 	if len(jniLibs) > 0 {
@@ -363,7 +363,7 @@
 				config.Apex_config.Apex_embedded_apk_config,
 				ApkConfig{
 					Package_name: packageName,
-					Apk_path:     fi.Path(),
+					Apk_path:     fi.path(),
 				})
 		}
 	}
@@ -396,15 +396,15 @@
 	// TODO(jiyong): construct the copy rules using RuleBuilder
 	var copyCommands []string
 	for _, fi := range a.filesInfo {
-		destPath := android.PathForModuleOut(ctx, "image"+suffix, fi.Path()).String()
+		destPath := android.PathForModuleOut(ctx, "image"+suffix, fi.path()).String()
 		destPathDir := filepath.Dir(destPath)
 		if fi.class == appSet {
 			copyCommands = append(copyCommands, "rm -rf "+destPathDir)
 		}
 		copyCommands = append(copyCommands, "mkdir -p "+destPathDir)
-		if a.linkToSystemLib && fi.transitiveDep && fi.AvailableToPlatform() {
+		if a.linkToSystemLib && fi.transitiveDep && fi.availableToPlatform() {
 			// TODO(jiyong): pathOnDevice should come from fi.module, not being calculated here
-			pathOnDevice := filepath.Join("/system", fi.Path())
+			pathOnDevice := filepath.Join("/system", fi.path())
 			copyCommands = append(copyCommands, "ln -sfn "+pathOnDevice+" "+destPath)
 		} else {
 			if fi.class == appSet {
@@ -416,7 +416,7 @@
 			implicitInputs = append(implicitInputs, fi.builtFile)
 		}
 		// create additional symlinks pointing the file inside the APEX
-		for _, symlinkPath := range fi.SymlinkPaths() {
+		for _, symlinkPath := range fi.symlinkPaths() {
 			symlinkDest := android.PathForModuleOut(ctx, "image"+suffix, symlinkPath).String()
 			copyCommands = append(copyCommands, "ln -sfn "+filepath.Base(destPath)+" "+symlinkDest)
 		}
@@ -444,7 +444,7 @@
 		emitCommands = append(emitCommands, "echo ./apex_manifest.json >> "+imageContentFile.String())
 	}
 	for _, fi := range a.filesInfo {
-		emitCommands = append(emitCommands, "echo './"+fi.Path()+"' >> "+imageContentFile.String())
+		emitCommands = append(emitCommands, "echo './"+fi.path()+"' >> "+imageContentFile.String())
 	}
 	emitCommands = append(emitCommands, "sort -o "+imageContentFile.String()+" "+imageContentFile.String())
 	implicitInputs = append(implicitInputs, a.manifestPbOut)
@@ -489,7 +489,7 @@
 		var extractedAppSetPaths android.Paths
 		var extractedAppSetDirs []string
 		for _, f := range a.filesInfo {
-			pathInApex := f.Path()
+			pathInApex := f.path()
 			if f.installDir == "bin" || strings.HasPrefix(f.installDir, "bin/") {
 				executablePaths = append(executablePaths, pathInApex)
 				for _, d := range f.dataPaths {
@@ -697,6 +697,15 @@
 	a.installedFilesFile = a.buildInstalledFilesFile(ctx, a.outputFile, imageDir)
 }
 
+// Context "decorator", overriding the InstallBypassMake method to always reply `true`.
+type flattenedApexContext struct {
+	android.ModuleContext
+}
+
+func (c *flattenedApexContext) InstallBypassMake() bool {
+	return true
+}
+
 func (a *apexBundle) buildFlattenedApex(ctx android.ModuleContext) {
 	// Temporarily wrap the original `ctx` into a `flattenedApexContext` to have it
 	// reply true to `InstallBypassMake()` (thus making the call
@@ -742,7 +751,7 @@
 			apexBundleName := a.Name()
 			for _, fi := range a.filesInfo {
 				dir := filepath.Join("apex", apexBundleName, fi.installDir)
-				target := ctx.InstallFile(android.PathForModuleInstall(ctx, dir), fi.Stem(), fi.builtFile)
+				target := ctx.InstallFile(android.PathForModuleInstall(ctx, dir), fi.stem(), fi.builtFile)
 				for _, sym := range fi.symlinks {
 					ctx.InstallSymlink(android.PathForModuleInstall(ctx, dir), sym, target)
 				}
diff --git a/bazel/Android.bp b/bazel/Android.bp
new file mode 100644
index 0000000..0113726
--- /dev/null
+++ b/bazel/Android.bp
@@ -0,0 +1,10 @@
+bootstrap_go_package {
+    name: "soong-bazel",
+    pkgPath: "android/soong/bazel",
+    srcs: [
+        "properties.go",
+    ],
+    pluginFor: [
+        "soong_build",
+    ],
+}
diff --git a/bazel/properties.go b/bazel/properties.go
new file mode 100644
index 0000000..8bb1956
--- /dev/null
+++ b/bazel/properties.go
@@ -0,0 +1,27 @@
+// Copyright 2020 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package bazel
+
+type bazelModuleProperties struct {
+	// The label of the Bazel target replacing this Soong module.
+	Label string
+}
+
+// Properties contains common module properties for migration purposes.
+type Properties struct {
+	// In USE_BAZEL_ANALYSIS=1 mode, this represents the Bazel target replacing
+	// this Soong module.
+	Bazel_module bazelModuleProperties
+}
diff --git a/cc/binary.go b/cc/binary.go
index da29412..fbd293e 100644
--- a/cc/binary.go
+++ b/cc/binary.go
@@ -42,6 +42,7 @@
 	// extension (if any) appended
 	Symlinks []string `android:"arch_variant"`
 
+	// override the dynamic linker
 	DynamicLinker string `blueprint:"mutated"`
 
 	// Names of modules to be overridden. Listed modules can only be other binaries
@@ -80,6 +81,7 @@
 // Executables
 //
 
+// binaryDecorator is a decorator containing information for C++ binary modules.
 type binaryDecorator struct {
 	*baseLinker
 	*baseInstaller
@@ -105,11 +107,15 @@
 	// Location of the files that should be copied to dist dir when requested
 	distFiles android.TaggedDistFiles
 
+	// Action command lines to run directly after the binary is installed. For example,
+	// may be used to symlink runtime dependencies (such as bionic) alongside installation.
 	post_install_cmds []string
 }
 
 var _ linker = (*binaryDecorator)(nil)
 
+// linkerProps returns the list of individual properties objects relevant
+// for this binary.
 func (binary *binaryDecorator) linkerProps() []interface{} {
 	return append(binary.baseLinker.linkerProps(),
 		&binary.Properties,
@@ -117,6 +123,10 @@
 
 }
 
+// getStemWithoutSuffix returns the main section of the name to use for the symlink of
+// the main output file of this binary module. This may be derived from the module name
+// or other property overrides.
+// For the full symlink name, the `Suffix` property of a binary module must be appended.
 func (binary *binaryDecorator) getStemWithoutSuffix(ctx BaseModuleContext) string {
 	stem := ctx.baseModuleName()
 	if String(binary.Properties.Stem) != "" {
@@ -126,10 +136,14 @@
 	return stem
 }
 
+// getStem returns the full name to use for the symlink of the main output file of this binary
+// module. This may be derived from the module name and/or other property overrides.
 func (binary *binaryDecorator) getStem(ctx BaseModuleContext) string {
 	return binary.getStemWithoutSuffix(ctx) + String(binary.Properties.Suffix)
 }
 
+// linkerDeps augments and returns the given `deps` to contain dependencies on
+// modules common to most binaries, such as bionic libraries.
 func (binary *binaryDecorator) linkerDeps(ctx DepsContext, deps Deps) Deps {
 	deps = binary.baseLinker.linkerDeps(ctx, deps)
 	if ctx.toolchain().Bionic() {
@@ -155,6 +169,13 @@
 			deps.LateStaticLibs = append(groupLibs, deps.LateStaticLibs...)
 		}
 
+		// Embed the linker into host bionic binaries. This is needed to support host bionic,
+		// as the linux kernel requires that the ELF interpreter referenced by PT_INTERP be
+		// either an absolute path, or relative from CWD. To work around this, we extract
+		// the load sections from the runtime linker ELF binary and embed them into each host
+		// bionic binary, omitting the PT_INTERP declaration. The kernel will treat it as a static
+		// binary, and then we use a special entry point to fix up the arguments passed by
+		// the kernel before jumping to the embedded linker.
 		if ctx.Os() == android.LinuxBionic && !binary.static() {
 			deps.DynamicLinker = "linker"
 			deps.LinkerFlagsFile = "host_bionic_linker_flags"
@@ -170,9 +191,13 @@
 }
 
 func (binary *binaryDecorator) isDependencyRoot() bool {
+	// Binaries are always the dependency root.
 	return true
 }
 
+// NewBinary builds and returns a new Module corresponding to a C++ binary.
+// Individual module implementations which comprise a C++ binary should call this function,
+// set some fields on the result, and then call the Init function.
 func NewBinary(hod android.HostOrDeviceSupported) (*Module, *binaryDecorator) {
 	module := newModule(hod, android.MultilibFirst)
 	binary := &binaryDecorator{
@@ -190,11 +215,15 @@
 	return module, binary
 }
 
+// linkerInit initializes dynamic properties of the linker (such as runpath) based
+// on properties of this binary.
 func (binary *binaryDecorator) linkerInit(ctx BaseModuleContext) {
 	binary.baseLinker.linkerInit(ctx)
 
 	if !ctx.toolchain().Bionic() {
 		if ctx.Os() == android.Linux {
+			// Unless explicitly specified otherwise, host static binaries are built with -static
+			// if HostStaticBinaries is true for the product configuration.
 			if binary.Properties.Static_executable == nil && ctx.Config().HostStaticBinaries() {
 				binary.Properties.Static_executable = BoolPtr(true)
 			}
@@ -217,9 +246,13 @@
 	return true
 }
 
+// linkerFlags returns a Flags object containing linker flags that are defined
+// by this binary, or that are implied by attributes of this binary. These flags are
+// combined with the given flags.
 func (binary *binaryDecorator) linkerFlags(ctx ModuleContext, flags Flags) Flags {
 	flags = binary.baseLinker.linkerFlags(ctx, flags)
 
+	// Passing -pie to clang for Windows binaries causes a warning that -pie is unused.
 	if ctx.Host() && !ctx.Windows() && !binary.static() {
 		if !ctx.Config().IsEnvTrue("DISABLE_HOST_PIE") {
 			flags.Global.LdFlags = append(flags.Global.LdFlags, "-pie")
@@ -248,7 +281,7 @@
 				"-Bstatic",
 				"-Wl,--gc-sections",
 			)
-		} else {
+		} else { // not static
 			if flags.DynamicLinker == "" {
 				if binary.Properties.DynamicLinker != "" {
 					flags.DynamicLinker = binary.Properties.DynamicLinker
@@ -288,7 +321,7 @@
 				"-Wl,-z,nocopyreloc",
 			)
 		}
-	} else {
+	} else { // not bionic
 		if binary.static() {
 			flags.Global.LdFlags = append(flags.Global.LdFlags, "-static")
 		}
@@ -300,6 +333,9 @@
 	return flags
 }
 
+// link registers actions to link this binary, and sets various fields
+// on this binary to reflect information that should be exported up the build
+// tree (for example, exported flags and include paths).
 func (binary *binaryDecorator) link(ctx ModuleContext,
 	flags Flags, deps PathDeps, objs Objects) android.Path {
 
@@ -309,6 +345,7 @@
 
 	var linkerDeps android.Paths
 
+	// Add flags from linker flags file.
 	if deps.LinkerFlagsFile.Valid() {
 		flags.Local.LdFlags = append(flags.Local.LdFlags, "$$(cat "+deps.LinkerFlagsFile.String()+")")
 		linkerDeps = append(linkerDeps, deps.LinkerFlagsFile.Path())
@@ -342,12 +379,15 @@
 
 	outputFile = maybeInjectBoringSSLHash(ctx, outputFile, binary.Properties.Inject_bssl_hash, fileName)
 
+	// If use_version_lib is true, make an android::build::GetBuildNumber() function available.
 	if Bool(binary.baseLinker.Properties.Use_version_lib) {
 		if ctx.Host() {
 			versionedOutputFile := outputFile
 			outputFile = android.PathForModuleOut(ctx, "unversioned", fileName)
 			binary.injectVersionSymbol(ctx, outputFile, versionedOutputFile)
 		} else {
+			// When dist'ing a library or binary that has use_version_lib set, always
+			// distribute the stamped version, even for the device.
 			versionedOutputFile := android.PathForModuleOut(ctx, "versioned", fileName)
 			binary.distFiles = android.MakeDefaultDistFiles(versionedOutputFile)
 
@@ -361,6 +401,7 @@
 		}
 	}
 
+	// Handle host bionic linker symbols.
 	if ctx.Os() == android.LinuxBionic && !binary.static() {
 		injectedOutputFile := outputFile
 		outputFile = android.PathForModuleOut(ctx, "prelinker", fileName)
@@ -386,6 +427,7 @@
 	linkerDeps = append(linkerDeps, objs.tidyFiles...)
 	linkerDeps = append(linkerDeps, flags.LdFlagsDeps...)
 
+	// Register link action.
 	TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs, deps.StaticLibs,
 		deps.LateStaticLibs, deps.WholeStaticLibs, linkerDeps, deps.CrtBegin, deps.CrtEnd, true,
 		builderFlags, outputFile, nil)
@@ -406,6 +448,7 @@
 			ctx.PropertyErrorf("symlink_preferred_arch", "must also specify suffix")
 		}
 		if ctx.TargetPrimary() {
+			// Install a symlink to the preferred architecture
 			symlinkName := binary.getStemWithoutSuffix(ctx)
 			binary.symlinks = append(binary.symlinks, symlinkName)
 			binary.preferredArchSymlink = symlinkName
diff --git a/cc/cc.go b/cc/cc.go
index bd6e5d5..6deb1b4 100644
--- a/cc/cc.go
+++ b/cc/cc.go
@@ -551,7 +551,15 @@
 	return d.Kind == staticLibraryDependency
 }
 
-// dependencyTag is used for tagging miscellanous dependency types that don't fit into
+// InstallDepNeeded returns true for shared libraries so that shared library dependencies of
+// binaries or other shared libraries are installed as dependencies.
+func (d libraryDependencyTag) InstallDepNeeded() bool {
+	return d.shared()
+}
+
+var _ android.InstallNeededDependencyTag = libraryDependencyTag{}
+
+// dependencyTag is used for tagging miscellaneous dependency types that don't fit into
 // libraryDependencyTag.  Each tag object is created globally and reused for multiple
 // dependencies (although since the object contains no references, assigning a tag to a
 // variable and modifying it will not modify the original).  Users can compare the tag
@@ -561,6 +569,15 @@
 	name string
 }
 
+// installDependencyTag is used for tagging miscellaneous dependency types that don't fit into
+// libraryDependencyTag, but where the dependency needs to be installed when the parent is
+// installed.
+type installDependencyTag struct {
+	blueprint.BaseDependencyTag
+	android.InstallAlwaysNeededDependencyTag
+	name string
+}
+
 var (
 	genSourceDepTag       = dependencyTag{name: "gen source"}
 	genHeaderDepTag       = dependencyTag{name: "gen header"}
@@ -572,7 +589,7 @@
 	staticVariantTag      = dependencyTag{name: "static variant"}
 	vndkExtDepTag         = dependencyTag{name: "vndk extends"}
 	dataLibDepTag         = dependencyTag{name: "data lib"}
-	runtimeDepTag         = dependencyTag{name: "runtime lib"}
+	runtimeDepTag         = installDependencyTag{name: "runtime lib"}
 	testPerSrcDepTag      = dependencyTag{name: "test_per_src"}
 	stubImplDepTag        = dependencyTag{name: "stub_impl"}
 )
@@ -599,8 +616,7 @@
 }
 
 func IsRuntimeDepTag(depTag blueprint.DependencyTag) bool {
-	ccDepTag, ok := depTag.(dependencyTag)
-	return ok && ccDepTag == runtimeDepTag
+	return depTag == runtimeDepTag
 }
 
 func IsTestPerSrcDepTag(depTag blueprint.DependencyTag) bool {
@@ -1841,6 +1857,11 @@
 		return
 	}
 
+	// sysprop_library has to support both C++ and Java. So sysprop_library internally creates one
+	// C++ implementation library and one Java implementation library. When a module links against
+	// sysprop_library, the C++ implementation library has to be linked. syspropImplLibraries is a
+	// map from sysprop_library to implementation library; it will be used in whole_static_libs,
+	// static_libs, and shared_libs.
 	syspropImplLibraries := syspropImplLibraries(actx.Config())
 	vendorSnapshotStaticLibs := vendorSnapshotStaticLibs(actx.Config())
 
diff --git a/cc/cc_test.go b/cc/cc_test.go
index f5ce867..7c98585 100644
--- a/cc/cc_test.go
+++ b/cc/cc_test.go
@@ -4069,3 +4069,98 @@
 	}
 
 }
+
+func TestInstallSharedLibs(t *testing.T) {
+	bp := `
+		cc_binary {
+			name: "bin",
+			host_supported: true,
+			shared_libs: ["libshared"],
+			runtime_libs: ["libruntime"],
+			srcs: [":gen"],
+		}
+
+		cc_library_shared {
+			name: "libshared",
+			host_supported: true,
+			shared_libs: ["libtransitive"],
+		}
+
+		cc_library_shared {
+			name: "libtransitive",
+			host_supported: true,
+		}
+
+		cc_library_shared {
+			name: "libruntime",
+			host_supported: true,
+		}
+
+		cc_binary_host {
+			name: "tool",
+			srcs: ["foo.cpp"],
+		}
+
+		genrule {
+			name: "gen",
+			tools: ["tool"],
+			out: ["gen.cpp"],
+			cmd: "$(location tool) $(out)",
+		}
+	`
+
+	config := TestConfig(buildDir, android.Android, nil, bp, nil)
+	ctx := testCcWithConfig(t, config)
+
+	hostBin := ctx.ModuleForTests("bin", config.BuildOSTarget.String()).Description("install")
+	hostShared := ctx.ModuleForTests("libshared", config.BuildOSTarget.String()+"_shared").Description("install")
+	hostRuntime := ctx.ModuleForTests("libruntime", config.BuildOSTarget.String()+"_shared").Description("install")
+	hostTransitive := ctx.ModuleForTests("libtransitive", config.BuildOSTarget.String()+"_shared").Description("install")
+	hostTool := ctx.ModuleForTests("tool", config.BuildOSTarget.String()).Description("install")
+
+	if g, w := hostBin.Implicits.Strings(), hostShared.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected host bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := hostBin.Implicits.Strings(), hostTransitive.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected host bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := hostShared.Implicits.Strings(), hostTransitive.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected host bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := hostBin.Implicits.Strings(), hostRuntime.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected host bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := hostBin.Implicits.Strings(), hostTool.Output.String(); android.InList(w, g) {
+		t.Errorf("expected no host bin dependency %q, got %q", w, g)
+	}
+
+	deviceBin := ctx.ModuleForTests("bin", "android_arm64_armv8-a").Description("install")
+	deviceShared := ctx.ModuleForTests("libshared", "android_arm64_armv8-a_shared").Description("install")
+	deviceTransitive := ctx.ModuleForTests("libtransitive", "android_arm64_armv8-a_shared").Description("install")
+	deviceRuntime := ctx.ModuleForTests("libruntime", "android_arm64_armv8-a_shared").Description("install")
+
+	if g, w := deviceBin.OrderOnly.Strings(), deviceShared.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected device bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := deviceBin.OrderOnly.Strings(), deviceTransitive.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected device bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := deviceShared.OrderOnly.Strings(), deviceTransitive.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected device bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := deviceBin.OrderOnly.Strings(), deviceRuntime.Output.String(); !android.InList(w, g) {
+		t.Errorf("expected device bin dependency %q, got %q", w, g)
+	}
+
+	if g, w := deviceBin.OrderOnly.Strings(), hostTool.Output.String(); android.InList(w, g) {
+		t.Errorf("expected no device bin dependency %q, got %q", w, g)
+	}
+
+}
diff --git a/cc/config/x86_darwin_host.go b/cc/config/x86_darwin_host.go
index 81c907d..d7ff580 100644
--- a/cc/config/x86_darwin_host.go
+++ b/cc/config/x86_darwin_host.go
@@ -135,7 +135,7 @@
 
 func getMacTools(ctx android.PackageVarContext) *macPlatformTools {
 	macTools.once.Do(func() {
-		xcrunTool := ctx.Config().HostSystemTool("xcrun")
+		xcrunTool := ctx.Config().NonHermeticHostSystemTool("xcrun")
 
 		xcrun := func(args ...string) string {
 			if macTools.err != nil {
diff --git a/cc/gen.go b/cc/gen.go
index 134d6d9..5895b31 100644
--- a/cc/gen.go
+++ b/cc/gen.go
@@ -232,7 +232,8 @@
 	var yaccRule_ *android.RuleBuilder
 	yaccRule := func() *android.RuleBuilder {
 		if yaccRule_ == nil {
-			yaccRule_ = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "yacc"))
+			yaccRule_ = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "yacc"),
+				android.PathForModuleGen(ctx, "yacc.sbox.textproto"))
 		}
 		return yaccRule_
 	}
@@ -261,7 +262,8 @@
 			deps = append(deps, headerFile)
 		case ".aidl":
 			if aidlRule == nil {
-				aidlRule = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "aidl"))
+				aidlRule = android.NewRuleBuilder().Sbox(android.PathForModuleGen(ctx, "aidl"),
+					android.PathForModuleGen(ctx, "aidl.sbox.textproto"))
 			}
 			cppFile := android.GenPathWithExt(ctx, "aidl", srcFile, "cpp")
 			depFile := android.GenPathWithExt(ctx, "aidl", srcFile, "cpp.d")
diff --git a/cc/gen_test.go b/cc/gen_test.go
index 4b9a36e..41ef95c 100644
--- a/cc/gen_test.go
+++ b/cc/gen_test.go
@@ -18,6 +18,8 @@
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"android/soong/android"
 )
 
 func TestGen(t *testing.T) {
@@ -56,13 +58,14 @@
 		}`)
 
 		aidl := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_shared").Rule("aidl")
+		aidlManifest := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_shared").Output("aidl.sbox.textproto")
 		libfoo := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_shared").Module().(*Module)
 
 		if !inList("-I"+filepath.Dir(aidl.Output.String()), libfoo.flags.Local.CommonFlags) {
 			t.Errorf("missing aidl includes in global flags")
 		}
 
-		aidlCommand := aidl.RuleParams.Command
+		aidlCommand := android.RuleBuilderSboxProtoForTests(t, aidlManifest).Commands[0].GetCommand()
 		if !strings.Contains(aidlCommand, "-Isub") {
 			t.Errorf("aidl command for c.aidl should contain \"-Isub\", but was %q", aidlCommand)
 		}
diff --git a/cc/library.go b/cc/library.go
index 2127c08..7ae75f2 100644
--- a/cc/library.go
+++ b/cc/library.go
@@ -1202,15 +1202,21 @@
 		}
 	}
 
+	// If the library is sysprop_library, expose either public or internal header selectively.
 	if library.baseCompiler.hasSrcExt(".sysprop") {
 		dir := android.PathForModuleGen(ctx, "sysprop", "include")
 		if library.Properties.Sysprop.Platform != nil {
-			isProduct := ctx.ProductSpecific() && !ctx.useVndk()
-			isVendor := ctx.useVndk()
+			isClientProduct := ctx.ProductSpecific() && !ctx.useVndk()
+			isClientVendor := ctx.useVndk()
 			isOwnerPlatform := Bool(library.Properties.Sysprop.Platform)
 
+			// If the owner is different from the user, expose public header. That is,
+			// 1) if the user is product (as owner can only be platform / vendor)
+			// 2) if one is platform and the other is vendor
+			// Exceptions are ramdisk and recovery. They are not enforced at all. So
+			// they always use internal header.
 			if !ctx.inRamdisk() && !ctx.inVendorRamdisk() && !ctx.inRecovery() &&
-				(isProduct || (isOwnerPlatform == isVendor)) {
+				(isClientProduct || (isOwnerPlatform == isClientVendor)) {
 				dir = android.PathForModuleGen(ctx, "sysprop/public", "include")
 			}
 		}
diff --git a/cc/linker.go b/cc/linker.go
index cbf8898..9d4a643 100644
--- a/cc/linker.go
+++ b/cc/linker.go
@@ -211,6 +211,7 @@
 	linker.Properties.Ldflags = append(linker.Properties.Ldflags, flags...)
 }
 
+// linkerInit initializes dynamic properties of the linker (such as runpath).
 func (linker *baseLinker) linkerInit(ctx BaseModuleContext) {
 	if ctx.toolchain().Is64Bit() {
 		linker.dynamicProperties.RunPaths = append(linker.dynamicProperties.RunPaths, "../lib64", "lib64")
diff --git a/cc/prebuilt_test.go b/cc/prebuilt_test.go
index 5bf334e..ee4de6e 100644
--- a/cc/prebuilt_test.go
+++ b/cc/prebuilt_test.go
@@ -32,7 +32,7 @@
 	// * Configure that we are inside make
 	// * Add CommonOS to ensure that androidmk processing works.
 	android.RegisterAndroidMkBuildComponents(ctx)
-	android.SetInMakeForTests(config)
+	android.SetKatiEnabledForTests(config)
 
 	for _, handler := range handlers {
 		handler(config)
diff --git a/cc/strip.go b/cc/strip.go
index 18150dc..e9aec91 100644
--- a/cc/strip.go
+++ b/cc/strip.go
@@ -20,23 +20,35 @@
 	"android/soong/android"
 )
 
+// StripProperties defines the type of stripping applied to the module.
 type StripProperties struct {
 	Strip struct {
-		None                         *bool    `android:"arch_variant"`
-		All                          *bool    `android:"arch_variant"`
-		Keep_symbols                 *bool    `android:"arch_variant"`
-		Keep_symbols_list            []string `android:"arch_variant"`
-		Keep_symbols_and_debug_frame *bool    `android:"arch_variant"`
+		// whether to disable all stripping.
+		None *bool `android:"arch_variant"`
+
+		// whether to strip everything, including the mini debug info.
+		All *bool `android:"arch_variant"`
+
+		// whether to keep the symbols.
+		Keep_symbols *bool `android:"arch_variant"`
+
+		// keeps only the symbols defined here.
+		Keep_symbols_list []string `android:"arch_variant"`
+
+		// whether to keep the symbols and the debug frames.
+		Keep_symbols_and_debug_frame *bool `android:"arch_variant"`
 	} `android:"arch_variant"`
 }
 
+// Stripper defines the stripping actions and properties for a module.
 type Stripper struct {
 	StripProperties StripProperties
 }
 
+// NeedsStrip determines if stripping is required for a module.
 func (stripper *Stripper) NeedsStrip(actx android.ModuleContext) bool {
-	// TODO(ccross): enable host stripping when embedded in make?  Make never had support for stripping host binaries.
-	return (!actx.Config().EmbeddedInMake() || actx.Device()) && !Bool(stripper.StripProperties.Strip.None)
+	// TODO(ccross): enable host stripping when Kati is enabled? Make never had support for stripping host binaries.
+	return (!actx.Config().KatiEnabled() || actx.Device()) && !Bool(stripper.StripProperties.Strip.None)
 }
 
 func (stripper *Stripper) strip(actx android.ModuleContext, in android.Path, out android.ModuleOutPath,
@@ -60,11 +72,17 @@
 	}
 }
 
+// StripExecutableOrSharedLib strips a binary or shared library from its debug
+// symbols and other debugging information. The helper function
+// flagsToStripFlags may be used to generate the flags argument.
 func (stripper *Stripper) StripExecutableOrSharedLib(actx android.ModuleContext, in android.Path,
 	out android.ModuleOutPath, flags StripFlags) {
 	stripper.strip(actx, in, out, flags, false)
 }
 
+// StripStaticLib strips a static library from its debug symbols and other
+// debugging information. The helper function flagsToStripFlags may be used to
+// generate the flags argument.
 func (stripper *Stripper) StripStaticLib(actx android.ModuleContext, in android.Path, out android.ModuleOutPath,
 	flags StripFlags) {
 	stripper.strip(actx, in, out, flags, true)
diff --git a/cc/sysprop.go b/cc/sysprop.go
index 6cac7fb..f578b50 100644
--- a/cc/sysprop.go
+++ b/cc/sysprop.go
@@ -14,6 +14,25 @@
 
 package cc
 
+// This file contains a map to redirect dependencies towards sysprop_library.
+// As sysprop_library has to support both Java and C++, sysprop_library internally
+// generates cc_library and java_library. For example, the following sysprop_library
+//
+//     sysprop_library {
+//         name: "foo",
+//     }
+//
+// will internally generate with prefix "lib"
+//
+//     cc_library {
+//         name: "libfoo",
+//     }
+//
+// When a cc module links against "foo", build system will redirect the
+// dependency to "libfoo". To do that, SyspropMutator gathers all sysprop_library,
+// records their cc implementation library names to a map. The map will be used in
+// cc.Module.DepsMutator.
+
 import (
 	"sync"
 
@@ -22,7 +41,7 @@
 
 type syspropLibraryInterface interface {
 	BaseModuleName() string
-	CcModuleName() string
+	CcImplementationModuleName() string
 }
 
 var (
@@ -43,6 +62,8 @@
 		syspropImplLibrariesLock.Lock()
 		defer syspropImplLibrariesLock.Unlock()
 
-		syspropImplLibraries[m.BaseModuleName()] = m.CcModuleName()
+		// BaseModuleName is the name of sysprop_library
+		// CcImplementationModuleName is the name of cc_library generated by sysprop_library
+		syspropImplLibraries[m.BaseModuleName()] = m.CcImplementationModuleName()
 	}
 }
diff --git a/cc/testing.go b/cc/testing.go
index 7161313..198764b 100644
--- a/cc/testing.go
+++ b/cc/testing.go
@@ -16,6 +16,7 @@
 
 import (
 	"android/soong/android"
+	"android/soong/genrule"
 )
 
 func RegisterRequiredBuildComponentsForTest(ctx android.RegistrationContext) {
@@ -24,6 +25,7 @@
 	RegisterBinaryBuildComponents(ctx)
 	RegisterLibraryBuildComponents(ctx)
 	RegisterLibraryHeadersBuildComponents(ctx)
+	genrule.RegisterGenruleBuildComponents(ctx)
 
 	ctx.RegisterModuleType("toolchain_library", ToolchainLibraryFactory)
 	ctx.RegisterModuleType("llndk_library", LlndkLibraryFactory)
diff --git a/cmd/sbox/Android.bp b/cmd/sbox/Android.bp
index 6fa304e..f5e87c0 100644
--- a/cmd/sbox/Android.bp
+++ b/cmd/sbox/Android.bp
@@ -14,8 +14,20 @@
 
 blueprint_go_binary {
     name: "sbox",
-    deps: ["soong-makedeps"],
+    deps: [
+        "sbox_proto",
+        "soong-makedeps",
+    ],
     srcs: [
         "sbox.go",
     ],
 }
+
+bootstrap_go_package {
+    name: "sbox_proto",
+    pkgPath: "android/soong/cmd/sbox/sbox_proto",
+    deps: ["golang-protobuf-proto"],
+    srcs: [
+        "sbox_proto/sbox.pb.go",
+    ],
+}
diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go
index 65a34fd..db483f1 100644
--- a/cmd/sbox/sbox.go
+++ b/cmd/sbox/sbox.go
@@ -19,41 +19,39 @@
 	"errors"
 	"flag"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
-	"path"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
+	"android/soong/cmd/sbox/sbox_proto"
 	"android/soong/makedeps"
+
+	"github.com/golang/protobuf/proto"
 )
 
 var (
 	sandboxesRoot string
-	rawCommand    string
-	outputRoot    string
+	manifestFile  string
 	keepOutDir    bool
-	depfileOut    string
-	inputHash     string
+)
+
+const (
+	depFilePlaceholder    = "__SBOX_DEPFILE__"
+	sandboxDirPlaceholder = "__SBOX_SANDBOX_DIR__"
 )
 
 func init() {
 	flag.StringVar(&sandboxesRoot, "sandbox-path", "",
 		"root of temp directory to put the sandbox into")
-	flag.StringVar(&rawCommand, "c", "",
-		"command to run")
-	flag.StringVar(&outputRoot, "output-root", "",
-		"root of directory to copy outputs into")
+	flag.StringVar(&manifestFile, "manifest", "",
+		"textproto manifest describing the sandboxed command(s)")
 	flag.BoolVar(&keepOutDir, "keep-out-dir", false,
 		"whether to keep the sandbox directory when done")
-
-	flag.StringVar(&depfileOut, "depfile-out", "",
-		"file path of the depfile to generate. This value will replace '__SBOX_DEPFILE__' in the command and will be treated as an output but won't be added to __SBOX_OUT_FILES__")
-
-	flag.StringVar(&inputHash, "input-hash", "",
-		"This option is ignored. Typical usage is to supply a hash of the list of input names so that the module will be rebuilt if the list (and thus the hash) changes.")
 }
 
 func usageViolation(violation string) {
@@ -62,11 +60,7 @@
 	}
 
 	fmt.Fprintf(os.Stderr,
-		"Usage: sbox -c <commandToRun> --sandbox-path <sandboxPath> --output-root <outputRoot> [--depfile-out depFile] [--input-hash hash] <outputFile> [<outputFile>...]\n"+
-			"\n"+
-			"Deletes <outputRoot>,"+
-			"runs <commandToRun>,"+
-			"and moves each <outputFile> out of <sandboxPath> and into <outputRoot>\n")
+		"Usage: sbox --manifest <manifest> --sandbox-path <sandboxPath>\n")
 
 	flag.PrintDefaults()
 
@@ -103,8 +97,8 @@
 }
 
 func run() error {
-	if rawCommand == "" {
-		usageViolation("-c <commandToRun> is required and must be non-empty")
+	if manifestFile == "" {
+		usageViolation("--manifest <manifest> is required and must be non-empty")
 	}
 	if sandboxesRoot == "" {
 		// In practice, the value of sandboxesRoot will mostly likely be at a fixed location relative to OUT_DIR,
@@ -114,61 +108,28 @@
 		// and by passing it as a parameter we don't need to duplicate its value
 		usageViolation("--sandbox-path <sandboxPath> is required and must be non-empty")
 	}
-	if len(outputRoot) == 0 {
-		usageViolation("--output-root <outputRoot> is required and must be non-empty")
+
+	manifest, err := readManifest(manifestFile)
+
+	if len(manifest.Commands) == 0 {
+		return fmt.Errorf("at least one commands entry is required in %q", manifestFile)
 	}
 
-	// the contents of the __SBOX_OUT_FILES__ variable
-	outputsVarEntries := flag.Args()
-	if len(outputsVarEntries) == 0 {
-		usageViolation("at least one output file must be given")
-	}
-
-	// all outputs
-	var allOutputs []string
-
-	// setup directories
-	err := os.MkdirAll(sandboxesRoot, 0777)
+	// setup sandbox directory
+	err = os.MkdirAll(sandboxesRoot, 0777)
 	if err != nil {
-		return err
-	}
-	err = os.RemoveAll(outputRoot)
-	if err != nil {
-		return err
-	}
-	err = os.MkdirAll(outputRoot, 0777)
-	if err != nil {
-		return err
+		return fmt.Errorf("failed to create %q: %w", sandboxesRoot, err)
 	}
 
 	tempDir, err := ioutil.TempDir(sandboxesRoot, "sbox")
-
-	for i, filePath := range outputsVarEntries {
-		if !strings.HasPrefix(filePath, "__SBOX_OUT_DIR__/") {
-			return fmt.Errorf("output files must start with `__SBOX_OUT_DIR__/`")
-		}
-		outputsVarEntries[i] = strings.TrimPrefix(filePath, "__SBOX_OUT_DIR__/")
-	}
-
-	allOutputs = append([]string(nil), outputsVarEntries...)
-
-	if depfileOut != "" {
-		sandboxedDepfile, err := filepath.Rel(outputRoot, depfileOut)
-		if err != nil {
-			return err
-		}
-		allOutputs = append(allOutputs, sandboxedDepfile)
-		rawCommand = strings.Replace(rawCommand, "__SBOX_DEPFILE__", filepath.Join(tempDir, sandboxedDepfile), -1)
-
-	}
-
 	if err != nil {
-		return fmt.Errorf("Failed to create temp dir: %s", err)
+		return fmt.Errorf("failed to create temporary dir in %q: %w", sandboxesRoot, err)
 	}
 
 	// In the common case, the following line of code is what removes the sandbox
 	// If a fatal error occurs (such as if our Go process is killed unexpectedly),
-	// then at the beginning of the next build, Soong will retry the cleanup
+	// then at the beginning of the next build, Soong will wipe the temporary
+	// directory.
 	defer func() {
 		// in some cases we decline to remove the temp dir, to facilitate debugging
 		if !keepOutDir {
@@ -176,27 +137,95 @@
 		}
 	}()
 
-	if strings.Contains(rawCommand, "__SBOX_OUT_DIR__") {
-		rawCommand = strings.Replace(rawCommand, "__SBOX_OUT_DIR__", tempDir, -1)
-	}
+	// If there is more than one command in the manifest use a separate directory for each one.
+	useSubDir := len(manifest.Commands) > 1
+	var commandDepFiles []string
 
-	if strings.Contains(rawCommand, "__SBOX_OUT_FILES__") {
-		// expands into a space-separated list of output files to be generated into the sandbox directory
-		tempOutPaths := []string{}
-		for _, outputPath := range outputsVarEntries {
-			tempOutPath := path.Join(tempDir, outputPath)
-			tempOutPaths = append(tempOutPaths, tempOutPath)
+	for i, command := range manifest.Commands {
+		localTempDir := tempDir
+		if useSubDir {
+			localTempDir = filepath.Join(localTempDir, strconv.Itoa(i))
 		}
-		pathsText := strings.Join(tempOutPaths, " ")
-		rawCommand = strings.Replace(rawCommand, "__SBOX_OUT_FILES__", pathsText, -1)
-	}
-
-	for _, filePath := range allOutputs {
-		dir := path.Join(tempDir, filepath.Dir(filePath))
-		err = os.MkdirAll(dir, 0777)
+		depFile, err := runCommand(command, localTempDir)
 		if err != nil {
+			// Running the command failed, keep the temporary output directory around in
+			// case a user wants to inspect it for debugging purposes.  Soong will delete
+			// it at the beginning of the next build anyway.
+			keepOutDir = true
 			return err
 		}
+		if depFile != "" {
+			commandDepFiles = append(commandDepFiles, depFile)
+		}
+	}
+
+	outputDepFile := manifest.GetOutputDepfile()
+	if len(commandDepFiles) > 0 && outputDepFile == "" {
+		return fmt.Errorf("Sandboxed commands used %s but output depfile is not set in manifest file",
+			depFilePlaceholder)
+	}
+
+	if outputDepFile != "" {
+		// Merge the depfiles from each command in the manifest to a single output depfile.
+		err = rewriteDepFiles(commandDepFiles, outputDepFile)
+		if err != nil {
+			return fmt.Errorf("failed merging depfiles: %w", err)
+		}
+	}
+
+	return nil
+}
+
+// readManifest reads an sbox manifest from a textproto file.
+func readManifest(file string) (*sbox_proto.Manifest, error) {
+	manifestData, err := ioutil.ReadFile(file)
+	if err != nil {
+		return nil, fmt.Errorf("error reading manifest %q: %w", file, err)
+	}
+
+	manifest := sbox_proto.Manifest{}
+
+	err = proto.UnmarshalText(string(manifestData), &manifest)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing manifest %q: %w", file, err)
+	}
+
+	return &manifest, nil
+}
+
+// runCommand runs a single command from a manifest.  If the command references the
+// __SBOX_DEPFILE__ placeholder it returns the name of the depfile that was used.
+func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, err error) {
+	rawCommand := command.GetCommand()
+	if rawCommand == "" {
+		return "", fmt.Errorf("command is required")
+	}
+
+	err = os.MkdirAll(tempDir, 0777)
+	if err != nil {
+		return "", fmt.Errorf("failed to create %q: %w", tempDir, err)
+	}
+
+	// Copy in any files specified by the manifest.
+	err = linkOrCopyFiles(command.CopyBefore, "", tempDir)
+	if err != nil {
+		return "", err
+	}
+
+	if strings.Contains(rawCommand, depFilePlaceholder) {
+		depFile = filepath.Join(tempDir, "deps.d")
+		rawCommand = strings.Replace(rawCommand, depFilePlaceholder, depFile, -1)
+	}
+
+	if strings.Contains(rawCommand, sandboxDirPlaceholder) {
+		rawCommand = strings.Replace(rawCommand, sandboxDirPlaceholder, tempDir, -1)
+	}
+
+	// Emulate ninja's behavior of creating the directories for any output files before
+	// running the command.
+	err = makeOutputDirs(command.CopyAfter, tempDir)
+	if err != nil {
+		return "", err
 	}
 
 	commandDescription := rawCommand
@@ -205,27 +234,20 @@
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
+
+	if command.GetChdir() {
+		cmd.Dir = tempDir
+	}
 	err = cmd.Run()
 
 	if exit, ok := err.(*exec.ExitError); ok && !exit.Success() {
-		return fmt.Errorf("sbox command (%s) failed with err %#v\n", commandDescription, err.Error())
+		return "", fmt.Errorf("sbox command failed with err:\n%s\n%w\n", commandDescription, err)
 	} else if err != nil {
-		return err
+		return "", err
 	}
 
-	// validate that all files are created properly
-	var missingOutputErrors []string
-	for _, filePath := range allOutputs {
-		tempPath := filepath.Join(tempDir, filePath)
-		fileInfo, err := os.Stat(tempPath)
-		if err != nil {
-			missingOutputErrors = append(missingOutputErrors, fmt.Sprintf("%s: does not exist", filePath))
-			continue
-		}
-		if fileInfo.IsDir() {
-			missingOutputErrors = append(missingOutputErrors, fmt.Sprintf("%s: not a file", filePath))
-		}
-	}
+	missingOutputErrors := validateOutputFiles(command.CopyAfter, tempDir)
+
 	if len(missingOutputErrors) > 0 {
 		// find all created files for making a more informative error message
 		createdFiles := findAllFilesUnder(tempDir)
@@ -236,7 +258,7 @@
 		errorMessage += "in sandbox " + tempDir + ",\n"
 		errorMessage += fmt.Sprintf("failed to create %v files:\n", len(missingOutputErrors))
 		for _, missingOutputError := range missingOutputErrors {
-			errorMessage += "  " + missingOutputError + "\n"
+			errorMessage += "  " + missingOutputError.Error() + "\n"
 		}
 		if len(createdFiles) < 1 {
 			errorMessage += "created 0 files."
@@ -253,19 +275,137 @@
 			}
 		}
 
-		// Keep the temporary output directory around in case a user wants to inspect it for debugging purposes.
-		// Soong will delete it later anyway.
-		keepOutDir = true
-		return errors.New(errorMessage)
+		return "", errors.New(errorMessage)
 	}
 	// the created files match the declared files; now move them
-	for _, filePath := range allOutputs {
-		tempPath := filepath.Join(tempDir, filePath)
-		destPath := filePath
-		if len(outputRoot) != 0 {
-			destPath = filepath.Join(outputRoot, filePath)
+	err = moveFiles(command.CopyAfter, tempDir, "")
+
+	return depFile, nil
+}
+
+// makeOutputDirs creates directories in the sandbox dir for every file that has a rule to be copied
+// out of the sandbox.  This emulate's Ninja's behavior of creating directories for output files
+// so that the tools don't have to.
+func makeOutputDirs(copies []*sbox_proto.Copy, sandboxDir string) error {
+	for _, copyPair := range copies {
+		dir := joinPath(sandboxDir, filepath.Dir(copyPair.GetFrom()))
+		err := os.MkdirAll(dir, 0777)
+		if err != nil {
+			return err
 		}
-		err := os.MkdirAll(filepath.Dir(destPath), 0777)
+	}
+	return nil
+}
+
+// validateOutputFiles verifies that all files that have a rule to be copied out of the sandbox
+// were created by the command.
+func validateOutputFiles(copies []*sbox_proto.Copy, sandboxDir string) []error {
+	var missingOutputErrors []error
+	for _, copyPair := range copies {
+		fromPath := joinPath(sandboxDir, copyPair.GetFrom())
+		fileInfo, err := os.Stat(fromPath)
+		if err != nil {
+			missingOutputErrors = append(missingOutputErrors, fmt.Errorf("%s: does not exist", fromPath))
+			continue
+		}
+		if fileInfo.IsDir() {
+			missingOutputErrors = append(missingOutputErrors, fmt.Errorf("%s: not a file", fromPath))
+		}
+	}
+	return missingOutputErrors
+}
+
+// linkOrCopyFiles hardlinks or copies files in or out of the sandbox.
+func linkOrCopyFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error {
+	for _, copyPair := range copies {
+		fromPath := joinPath(fromDir, copyPair.GetFrom())
+		toPath := joinPath(toDir, copyPair.GetTo())
+		err := linkOrCopyOneFile(fromPath, toPath)
+		if err != nil {
+			return fmt.Errorf("error copying %q to %q: %w", fromPath, toPath, err)
+		}
+	}
+	return nil
+}
+
+// linkOrCopyOneFile first attempts to hardlink a file to a destination, and falls back to making
+// a copy if the hardlink fails.
+func linkOrCopyOneFile(from string, to string) error {
+	err := os.MkdirAll(filepath.Dir(to), 0777)
+	if err != nil {
+		return err
+	}
+
+	// First try hardlinking
+	err = os.Link(from, to)
+	if err != nil {
+		// Retry with copying in case the source an destination are on different filesystems.
+		// TODO: check for specific hardlink error?
+		err = copyOneFile(from, to)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// copyOneFile copies a file.
+func copyOneFile(from string, to string) error {
+	stat, err := os.Stat(from)
+	if err != nil {
+		return err
+	}
+
+	perm := stat.Mode()
+
+	in, err := os.Open(from)
+	if err != nil {
+		return err
+	}
+	defer in.Close()
+
+	out, err := os.Create(to)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		out.Close()
+		if err != nil {
+			os.Remove(to)
+		}
+	}()
+
+	_, err = io.Copy(out, in)
+	if err != nil {
+		return err
+	}
+
+	if err = out.Close(); err != nil {
+		return err
+	}
+
+	if err = os.Chmod(to, perm); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// moveFiles moves files specified by a set of copy rules.  It uses os.Rename, so it is restricted
+// to moving files where the source and destination are in the same filesystem.  This is OK for
+// sbox because the temporary directory is inside the out directory.  It updates the timestamp
+// of the new file.
+func moveFiles(copies []*sbox_proto.Copy, fromDir, toDir string) error {
+	for _, copyPair := range copies {
+		fromPath := joinPath(fromDir, copyPair.GetFrom())
+		toPath := joinPath(toDir, copyPair.GetTo())
+		err := os.MkdirAll(filepath.Dir(toPath), 0777)
+		if err != nil {
+			return err
+		}
+
+		err = os.Rename(fromPath, toPath)
 		if err != nil {
 			return err
 		}
@@ -273,37 +413,53 @@
 		// Update the timestamp of the output file in case the tool wrote an old timestamp (for example, tar can extract
 		// files with old timestamps).
 		now := time.Now()
-		err = os.Chtimes(tempPath, now, now)
-		if err != nil {
-			return err
-		}
-
-		err = os.Rename(tempPath, destPath)
+		err = os.Chtimes(toPath, now, now)
 		if err != nil {
 			return err
 		}
 	}
-
-	// Rewrite the depfile so that it doesn't include the (randomized) sandbox directory
-	if depfileOut != "" {
-		in, err := ioutil.ReadFile(depfileOut)
-		if err != nil {
-			return err
-		}
-
-		deps, err := makedeps.Parse(depfileOut, bytes.NewBuffer(in))
-		if err != nil {
-			return err
-		}
-
-		deps.Output = "outputfile"
-
-		err = ioutil.WriteFile(depfileOut, deps.Print(), 0666)
-		if err != nil {
-			return err
-		}
-	}
-
-	// TODO(jeffrygaston) if a process creates more output files than it declares, should there be a warning?
 	return nil
 }
+
+// Rewrite one or more depfiles so that it doesn't include the (randomized) sandbox directory
+// to an output file.
+func rewriteDepFiles(ins []string, out string) error {
+	var mergedDeps []string
+	for _, in := range ins {
+		data, err := ioutil.ReadFile(in)
+		if err != nil {
+			return err
+		}
+
+		deps, err := makedeps.Parse(in, bytes.NewBuffer(data))
+		if err != nil {
+			return err
+		}
+		mergedDeps = append(mergedDeps, deps.Inputs...)
+	}
+
+	deps := makedeps.Deps{
+		// Ninja doesn't care what the output file is, so we can use any string here.
+		Output: "outputfile",
+		Inputs: mergedDeps,
+	}
+
+	// Make the directory for the output depfile in case it is in a different directory
+	// than any of the output files.
+	outDir := filepath.Dir(out)
+	err := os.MkdirAll(outDir, 0777)
+	if err != nil {
+		return fmt.Errorf("failed to create %q: %w", outDir, err)
+	}
+
+	return ioutil.WriteFile(out, deps.Print(), 0666)
+}
+
+// joinPath wraps filepath.Join but returns file without appending to dir if file is
+// absolute.
+func joinPath(dir, file string) string {
+	if filepath.IsAbs(file) {
+		return file
+	}
+	return filepath.Join(dir, file)
+}
diff --git a/cmd/sbox/sbox_proto/sbox.pb.go b/cmd/sbox/sbox_proto/sbox.pb.go
new file mode 100644
index 0000000..6584bdf
--- /dev/null
+++ b/cmd/sbox/sbox_proto/sbox.pb.go
@@ -0,0 +1,233 @@
+// Code generated by protoc-gen-go. DO NOT EDIT.
+// source: sbox.proto
+
+package sbox_proto
+
+import (
+	fmt "fmt"
+	proto "github.com/golang/protobuf/proto"
+	math "math"
+)
+
+// Reference imports to suppress errors if they are not otherwise used.
+var _ = proto.Marshal
+var _ = fmt.Errorf
+var _ = math.Inf
+
+// This is a compile-time assertion to ensure that this generated file
+// is compatible with the proto package it is being compiled against.
+// A compilation error at this line likely means your copy of the
+// proto package needs to be updated.
+const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package
+
+// A set of commands to run in a sandbox.
+type Manifest struct {
+	// A list of commands to run in the sandbox.
+	Commands []*Command `protobuf:"bytes,1,rep,name=commands" json:"commands,omitempty"`
+	// If set, GCC-style dependency files from any command that references __SBOX_DEPFILE__ will be
+	// merged into the given output file relative to the $PWD when sbox was started.
+	OutputDepfile        *string  `protobuf:"bytes,2,opt,name=output_depfile,json=outputDepfile" json:"output_depfile,omitempty"`
+	XXX_NoUnkeyedLiteral struct{} `json:"-"`
+	XXX_unrecognized     []byte   `json:"-"`
+	XXX_sizecache        int32    `json:"-"`
+}
+
+func (m *Manifest) Reset()         { *m = Manifest{} }
+func (m *Manifest) String() string { return proto.CompactTextString(m) }
+func (*Manifest) ProtoMessage()    {}
+func (*Manifest) Descriptor() ([]byte, []int) {
+	return fileDescriptor_9d0425bf0de86ed1, []int{0}
+}
+
+func (m *Manifest) XXX_Unmarshal(b []byte) error {
+	return xxx_messageInfo_Manifest.Unmarshal(m, b)
+}
+func (m *Manifest) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) {
+	return xxx_messageInfo_Manifest.Marshal(b, m, deterministic)
+}
+func (m *Manifest) XXX_Merge(src proto.Message) {
+	xxx_messageInfo_Manifest.Merge(m, src)
+}
+func (m *Manifest) XXX_Size() int {
+	return xxx_messageInfo_Manifest.Size(m)
+}
+func (m *Manifest) XXX_DiscardUnknown() {
+	xxx_messageInfo_Manifest.DiscardUnknown(m)
+}
+
+var xxx_messageInfo_Manifest proto.InternalMessageInfo
+
+func (m *Manifest) GetCommands() []*Command {
+	if m != nil {
+		return m.Commands
+	}
+	return nil
+}
+
+func (m *Manifest) GetOutputDepfile() string {
+	if m != nil && m.OutputDepfile != nil {
+		return *m.OutputDepfile
+	}
+	return ""
+}
+
+// SandboxManifest describes a command to run in the sandbox.
+type Command struct {
+	// A list of copy rules to run before the sandboxed command.  The from field is relative to the
+	// $PWD when sbox was run, the to field is relative to the top of the temporary sandbox directory.
+	CopyBefore []*Copy `protobuf:"bytes,1,rep,name=copy_before,json=copyBefore" json:"copy_before,omitempty"`
+	// If true, change the working directory to the top of the temporary sandbox directory before
+	// running the command.  If false, leave the working directory where it was when sbox was started.
+	Chdir *bool `protobuf:"varint,2,opt,name=chdir" json:"chdir,omitempty"`
+	// The command to run.
+	Command *string `protobuf:"bytes,3,req,name=command" json:"command,omitempty"`
+	// A list of copy rules to run after the sandboxed command.  The from field is relative to the
+	// top of the temporary sandbox directory, the to field is relative to the $PWD when sbox was run.
+	CopyAfter []*Copy `protobuf:"bytes,4,rep,name=copy_after,json=copyAfter" json:"copy_after,omitempty"`
+	// An optional hash of the input files to ensure the textproto files and the sbox rule reruns
+	// when the lists of inputs changes, even if the inputs are not on the command line.
+	InputHash            *string  `protobuf:"bytes,5,opt,name=input_hash,json=inputHash" json:"input_hash,omitempty"`
+	XXX_NoUnkeyedLiteral struct{} `json:"-"`
+	XXX_unrecognized     []byte   `json:"-"`
+	XXX_sizecache        int32    `json:"-"`
+}
+
+func (m *Command) Reset()         { *m = Command{} }
+func (m *Command) String() string { return proto.CompactTextString(m) }
+func (*Command) ProtoMessage()    {}
+func (*Command) Descriptor() ([]byte, []int) {
+	return fileDescriptor_9d0425bf0de86ed1, []int{1}
+}
+
+func (m *Command) XXX_Unmarshal(b []byte) error {
+	return xxx_messageInfo_Command.Unmarshal(m, b)
+}
+func (m *Command) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) {
+	return xxx_messageInfo_Command.Marshal(b, m, deterministic)
+}
+func (m *Command) XXX_Merge(src proto.Message) {
+	xxx_messageInfo_Command.Merge(m, src)
+}
+func (m *Command) XXX_Size() int {
+	return xxx_messageInfo_Command.Size(m)
+}
+func (m *Command) XXX_DiscardUnknown() {
+	xxx_messageInfo_Command.DiscardUnknown(m)
+}
+
+var xxx_messageInfo_Command proto.InternalMessageInfo
+
+func (m *Command) GetCopyBefore() []*Copy {
+	if m != nil {
+		return m.CopyBefore
+	}
+	return nil
+}
+
+func (m *Command) GetChdir() bool {
+	if m != nil && m.Chdir != nil {
+		return *m.Chdir
+	}
+	return false
+}
+
+func (m *Command) GetCommand() string {
+	if m != nil && m.Command != nil {
+		return *m.Command
+	}
+	return ""
+}
+
+func (m *Command) GetCopyAfter() []*Copy {
+	if m != nil {
+		return m.CopyAfter
+	}
+	return nil
+}
+
+func (m *Command) GetInputHash() string {
+	if m != nil && m.InputHash != nil {
+		return *m.InputHash
+	}
+	return ""
+}
+
+// Copy describes a from-to pair of files to copy.  The paths may be relative, the root that they
+// are relative to is specific to the context the Copy is used in and will be different for
+// from and to.
+type Copy struct {
+	From                 *string  `protobuf:"bytes,1,req,name=from" json:"from,omitempty"`
+	To                   *string  `protobuf:"bytes,2,req,name=to" json:"to,omitempty"`
+	XXX_NoUnkeyedLiteral struct{} `json:"-"`
+	XXX_unrecognized     []byte   `json:"-"`
+	XXX_sizecache        int32    `json:"-"`
+}
+
+func (m *Copy) Reset()         { *m = Copy{} }
+func (m *Copy) String() string { return proto.CompactTextString(m) }
+func (*Copy) ProtoMessage()    {}
+func (*Copy) Descriptor() ([]byte, []int) {
+	return fileDescriptor_9d0425bf0de86ed1, []int{2}
+}
+
+func (m *Copy) XXX_Unmarshal(b []byte) error {
+	return xxx_messageInfo_Copy.Unmarshal(m, b)
+}
+func (m *Copy) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) {
+	return xxx_messageInfo_Copy.Marshal(b, m, deterministic)
+}
+func (m *Copy) XXX_Merge(src proto.Message) {
+	xxx_messageInfo_Copy.Merge(m, src)
+}
+func (m *Copy) XXX_Size() int {
+	return xxx_messageInfo_Copy.Size(m)
+}
+func (m *Copy) XXX_DiscardUnknown() {
+	xxx_messageInfo_Copy.DiscardUnknown(m)
+}
+
+var xxx_messageInfo_Copy proto.InternalMessageInfo
+
+func (m *Copy) GetFrom() string {
+	if m != nil && m.From != nil {
+		return *m.From
+	}
+	return ""
+}
+
+func (m *Copy) GetTo() string {
+	if m != nil && m.To != nil {
+		return *m.To
+	}
+	return ""
+}
+
+func init() {
+	proto.RegisterType((*Manifest)(nil), "sbox.Manifest")
+	proto.RegisterType((*Command)(nil), "sbox.Command")
+	proto.RegisterType((*Copy)(nil), "sbox.Copy")
+}
+
+func init() {
+	proto.RegisterFile("sbox.proto", fileDescriptor_9d0425bf0de86ed1)
+}
+
+var fileDescriptor_9d0425bf0de86ed1 = []byte{
+	// 252 bytes of a gzipped FileDescriptorProto
+	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x64, 0x90, 0x41, 0x4b, 0xc3, 0x40,
+	0x10, 0x85, 0x49, 0x9a, 0xd2, 0x66, 0x6a, 0x7b, 0x18, 0x3c, 0xec, 0x45, 0x08, 0x01, 0x21, 0x55,
+	0xe8, 0xc1, 0x7f, 0x60, 0xf5, 0xe0, 0xc5, 0xcb, 0x1e, 0x45, 0x08, 0xdb, 0x64, 0x97, 0x04, 0x4c,
+	0x66, 0xd9, 0xdd, 0x82, 0xfd, 0x57, 0xfe, 0x44, 0xd9, 0x49, 0xea, 0xc5, 0xdb, 0xcc, 0xfb, 0x78,
+	0xf3, 0x1e, 0x03, 0xe0, 0x4f, 0xf4, 0x7d, 0xb0, 0x8e, 0x02, 0x61, 0x16, 0xe7, 0xf2, 0x13, 0xd6,
+	0xef, 0x6a, 0xec, 0x8d, 0xf6, 0x01, 0xf7, 0xb0, 0x6e, 0x68, 0x18, 0xd4, 0xd8, 0x7a, 0x91, 0x14,
+	0x8b, 0x6a, 0xf3, 0xb4, 0x3d, 0xb0, 0xe1, 0x65, 0x52, 0xe5, 0x1f, 0xc6, 0x7b, 0xd8, 0xd1, 0x39,
+	0xd8, 0x73, 0xa8, 0x5b, 0x6d, 0x4d, 0xff, 0xa5, 0x45, 0x5a, 0x24, 0x55, 0x2e, 0xb7, 0x93, 0xfa,
+	0x3a, 0x89, 0xe5, 0x4f, 0x02, 0xab, 0xd9, 0x8c, 0x8f, 0xb0, 0x69, 0xc8, 0x5e, 0xea, 0x93, 0x36,
+	0xe4, 0xf4, 0x1c, 0x00, 0xd7, 0x00, 0x7b, 0x91, 0x10, 0xf1, 0x91, 0x29, 0xde, 0xc2, 0xb2, 0xe9,
+	0xda, 0xde, 0xf1, 0xd9, 0xb5, 0x9c, 0x16, 0x14, 0xb0, 0x9a, 0x1b, 0x88, 0x45, 0x91, 0x56, 0xb9,
+	0xbc, 0xae, 0xb8, 0x07, 0x76, 0xd7, 0xca, 0x04, 0xed, 0x44, 0xf6, 0xef, 0x76, 0x1e, 0xe9, 0x73,
+	0x84, 0x78, 0x07, 0xd0, 0x8f, 0xb1, 0x79, 0xa7, 0x7c, 0x27, 0x96, 0x5c, 0x3b, 0x67, 0xe5, 0x4d,
+	0xf9, 0xae, 0x7c, 0x80, 0x2c, 0x3a, 0x10, 0x21, 0x33, 0x8e, 0x06, 0x91, 0x70, 0x10, 0xcf, 0xb8,
+	0x83, 0x34, 0x90, 0x48, 0x59, 0x49, 0x03, 0x1d, 0x6f, 0x3e, 0xf8, 0xa1, 0x35, 0x3f, 0xf4, 0x37,
+	0x00, 0x00, 0xff, 0xff, 0x95, 0x4d, 0xee, 0x7d, 0x5d, 0x01, 0x00, 0x00,
+}
diff --git a/cmd/sbox/sbox_proto/sbox.proto b/cmd/sbox/sbox_proto/sbox.proto
new file mode 100644
index 0000000..ab95545
--- /dev/null
+++ b/cmd/sbox/sbox_proto/sbox.proto
@@ -0,0 +1,58 @@
+// Copyright 2020 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+syntax = "proto2";
+
+package sbox;
+option go_package = "sbox_proto";
+
+// A set of commands to run in a sandbox.
+message Manifest {
+  // A list of commands to run in the sandbox.
+  repeated Command commands = 1;
+
+  // If set, GCC-style dependency files from any command that references __SBOX_DEPFILE__ will be
+  // merged into the given output file relative to the $PWD when sbox was started.
+  optional string output_depfile = 2;
+}
+
+// SandboxManifest describes a command to run in the sandbox.
+message Command {
+  // A list of copy rules to run before the sandboxed command.  The from field is relative to the
+  // $PWD when sbox was run, the to field is relative to the top of the temporary sandbox directory.
+  repeated Copy copy_before = 1;
+
+  // If true, change the working directory to the top of the temporary sandbox directory before
+  // running the command.  If false, leave the working directory where it was when sbox was started.
+  optional bool chdir = 2;
+
+  // The command to run.
+  required string command = 3;
+
+  // A list of copy rules to run after the sandboxed command.  The from field is relative to the
+  // top of the temporary sandbox directory, the to field is relative to the $PWD when sbox was run.
+  repeated Copy copy_after = 4;
+
+  // An optional hash of the input files to ensure the textproto files and the sbox rule reruns
+  // when the lists of inputs changes, even if the inputs are not on the command line.
+  optional string input_hash = 5;
+}
+
+// Copy describes a from-to pair of files to copy.  The paths may be relative, the root that they
+// are relative to is specific to the context the Copy is used in and will be different for
+// from and to.
+message Copy {
+  required string from = 1;
+  required string to = 2;
+}
\ No newline at end of file
diff --git a/cmd/soong_build/Android.bp b/cmd/soong_build/Android.bp
index 680c00a..441ea0d 100644
--- a/cmd/soong_build/Android.bp
+++ b/cmd/soong_build/Android.bp
@@ -27,6 +27,7 @@
         "main.go",
         "writedocs.go",
         "queryview.go",
+        "queryview_templates.go",
     ],
     testSrcs: [
         "queryview_test.go",
diff --git a/cmd/soong_build/queryview.go b/cmd/soong_build/queryview.go
index 27856b5..f5aa685 100644
--- a/cmd/soong_build/queryview.go
+++ b/cmd/soong_build/queryview.go
@@ -28,115 +28,6 @@
 	"github.com/google/blueprint/proptools"
 )
 
-const (
-	// The default `load` preamble for every generated BUILD file.
-	soongModuleLoad = `package(default_visibility = ["//visibility:public"])
-load("//build/bazel/queryview_rules:soong_module.bzl", "soong_module")
-
-`
-
-	// A macro call in the BUILD file representing a Soong module, with space
-	// for expanding more attributes.
-	soongModuleTarget = `soong_module(
-    name = "%s",
-    module_name = "%s",
-    module_type = "%s",
-    module_variant = "%s",
-    module_deps = %s,
-%s)`
-
-	// A simple provider to mark and differentiate Soong module rule shims from
-	// regular Bazel rules. Every Soong module rule shim returns a
-	// SoongModuleInfo provider, and can only depend on rules returning
-	// SoongModuleInfo in the `module_deps` attribute.
-	providersBzl = `SoongModuleInfo = provider(
-    fields = {
-        "name": "Name of module",
-        "type": "Type of module",
-        "variant": "Variant of module",
-    },
-)
-`
-
-	// The soong_module rule implementation in a .bzl file.
-	soongModuleBzl = `
-%s
-
-load("//build/bazel/queryview_rules:providers.bzl", "SoongModuleInfo")
-
-def _generic_soong_module_impl(ctx):
-    return [
-        SoongModuleInfo(
-            name = ctx.attr.module_name,
-            type = ctx.attr.module_type,
-            variant = ctx.attr.module_variant,
-        ),
-    ]
-
-generic_soong_module = rule(
-    implementation = _generic_soong_module_impl,
-    attrs = {
-        "module_name": attr.string(mandatory = True),
-        "module_type": attr.string(mandatory = True),
-        "module_variant": attr.string(),
-        "module_deps": attr.label_list(providers = [SoongModuleInfo]),
-    },
-)
-
-soong_module_rule_map = {
-%s}
-
-_SUPPORTED_TYPES = ["bool", "int", "string"]
-
-def _is_supported_type(value):
-    if type(value) in _SUPPORTED_TYPES:
-        return True
-    elif type(value) == "list":
-        supported = True
-        for v in value:
-            supported = supported and type(v) in _SUPPORTED_TYPES
-        return supported
-    else:
-        return False
-
-# soong_module is a macro that supports arbitrary kwargs, and uses module_type to
-# expand to the right underlying shim.
-def soong_module(name, module_type, **kwargs):
-    soong_module_rule = soong_module_rule_map.get(module_type)
-
-    if soong_module_rule == None:
-        # This module type does not have an existing rule to map to, so use the
-        # generic_soong_module rule instead.
-        generic_soong_module(
-            name = name,
-            module_type = module_type,
-            module_name = kwargs.pop("module_name", ""),
-            module_variant = kwargs.pop("module_variant", ""),
-            module_deps = kwargs.pop("module_deps", []),
-        )
-    else:
-        supported_kwargs = dict()
-        for key, value in kwargs.items():
-            if _is_supported_type(value):
-                supported_kwargs[key] = value
-        soong_module_rule(
-            name = name,
-            **supported_kwargs,
-        )
-`
-
-	// A rule shim for representing a Soong module type and its properties.
-	moduleRuleShim = `
-def _%[1]s_impl(ctx):
-    return [SoongModuleInfo()]
-
-%[1]s = rule(
-    implementation = _%[1]s_impl,
-    attrs = %[2]s
-)
-`
-)
-
 var (
 	// An allowlist of prop types that are surfaced from module props to rule
 	// attributes. (nested) dictionaries are notably absent here, because while
diff --git a/cmd/soong_build/queryview_templates.go b/cmd/soong_build/queryview_templates.go
new file mode 100644
index 0000000..359c0d8
--- /dev/null
+++ b/cmd/soong_build/queryview_templates.go
@@ -0,0 +1,124 @@
+// Copyright 2020 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+const (
+	// The default `load` preamble for every generated BUILD file.
+	soongModuleLoad = `package(default_visibility = ["//visibility:public"])
+load("//build/bazel/queryview_rules:soong_module.bzl", "soong_module")
+
+`
+
+	// A macro call in the BUILD file representing a Soong module, with space
+	// for expanding more attributes.
+	soongModuleTarget = `soong_module(
+    name = "%s",
+    module_name = "%s",
+    module_type = "%s",
+    module_variant = "%s",
+    module_deps = %s,
+%s)`
+
+	// A simple provider to mark and differentiate Soong module rule shims from
+	// regular Bazel rules. Every Soong module rule shim returns a
+	// SoongModuleInfo provider, and can only depend on rules returning
+	// SoongModuleInfo in the `module_deps` attribute.
+	providersBzl = `SoongModuleInfo = provider(
+    fields = {
+        "name": "Name of module",
+        "type": "Type of module",
+        "variant": "Variant of module",
+    },
+)
+`
+
+	// The soong_module rule implementation in a .bzl file.
+	soongModuleBzl = `
+%s
+
+load("//build/bazel/queryview_rules:providers.bzl", "SoongModuleInfo")
+
+def _generic_soong_module_impl(ctx):
+    return [
+        SoongModuleInfo(
+            name = ctx.attr.module_name,
+            type = ctx.attr.module_type,
+            variant = ctx.attr.module_variant,
+        ),
+    ]
+
+generic_soong_module = rule(
+    implementation = _generic_soong_module_impl,
+    attrs = {
+        "module_name": attr.string(mandatory = True),
+        "module_type": attr.string(mandatory = True),
+        "module_variant": attr.string(),
+        "module_deps": attr.label_list(providers = [SoongModuleInfo]),
+    },
+)
+
+soong_module_rule_map = {
+%s}
+
+_SUPPORTED_TYPES = ["bool", "int", "string"]
+
+def _is_supported_type(value):
+    if type(value) in _SUPPORTED_TYPES:
+        return True
+    elif type(value) == "list":
+        supported = True
+        for v in value:
+            supported = supported and type(v) in _SUPPORTED_TYPES
+        return supported
+    else:
+        return False
+
+# soong_module is a macro that supports arbitrary kwargs, and uses module_type to
+# expand to the right underlying shim.
+def soong_module(name, module_type, **kwargs):
+    soong_module_rule = soong_module_rule_map.get(module_type)
+
+    if soong_module_rule == None:
+        # This module type does not have an existing rule to map to, so use the
+        # generic_soong_module rule instead.
+        generic_soong_module(
+            name = name,
+            module_type = module_type,
+            module_name = kwargs.pop("module_name", ""),
+            module_variant = kwargs.pop("module_variant", ""),
+            module_deps = kwargs.pop("module_deps", []),
+        )
+    else:
+        supported_kwargs = dict()
+        for key, value in kwargs.items():
+            if _is_supported_type(value):
+                supported_kwargs[key] = value
+        soong_module_rule(
+            name = name,
+            **supported_kwargs,
+        )
+`
+
+	// A rule shim for representing a Soong module type and its properties.
+	moduleRuleShim = `
+def _%[1]s_impl(ctx):
+    return [SoongModuleInfo()]
+
+%[1]s = rule(
+    implementation = _%[1]s_impl,
+    attrs = %[2]s
+)
+`
+)
diff --git a/genrule/Android.bp b/genrule/Android.bp
index ff543a6..0e27d4e 100644
--- a/genrule/Android.bp
+++ b/genrule/Android.bp
@@ -4,8 +4,10 @@
     deps: [
         "blueprint",
         "blueprint-pathtools",
+        "sbox_proto",
         "soong",
         "soong-android",
+        "soong-bazel",
         "soong-shared",
     ],
     srcs: [
diff --git a/genrule/genrule.go b/genrule/genrule.go
index f85146c..cd7c52f 100644
--- a/genrule/genrule.go
+++ b/genrule/genrule.go
@@ -26,13 +26,14 @@
 	"github.com/google/blueprint/proptools"
 
 	"android/soong/android"
+	"android/soong/bazel"
 )
 
 func init() {
-	registerGenruleBuildComponents(android.InitRegistrationContext)
+	RegisterGenruleBuildComponents(android.InitRegistrationContext)
 }
 
-func registerGenruleBuildComponents(ctx android.RegistrationContext) {
+func RegisterGenruleBuildComponents(ctx android.RegistrationContext) {
 	ctx.RegisterModuleType("genrule_defaults", defaultsFactory)
 
 	ctx.RegisterModuleType("gensrcs", GenSrcsFactory)
@@ -78,13 +79,6 @@
 	blueprint.BaseDependencyTag
 	label string
 }
-
-// TODO(cparsons): Move to a common location when there is more than just
-// genrule with a bazel_module property.
-type bazelModuleProperties struct {
-	Label string
-}
-
 type generatorProperties struct {
 	// The command to run on one or more input files. Cmd supports substitution of a few variables
 	//
@@ -118,8 +112,8 @@
 	// input files to exclude
 	Exclude_srcs []string `android:"path,arch_variant"`
 
-	// in bazel-enabled mode, the bazel label to evaluate instead of this module
-	Bazel_module bazelModuleProperties
+	// Properties for Bazel migration purposes.
+	bazel.Properties
 }
 type Module struct {
 	android.ModuleBase
@@ -206,6 +200,7 @@
 	}
 	return ok
 }
+
 func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	g.subName = ctx.ModuleSubDir()
 
@@ -417,18 +412,23 @@
 		}
 		g.rawCommands = append(g.rawCommands, rawCommand)
 
-		// Pick a unique rule name and the user-visible description.
+		// Pick a unique path outside the task.genDir for the sbox manifest textproto,
+		// a unique rule name, and the user-visible description.
+		manifestName := "genrule.sbox.textproto"
 		desc := "generate"
 		name := "generator"
 		if task.shards > 0 {
+			manifestName = "genrule_" + strconv.Itoa(task.shard) + ".sbox.textproto"
 			desc += " " + strconv.Itoa(task.shard)
 			name += strconv.Itoa(task.shard)
 		} else if len(task.out) == 1 {
 			desc += " " + task.out[0].Base()
 		}
 
+		manifestPath := android.PathForModuleOut(ctx, manifestName)
+
 		// Use a RuleBuilder to create a rule that runs the command inside an sbox sandbox.
-		rule := android.NewRuleBuilder().Sbox(task.genDir)
+		rule := android.NewRuleBuilder().Sbox(task.genDir, manifestPath)
 		cmd := rule.Command()
 		cmd.Text(rawCommand)
 		cmd.ImplicitOutputs(task.out)
@@ -461,9 +461,10 @@
 		// 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,
-			Outputs:   outputFiles,
+			Rule:        gensrcsMerge,
+			Implicits:   copyFrom,
+			Outputs:     outputFiles,
+			Description: "merge shards",
 			Args: map[string]string{
 				"zipArgs": zipArgs.String(),
 				"tmpZip":  android.PathForModuleGen(ctx, g.subDir+".zip").String(),
@@ -564,13 +565,19 @@
 func NewGenSrcs() *Module {
 	properties := &genSrcsProperties{}
 
+	// finalSubDir is the name of the subdirectory that output files will be generated into.
+	// It is used so that per-shard directories can be placed alongside it an then finally
+	// merged into it.
+	const finalSubDir = "gensrcs"
+
 	taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask {
-		genDir := android.PathForModuleGen(ctx, "gensrcs")
 		shardSize := defaultShardSize
 		if s := properties.Shard_size; s != nil {
 			shardSize = int(*s)
 		}
 
+		// gensrcs rules can easily hit command line limits by repeating the command for
+		// every input file.  Shard the input files into groups.
 		shards := android.ShardPaths(srcFiles, shardSize)
 		var generateTasks []generateTask
 
@@ -578,35 +585,45 @@
 			var commands []string
 			var outFiles android.WritablePaths
 			var copyTo android.WritablePaths
-			var shardDir android.WritablePath
 			var depFile android.WritablePath
 
+			// When sharding is enabled (i.e. len(shards) > 1), the sbox rules for each
+			// shard will be write to their own directories and then be merged together
+			// into finalSubDir.  If sharding is not enabled (i.e. len(shards) == 1),
+			// the sbox rule will write directly to finalSubDir.
+			genSubDir := finalSubDir
 			if len(shards) > 1 {
-				shardDir = android.PathForModuleGen(ctx, strconv.Itoa(i))
-			} else {
-				shardDir = genDir
+				genSubDir = strconv.Itoa(i)
 			}
 
-			for j, in := range shard {
-				outFile := android.GenPathWithExt(ctx, "gensrcs", in, String(properties.Output_extension))
-				if j == 0 {
-					depFile = outFile.ReplaceExtension(ctx, "d")
-				}
+			genDir := android.PathForModuleGen(ctx, genSubDir)
 
+			for j, in := range shard {
+				outFile := android.GenPathWithExt(ctx, finalSubDir, in, String(properties.Output_extension))
+
+				// If sharding is enabled, then outFile is the path to the output file in
+				// the shard directory, and copyTo is the path to the output file in the
+				// final directory.
 				if len(shards) > 1 {
-					shardFile := android.GenPathWithExt(ctx, strconv.Itoa(i), in, String(properties.Output_extension))
+					shardFile := android.GenPathWithExt(ctx, genSubDir, in, String(properties.Output_extension))
 					copyTo = append(copyTo, outFile)
 					outFile = shardFile
 				}
 
+				if j == 0 {
+					depFile = outFile.ReplaceExtension(ctx, "d")
+				}
+
 				outFiles = append(outFiles, outFile)
 
+				// pre-expand the command line to replace $in and $out with references to
+				// a single input and output file.
 				command, err := android.Expand(rawCommand, func(name string) (string, error) {
 					switch name {
 					case "in":
 						return in.String(), nil
 					case "out":
-						return android.SboxPathForOutput(outFile, shardDir), nil
+						return android.SboxPathForOutput(outFile, genDir), nil
 					default:
 						return "$(" + name + ")", nil
 					}
@@ -626,7 +643,7 @@
 				out:     outFiles,
 				depFile: depFile,
 				copyTo:  copyTo,
-				genDir:  shardDir,
+				genDir:  genDir,
 				cmd:     fullCommand,
 				shard:   i,
 				shards:  len(shards),
@@ -637,7 +654,7 @@
 	}
 
 	g := generatorFactory(taskGenerator, properties)
-	g.subDir = "gensrcs"
+	g.subDir = finalSubDir
 	return g
 }
 
diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go
index 6779c73..8d3cfcb 100644
--- a/genrule/genrule_test.go
+++ b/genrule/genrule_test.go
@@ -57,7 +57,7 @@
 	ctx.RegisterModuleType("filegroup", android.FileGroupFactory)
 	ctx.RegisterModuleType("tool", toolFactory)
 
-	registerGenruleBuildComponents(ctx)
+	RegisterGenruleBuildComponents(ctx)
 
 	ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators)
 	ctx.Register()
@@ -141,7 +141,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_DIR__/out",
+			expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "empty location tool2",
@@ -150,7 +150,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_DIR__/out",
+			expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "empty location tool file",
@@ -159,7 +159,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
+			expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "empty location tool file fg",
@@ -168,7 +168,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
+			expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "empty location tool and tool file",
@@ -178,7 +178,7 @@
 				out: ["out"],
 				cmd: "$(location) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_DIR__/out",
+			expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "tool",
@@ -187,7 +187,7 @@
 				out: ["out"],
 				cmd: "$(location tool) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_DIR__/out",
+			expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "tool2",
@@ -196,7 +196,7 @@
 				out: ["out"],
 				cmd: "$(location :tool) > $(out)",
 			`,
-			expect: "out/tool > __SBOX_OUT_DIR__/out",
+			expect: "out/tool > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "tool file",
@@ -205,7 +205,7 @@
 				out: ["out"],
 				cmd: "$(location tool_file1) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
+			expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "tool file fg",
@@ -214,7 +214,7 @@
 				out: ["out"],
 				cmd: "$(location :1tool_file) > $(out)",
 			`,
-			expect: "tool_file1 > __SBOX_OUT_DIR__/out",
+			expect: "tool_file1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "tool files",
@@ -223,7 +223,7 @@
 				out: ["out"],
 				cmd: "$(locations :tool_files) > $(out)",
 			`,
-			expect: "tool_file1 tool_file2 > __SBOX_OUT_DIR__/out",
+			expect: "tool_file1 tool_file2 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "in1",
@@ -232,7 +232,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "in1 fg",
@@ -241,7 +241,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "ins",
@@ -250,7 +250,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat in1 in2 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 in2 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "ins fg",
@@ -259,7 +259,7 @@
 				out: ["out"],
 				cmd: "cat $(in) > $(out)",
 			`,
-			expect: "cat in1 in2 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 in2 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "location in1",
@@ -268,7 +268,7 @@
 				out: ["out"],
 				cmd: "cat $(location in1) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "location in1 fg",
@@ -277,7 +277,7 @@
 				out: ["out"],
 				cmd: "cat $(location :1in) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "location ins",
@@ -286,7 +286,7 @@
 				out: ["out"],
 				cmd: "cat $(location in1) > $(out)",
 			`,
-			expect: "cat in1 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "location ins fg",
@@ -295,7 +295,7 @@
 				out: ["out"],
 				cmd: "cat $(locations :ins) > $(out)",
 			`,
-			expect: "cat in1 in2 > __SBOX_OUT_DIR__/out",
+			expect: "cat in1 in2 > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "outs",
@@ -303,7 +303,7 @@
 				out: ["out", "out2"],
 				cmd: "echo foo > $(out)",
 			`,
-			expect: "echo foo > __SBOX_OUT_DIR__/out __SBOX_OUT_DIR__/out2",
+			expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out __SBOX_SANDBOX_DIR__/out/out2",
 		},
 		{
 			name: "location out",
@@ -311,7 +311,7 @@
 				out: ["out", "out2"],
 				cmd: "echo foo > $(location out2)",
 			`,
-			expect: "echo foo > __SBOX_OUT_DIR__/out2",
+			expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out2",
 		},
 		{
 			name: "depfile",
@@ -320,7 +320,7 @@
 				depfile: true,
 				cmd: "echo foo > $(out) && touch $(depfile)",
 			`,
-			expect: "echo foo > __SBOX_OUT_DIR__/out && touch __SBOX_DEPFILE__",
+			expect: "echo foo > __SBOX_SANDBOX_DIR__/out/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_DIR__/out",
+			expect: "echo foo > __SBOX_SANDBOX_DIR__/out/foo && cp __SBOX_SANDBOX_DIR__/out/foo __SBOX_SANDBOX_DIR__/out/out",
 		},
 
 		{
@@ -443,7 +443,7 @@
 
 			allowMissingDependencies: true,
 
-			expect: "cat ***missing srcs :missing*** > __SBOX_OUT_DIR__/out",
+			expect: "cat ***missing srcs :missing*** > __SBOX_SANDBOX_DIR__/out/out",
 		},
 		{
 			name: "tool allow missing dependencies",
@@ -455,7 +455,7 @@
 
 			allowMissingDependencies: true,
 
-			expect: "***missing tool :missing*** > __SBOX_OUT_DIR__/out",
+			expect: "***missing tool :missing*** > __SBOX_SANDBOX_DIR__/out/out",
 		},
 	}
 
@@ -570,20 +570,11 @@
 	for _, test := range testcases {
 		t.Run(test.name, func(t *testing.T) {
 			gen := ctx.ModuleForTests(test.name, "")
-			command := gen.Rule("generator").RuleParams.Command
+			manifest := android.RuleBuilderSboxProtoForTests(t, gen.Output("genrule.sbox.textproto"))
+			hash := manifest.Commands[0].GetInputHash()
 
-			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
-
-				if !strings.Contains(command, expectedInputHashOption) {
-					t.Errorf("Expected command \"%s\" to contain \"%s\"", command, expectedInputHashOption)
-				}
-			} else {
-				if strings.Contains(command, "--input-hash") {
-					t.Errorf("Unexpected \"--input-hash\" found in command: \"%s\"", command)
-				}
+			if g, w := hash, test.expectedHash; g != w {
+				t.Errorf("Expected has %q, got %q", w, g)
 			}
 		})
 	}
@@ -609,7 +600,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_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'",
 			},
 			deps:  []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"},
 			files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"},
@@ -623,8 +614,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_SANDBOX_DIR__/out/in1.h' && bash -c 'out/tool in2.txt > __SBOX_SANDBOX_DIR__/out/in2.h'",
+				"bash -c 'out/tool in3.txt > __SBOX_SANDBOX_DIR__/out/in3.h'",
 			},
 			deps:  []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"},
 			files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"},
@@ -710,7 +701,7 @@
 	}
 	gen := ctx.ModuleForTests("gen", "").Module().(*Module)
 
-	expectedCmd := "cp in1 __SBOX_OUT_DIR__/out"
+	expectedCmd := "cp in1 __SBOX_SANDBOX_DIR__/out/out"
 	if gen.rawCommands[0] != expectedCmd {
 		t.Errorf("Expected cmd: %q, actual: %q", expectedCmd, gen.rawCommands[0])
 	}
diff --git a/java/aar.go b/java/aar.go
index 051715d..799e763 100644
--- a/java/aar.go
+++ b/java/aar.go
@@ -836,8 +836,8 @@
 	return nil
 }
 
-func (d *AARImport) ExportedPlugins() (android.Paths, []string) {
-	return nil, nil
+func (d *AARImport) ExportedPlugins() (android.Paths, []string, bool) {
+	return nil, nil, false
 }
 
 func (a *AARImport) SrcJarArgs() ([]string, android.Paths) {
diff --git a/java/device_host_converter.go b/java/device_host_converter.go
index cd395b1..4914d74 100644
--- a/java/device_host_converter.go
+++ b/java/device_host_converter.go
@@ -167,8 +167,8 @@
 	return nil
 }
 
-func (d *DeviceHostConverter) ExportedPlugins() (android.Paths, []string) {
-	return nil, nil
+func (d *DeviceHostConverter) ExportedPlugins() (android.Paths, []string, bool) {
+	return nil, nil, false
 }
 
 func (d *DeviceHostConverter) SrcJarArgs() ([]string, android.Paths) {
diff --git a/java/java.go b/java/java.go
index 7cf04fa..5012279 100644
--- a/java/java.go
+++ b/java/java.go
@@ -201,7 +201,10 @@
 	// List of modules to use as annotation processors
 	Plugins []string
 
-	// List of modules to export to libraries that directly depend on this library as annotation processors
+	// List of modules to export to libraries that directly depend on this library as annotation
+	// processors.  Note that if the plugins set generates_api: true this will disable the turbine
+	// optimization on modules that depend on this module, which will reduce parallelism and cause
+	// more recompilation.
 	Exported_plugins []string
 
 	// The number of Java source entries each Javac instance can process
@@ -428,6 +431,9 @@
 	// list of plugins that this java module is exporting
 	exportedPluginClasses []string
 
+	// if true, the exported plugins generate API and require disabling turbine.
+	exportedDisableTurbine bool
+
 	// list of source files, collected from srcFiles with unique java and all kt files,
 	// will be used by android.IDEInfo struct
 	expandIDEInfoCompiledSrcs []string
@@ -513,7 +519,7 @@
 	ResourceJars() android.Paths
 	AidlIncludeDirs() android.Paths
 	ClassLoaderContexts() dexpreopt.ClassLoaderContextMap
-	ExportedPlugins() (android.Paths, []string)
+	ExportedPlugins() (android.Paths, []string, bool)
 	SrcJarArgs() ([]string, android.Paths)
 	BaseModuleName() string
 	JacocoReportClassesFile() android.Path
@@ -550,6 +556,14 @@
 	name string
 }
 
+// installDependencyTag is a dependency tag that is annotated to cause the installed files of the
+// dependency to be installed when the parent module is installed.
+type installDependencyTag struct {
+	blueprint.BaseDependencyTag
+	android.InstallAlwaysNeededDependencyTag
+	name string
+}
+
 type usesLibraryDependencyTag struct {
 	dependencyTag
 	sdkVersion int // SDK version in which the library appared as a standalone library.
@@ -584,6 +598,8 @@
 	instrumentationForTag = dependencyTag{name: "instrumentation_for"}
 	extraLintCheckTag     = dependencyTag{name: "extra-lint-check"}
 	jniLibTag             = dependencyTag{name: "jnilib"}
+	jniInstallTag         = installDependencyTag{name: "jni install"}
+	binaryInstallTag      = installDependencyTag{name: "binary install"}
 	usesLibTag            = makeUsesLibraryDependencyTag(dexpreopt.AnySdkVersion)
 	usesLibCompat28Tag    = makeUsesLibraryDependencyTag(28)
 	usesLibCompat29Tag    = makeUsesLibraryDependencyTag(29)
@@ -1049,8 +1065,9 @@
 				// sdk lib names from dependencies are re-exported
 				j.classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), otherName)
 				deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...)
-				pluginJars, pluginClasses := dep.ExportedPlugins()
+				pluginJars, pluginClasses, disableTurbine := dep.ExportedPlugins()
 				addPlugins(&deps, pluginJars, pluginClasses...)
+				deps.disableTurbine = deps.disableTurbine || disableTurbine
 			case java9LibTag:
 				deps.java9Classpath = append(deps.java9Classpath, dep.HeaderJars()...)
 			case staticLibTag:
@@ -1061,8 +1078,12 @@
 				// sdk lib names from dependencies are re-exported
 				j.classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), otherName)
 				deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...)
-				pluginJars, pluginClasses := dep.ExportedPlugins()
+				pluginJars, pluginClasses, disableTurbine := dep.ExportedPlugins()
 				addPlugins(&deps, pluginJars, pluginClasses...)
+				// Turbine doesn't run annotation processors, so any module that uses an
+				// annotation processor that generates API is incompatible with the turbine
+				// optimization.
+				deps.disableTurbine = deps.disableTurbine || disableTurbine
 			case pluginTag:
 				if plugin, ok := dep.(*Plugin); ok {
 					if plugin.pluginProperties.Processor_class != nil {
@@ -1070,6 +1091,9 @@
 					} else {
 						addPlugins(&deps, plugin.ImplementationAndResourcesJars())
 					}
+					// Turbine doesn't run annotation processors, so any module that uses an
+					// annotation processor that generates API is incompatible with the turbine
+					// optimization.
 					deps.disableTurbine = deps.disableTurbine || Bool(plugin.pluginProperties.Generates_api)
 				} else {
 					ctx.PropertyErrorf("plugins", "%q is not a java_plugin module", otherName)
@@ -1082,13 +1106,14 @@
 				}
 			case exportedPluginTag:
 				if plugin, ok := dep.(*Plugin); ok {
-					if plugin.pluginProperties.Generates_api != nil && *plugin.pluginProperties.Generates_api {
-						ctx.PropertyErrorf("exported_plugins", "Cannot export plugins with generates_api = true, found %v", otherName)
-					}
 					j.exportedPluginJars = append(j.exportedPluginJars, plugin.ImplementationAndResourcesJars()...)
 					if plugin.pluginProperties.Processor_class != nil {
 						j.exportedPluginClasses = append(j.exportedPluginClasses, *plugin.pluginProperties.Processor_class)
 					}
+					// Turbine doesn't run annotation processors, so any module that uses an
+					// annotation processor that generates API is incompatible with the turbine
+					// optimization.
+					j.exportedDisableTurbine = Bool(plugin.pluginProperties.Generates_api)
 				} else {
 					ctx.PropertyErrorf("exported_plugins", "%q is not a java_plugin module", otherName)
 				}
@@ -1922,8 +1947,11 @@
 	return j.classLoaderContexts
 }
 
-func (j *Module) ExportedPlugins() (android.Paths, []string) {
-	return j.exportedPluginJars, j.exportedPluginClasses
+// ExportedPlugins returns the list of jars needed to run the exported plugins, the list of
+// classes for the plugins, and a boolean for whether turbine needs to be disabled due to plugins
+// that generate APIs.
+func (j *Module) ExportedPlugins() (android.Paths, []string, bool) {
+	return j.exportedPluginJars, j.exportedPluginClasses, j.exportedDisableTurbine
 }
 
 func (j *Module) SrcJarArgs() ([]string, android.Paths) {
@@ -2568,9 +2596,12 @@
 	if ctx.Arch().ArchType == android.Common {
 		j.deps(ctx)
 	} else {
-		// This dependency ensures the host installation rules will install the jni libraries
-		// when the wrapper is installed.
-		ctx.AddVariationDependencies(nil, jniLibTag, j.binaryProperties.Jni_libs...)
+		// These dependencies ensure the host installation rules will install the jar file and
+		// the jni libraries when the wrapper is installed.
+		ctx.AddVariationDependencies(nil, jniInstallTag, j.binaryProperties.Jni_libs...)
+		ctx.AddVariationDependencies(
+			[]blueprint.Variation{{Mutator: "arch", Variation: android.CommonArch.String()}},
+			binaryInstallTag, ctx.ModuleName())
 	}
 }
 
@@ -2865,8 +2896,8 @@
 	return j.classLoaderContexts
 }
 
-func (j *Import) ExportedPlugins() (android.Paths, []string) {
-	return nil, nil
+func (j *Import) ExportedPlugins() (android.Paths, []string, bool) {
+	return nil, nil, false
 }
 
 func (j *Import) SrcJarArgs() ([]string, android.Paths) {
diff --git a/java/java_test.go b/java/java_test.go
index cf56e66..83db443 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -30,7 +30,6 @@
 	"android/soong/android"
 	"android/soong/cc"
 	"android/soong/dexpreopt"
-	"android/soong/genrule"
 	"android/soong/python"
 )
 
@@ -81,7 +80,6 @@
 	RegisterSystemModulesBuildComponents(ctx)
 	ctx.RegisterModuleType("java_plugin", PluginFactory)
 	ctx.RegisterModuleType("filegroup", android.FileGroupFactory)
-	ctx.RegisterModuleType("genrule", genrule.GenRuleFactory)
 	ctx.RegisterModuleType("python_binary_host", python.PythonBinaryHostFactory)
 	RegisterDocsBuildComponents(ctx)
 	RegisterStubsBuildComponents(ctx)
@@ -315,8 +313,9 @@
 
 func TestExportedPlugins(t *testing.T) {
 	type Result struct {
-		library    string
-		processors string
+		library        string
+		processors     string
+		disableTurbine bool
 	}
 	var tests = []struct {
 		name    string
@@ -375,6 +374,18 @@
 				{library: "foo", processors: "-processor com.android.TestPlugin,com.android.TestPlugin2"},
 			},
 		},
+		{
+			name: "Exports plugin to with generates_api to dependee",
+			extra: `
+				java_library{name: "exports", exported_plugins: ["plugin_generates_api"]}
+				java_library{name: "foo", srcs: ["a.java"], libs: ["exports"]}
+				java_library{name: "bar", srcs: ["a.java"], static_libs: ["exports"]}
+			`,
+			results: []Result{
+				{library: "foo", processors: "-processor com.android.TestPlugin", disableTurbine: true},
+				{library: "bar", processors: "-processor com.android.TestPlugin", disableTurbine: true},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -384,6 +395,11 @@
 					name: "plugin",
 					processor_class: "com.android.TestPlugin",
 				}
+				java_plugin {
+					name: "plugin_generates_api",
+					generates_api: true,
+					processor_class: "com.android.TestPlugin",
+				}
 			`+test.extra)
 
 			for _, want := range test.results {
@@ -391,6 +407,11 @@
 				if javac.Args["processor"] != want.processors {
 					t.Errorf("For library %v, expected %v, found %v", want.library, want.processors, javac.Args["processor"])
 				}
+				turbine := ctx.ModuleForTests(want.library, "android_common").MaybeRule("turbine")
+				disableTurbine := turbine.BuildParams.Rule == nil
+				if disableTurbine != want.disableTurbine {
+					t.Errorf("For library %v, expected disableTurbine %v, found %v", want.library, want.disableTurbine, disableTurbine)
+				}
 			}
 		})
 	}
diff --git a/java/lint.go b/java/lint.go
index 3df582f..11f92e5 100644
--- a/java/lint.go
+++ b/java/lint.go
@@ -447,7 +447,7 @@
 	var outputs []*lintOutputs
 	var dirs []string
 	ctx.VisitAllModules(func(m android.Module) {
-		if ctx.Config().EmbeddedInMake() && !m.ExportedToMake() {
+		if ctx.Config().KatiEnabled() && !m.ExportedToMake() {
 			return
 		}
 
diff --git a/java/sysprop.go b/java/sysprop.go
index 1a70499..e41aef6 100644
--- a/java/sysprop.go
+++ b/java/sysprop.go
@@ -14,6 +14,10 @@
 
 package java
 
+// This file contains a map to redirect dependencies towards sysprop_library. If a sysprop_library
+// is owned by Platform, and the client module links against system API, the public stub of the
+// sysprop_library should be used. The map will contain public stub names of sysprop_libraries.
+
 import (
 	"sync"
 
diff --git a/python/python.go b/python/python.go
index d7b1bba..c27a096 100644
--- a/python/python.go
+++ b/python/python.go
@@ -217,11 +217,17 @@
 	name string
 }
 
+type installDependencyTag struct {
+	blueprint.BaseDependencyTag
+	android.InstallAlwaysNeededDependencyTag
+	name string
+}
+
 var (
 	pythonLibTag         = dependencyTag{name: "pythonLib"}
 	javaDataTag          = dependencyTag{name: "javaData"}
 	launcherTag          = dependencyTag{name: "launcher"}
-	launcherSharedLibTag = dependencyTag{name: "launcherSharedLib"}
+	launcherSharedLibTag = installDependencyTag{name: "launcherSharedLib"}
 	pyIdentifierRegexp   = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_-]*$`)
 	pyExt                = ".py"
 	protoExt             = ".proto"
@@ -336,6 +342,7 @@
 			// cannot read the property at this stage and it will be too late to add
 			// dependencies later.
 			ctx.AddFarVariationDependencies(ctx.Target().Variations(), launcherSharedLibTag, "libsqlite")
+			ctx.AddFarVariationDependencies(ctx.Target().Variations(), launcherSharedLibTag, "libc++")
 
 			if ctx.Target().Os.Bionic() {
 				ctx.AddFarVariationDependencies(ctx.Target().Variations(), launcherSharedLibTag,
diff --git a/rust/rust_test.go b/rust/rust_test.go
index 14bbd0b..187f0b6 100644
--- a/rust/rust_test.go
+++ b/rust/rust_test.go
@@ -286,6 +286,12 @@
 			srcs: ["src/any.h"],
 			out: ["src/any.rs"],
 		}
+		rust_binary_host {
+			name: "any_rust_binary",
+			srcs: [
+				"foo.rs",
+			],
+		}
 		rust_bindgen {
 			name: "libbindings",
 			crate_name: "bindings",
diff --git a/rust/strip.go b/rust/strip.go
index d1bbba6..110e3cc 100644
--- a/rust/strip.go
+++ b/rust/strip.go
@@ -19,11 +19,14 @@
 	"android/soong/cc"
 )
 
-// Stripper encapsulates cc.Stripper.
+// Stripper defines the stripping actions and properties for a module. The Rust
+// implementation reuses the C++ implementation.
 type Stripper struct {
 	cc.Stripper
 }
 
+// StripExecutableOrSharedLib strips a binary or shared library from its debug
+// symbols and other debug information.
 func (s *Stripper) StripExecutableOrSharedLib(ctx ModuleContext, in android.Path, out android.ModuleOutPath) {
 	ccFlags := cc.StripFlags{Toolchain: ctx.RustModule().ccToolchain(ctx)}
 	s.Stripper.StripExecutableOrSharedLib(ctx, in, out, ccFlags)
diff --git a/rust/testing.go b/rust/testing.go
index 66877a9..001f322 100644
--- a/rust/testing.go
+++ b/rust/testing.go
@@ -17,7 +17,6 @@
 import (
 	"android/soong/android"
 	"android/soong/cc"
-	"android/soong/genrule"
 )
 
 func GatherRequiredDepsForTest() string {
@@ -132,7 +131,6 @@
 	android.RegisterPrebuiltMutators(ctx)
 	ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators)
 	cc.RegisterRequiredBuildComponentsForTest(ctx)
-	ctx.RegisterModuleType("genrule", genrule.GenRuleFactory)
 	ctx.RegisterModuleType("rust_binary", RustBinaryFactory)
 	ctx.RegisterModuleType("rust_binary_host", RustBinaryHostFactory)
 	ctx.RegisterModuleType("rust_bindgen", RustBindgenFactory)
diff --git a/scripts/build-aml-prebuilts.sh b/scripts/build-aml-prebuilts.sh
index 89fb1a5..0c868ea 100755
--- a/scripts/build-aml-prebuilts.sh
+++ b/scripts/build-aml-prebuilts.sh
@@ -12,7 +12,7 @@
 
 export OUT_DIR=${OUT_DIR:-out}
 
-if [ -e ${OUT_DIR}/soong/.soong.in_make ]; then
+if [ -e ${OUT_DIR}/soong/.soong.kati_enabled ]; then
   # If ${OUT_DIR} has been created without --skip-make, Soong will create an
   # ${OUT_DIR}/soong/build.ninja that leaves out many targets which are
   # expected to be supplied by the .mk files, and that might cause errors in
@@ -32,8 +32,8 @@
 
 my_get_build_var() {
   # get_build_var will run Soong in normal in-make mode where it creates
-  # .soong.in_make. That would clobber our real out directory, so we need to
-  # run it in a different one.
+  # .soong.kati_enabled. That would clobber our real out directory, so we need
+  # to run it in a different one.
   OUT_DIR=${OUT_DIR}/get_build_var get_build_var "$@"
 }
 
diff --git a/sdk/testing.go b/sdk/testing.go
index 5f520e5..91aa879 100644
--- a/sdk/testing.go
+++ b/sdk/testing.go
@@ -87,7 +87,7 @@
 	// * Configure that we are inside make
 	// * Add CommonOS to ensure that androidmk processing works.
 	android.RegisterAndroidMkBuildComponents(ctx)
-	android.SetInMakeForTests(config)
+	android.SetKatiEnabledForTests(config)
 	config.Targets[android.CommonOS] = []android.Target{
 		{android.CommonOS, android.Arch{ArchType: android.Common}, android.NativeBridgeDisabled, "", "", true},
 	}
diff --git a/sysprop/sysprop_library.go b/sysprop/sysprop_library.go
index 1740ba8..828d1cf 100644
--- a/sysprop/sysprop_library.go
+++ b/sysprop/sysprop_library.go
@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// sysprop package defines a module named sysprop_library that can implement sysprop as API
+// See https://source.android.com/devices/architecture/sysprops-apis for details
 package sysprop
 
 import (
@@ -71,6 +73,8 @@
 	})
 }
 
+// syspropJavaGenRule module generates srcjar containing generated java APIs.
+// It also depends on check api rule, so api check has to pass to use sysprop_library.
 func (g *syspropJavaGenRule) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	var checkApiFileTimeStamp android.WritablePath
 
@@ -170,6 +174,7 @@
 	syspropLibrariesLock sync.Mutex
 )
 
+// List of sysprop_library used by property_contexts to perform type check.
 func syspropLibraries(config android.Config) *[]string {
 	return config.Once(syspropLibrariesKey, func() interface{} {
 		return &[]string{}
@@ -192,7 +197,7 @@
 	return m.properties.Property_owner
 }
 
-func (m *syspropLibrary) CcModuleName() string {
+func (m *syspropLibrary) CcImplementationModuleName() string {
 	return "lib" + m.BaseModuleName()
 }
 
@@ -223,6 +228,8 @@
 	return m.currentApiFile
 }
 
+// GenerateAndroidBuildActions of sysprop_library handles API dump and API check.
+// generated java_library will depend on these API files.
 func (m *syspropLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	baseModuleName := m.BaseModuleName()
 
@@ -251,7 +258,8 @@
 	// check API rule
 	rule = android.NewRuleBuilder()
 
-	// 1. current.txt <-> api_dump.txt
+	// 1. compares current.txt to api-dump.txt
+	// current.txt should be identical to api-dump.txt.
 	msg := fmt.Sprintf(`\n******************************\n`+
 		`API of sysprop_library %s doesn't match with current.txt\n`+
 		`Please update current.txt by:\n`+
@@ -267,7 +275,8 @@
 		Flag(`"` + msg + `"`).
 		Text("; exit 38) )")
 
-	// 2. current.txt <-> latest.txt
+	// 2. compares current.txt to latest.txt (frozen API)
+	// current.txt should be compatible with latest.txt
 	msg = fmt.Sprintf(`\n******************************\n`+
 		`API of sysprop_library %s doesn't match with latest version\n`+
 		`Please fix the breakage and rebuild.\n`+
@@ -410,14 +419,16 @@
 	installedInVendorOrOdm := ctx.SocSpecific() || ctx.DeviceSpecific()
 	installedInProduct := ctx.ProductSpecific()
 	isOwnerPlatform := false
-	var stub string
+	var javaSyspropStub string
 
+	// javaSyspropStub contains stub libraries used by generated APIs, instead of framework stub.
+	// This is to make sysprop_library link against core_current.
 	if installedInVendorOrOdm {
-		stub = "sysprop-library-stub-vendor"
+		javaSyspropStub = "sysprop-library-stub-vendor"
 	} else if installedInProduct {
-		stub = "sysprop-library-stub-product"
+		javaSyspropStub = "sysprop-library-stub-product"
 	} else {
-		stub = "sysprop-library-stub-platform"
+		javaSyspropStub = "sysprop-library-stub-platform"
 	}
 
 	switch m.Owner() {
@@ -441,8 +452,10 @@
 			"Unknown value %s: must be one of Platform, Vendor or Odm", m.Owner())
 	}
 
+	// Generate a C++ implementation library.
+	// cc_library can receive *.sysprop files as their srcs, generating sources itself.
 	ccProps := ccLibraryProperties{}
-	ccProps.Name = proptools.StringPtr(m.CcModuleName())
+	ccProps.Name = proptools.StringPtr(m.CcImplementationModuleName())
 	ccProps.Srcs = m.properties.Srcs
 	ccProps.Soc_specific = proptools.BoolPtr(ctx.SocSpecific())
 	ccProps.Device_specific = proptools.BoolPtr(ctx.DeviceSpecific())
@@ -463,15 +476,17 @@
 
 	// We need to only use public version, if the partition where sysprop_library will be installed
 	// is different from owner.
-
 	if ctx.ProductSpecific() {
-		// Currently product partition can't own any sysprop_library.
+		// Currently product partition can't own any sysprop_library. So product always uses public.
 		scope = "public"
 	} else if isOwnerPlatform && installedInVendorOrOdm {
 		// Vendor or Odm should use public version of Platform's sysprop_library.
 		scope = "public"
 	}
 
+	// Generate a Java implementation library.
+	// Contrast to C++, syspropJavaGenRule module will generate srcjar and the srcjar will be fed
+	// to Java implementation library.
 	ctx.CreateModule(syspropJavaGenFactory, &syspropGenProperties{
 		Srcs:  m.properties.Srcs,
 		Scope: scope,
@@ -486,7 +501,7 @@
 		Product_specific: proptools.BoolPtr(ctx.ProductSpecific()),
 		Installable:      m.properties.Installable,
 		Sdk_version:      proptools.StringPtr("core_current"),
-		Libs:             []string{stub},
+		Libs:             []string{javaSyspropStub},
 	})
 
 	// if platform sysprop_library is installed in /system or /system-ext, we regard it as an API
@@ -505,11 +520,13 @@
 			Srcs:        []string{":" + m.javaGenPublicStubName()},
 			Installable: proptools.BoolPtr(false),
 			Sdk_version: proptools.StringPtr("core_current"),
-			Libs:        []string{stub},
+			Libs:        []string{javaSyspropStub},
 			Stem:        proptools.StringPtr(m.BaseModuleName()),
 		})
 	}
 
+	// syspropLibraries will be used by property_contexts to check types.
+	// Record absolute paths of sysprop_library to prevent soong_namespace problem.
 	if m.ExportedToMake() {
 		syspropLibrariesLock.Lock()
 		defer syspropLibrariesLock.Unlock()
@@ -519,6 +536,8 @@
 	}
 }
 
+// syspropDepsMutator adds dependencies from java implementation library to sysprop library.
+// java implementation library then depends on check API rule of sysprop library.
 func syspropDepsMutator(ctx android.BottomUpMutatorContext) {
 	if m, ok := ctx.Module().(*syspropLibrary); ok {
 		ctx.AddReverseDependency(m, nil, m.javaGenModuleName())
diff --git a/ui/build/build.go b/ui/build/build.go
index 1cf2023..cfd0b83 100644
--- a/ui/build/build.go
+++ b/ui/build/build.go
@@ -23,13 +23,20 @@
 	"android/soong/ui/metrics"
 )
 
-// Ensures the out directory exists, and has the proper files to prevent kati
-// from recursing into it.
+// SetupOutDir ensures the out directory exists, and has the proper files to
+// prevent kati from recursing into it.
 func SetupOutDir(ctx Context, config Config) {
 	ensureEmptyFileExists(ctx, filepath.Join(config.OutDir(), "Android.mk"))
 	ensureEmptyFileExists(ctx, filepath.Join(config.OutDir(), "CleanSpec.mk"))
 	if !config.SkipMake() {
-		ensureEmptyFileExists(ctx, filepath.Join(config.SoongOutDir(), ".soong.in_make"))
+		// Run soong_build with Kati for a hybrid build, e.g. running the
+		// AndroidMk singleton and postinstall commands. Communicate this to
+		// soong_build by writing an empty .soong.kati_enabled marker file in the
+		// soong_build output directory for the soong_build primary builder to
+		// know if the user wants to run Kati after.
+		//
+		// This does not preclude running Kati for *product configuration purposes*.
+		ensureEmptyFileExists(ctx, filepath.Join(config.SoongOutDir(), ".soong.kati_enabled"))
 	}
 	// The ninja_build file is used by our buildbots to understand that the output
 	// can be parsed as ninja output.
@@ -176,7 +183,7 @@
 		help(ctx, config, what)
 		return
 	} else if inList("clean", config.Arguments()) || inList("clobber", config.Arguments()) {
-		clean(ctx, config, what)
+		clean(ctx, config)
 		return
 	}
 
@@ -211,12 +218,12 @@
 
 	if inList("installclean", config.Arguments()) ||
 		inList("install-clean", config.Arguments()) {
-		installClean(ctx, config, what)
+		installClean(ctx, config)
 		ctx.Println("Deleted images and staging directories.")
 		return
 	} else if inList("dataclean", config.Arguments()) ||
 		inList("data-clean", config.Arguments()) {
-		dataClean(ctx, config, what)
+		dataClean(ctx, config)
 		ctx.Println("Deleted data files.")
 		return
 	}
diff --git a/ui/build/cleanbuild.go b/ui/build/cleanbuild.go
index 03e884a..06f6c63 100644
--- a/ui/build/cleanbuild.go
+++ b/ui/build/cleanbuild.go
@@ -26,8 +26,11 @@
 	"android/soong/ui/metrics"
 )
 
+// Given a series of glob patterns, remove matching files and directories from the filesystem.
+// For example, "malware*" would remove all files and directories in the current directory that begin with "malware".
 func removeGlobs(ctx Context, globs ...string) {
 	for _, glob := range globs {
+		// Find files and directories that match this glob pattern.
 		files, err := filepath.Glob(glob)
 		if err != nil {
 			// Only possible error is ErrBadPattern
@@ -45,13 +48,15 @@
 
 // Remove everything under the out directory. Don't remove the out directory
 // itself in case it's a symlink.
-func clean(ctx Context, config Config, what int) {
+func clean(ctx Context, config Config) {
 	removeGlobs(ctx, filepath.Join(config.OutDir(), "*"))
 	ctx.Println("Entire build directory removed.")
 }
 
-func dataClean(ctx Context, config Config, what int) {
+// Remove everything in the data directory.
+func dataClean(ctx Context, config Config) {
 	removeGlobs(ctx, filepath.Join(config.ProductOut(), "data", "*"))
+	ctx.Println("Entire data directory removed.")
 }
 
 // installClean deletes all of the installed files -- the intent is to remove
@@ -61,8 +66,8 @@
 //
 // This is faster than a full clean, since we're not deleting the
 // intermediates.  Instead of recompiling, we can just copy the results.
-func installClean(ctx Context, config Config, what int) {
-	dataClean(ctx, config, what)
+func installClean(ctx Context, config Config) {
+	dataClean(ctx, config)
 
 	if hostCrossOutPath := config.hostCrossOut(); hostCrossOutPath != "" {
 		hostCrossOut := func(path string) string {
@@ -145,85 +150,95 @@
 // Since products and build variants (unfortunately) shared the same
 // PRODUCT_OUT staging directory, things can get out of sync if different
 // build configurations are built in the same tree. This function will
-// notice when the configuration has changed and call installclean to
+// notice when the configuration has changed and call installClean to
 // remove the files necessary to keep things consistent.
 func installCleanIfNecessary(ctx Context, config Config) {
 	configFile := config.DevicePreviousProductConfig()
 	prefix := "PREVIOUS_BUILD_CONFIG := "
 	suffix := "\n"
-	currentProduct := prefix + config.TargetProduct() + "-" + config.TargetBuildVariant() + suffix
+	currentConfig := prefix + config.TargetProduct() + "-" + config.TargetBuildVariant() + suffix
 
 	ensureDirectoriesExist(ctx, filepath.Dir(configFile))
 
 	writeConfig := func() {
-		err := ioutil.WriteFile(configFile, []byte(currentProduct), 0666)
+		err := ioutil.WriteFile(configFile, []byte(currentConfig), 0666) // a+rw
 		if err != nil {
 			ctx.Fatalln("Failed to write product config:", err)
 		}
 	}
 
-	prev, err := ioutil.ReadFile(configFile)
+	previousConfigBytes, err := ioutil.ReadFile(configFile)
 	if err != nil {
 		if os.IsNotExist(err) {
+			// Just write the new config file, no old config file to worry about.
 			writeConfig()
 			return
 		} else {
 			ctx.Fatalln("Failed to read previous product config:", err)
 		}
-	} else if string(prev) == currentProduct {
+	}
+
+	previousConfig := string(previousConfigBytes)
+	if previousConfig == currentConfig {
+		// Same config as before - nothing to clean.
 		return
 	}
 
-	if disable, _ := config.Environment().Get("DISABLE_AUTO_INSTALLCLEAN"); disable == "true" {
-		ctx.Println("DISABLE_AUTO_INSTALLCLEAN is set; skipping auto-clean. Your tree may be in an inconsistent state.")
+	if config.Environment().IsEnvTrue("DISABLE_AUTO_INSTALLCLEAN") {
+		ctx.Println("DISABLE_AUTO_INSTALLCLEAN is set and true; skipping auto-clean. Your tree may be in an inconsistent state.")
 		return
 	}
 
 	ctx.BeginTrace(metrics.PrimaryNinja, "installclean")
 	defer ctx.EndTrace()
 
-	prevConfig := strings.TrimPrefix(strings.TrimSuffix(string(prev), suffix), prefix)
-	currentConfig := strings.TrimPrefix(strings.TrimSuffix(currentProduct, suffix), prefix)
+	previousProductAndVariant := strings.TrimPrefix(strings.TrimSuffix(previousConfig, suffix), prefix)
+	currentProductAndVariant := strings.TrimPrefix(strings.TrimSuffix(currentConfig, suffix), prefix)
 
-	ctx.Printf("Build configuration changed: %q -> %q, forcing installclean\n", prevConfig, currentConfig)
+	ctx.Printf("Build configuration changed: %q -> %q, forcing installclean\n", previousProductAndVariant, currentProductAndVariant)
 
-	installClean(ctx, config, 0)
+	installClean(ctx, config)
 
 	writeConfig()
 }
 
 // cleanOldFiles takes an input file (with all paths relative to basePath), and removes files from
 // the filesystem if they were removed from the input file since the last execution.
-func cleanOldFiles(ctx Context, basePath, file string) {
-	file = filepath.Join(basePath, file)
-	oldFile := file + ".previous"
+func cleanOldFiles(ctx Context, basePath, newFile string) {
+	newFile = filepath.Join(basePath, newFile)
+	oldFile := newFile + ".previous"
 
-	if _, err := os.Stat(file); err != nil {
-		ctx.Fatalf("Expected %q to be readable", file)
+	if _, err := os.Stat(newFile); err != nil {
+		ctx.Fatalf("Expected %q to be readable", newFile)
 	}
 
 	if _, err := os.Stat(oldFile); os.IsNotExist(err) {
-		if err := os.Rename(file, oldFile); err != nil {
-			ctx.Fatalf("Failed to rename file list (%q->%q): %v", file, oldFile, err)
+		if err := os.Rename(newFile, oldFile); err != nil {
+			ctx.Fatalf("Failed to rename file list (%q->%q): %v", newFile, oldFile, err)
 		}
 		return
 	}
 
-	var newPaths, oldPaths []string
-	if newData, err := ioutil.ReadFile(file); err == nil {
-		if oldData, err := ioutil.ReadFile(oldFile); err == nil {
-			// Common case: nothing has changed
-			if bytes.Equal(newData, oldData) {
-				return
-			}
-			newPaths = strings.Fields(string(newData))
-			oldPaths = strings.Fields(string(oldData))
-		} else {
-			ctx.Fatalf("Failed to read list of installable files (%q): %v", oldFile, err)
-		}
+	var newData, oldData []byte
+	if data, err := ioutil.ReadFile(newFile); err == nil {
+		newData = data
 	} else {
-		ctx.Fatalf("Failed to read list of installable files (%q): %v", file, err)
+		ctx.Fatalf("Failed to read list of installable files (%q): %v", newFile, err)
 	}
+	if data, err := ioutil.ReadFile(oldFile); err == nil {
+		oldData = data
+	} else {
+		ctx.Fatalf("Failed to read list of installable files (%q): %v", oldFile, err)
+	}
+
+	// Common case: nothing has changed
+	if bytes.Equal(newData, oldData) {
+		return
+	}
+
+	var newPaths, oldPaths []string
+	newPaths = strings.Fields(string(newData))
+	oldPaths = strings.Fields(string(oldData))
 
 	// These should be mostly sorted by make already, but better make sure Go concurs
 	sort.Strings(newPaths)
@@ -242,42 +257,55 @@
 				continue
 			}
 		}
+
 		// File only exists in the old list; remove if it exists
-		old := filepath.Join(basePath, oldPaths[0])
+		oldPath := filepath.Join(basePath, oldPaths[0])
 		oldPaths = oldPaths[1:]
-		if fi, err := os.Stat(old); err == nil {
-			if fi.IsDir() {
-				if err := os.Remove(old); err == nil {
-					ctx.Println("Removed directory that is no longer installed: ", old)
-					cleanEmptyDirs(ctx, filepath.Dir(old))
+
+		if oldFile, err := os.Stat(oldPath); err == nil {
+			if oldFile.IsDir() {
+				if err := os.Remove(oldPath); err == nil {
+					ctx.Println("Removed directory that is no longer installed: ", oldPath)
+					cleanEmptyDirs(ctx, filepath.Dir(oldPath))
 				} else {
-					ctx.Println("Failed to remove directory that is no longer installed (%q): %v", old, err)
+					ctx.Println("Failed to remove directory that is no longer installed (%q): %v", oldPath, err)
 					ctx.Println("It's recommended to run `m installclean`")
 				}
 			} else {
-				if err := os.Remove(old); err == nil {
-					ctx.Println("Removed file that is no longer installed: ", old)
-					cleanEmptyDirs(ctx, filepath.Dir(old))
+				// Removing a file, not a directory.
+				if err := os.Remove(oldPath); err == nil {
+					ctx.Println("Removed file that is no longer installed: ", oldPath)
+					cleanEmptyDirs(ctx, filepath.Dir(oldPath))
 				} else if !os.IsNotExist(err) {
-					ctx.Fatalf("Failed to remove file that is no longer installed (%q): %v", old, err)
+					ctx.Fatalf("Failed to remove file that is no longer installed (%q): %v", oldPath, err)
 				}
 			}
 		}
 	}
 
 	// Use the new list as the base for the next build
-	os.Rename(file, oldFile)
+	os.Rename(newFile, oldFile)
 }
 
+// cleanEmptyDirs will delete a directory if it contains no files.
+// If a deletion occurs, then it also recurses upwards to try and delete empty parent directories.
 func cleanEmptyDirs(ctx Context, dir string) {
 	files, err := ioutil.ReadDir(dir)
-	if err != nil || len(files) > 0 {
+	if err != nil {
+		ctx.Println("Could not read directory while trying to clean empty dirs: ", dir)
 		return
 	}
-	if err := os.Remove(dir); err == nil {
-		ctx.Println("Removed directory that is no longer installed: ", dir)
-	} else {
-		ctx.Fatalf("Failed to remove directory that is no longer installed (%q): %v", dir, err)
+	if len(files) > 0 {
+		// Directory is not empty.
+		return
 	}
+
+	if err := os.Remove(dir); err == nil {
+		ctx.Println("Removed empty directory (may no longer be installed?): ", dir)
+	} else {
+		ctx.Fatalf("Failed to remove empty directory (which may no longer be installed?) %q: (%v)", dir, err)
+	}
+
+	// Try and delete empty parent directories too.
 	cleanEmptyDirs(ctx, filepath.Dir(dir))
 }
diff --git a/ui/build/environment.go b/ui/build/environment.go
index 9bca7c0..6d8a28f 100644
--- a/ui/build/environment.go
+++ b/ui/build/environment.go
@@ -37,8 +37,8 @@
 // It's equivalent to the os.LookupEnv function, but with this copy of the
 // Environment.
 func (e *Environment) Get(key string) (string, bool) {
-	for _, env := range *e {
-		if k, v, ok := decodeKeyValue(env); ok && k == key {
+	for _, envVar := range *e {
+		if k, v, ok := decodeKeyValue(envVar); ok && k == key {
 			return v, true
 		}
 	}
@@ -65,37 +65,40 @@
 
 // Unset removes the specified keys from the Environment.
 func (e *Environment) Unset(keys ...string) {
-	out := (*e)[:0]
-	for _, env := range *e {
-		if key, _, ok := decodeKeyValue(env); ok && inList(key, keys) {
+	newEnv := (*e)[:0]
+	for _, envVar := range *e {
+		if key, _, ok := decodeKeyValue(envVar); ok && inList(key, keys) {
+			// Delete this key.
 			continue
 		}
-		out = append(out, env)
+		newEnv = append(newEnv, envVar)
 	}
-	*e = out
+	*e = newEnv
 }
 
 // UnsetWithPrefix removes all keys that start with prefix.
 func (e *Environment) UnsetWithPrefix(prefix string) {
-	out := (*e)[:0]
-	for _, env := range *e {
-		if key, _, ok := decodeKeyValue(env); ok && strings.HasPrefix(key, prefix) {
+	newEnv := (*e)[:0]
+	for _, envVar := range *e {
+		if key, _, ok := decodeKeyValue(envVar); ok && strings.HasPrefix(key, prefix) {
+			// Delete this key.
 			continue
 		}
-		out = append(out, env)
+		newEnv = append(newEnv, envVar)
 	}
-	*e = out
+	*e = newEnv
 }
 
 // Allow removes all keys that are not present in the input list
 func (e *Environment) Allow(keys ...string) {
-	out := (*e)[:0]
-	for _, env := range *e {
-		if key, _, ok := decodeKeyValue(env); ok && inList(key, keys) {
-			out = append(out, env)
+	newEnv := (*e)[:0]
+	for _, envVar := range *e {
+		if key, _, ok := decodeKeyValue(envVar); ok && inList(key, keys) {
+			// Keep this key.
+			newEnv = append(newEnv, envVar)
 		}
 	}
-	*e = out
+	*e = newEnv
 }
 
 // Environ returns the []string required for exec.Cmd.Env
@@ -105,11 +108,11 @@
 
 // Copy returns a copy of the Environment so that independent changes may be made.
 func (e *Environment) Copy() *Environment {
-	ret := Environment(make([]string, len(*e)))
-	for i, v := range *e {
-		ret[i] = v
+	envCopy := Environment(make([]string, len(*e)))
+	for i, envVar := range *e {
+		envCopy[i] = envVar
 	}
-	return &ret
+	return &envCopy
 }
 
 // IsTrue returns whether an environment variable is set to a positive value (1,y,yes,on,true)
@@ -140,15 +143,20 @@
 	return e.appendFromKati(file)
 }
 
+// Helper function for AppendFromKati. Accepts an io.Reader to make testing easier.
 func (e *Environment) appendFromKati(reader io.Reader) error {
 	scanner := bufio.NewScanner(reader)
 	for scanner.Scan() {
 		text := strings.TrimSpace(scanner.Text())
 
 		if len(text) == 0 || text[0] == '#' {
+			// Skip blank lines and comments.
 			continue
 		}
 
+		// We expect two space-delimited strings, like:
+		// unset 'HOME'
+		// export 'BEST_PIZZA_CITY'='NYC'
 		cmd := strings.SplitN(text, " ", 2)
 		if len(cmd) != 2 {
 			return fmt.Errorf("Unknown kati environment line: %q", text)
@@ -159,6 +167,8 @@
 			if !ok {
 				return fmt.Errorf("Failed to unquote kati line: %q", text)
 			}
+
+			// Actually unset it.
 			e.Unset(str)
 		} else if cmd[0] == "export" {
 			key, value, ok := decodeKeyValue(cmd[1])
@@ -175,6 +185,7 @@
 				return fmt.Errorf("Failed to unquote kati line: %q", text)
 			}
 
+			// Actually set it.
 			e.Set(key, value)
 		} else {
 			return fmt.Errorf("Unknown kati environment command: %q", text)