Don't create unnecessary APEX variations

This change fixes a problem that APEX variations are created for the
modules that actually shouldn't built for any APEX. For example,
consider this case.

apex { name: "myapex", native_shared_libs: ["mylib"],}

cc_library { name: "mylib", shared_libs: ["libfoo#10"],}
cc_library { name: "libfoo",
             shared_libs: ["libbar"],
             stubs: { versions: ["10"], }, }
cc_library { name: "libbar", ...}

Before this change, both the stubs and non-stubs variations of libfoo
were mutated with apexMuator, which is incorrect for the non-stubs
varia; there is no dependency chain from the apex "myapex" to the
non-stubs variation, but to the stubs variation due to the #10 syntax.

This was happening becauses we used the name of the module to determine
whether it should be built for APEX or not. Both stubs and non-stubs
variations have the same module name "libfoo".

Fixing this issue by recording the list of APEX variations required
directly on the module. So, the stubs variation of libfoo has myapex in
its apex variations list, but the non-stubs variation doesn't, and thus
apexMutator does not pick up the non-stubs variation.

Test: m (apex_test updated and passing)
Test: cherry-pick ag/5747464 and m
Change-Id: I31e618626809a828a55fff513ef5f81f79637afa

1 files changed, 109 insertions, 60 deletions

diff --git a/android/apex.go b/android/apex.go
index 9d65c082..d1fb6f4e 100644
--- a/android/apex.go
+++ b/android/apex.go
@@ -14,7 +14,11 @@
 
 package android
 
-import "sync"
+import (
+	"sync"
+
+	"github.com/google/blueprint"
+)
 
 // 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
@@ -25,7 +29,7 @@ import "sync"
 // or C APIs from other APEXs.
 //
 // A module implementing this interface will be mutated into multiple
-// variations by the apex mutator if it is directly or indirectly included
+// 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
@@ -35,31 +39,44 @@ type ApexModule interface {
 	Module
 	apexModuleBase() *ApexModuleBase
 
-	// Marks that this module should be built for the APEX of the specified name
+	// Marks that this module should be built for the APEX of the specified name.
+	// Call this before apex.apexMutator is run.
 	BuildForApex(apexName string)
 
-	// Tests whether this module will be built for the platform or not (= APEXs)
-	IsForPlatform() bool
-
 	// Returns the name of APEX that this module will be built for. Empty string
 	// is returned when 'IsForPlatform() == true'. Note that a module can be
-	// included to multiple APEXs, in which case, the module is mutated into
+	// included in multiple APEXes, in which case, the module is mutated into
 	// multiple modules each of which for an APEX. This method returns the
 	// name of the APEX that a variant module is for.
+	// Call this after apex.apexMutator is run.
 	ApexName() string
 
-	// Tests if this module can have APEX variants. APEX variants are
+	// Tests whether this module will be built for the platform or not.
+	// This is a shortcut for ApexName() == ""
+	IsForPlatform() 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 shared libraries such as NDK stubs.
+	// 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.
 	IsInstallableToApex() bool
+
+	// Mutate this module into one or more variants each of which is built
+	// for an APEX marked via BuildForApex().
+	CreateApexVariations(mctx BottomUpMutatorContext) []blueprint.Module
+
+	// Sets the name of the apex variant of this module. Called inside
+	// CreateApexVariations.
+	setApexName(apexName string)
 }
 
 type ApexProperties struct {
+	// Name of the apex variant that this module is mutated into
 	ApexName string `blueprint:"mutated"`
 }
 
