Merge changes I0f2f0e41,Ib034140c,I4b2ca283

* changes:
  Add java_data parameter to python modules
  Use local variations for python version splits
  Support SourceFileProducer in android.OutputFilesForModule
diff --git a/android/apex.go b/android/apex.go
index 276f7a4..a4ff0f9 100644
--- a/android/apex.go
+++ b/android/apex.go
@@ -25,32 +25,54 @@
 )
 
 var (
+	// This is the sdk version when APEX was first introduced
 	SdkVersion_Android10 = uncheckedFinalApiLevel(29)
 )
 
-// ApexInfo describes the metadata common to all modules in an apexBundle.
+// ApexInfo describes the metadata about one or more apexBundles that an apex variant of a module is
+// part of.  When an apex variant is created, the variant is associated with one apexBundle. But
+// when multiple apex variants are merged for deduping (see mergeApexVariations), this holds the
+// information about the apexBundles that are merged together.
+// Accessible via `ctx.Provider(android.ApexInfoProvider).(android.ApexInfo)`
 type ApexInfo struct {
-	// Name of the apex variation that this module is mutated into, or "" for
-	// a platform variant.  Note that a module can be included in multiple APEXes,
-	// in which case, the module is mutated into one or more variants, each of
-	// which is for one or more APEXes.
+	// Name of the apex variation that this module (i.e. the apex variant of the module) is
+	// mutated into, or "" for a platform (i.e. non-APEX) variant. Note that a module can be
+	// included in multiple APEXes, in which case, the module is mutated into one or more
+	// variants, each of which is for an APEX. The variants then can later be deduped if they
+	// don't need to be compiled differently. This is an optimization done in
+	// mergeApexVariations.
 	ApexVariationName string
 
-	// Serialized ApiLevel. Use via MinSdkVersion() method. Cannot be stored in
-	// its struct form because this is cloned into properties structs, and
-	// ApiLevel has private members.
+	// Serialized ApiLevel that this module has to support at minimum. Should be accessed via
+	// MinSdkVersion() method. Cannot be stored in its struct form because this is cloned into
+	// properties structs, and ApiLevel has private members.
 	MinSdkVersionStr string
 
-	// True if the module comes from an updatable APEX.
-	Updatable    bool
+	// True if this module comes from an updatable apexBundle.
+	Updatable bool
+
+	// The list of SDK modules that the containing apexBundle depends on.
 	RequiredSdks SdkRefs
 
-	InApexes     []string
+	// List of apexBundles that this apex variant of the module is associated with. Initially,
+	// the size of this list is one because one apex variant is associated with one apexBundle.
+	// When multiple apex variants are merged in mergeApexVariations, ApexInfo struct of the
+	// merged variant holds the list of apexBundles that are merged together.
+	InApexes []string
+
+	// Pointers to the ApexContents struct each of which is for apexBundle modules that this
+	// module is part of. The ApexContents gives information about which modules the apexBundle
+	// has and whether a module became part of the apexBundle via a direct dependency or not.
 	ApexContents []*ApexContents
 }
 
 var ApexInfoProvider = blueprint.NewMutatorProvider(ApexInfo{}, "apex")
 
+// mergedName gives the name of the alias variation that will be used when multiple apex variations
+// of a module can be deduped into one variation. For example, if libfoo is included in both apex.a
+// and apex.b, and if the two APEXes have the same min_sdk_version (say 29), then libfoo doesn't
+// have to be built twice, but only once. In that case, the two apex variations apex.a and apex.b
+// are configured to have the same alias variation named apex29.
 func (i ApexInfo) mergedName(ctx PathContext) string {
 	name := "apex" + strconv.Itoa(i.MinSdkVersion(ctx).FinalOrFutureInt())
 	for _, sdk := range i.RequiredSdks {
@@ -59,14 +81,19 @@
 	return name
 }
 
-func (this *ApexInfo) MinSdkVersion(ctx PathContext) ApiLevel {
-	return ApiLevelOrPanic(ctx, this.MinSdkVersionStr)
+// MinSdkVersion gives the api level that this module has to support at minimum. This is from the
+// min_sdk_version property of the containing apexBundle.
+func (i ApexInfo) MinSdkVersion(ctx PathContext) ApiLevel {
+	return ApiLevelOrPanic(ctx, i.MinSdkVersionStr)
 }
 
+// IsForPlatform tells whether this module is for the platform or not. If false is returned, it
+// means that this apex variant of the module is built for an APEX.
 func (i ApexInfo) IsForPlatform() bool {
 	return i.ApexVariationName == ""
 }
 
+// InApex tells whether this apex variant of the module is part of the given apexBundle or not.
 func (i ApexInfo) InApex(apex string) bool {
 	for _, a := range i.InApexes {
 		if a == apex {
@@ -76,102 +103,111 @@
 	return false
 }
 
-// ApexTestForInfo stores the contents of APEXes for which this module is a test and thus has
-// access to APEX internals.
+// ApexTestForInfo stores the contents of APEXes for which this module is a test - although this
+// module is not part of the APEX - and thus has access to APEX internals.
 type ApexTestForInfo struct {
 	ApexContents []*ApexContents
 }
 
 var ApexTestForInfoProvider = blueprint.NewMutatorProvider(ApexTestForInfo{}, "apex_test_for")
 
-// Extracted from ApexModule to make it easier to define custom subsets of the
-// ApexModule interface and improve code navigation within the IDE.
+// DepIsInSameApex defines an interface that should be used to determine whether a given dependency
+// should be considered as part of the same APEX as the current module or not. Note: this was
+// extracted from ApexModule to make it easier to define custom subsets of the ApexModule interface
+// and improve code navigation within the IDE.
 type DepIsInSameApex interface {
-	// DepIsInSameApex tests if the other module 'dep' is installed to the same
-	// APEX as this module
+	// DepIsInSameApex tests if the other module 'dep' is considered as part of the same APEX as
+	// this module. For example, a static lib dependency usually returns true here, while a
+	// shared lib dependency to a stub library returns false.
 	DepIsInSameApex(ctx BaseModuleContext, dep Module) bool
 }
 
-// ApexModule is the interface that a module type is expected to implement if
-// the module has to be built differently depending on whether the module
-// is destined for an apex or not (installed to one of the regular partitions).
+// ApexModule is the interface that a module type is expected to implement if the module has to be
+// built differently depending on whether the module is destined for an APEX or not (i.e., installed
+// to one of the regular partitions).
 //
-// Native shared libraries are one such module type; when it is built for an
-// APEX, it should depend only on stable interfaces such as NDK, stable AIDL,
-// or C APIs from other APEXs.
+// Native shared libraries are one such module type; when it is built for an APEX, it should depend
+// only on stable interfaces such as NDK, stable AIDL, or C APIs from other APEXes.
 //
-// A module implementing this interface will be mutated into multiple
-// variations by apex.apexMutator if it is directly or indirectly included
-// in one or more APEXs. Specifically, if a module is included in apex.foo and
-// apex.bar then three apex variants are created: platform, apex.foo and
-// apex.bar. The platform variant is for the regular partitions
-// (e.g., /system or /vendor, etc.) while the other two are for the APEXs,
-// respectively.
+// A module implementing this interface will be mutated into multiple variations by apex.apexMutator
+// if it is directly or indirectly included in one or more APEXes. Specifically, if a module is
+// included in apex.foo and apex.bar then three apex variants are created: platform, apex.foo and
+// apex.bar. The platform variant is for the regular partitions (e.g., /system or /vendor, etc.)
+// while the other two are for the APEXs, respectively. The latter two variations can be merged (see
+// mergedName) when the two APEXes have the same min_sdk_version requirement.
 type ApexModule interface {
 	Module
 	DepIsInSameApex
 
 	apexModuleBase() *ApexModuleBase
 
-	// Marks that this module should be built for the specified APEX.
-	// Call this before apex.apexMutator is run.
+	// Marks that this module should be built for the specified APEX. Call this BEFORE
+	// apex.apexMutator is run.
 	BuildForApex(apex ApexInfo)
 
-	// Returns true if this module is present in any APEXes
-	// directly or indirectly.
-	// Call this after apex.apexMutator is run.
+	// Returns true if this module is present in any APEX either directly or indirectly. Call
+	// this after apex.apexMutator is run.
 	InAnyApex() bool
 
-	// Returns true if this module is directly in any APEXes.
-	// Call this after apex.apexMutator is run.
+	// Returns true if this module is directly in any APEX. Call this AFTER apex.apexMutator is
+	// run.
 	DirectlyInAnyApex() bool
 
-	// Returns true if any variant of this module is directly in any APEXes.
-	// Call this after apex.apexMutator is run.
+	// Returns true in the primary variant of a module if _any_ variant of the module is
+	// directly in any apex. This includes host, arch, asan, etc. variants. It is unused in any
+	// variant that is not the primary variant. Ideally this wouldn't be used, as it incorrectly
+	// mixes arch variants if only one arch is in an apex, but a few places depend on it, for
+	// example when an ASAN variant is created before the apexMutator. Call this after
+	// apex.apexMutator is run.
 	AnyVariantDirectlyInAnyApex() bool
 
-	// Tests if this module could have APEX variants. APEX variants are
-	// created only for the modules that returns true here. This is useful
-	// for not creating APEX variants for certain types of shared libraries
-	// such as NDK stubs.
+	// Tests if this module could have APEX variants. Even when a module type implements
+	// ApexModule interface, APEX variants are created only for the module instances that return
+	// true here. This is useful for not creating APEX variants for certain types of shared
+	// libraries such as NDK stubs.
 	CanHaveApexVariants() bool
 
-	// Tests if this module can be installed to APEX as a file. For example,
-	// this would return true for shared libs while return false for static
-	// libs.
+	// Tests if this module can be installed to APEX as a file. For example, this would return
+	// true for shared libs while return false for static libs because static libs are not
+	// installable module (but it can still be mutated for APEX)
 	IsInstallableToApex() bool
 
-	// Tests if this module is available for the specified APEX or ":platform"
+	// Tests if this module is available for the specified APEX or ":platform". This is from the
+	// apex_available property of the module.
 	AvailableFor(what string) bool
 
-	// Return true if this module is not available to platform (i.e. apex_available
-	// property doesn't have "//apex_available:platform"), or shouldn't be available
-	// to platform, which is the case when this module depends on other module that
-	// isn't available to platform.
+	// Returns true if this module is not available to platform (i.e. apex_available property
+	// doesn't have "//apex_available:platform"), or shouldn't be available to platform, which
+	// is the case when this module depends on other module that isn't available to platform.
 	NotAvailableForPlatform() bool
 
-	// Mark that this module is not available to platform. Set by the
+	// Marks that this module is not available to platform. Set by the
 	// check-platform-availability mutator in the apex package.
 	SetNotAvailableForPlatform()
 
-	// List of APEXes that this module tests. The module has access to
-	// the private part of the listed APEXes even when it is not included in the
-	// APEXes.
+	// Returns the list of APEXes that this module is a test for. The module has access to the
+	// private part of the listed APEXes even when it is not included in the APEXes. This by
+	// default returns nil. A module type should override the default implementation. For
+	// example, cc_test module type returns the value of test_for here.
 	TestFor() []string
 
-	// Returns nil if this module supports sdkVersion
-	// Otherwise, returns error with reason
+	// Returns nil (success) if this module should support the given sdk version. Returns an
+	// error if not. No default implementation is provided for this method. A module type
+	// implementing this interface should provide an implementation. A module supports an sdk
+	// version when the module's min_sdk_version is equal to or less than the given sdk version.
 	ShouldSupportSdkVersion(ctx BaseModuleContext, sdkVersion ApiLevel) error
 
-	// Returns true if this module needs a unique variation per apex, for example if
-	// use_apex_name_macro is set.
+	// Returns true if this module needs a unique variation per apex, effectively disabling the
+	// deduping. This is turned on when, for example if use_apex_name_macro is set so that each
+	// apex variant should be built with different macro definitions.
 	UniqueApexVariations() bool
 }
 
+// Properties that are common to all module types implementing ApexModule interface.
 type ApexProperties struct {
-	// Availability of this module in APEXes. Only the listed APEXes can contain
-	// this module. If the module has stubs then other APEXes and the platform may
-	// access it through them (subject to visibility).
+	// Availability of this module in APEXes. Only the listed APEXes can contain this module. If
+	// the module has stubs then other APEXes and the platform may access it through them
+	// (subject to visibility).
 	//
 	// "//apex_available:anyapex" is a pseudo APEX name that matches to any APEX.
 	// "//apex_available:platform" refers to non-APEX partitions like "system.img".
@@ -179,29 +215,23 @@
 	// Default is ["//apex_available:platform"].
 	Apex_available []string
 
-	// AnyVariantDirectlyInAnyApex is true in the primary variant of a module if _any_ variant
-	// of the module is directly in any apex.  This includes host, arch, asan, etc. variants.
-	// It is unused in any variant that is not the primary variant.
-	// Ideally this wouldn't be used, as it incorrectly mixes arch variants if only one arch
-	// is in an apex, but a few places depend on it, for example when an ASAN variant is
-	// created before the apexMutator.
-	AnyVariantDirectlyInAnyApex bool `blueprint:"mutated"`
-
-	// DirectlyInAnyApex is true if any APEX variant (including the "" variant used for the
-	// platform) of this module is directly in any APEX.
-	DirectlyInAnyApex bool `blueprint:"mutated"`
-
-	// DirectlyInAnyApex is true if any APEX variant (including the "" variant used for the
-	// platform) of this module is directly or indirectly in any APEX.
+	// See ApexModule.InAnyApex()
 	InAnyApex bool `blueprint:"mutated"`
 
+	// See ApexModule.DirectlyInAnyApex()
+	DirectlyInAnyApex bool `blueprint:"mutated"`
+
+	// See ApexModule.AnyVariantDirectlyInAnyApex()
+	AnyVariantDirectlyInAnyApex bool `blueprint:"mutated"`
+
+	// See ApexModule.NotAvailableForPlatform()
 	NotAvailableForPlatform bool `blueprint:"mutated"`
 
+	// See ApexModule.UniqueApexVariants()
 	UniqueApexVariationsForDeps bool `blueprint:"mutated"`
 }
 
-// Marker interface that identifies dependencies that are excluded from APEX
-// contents.
+// Marker interface that identifies dependencies that are excluded from APEX contents.
 type ExcludeFromApexContentsTag interface {
 	blueprint.DependencyTag
 
@@ -209,85 +239,122 @@
 	ExcludeFromApexContents()
 }
 
-// Marker interface that identifies dependencies that should inherit the DirectlyInAnyApex
-// state from the parent to the child.  For example, stubs libraries are marked as
-// DirectlyInAnyApex if their implementation is in an apex.
+// Marker interface that identifies dependencies that should inherit the DirectlyInAnyApex state
+// from the parent to the child. For example, stubs libraries are marked as DirectlyInAnyApex if
+// their implementation is in an apex.
 type CopyDirectlyInAnyApexTag interface {
 	blueprint.DependencyTag
 
+	// Method that differentiates this interface from others.
 	CopyDirectlyInAnyApex()
 }
 
-// Provides default implementation for the ApexModule interface. APEX-aware
+// ApexModuleBase provides the default implementation for the ApexModule interface. APEX-aware
 // modules are expected to include this struct and call InitApexModule().
 type ApexModuleBase struct {
 	ApexProperties ApexProperties
 
 	canHaveApexVariants bool
 
-	apexVariationsLock sync.Mutex // protects apexVariations during parallel apexDepsMutator
-	apexVariations     []ApexInfo
+	apexInfos     []ApexInfo
+	apexInfosLock sync.Mutex // protects apexInfos during parallel apexDepsMutator
 }
 
+// Initializes ApexModuleBase struct. Not calling this (even when inheriting from ApexModuleBase)
+// prevents the module from being mutated for apexBundle.
+func InitApexModule(m ApexModule) {
+	base := m.apexModuleBase()
+	base.canHaveApexVariants = true
+
+	m.AddProperties(&base.ApexProperties)
+}
+
+// Implements ApexModule
 func (m *ApexModuleBase) apexModuleBase() *ApexModuleBase {
 	return m
 }
 
+// Implements ApexModule
 func (m *ApexModuleBase) ApexAvailable() []string {
 	return m.ApexProperties.Apex_available
 }
 
-func (m *ApexModuleBase) TestFor() []string {
-	// To be implemented by concrete types inheriting ApexModuleBase
-	return nil
-}
-
-func (m *ApexModuleBase) UniqueApexVariations() bool {
-	return false
-}
-
+// Implements ApexModule
 func (m *ApexModuleBase) BuildForApex(apex ApexInfo) {
-	m.apexVariationsLock.Lock()
-	defer m.apexVariationsLock.Unlock()
-	for _, v := range m.apexVariations {
+	m.apexInfosLock.Lock()
+	defer m.apexInfosLock.Unlock()
+	for _, v := range m.apexInfos {
 		if v.ApexVariationName == apex.ApexVariationName {
 			return
 		}
 	}
-	m.apexVariations = append(m.apexVariations, apex)
+	m.apexInfos = append(m.apexInfos, apex)
 }
 
-func (m *ApexModuleBase) DirectlyInAnyApex() bool {
-	return m.ApexProperties.DirectlyInAnyApex
-}
-
-func (m *ApexModuleBase) AnyVariantDirectlyInAnyApex() bool {
-	return m.ApexProperties.AnyVariantDirectlyInAnyApex
-}
-
+// Implements ApexModule
 func (m *ApexModuleBase) InAnyApex() bool {
 	return m.ApexProperties.InAnyApex
 }
 
+// Implements ApexModule
+func (m *ApexModuleBase) DirectlyInAnyApex() bool {
+	return m.ApexProperties.DirectlyInAnyApex
+}
+
+// Implements ApexModule
+func (m *ApexModuleBase) AnyVariantDirectlyInAnyApex() bool {
+	return m.ApexProperties.AnyVariantDirectlyInAnyApex
+}
+
+// Implements ApexModule
 func (m *ApexModuleBase) CanHaveApexVariants() bool {
 	return m.canHaveApexVariants
 }
 
+// Implements ApexModule
 func (m *ApexModuleBase) IsInstallableToApex() bool {
-	// should be overriden if needed
+	// If needed, this will bel overridden by concrete types inheriting
+	// ApexModuleBase
 	return false
 }
 
+// Implements ApexModule
+func (m *ApexModuleBase) TestFor() []string {
+	// If needed, this will be overridden by concrete types inheriting
+	// ApexModuleBase
+	return nil
+}
+
+// Implements ApexModule
+func (m *ApexModuleBase) UniqueApexVariations() bool {
+	// If needed, this will bel overridden by concrete types inheriting
+	// ApexModuleBase
+	return false
+}
+
+// Implements ApexModule
+func (m *ApexModuleBase) DepIsInSameApex(ctx BaseModuleContext, dep Module) bool {
+	// By default, if there is a dependency from A to B, we try to include both in the same
+	// APEX, unless B is explicitly from outside of the APEX (i.e. a stubs lib). Thus, returning
+	// true. This is overridden by some module types like apex.ApexBundle, cc.Module,
+	// java.Module, etc.
+	return true
+}
+
 const (
 	AvailableToPlatform = "//apex_available:platform"
 	AvailableToAnyApex  = "//apex_available:anyapex"
 	AvailableToGkiApex  = "com.android.gki.*"
 )
 
+// CheckAvailableForApex provides the default algorithm for checking the apex availability. When the
+// availability is empty, it defaults to ["//apex_available:platform"] which means "available to the
+// platform but not available to any APEX". When the list is not empty, `what` is matched against
+// the list. If there is any matching element in the list, thus function returns true. The special
+// availability "//apex_available:anyapex" matches with anything except for
+// "//apex_available:platform".
 func CheckAvailableForApex(what string, apex_available []string) bool {
 	if len(apex_available) == 0 {
-		// apex_available defaults to ["//apex_available:platform"],
-		// which means 'available to the platform but no apexes'.
 		return what == AvailableToPlatform
 	}
 	return InList(what, apex_available) ||
@@ -295,25 +362,22 @@
 		(strings.HasPrefix(what, "com.android.gki.") && InList(AvailableToGkiApex, apex_available))
 }
 
+// Implements ApexModule
 func (m *ApexModuleBase) AvailableFor(what string) bool {
 	return CheckAvailableForApex(what, m.ApexProperties.Apex_available)
 }
 
+// Implements ApexModule
 func (m *ApexModuleBase) NotAvailableForPlatform() bool {
 	return m.ApexProperties.NotAvailableForPlatform
 }
 
+// Implements ApexModule
 func (m *ApexModuleBase) SetNotAvailableForPlatform() {
 	m.ApexProperties.NotAvailableForPlatform = true
 }
 
-func (m *ApexModuleBase) DepIsInSameApex(ctx BaseModuleContext, dep Module) bool {
-	// By default, if there is a dependency from A to B, we try to include both in the same APEX,
-	// unless B is explicitly from outside of the APEX (i.e. a stubs lib). Thus, returning true.
-	// This is overridden by some module types like apex.ApexBundle, cc.Module, java.Module, etc.
-	return true
-}
-
+// This function makes sure that the apex_available property is valid
 func (m *ApexModuleBase) checkApexAvailableProperty(mctx BaseModuleContext) {
 	for _, n := range m.ApexProperties.Apex_available {
 		if n == AvailableToPlatform || n == AvailableToAnyApex || n == AvailableToGkiApex {
@@ -331,22 +395,23 @@
 func (a byApexName) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
 func (a byApexName) Less(i, j int) bool { return a[i].ApexVariationName < a[j].ApexVariationName }
 
-// mergeApexVariations deduplicates APEX variations that would build identically into a common
-// variation.  It returns the reduced list of variations and a list of aliases from the original
+// mergeApexVariations deduplicates apex variations that would build identically into a common
+// variation. It returns the reduced list of variations and a list of aliases from the original
 // variation names to the new variation names.
-func mergeApexVariations(ctx PathContext, apexVariations []ApexInfo) (merged []ApexInfo, aliases [][2]string) {
-	sort.Sort(byApexName(apexVariations))
+func mergeApexVariations(ctx PathContext, apexInfos []ApexInfo) (merged []ApexInfo, aliases [][2]string) {
+	sort.Sort(byApexName(apexInfos))
 	seen := make(map[string]int)
-	for _, apexInfo := range apexVariations {
+	for _, apexInfo := range apexInfos {
 		apexName := apexInfo.ApexVariationName
 		mergedName := apexInfo.mergedName(ctx)
 		if index, exists := seen[mergedName]; exists {
+			// Variants having the same mergedName are deduped
 			merged[index].InApexes = append(merged[index].InApexes, apexName)
 			merged[index].ApexContents = append(merged[index].ApexContents, apexInfo.ApexContents...)
 			merged[index].Updatable = merged[index].Updatable || apexInfo.Updatable
 		} else {
 			seen[mergedName] = len(merged)
-			apexInfo.ApexVariationName = apexInfo.mergedName(ctx)
+			apexInfo.ApexVariationName = mergedName
 			apexInfo.InApexes = CopyOf(apexInfo.InApexes)
 			apexInfo.ApexContents = append([]*ApexContents(nil), apexInfo.ApexContents...)
 			merged = append(merged, apexInfo)
@@ -356,75 +421,78 @@
 	return merged, aliases
 }
 
+// CreateApexVariations mutates a given module into multiple apex variants each of which is for an
+// apexBundle (and/or the platform) where the module is part of.
 func CreateApexVariations(mctx BottomUpMutatorContext, module ApexModule) []Module {
 	base := module.apexModuleBase()
-	if len(base.apexVariations) > 0 {
-		base.checkApexAvailableProperty(mctx)
 
-		var apexVariations []ApexInfo
-		var aliases [][2]string
-		if !mctx.Module().(ApexModule).UniqueApexVariations() && !base.ApexProperties.UniqueApexVariationsForDeps {
-			apexVariations, aliases = mergeApexVariations(mctx, base.apexVariations)
-		} else {
-			apexVariations = base.apexVariations
-		}
-		// base.apexVariations is only needed to propagate the list of apexes from
-		// apexDepsMutator to apexMutator.  It is no longer accurate after
-		// mergeApexVariations, and won't be copied to all but the first created
-		// variant.  Clear it so it doesn't accidentally get used later.
-		base.apexVariations = nil
-
-		sort.Sort(byApexName(apexVariations))
-		variations := []string{}
-		variations = append(variations, "") // Original variation for platform
-		for _, apex := range apexVariations {
-			variations = append(variations, apex.ApexVariationName)
-		}
-
-		defaultVariation := ""
-		mctx.SetDefaultDependencyVariation(&defaultVariation)
-
-		var inApex ApexMembership
-		for _, a := range apexVariations {
-			for _, apexContents := range a.ApexContents {
-				inApex = inApex.merge(apexContents.contents[mctx.ModuleName()])
-			}
-		}
-
-		base.ApexProperties.InAnyApex = true
-		base.ApexProperties.DirectlyInAnyApex = inApex == directlyInApex
-
-		modules := mctx.CreateVariations(variations...)
-		for i, mod := range modules {
-			platformVariation := i == 0
-			if platformVariation && !mctx.Host() && !mod.(ApexModule).AvailableFor(AvailableToPlatform) {
-				// Do not install the module for platform, but still allow it to output
-				// uninstallable AndroidMk entries in certain cases when they have
-				// side effects.
-				mod.MakeUninstallable()
-			}
-			if !platformVariation {
-				mctx.SetVariationProvider(mod, ApexInfoProvider, apexVariations[i-1])
-			}
-		}
-
-		for _, alias := range aliases {
-			mctx.CreateAliasVariation(alias[0], alias[1])
-		}
-
-		return modules
+	// Shortcut
+	if len(base.apexInfos) == 0 {
+		return nil
 	}
-	return nil
+
+	// Do some validity checks.
+	// TODO(jiyong): is this the right place?
+	base.checkApexAvailableProperty(mctx)
+
+	var apexInfos []ApexInfo
+	var aliases [][2]string
+	if !mctx.Module().(ApexModule).UniqueApexVariations() && !base.ApexProperties.UniqueApexVariationsForDeps {
+		apexInfos, aliases = mergeApexVariations(mctx, base.apexInfos)
+	} else {
+		apexInfos = base.apexInfos
+	}
+	// base.apexInfos is only needed to propagate the list of apexes from apexDepsMutator to
+	// apexMutator. It is no longer accurate after mergeApexVariations, and won't be copied to
+	// all but the first created variant. Clear it so it doesn't accidentally get used later.
+	base.apexInfos = nil
+	sort.Sort(byApexName(apexInfos))
+
+	var inApex ApexMembership
+	for _, a := range apexInfos {
+		for _, apexContents := range a.ApexContents {
+			inApex = inApex.merge(apexContents.contents[mctx.ModuleName()])
+		}
+	}
+	base.ApexProperties.InAnyApex = true
+	base.ApexProperties.DirectlyInAnyApex = inApex == directlyInApex
+
+	defaultVariation := ""
+	mctx.SetDefaultDependencyVariation(&defaultVariation)
+
+	variations := []string{defaultVariation}
+	for _, a := range apexInfos {
+		variations = append(variations, a.ApexVariationName)
+	}
+	modules := mctx.CreateVariations(variations...)
+	for i, mod := range modules {
+		platformVariation := i == 0
+		if platformVariation && !mctx.Host() && !mod.(ApexModule).AvailableFor(AvailableToPlatform) {
+			// Do not install the module for platform, but still allow it to output
+			// uninstallable AndroidMk entries in certain cases when they have side
+			// effects.  TODO(jiyong): move this routine to somewhere else
+			mod.MakeUninstallable()
+		}
+		if !platformVariation {
+			mctx.SetVariationProvider(mod, ApexInfoProvider, apexInfos[i-1])
+		}
+	}
+
+	for _, alias := range aliases {
+		mctx.CreateAliasVariation(alias[0], alias[1])
+	}
+
+	return modules
 }
 
-// UpdateUniqueApexVariationsForDeps sets UniqueApexVariationsForDeps if any dependencies
-// that are in the same APEX have unique APEX variations so that the module can link against
-// the right variant.
+// UpdateUniqueApexVariationsForDeps sets UniqueApexVariationsForDeps if any dependencies that are
+// in the same APEX have unique APEX variations so that the module can link against the right
+// variant.
 func UpdateUniqueApexVariationsForDeps(mctx BottomUpMutatorContext, am ApexModule) {
-	// anyInSameApex returns true if the two ApexInfo lists contain any values in an InApexes list
-	// in common.  It is used instead of DepIsInSameApex because it needs to determine if the dep
-	// is in the same APEX due to being directly included, not only if it is included _because_ it
-	// is a dependency.
+	// anyInSameApex returns true if the two ApexInfo lists contain any values in an InApexes
+	// list in common. It is used instead of DepIsInSameApex because it needs to determine if
+	// the dep is in the same APEX due to being directly included, not only if it is included
+	// _because_ it is a dependency.
 	anyInSameApex := func(a, b []ApexInfo) bool {
 		collectApexes := func(infos []ApexInfo) []string {
 			var ret []string
@@ -446,9 +514,10 @@
 		return false
 	}
 
+	// If any of the dependencies requires unique apex variations, so does this module.
 	mctx.VisitDirectDeps(func(dep Module) {
 		if depApexModule, ok := dep.(ApexModule); ok {
-			if anyInSameApex(depApexModule.apexModuleBase().apexVariations, am.apexModuleBase().apexVariations) &&
+			if anyInSameApex(depApexModule.apexModuleBase().apexInfos, am.apexModuleBase().apexInfos) &&
 				(depApexModule.UniqueApexVariations() ||
 					depApexModule.apexModuleBase().ApexProperties.UniqueApexVariationsForDeps) {
 				am.apexModuleBase().ApexProperties.UniqueApexVariationsForDeps = true
@@ -457,10 +526,10 @@
 	})
 }
 
-// UpdateDirectlyInAnyApex uses the final module to store if any variant of this
-// module is directly in any APEX, and then copies the final value to all the modules.
-// It also copies the DirectlyInAnyApex value to any direct dependencies with a
-// CopyDirectlyInAnyApexTag dependency tag.
+// UpdateDirectlyInAnyApex uses the final module to store if any variant of this module is directly
+// in any APEX, and then copies the final value to all the modules. It also copies the
+// DirectlyInAnyApex value to any direct dependencies with a CopyDirectlyInAnyApexTag dependency
+// tag.
 func UpdateDirectlyInAnyApex(mctx BottomUpMutatorContext, am ApexModule) {
 	base := am.apexModuleBase()
 	// Copy DirectlyInAnyApex and InAnyApex from any direct dependencies with a
@@ -475,14 +544,13 @@
 
 	if base.ApexProperties.DirectlyInAnyApex {
 		// Variants of a module are always visited sequentially in order, so it is safe to
-		// write to another variant of this module.
-		// For a BottomUpMutator the PrimaryModule() is visited first and FinalModule() is
-		// visited last.
+		// write to another variant of this module. For a BottomUpMutator the
+		// PrimaryModule() is visited first and FinalModule() is visited last.
 		mctx.FinalModule().(ApexModule).apexModuleBase().ApexProperties.AnyVariantDirectlyInAnyApex = true
 	}
 
-	// If this is the FinalModule (last visited module) copy AnyVariantDirectlyInAnyApex to
-	// all the other variants
+	// If this is the FinalModule (last visited module) copy
+	// AnyVariantDirectlyInAnyApex to all the other variants
 	if am == mctx.FinalModule().(ApexModule) {
 		mctx.VisitAllModuleVariants(func(variant Module) {
 			variant.(ApexModule).apexModuleBase().ApexProperties.AnyVariantDirectlyInAnyApex =
@@ -491,6 +559,7 @@
 	}
 }
 
+// ApexMembership tells how a module became part of an APEX.
 type ApexMembership int
 
 const (
@@ -499,20 +568,21 @@
 	directlyInApex
 )
 
-// Each apexBundle has an apexContents, and modules in that apex have a provider containing the
-// apexContents of each apexBundle they are part of.
+// ApexContents gives an information about member modules of an apexBundle.  Each apexBundle has an
+// apexContents, and modules in that apex have a provider containing the apexContents of each
+// apexBundle they are part of.
 type ApexContents struct {
-	ApexName string
+	// map from a module name to its membership to this apexBUndle
 	contents map[string]ApexMembership
 }
 
-func NewApexContents(name string, contents map[string]ApexMembership) *ApexContents {
+func NewApexContents(contents map[string]ApexMembership) *ApexContents {
 	return &ApexContents{
-		ApexName: name,
 		contents: contents,
 	}
 }
 
+// Updates an existing membership by adding a new direct (or indirect) membership
 func (i ApexMembership) Add(direct bool) ApexMembership {
 	if direct || i == directlyInApex {
 		return directlyInApex
@@ -520,6 +590,10 @@
 	return indirectlyInApex
 }
 
+// Merges two membership into one. Merging is needed because a module can be a part of an apexBundle
+// in many different paths. For example, it could be dependend on by the apexBundle directly, but at
+// the same time, there might be an indirect dependency to the module. In that case, the more
+// specific dependency (the direct one) is chosen.
 func (i ApexMembership) merge(other ApexMembership) ApexMembership {
 	if other == directlyInApex || i == directlyInApex {
 		return directlyInApex
@@ -531,16 +605,19 @@
 	return notInApex
 }
 
-func (ac *ApexContents) DirectlyInApex(name string) bool {
-	return ac.contents[name] == directlyInApex
+// Tests whether a module named moduleName is directly included in the apexBundle where this
+// ApexContents is tagged.
+func (ac *ApexContents) DirectlyInApex(moduleName string) bool {
+	return ac.contents[moduleName] == directlyInApex
 }
 
-func (ac *ApexContents) InApex(name string) bool {
-	return ac.contents[name] != notInApex
+// Tests whether a module named moduleName is included in the apexBundle where this ApexContent is
+// tagged.
+func (ac *ApexContents) InApex(moduleName string) bool {
+	return ac.contents[moduleName] != notInApex
 }
 
-// Tests whether a module named moduleName is directly depended on by all APEXes
-// in an ApexInfo.
+// Tests whether a module named moduleName is directly depended on by all APEXes in an ApexInfo.
 func DirectlyInAllApexes(apexInfo ApexInfo, moduleName string) bool {
 	for _, contents := range apexInfo.ApexContents {
 		if !contents.DirectlyInApex(moduleName) {
@@ -550,12 +627,15 @@
 	return true
 }
 
-func InitApexModule(m ApexModule) {
-	base := m.apexModuleBase()
-	base.canHaveApexVariants = true
-
-	m.AddProperties(&base.ApexProperties)
-}
+////////////////////////////////////////////////////////////////////////////////////////////////////
+//Below are routines for extra safety checks.
+//
+// BuildDepsInfoLists is to flatten the dependency graph for an apexBundle into a text file
+// (actually two in slightly different formats). The files are mostly for debugging, for example to
+// see why a certain module is included in an APEX via which dependency path.
+//
+// CheckMinSdkVersion is to make sure that all modules in an apexBundle satisfy the min_sdk_version
+// requirement of the apexBundle.
 
 // A dependency info for a single ApexModule, either direct or transitive.
 type ApexModuleDepInfo struct {
@@ -705,7 +785,8 @@
 	WalkPayloadDeps(ctx ModuleContext, do PayloadDepsCallback)
 }
 
-// CheckMinSdkVersion checks if every dependency of an updatable module sets min_sdk_version accordingly
+// CheckMinSdkVersion checks if every dependency of an updatable module sets min_sdk_version
+// accordingly
 func CheckMinSdkVersion(m UpdatableModule, ctx ModuleContext, minSdkVersion ApiLevel) {
 	// do not enforce min_sdk_version for host
 	if ctx.Host() {
@@ -725,8 +806,9 @@
 
 	m.WalkPayloadDeps(ctx, func(ctx ModuleContext, from blueprint.Module, to ApexModule, externalDep bool) bool {
 		if externalDep {
-			// external deps are outside the payload boundary, which is "stable" interface.
-			// We don't have to check min_sdk_version for external dependencies.
+			// external deps are outside the payload boundary, which is "stable"
+			// interface. We don't have to check min_sdk_version for external
+			// dependencies.
 			return false
 		}
 		if am, ok := from.(DepIsInSameApex); ok && !am.DepIsInSameApex(ctx, to) {
diff --git a/apex/apex.go b/apex/apex.go
index ce323ca..dff0855 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -709,7 +709,7 @@
 		return true
 	})
 
-	apexContents := android.NewApexContents(mctx.ModuleName(), contents)
+	apexContents := android.NewApexContents(contents)
 	mctx.SetProvider(ApexBundleInfoProvider, ApexBundleInfo{
 		Contents: apexContents,
 	})
diff --git a/etc/prebuilt_etc.go b/etc/prebuilt_etc.go
index a6d1fcf..850c8f9 100644
--- a/etc/prebuilt_etc.go
+++ b/etc/prebuilt_etc.go
@@ -56,16 +56,19 @@
 	// Source file of this prebuilt. Can reference a genrule type module with the ":module" syntax.
 	Src *string `android:"path,arch_variant"`
 
-	// optional subdirectory under which this file is installed into, cannot be specified with relative_install_path, prefer relative_install_path
+	// Optional subdirectory under which this file is installed into, cannot be specified with
+	// relative_install_path, prefer relative_install_path.
 	Sub_dir *string `android:"arch_variant"`
 
-	// optional subdirectory under which this file is installed into, cannot be specified with sub_dir
+	// Optional subdirectory under which this file is installed into, cannot be specified with
+	// sub_dir.
 	Relative_install_path *string `android:"arch_variant"`
 
-	// optional name for the installed file. If unspecified, name of the module is used as the file name
+	// Optional name for the installed file. If unspecified, name of the module is used as the file
+	// name.
 	Filename *string `android:"arch_variant"`
 
-	// when set to true, and filename property is not set, the name for the installed file
+	// When set to true, and filename property is not set, the name for the installed file
 	// is the same as the file name of the source file.
 	Filename_from_src *bool `android:"arch_variant"`
 
@@ -95,8 +98,15 @@
 
 type PrebuiltEtcModule interface {
 	android.Module
+
+	// Returns the base install directory, such as "etc", "usr/share".
 	BaseDir() string
+
+	// Returns the sub install directory relative to BaseDir().
 	SubDir() string
+
+	// Returns an android.OutputPath to the intermeidate file, which is the renamed prebuilt source
+	// file.
 	OutputFile() android.OutputPath
 }
 
@@ -109,7 +119,8 @@
 	outputFilePath android.OutputPath
 	// The base install location, e.g. "etc" for prebuilt_etc, "usr/share" for prebuilt_usr_share.
 	installDirBase string
-	// The base install location when soc_specific property is set to true, e.g. "firmware" for prebuilt_firmware.
+	// The base install location when soc_specific property is set to true, e.g. "firmware" for
+	// prebuilt_firmware.
 	socInstallDirBase      string
 	installDirPath         android.InstallPath
 	additionalDependencies *android.Paths
@@ -179,14 +190,8 @@
 func (p *PrebuiltEtc) SetImageVariation(ctx android.BaseModuleContext, variation string, module android.Module) {
 }
 
-func (p *PrebuiltEtc) DepsMutator(ctx android.BottomUpMutatorContext) {
-	if p.properties.Src == nil {
-		ctx.PropertyErrorf("src", "missing prebuilt source file")
-	}
-}
-
 func (p *PrebuiltEtc) SourceFilePath(ctx android.ModuleContext) android.Path {
-	return android.PathForModuleSrc(ctx, android.String(p.properties.Src))
+	return android.PathForModuleSrc(ctx, proptools.String(p.properties.Src))
 }
 
 func (p *PrebuiltEtc) InstallDirPath() android.InstallPath {
@@ -215,36 +220,41 @@
 }
 
 func (p *PrebuiltEtc) Installable() bool {
-	return p.properties.Installable == nil || android.Bool(p.properties.Installable)
+	return p.properties.Installable == nil || proptools.Bool(p.properties.Installable)
 }
 
 func (p *PrebuiltEtc) GenerateAndroidBuildActions(ctx android.ModuleContext) {
-	p.sourceFilePath = android.PathForModuleSrc(ctx, android.String(p.properties.Src))
+	if p.properties.Src == nil {
+		ctx.PropertyErrorf("src", "missing prebuilt source file")
+		return
+	}
+	p.sourceFilePath = android.PathForModuleSrc(ctx, proptools.String(p.properties.Src))
 
 	// Determine the output file basename.
 	// If Filename is set, use the name specified by the property.
 	// If Filename_from_src is set, use the source file name.
 	// Otherwise use the module name.
-	filename := android.String(p.properties.Filename)
-	filename_from_src := android.Bool(p.properties.Filename_from_src)
-	if filename == "" {
-		if filename_from_src {
-			filename = p.sourceFilePath.Base()
-		} else {
-			filename = ctx.ModuleName()
+	filename := proptools.String(p.properties.Filename)
+	filenameFromSrc := proptools.Bool(p.properties.Filename_from_src)
+	if filename != "" {
+		if filenameFromSrc {
+			ctx.PropertyErrorf("filename_from_src", "filename is set. filename_from_src can't be true")
+			return
 		}
-	} else if filename_from_src {
-		ctx.PropertyErrorf("filename_from_src", "filename is set. filename_from_src can't be true")
-		return
+	} else if filenameFromSrc {
+		filename = p.sourceFilePath.Base()
+	} else {
+		filename = ctx.ModuleName()
 	}
 	p.outputFilePath = android.PathForModuleOut(ctx, filename).OutputPath
 
+	// Check that `sub_dir` and `relative_install_path` are not set at the same time.
 	if p.properties.Sub_dir != nil && p.properties.Relative_install_path != nil {
 		ctx.PropertyErrorf("sub_dir", "relative_install_path is set. Cannot set sub_dir")
 	}
 
-	// If soc install dir was specified and SOC specific is set, set the installDirPath to the specified
-	// socInstallDirBase.
+	// If soc install dir was specified and SOC specific is set, set the installDirPath to the
+	// specified socInstallDirBase.
 	installBaseDir := p.installDirBase
 	if p.SocSpecific() && p.socInstallDirBase != "" {
 		installBaseDir = p.socInstallDirBase
@@ -353,9 +363,10 @@
 	return module
 }
 
-// prebuilt_firmware installs a firmware file to <partition>/etc/firmware directory for system image.
-// If soc_specific property is set to true, the firmware file is installed to the vendor <partition>/firmware
-// directory for vendor image.
+// prebuilt_firmware installs a firmware file to <partition>/etc/firmware directory for system
+// image.
+// If soc_specific property is set to true, the firmware file is installed to the
+// vendor <partition>/firmware directory for vendor image.
 func PrebuiltFirmwareFactory() android.Module {
 	module := &PrebuiltEtc{}
 	module.socInstallDirBase = "firmware"
@@ -366,8 +377,8 @@
 }
 
 // prebuilt_dsp installs a DSP related file to <partition>/etc/dsp directory for system image.
-// If soc_specific property is set to true, the DSP related file is installed to the vendor <partition>/dsp
-// directory for vendor image.
+// If soc_specific property is set to true, the DSP related file is installed to the
+// vendor <partition>/dsp directory for vendor image.
 func PrebuiltDSPFactory() android.Module {
 	module := &PrebuiltEtc{}
 	module.socInstallDirBase = "dsp"
diff --git a/java/hiddenapi_singleton.go b/java/hiddenapi_singleton.go
index 1f80e77..e57f323 100644
--- a/java/hiddenapi_singleton.go
+++ b/java/hiddenapi_singleton.go
@@ -255,7 +255,7 @@
 		FlagWithInput("--max-target-p ",
 			android.PathForSource(ctx, "frameworks/base/config/hiddenapi-max-target-p.txt")).
 		FlagWithInput("--max-target-o ", android.PathForSource(
-			ctx, "frameworks/base/config/hiddenapi-max-target-o.txt")).Flag("--ignore-conflicts ").
+			ctx, "frameworks/base/config/hiddenapi-max-target-o.txt")).Flag("--ignore-conflicts ").FlagWithArg("--tag ", "lo-prio").
 		FlagWithInput("--blocked ",
 			android.PathForSource(ctx, "frameworks/base/config/hiddenapi-force-blocked.txt")).
 		FlagWithInput("--blocked ",
diff --git a/java/sdk_library.go b/java/sdk_library.go
index ebf867d..603c808 100644
--- a/java/sdk_library.go
+++ b/java/sdk_library.go
@@ -438,6 +438,21 @@
 	// If set to true then don't create dist rules.
 	No_dist *bool
 
+	// The stem for the artifacts that are copied to the dist, if not specified
+	// then defaults to the base module name.
+	//
+	// For each scope the following artifacts are copied to the apistubs/<scope>
+	// directory in the dist.
+	// * stubs impl jar -> <dist-stem>.jar
+	// * API specification file -> api/<dist-stem>.txt
+	// * Removed API specification file -> api/<dist-stem>-removed.txt
+	//
+	// Also used to construct the name of the filegroup (created by prebuilt_apis)
+	// that references the latest released API and remove API specification files.
+	// * API specification filegroup -> <dist-stem>.api.<scope>.latest
+	// * Removed API specification filegroup -> <dist-stem>-removed.api.<scope>.latest
+	Dist_stem *string
+
 	// indicates whether system and test apis should be generated.
 	Generate_system_and_test_apis bool `blueprint:"mutated"`
 
@@ -1117,12 +1132,16 @@
 	}
 }
 
+func (module *SdkLibrary) distStem() string {
+	return proptools.StringDefault(module.sdkLibraryProperties.Dist_stem, module.BaseModuleName())
+}
+
 func (module *SdkLibrary) latestApiFilegroupName(apiScope *apiScope) string {
-	return ":" + module.BaseModuleName() + ".api." + apiScope.name + ".latest"
+	return ":" + module.distStem() + ".api." + apiScope.name + ".latest"
 }
 
 func (module *SdkLibrary) latestRemovedApiFilegroupName(apiScope *apiScope) string {
-	return ":" + module.BaseModuleName() + "-removed.api." + apiScope.name + ".latest"
+	return ":" + module.distStem() + "-removed.api." + apiScope.name + ".latest"
 }
 
 func childModuleVisibility(childVisibility []string) []string {
@@ -1228,7 +1247,7 @@
 	// Dist the class jar artifact for sdk builds.
 	if !Bool(module.sdkLibraryProperties.No_dist) {
 		props.Dist.Targets = []string{"sdk", "win_sdk"}
-		props.Dist.Dest = proptools.StringPtr(fmt.Sprintf("%v.jar", module.BaseModuleName()))
+		props.Dist.Dest = proptools.StringPtr(fmt.Sprintf("%v.jar", module.distStem()))
 		props.Dist.Dir = proptools.StringPtr(module.apiDistPath(apiScope))
 		props.Dist.Tag = proptools.StringPtr(".jar")
 	}
@@ -1377,7 +1396,7 @@
 	// Dist the api txt artifact for sdk builds.
 	if !Bool(module.sdkLibraryProperties.No_dist) {
 		props.Dist.Targets = []string{"sdk", "win_sdk"}
-		props.Dist.Dest = proptools.StringPtr(fmt.Sprintf("%v.txt", module.BaseModuleName()))
+		props.Dist.Dest = proptools.StringPtr(fmt.Sprintf("%v.txt", module.distStem()))
 		props.Dist.Dir = proptools.StringPtr(path.Join(module.apiDistPath(apiScope), "api"))
 	}
 
diff --git a/ui/build/ninja.go b/ui/build/ninja.go
index fa44cb1..ffd1ab9 100644
--- a/ui/build/ninja.go
+++ b/ui/build/ninja.go
@@ -27,10 +27,16 @@
 	"android/soong/ui/status"
 )
 
+// Constructs and runs the Ninja command line with a restricted set of
+// environment variables. It's important to restrict the environment Ninja runs
+// for hermeticity reasons, and to avoid spurious rebuilds.
 func runNinja(ctx Context, config Config) {
 	ctx.BeginTrace(metrics.PrimaryNinja, "ninja")
 	defer ctx.EndTrace()
 
+	// Sets up the FIFO status updater that reads the Ninja protobuf output, and
+	// translates it to the soong_ui status output, displaying real-time
+	// progress of the build.
 	fifo := filepath.Join(config.OutDir(), ".ninja_fifo")
 	nr := status.NewNinjaReader(ctx, ctx.Status.StartTool(), fifo)
 	defer nr.Close()
@@ -64,8 +70,12 @@
 		"-w", "missingdepfile=err")
 
 	cmd := Command(ctx, config, "ninja", executable, args...)
+
+	// Set up the nsjail sandbox Ninja runs in.
 	cmd.Sandbox = ninjaSandbox
 	if config.HasKatiSuffix() {
+		// Reads and executes a shell script from Kati that sets/unsets the
+		// environment Ninja runs in.
 		cmd.Environment.AppendFromKati(config.KatiEnvFile())
 	}
 
@@ -78,8 +88,8 @@
 		cmd.Args = append(cmd.Args, strings.Fields(extra)...)
 	}
 
-	logPath := filepath.Join(config.OutDir(), ".ninja_log")
 	ninjaHeartbeatDuration := time.Minute * 5
+	// Get the ninja heartbeat interval from the environment before it's filtered away later.
 	if overrideText, ok := cmd.Environment.Get("NINJA_HEARTBEAT_INTERVAL"); ok {
 		// For example, "1m"
 		overrideDuration, err := time.ParseDuration(overrideText)
@@ -88,18 +98,22 @@
 		}
 	}
 
-	// Filter the environment, as ninja does not rebuild files when environment variables change.
+	// Filter the environment, as ninja does not rebuild files when environment
+	// variables change.
 	//
-	// Anything listed here must not change the output of rules/actions when the value changes,
-	// otherwise incremental builds may be unsafe. Vars explicitly set to stable values
-	// elsewhere in soong_ui are fine.
+	// Anything listed here must not change the output of rules/actions when the
+	// value changes, otherwise incremental builds may be unsafe. Vars
+	// explicitly set to stable values elsewhere in soong_ui are fine.
 	//
-	// For the majority of cases, either Soong or the makefiles should be replicating any
-	// necessary environment variables in the command line of each action that needs it.
+	// For the majority of cases, either Soong or the makefiles should be
+	// replicating any necessary environment variables in the command line of
+	// each action that needs it.
 	if cmd.Environment.IsEnvTrue("ALLOW_NINJA_ENV") {
 		ctx.Println("Allowing all environment variables during ninja; incremental builds may be unsafe.")
 	} else {
 		cmd.Environment.Allow(append([]string{
+			// Set the path to a symbolizer (e.g. llvm-symbolizer) so ASAN-based
+			// tools can symbolize crashes.
 			"ASAN_SYMBOLIZER_PATH",
 			"HOME",
 			"JAVA_HOME",
@@ -108,14 +122,19 @@
 			"OUT_DIR",
 			"PATH",
 			"PWD",
+			// https://docs.python.org/3/using/cmdline.html#envvar-PYTHONDONTWRITEBYTECODE
 			"PYTHONDONTWRITEBYTECODE",
 			"TMPDIR",
 			"USER",
 
 			// TODO: remove these carefully
+			// Options for the address sanitizer.
 			"ASAN_OPTIONS",
+			// The list of Android app modules to be built in an unbundled manner.
 			"TARGET_BUILD_APPS",
+			// The variant of the product being built. e.g. eng, userdebug, debug.
 			"TARGET_BUILD_VARIANT",
+			// The product name of the product being built, e.g. aosp_arm, aosp_flame.
 			"TARGET_PRODUCT",
 			// b/147197813 - used by art-check-debug-apex-gen
 			"EMMA_INSTRUMENT_FRAMEWORK",
@@ -162,6 +181,7 @@
 	cmd.Environment.Set("DIST_DIR", config.DistDir())
 	cmd.Environment.Set("SHELL", "/bin/bash")
 
+	// Print the environment variables that Ninja is operating in.
 	ctx.Verboseln("Ninja environment: ")
 	envVars := cmd.Environment.Environ()
 	sort.Strings(envVars)
@@ -169,17 +189,21 @@
 		ctx.Verbosef("  %s", envVar)
 	}
 
-	// Poll the ninja log for updates; if it isn't updated enough, then we want to show some diagnostics
+	// Poll the Ninja log for updates regularly based on the heartbeat
+	// frequency. If it isn't updated enough, then we want to surface the
+	// possibility that Ninja is stuck, to the user.
 	done := make(chan struct{})
 	defer close(done)
 	ticker := time.NewTicker(ninjaHeartbeatDuration)
 	defer ticker.Stop()
-	checker := &statusChecker{}
+	ninjaChecker := &ninjaStucknessChecker{
+		logPath: filepath.Join(config.OutDir(), ".ninja_log"),
+	}
 	go func() {
 		for {
 			select {
 			case <-ticker.C:
-				checker.check(ctx, config, logPath)
+				ninjaChecker.check(ctx, config)
 			case <-done:
 				return
 			}
@@ -190,37 +214,36 @@
 	cmd.RunAndStreamOrFatal()
 }
 
-type statusChecker struct {
-	prevTime time.Time
+// A simple struct for checking if Ninja gets stuck, using timestamps.
+type ninjaStucknessChecker struct {
+	logPath     string
+	prevModTime time.Time
 }
 
-func (c *statusChecker) check(ctx Context, config Config, pathToCheck string) {
-	info, err := os.Stat(pathToCheck)
-	var newTime time.Time
+// Check that a file has been modified since the last time it was checked. If
+// the mod time hasn't changed, then assume that Ninja got stuck, and print
+// diagnostics for debugging.
+func (c *ninjaStucknessChecker) check(ctx Context, config Config) {
+	info, err := os.Stat(c.logPath)
+	var newModTime time.Time
 	if err == nil {
-		newTime = info.ModTime()
+		newModTime = info.ModTime()
 	}
-	if newTime == c.prevTime {
-		// ninja may be stuck
-		dumpStucknessDiagnostics(ctx, config, pathToCheck, newTime)
+	if newModTime == c.prevModTime {
+		// The Ninja file hasn't been modified since the last time it was
+		// checked, so Ninja could be stuck. Output some diagnostics.
+		ctx.Verbosef("ninja may be stuck; last update to %v was %v. dumping process tree...", c.logPath, newModTime)
+
+		// The "pstree" command doesn't exist on Mac, but "pstree" on Linux
+		// gives more convenient output than "ps" So, we try pstree first, and
+		// ps second
+		commandText := fmt.Sprintf("pstree -pal %v || ps -ef", os.Getpid())
+
+		cmd := Command(ctx, config, "dump process tree", "bash", "-c", commandText)
+		output := cmd.CombinedOutputOrFatal()
+		ctx.Verbose(string(output))
+
+		ctx.Verbosef("done\n")
 	}
-	c.prevTime = newTime
-}
-
-// dumpStucknessDiagnostics gets called when it is suspected that Ninja is stuck and we want to output some diagnostics
-func dumpStucknessDiagnostics(ctx Context, config Config, statusPath string, lastUpdated time.Time) {
-
-	ctx.Verbosef("ninja may be stuck; last update to %v was %v. dumping process tree...", statusPath, lastUpdated)
-
-	// The "pstree" command doesn't exist on Mac, but "pstree" on Linux gives more convenient output than "ps"
-	// So, we try pstree first, and ps second
-	pstreeCommandText := fmt.Sprintf("pstree -pal %v", os.Getpid())
-	psCommandText := "ps -ef"
-	commandText := pstreeCommandText + " || " + psCommandText
-
-	cmd := Command(ctx, config, "dump process tree", "bash", "-c", commandText)
-	output := cmd.CombinedOutputOrFatal()
-	ctx.Verbose(string(output))
-
-	ctx.Verbosef("done\n")
+	c.prevModTime = newModTime
 }