@@ -69,6 +86,7 @@ type ApexModuleBase struct {
 	ApexProperties ApexProperties
 
 	canHaveApexVariants bool
+	apexVariations      []string
 }
 
 func (m *ApexModuleBase) apexModuleBase() *ApexModuleBase {
@@ -76,15 +94,21 @@ func (m *ApexModuleBase) apexModuleBase() *ApexModuleBase {
 }
 
 func (m *ApexModuleBase) BuildForApex(apexName string) {
-	m.ApexProperties.ApexName = apexName
+	if !InList(apexName, m.apexVariations) {
+		m.apexVariations = append(m.apexVariations, apexName)
+	}
+}
+
+func (m *ApexModuleBase) ApexName() string {
+	return m.ApexProperties.ApexName
 }
 
 func (m *ApexModuleBase) IsForPlatform() bool {
 	return m.ApexProperties.ApexName == ""
 }
 
-func (m *ApexModuleBase) ApexName() string {
-	return m.ApexProperties.ApexName
+func (m *ApexModuleBase) setApexName(apexName string) {
+	m.ApexProperties.ApexName = apexName
 }
 
 func (m *ApexModuleBase) CanHaveApexVariants() bool {
@@ -96,61 +120,73 @@ func (m *ApexModuleBase) IsInstallableToApex() bool {
 	return false
 }
 
-// This structure maps a module name to the set of APEX bundle names that the module
-// should be built for. Examples:
+func (m *ApexModuleBase) CreateApexVariations(mctx BottomUpMutatorContext) []blueprint.Module {
+	if len(m.apexVariations) > 0 {
+		// The original module is mutated into "platform" variation.
+		variations := []string{"platform"}
+		for _, a := range m.apexVariations {
+			variations = append(variations, a)
+		}
+		modules := mctx.CreateVariations(variations...)
+		for i, m := range modules {
+			if i == 0 {
+				continue // platform
+			}
+			m.(ApexModule).setApexName(variations[i])
+		}
+		return modules
+	}
+	return nil
+}
+
+var apexData OncePer
+var apexNamesMapMutex sync.Mutex
+
+// This structure maintains the global mapping in between modules and APEXes.
+// Examples:
 //
-// ...["foo"]["bar"] == true: module foo is directly depended on by APEX bar
-// ...["foo"]["bar"] == false: module foo is indirectly depended on by APEX bar
-// ...["foo"]["bar"] doesn't exist: foo is not built for APEX bar
-// ...["foo"] doesn't exist: foo is not built for any APEX
-func apexBundleNamesMap(config Config) map[string]map[string]bool {
-	return config.Once("apexBundleNames", func() interface{} {
+// apexNamesMap()["foo"]["bar"] == true: module foo is directly depended on by APEX bar
+// apexNamesMap()["foo"]["bar"] == false: module foo is indirectly depended on by APEX bar
+// apexNamesMap()["foo"]["bar"] doesn't exist: foo is not built for APEX bar
+func apexNamesMap() map[string]map[string]bool {
+	return apexData.Once("apexNames", func() interface{} {
 		return make(map[string]map[string]bool)
 	}).(map[string]map[string]bool)
 }
 
-var bundleNamesMapMutex sync.Mutex
-
-// Mark that a module named moduleName should be built for an apex named bundleName
-// directDep should be set to true if the module is a direct dependency of the apex.
-func BuildModuleForApexBundle(ctx BaseModuleContext, moduleName string, bundleName string, directDep bool) {
-	bundleNamesMapMutex.Lock()
-	defer bundleNamesMapMutex.Unlock()
-	bundleNames, ok := apexBundleNamesMap(ctx.Config())[moduleName]
+// Update the map to mark that a module named moduleName is directly or indirectly
+// depended on by an APEX named apexName. Directly depending means that a module
+// is explicitly listed in the build definition of the APEX via properties like
+// native_shared_libs, java_libs, etc.
+func UpdateApexDependency(apexName string, moduleName string, directDep bool) {
+	apexNamesMapMutex.Lock()
+	defer apexNamesMapMutex.Unlock()
+	apexNames, ok := apexNamesMap()[moduleName]
 	if !ok {
-		bundleNames = make(map[string]bool)
-		apexBundleNamesMap(ctx.Config())[moduleName] = bundleNames
+		apexNames = make(map[string]bool)
+		apexNamesMap()[moduleName] = apexNames
 	}
-	bundleNames[bundleName] = bundleNames[bundleName] || directDep
-}
-
-// Returns the list of apex bundle names that the module named moduleName
-// should be built for.
-func GetApexBundlesForModule(ctx BaseModuleContext, moduleName string) map[string]bool {
-	bundleNamesMapMutex.Lock()
-	defer bundleNamesMapMutex.Unlock()
-	return apexBundleNamesMap(ctx.Config())[moduleName]
+	apexNames[apexName] = apexNames[apexName] || directDep
 }
 
-// Tests if moduleName is directly depended on by bundleName (i.e. referenced in
-// native_shared_libs, etc.)
-func DirectlyInApex(config Config, bundleName string, moduleName string) bool {
-	bundleNamesMapMutex.Lock()
-	defer bundleNamesMapMutex.Unlock()
-	if bundleNames, ok := apexBundleNamesMap(config)[moduleName]; ok {
-		return bundleNames[bundleName]
+// Tests whether a module named moduleName is directly depended on by an APEX
+// named apexName.
+func DirectlyInApex(apexName string, moduleName string) bool {
+	apexNamesMapMutex.Lock()
+	defer apexNamesMapMutex.Unlock()
+	if apexNames, ok := apexNamesMap()[moduleName]; ok {
+		return apexNames[apexName]
 	}
 	return false
 }
 
-// Tests if moduleName is directly depended on by any APEX. If this returns true,
-// that means the module is part of the platform.
-func DirectlyInAnyApex(config Config, moduleName string) bool {
-	bundleNamesMapMutex.Lock()
-	defer bundleNamesMapMutex.Unlock()
-	if bundleNames, ok := apexBundleNamesMap(config)[moduleName]; ok {
-		for bn := range bundleNames {
-			if bundleNames[bn] {
+// Tests whether a module named moduleName is directly depended on by any APEX.
+func DirectlyInAnyApex(moduleName string) bool {
+	apexNamesMapMutex.Lock()
+	defer apexNamesMapMutex.Unlock()
+	if apexNames, ok := apexNamesMap()[moduleName]; ok {
+		for an := range apexNames {
+			if apexNames[an] {
 				return true
 			}
 		}
@@ -158,12 +194,25 @@ func DirectlyInAnyApex(config Config, moduleName string) bool {
 	return false
 }
 
-// Tests if moduleName is included in any APEX.
-func InAnyApex(config Config, moduleName string) bool {
-	bundleNamesMapMutex.Lock()
-	defer bundleNamesMapMutex.Unlock()
-	bundleNames, ok := apexBundleNamesMap(config)[moduleName]
-	return ok && len(bundleNames) > 0
+// Tests whether a module named module is depended on (including both
+// direct and indirect dependencies) by any APEX.
+func InAnyApex(moduleName string) bool {
+	apexNamesMapMutex.Lock()
+	defer apexNamesMapMutex.Unlock()
+	apexNames, ok := apexNamesMap()[moduleName]
+	return ok && len(apexNames) > 0
+}
+
+func GetApexesForModule(moduleName string) []string {
+	ret := []string{}
+	apexNamesMapMutex.Lock()
+	defer apexNamesMapMutex.Unlock()
+	if apexNames, ok := apexNamesMap()[moduleName]; ok {
+		for an := range apexNames {
+			ret = append(ret, an)
+		}
+	}
+	return ret
 }
 
 func InitApexModule(m ApexModule) {