Fix inconsistencies in the apex names used for the apex variations.

Add an apex_name property to prebuilt APEX modules to allow specifying
the "runtime" name of the APEX, i.e. the one it gets mounted as in /apex/,
which is also the one used for the apex variations.

Introduce a callback to retrieve that name consistently for all APEX
modules (apex, override_apex, prebuilt_apex, and apex_set), and update
some apex mutator code paths to use it.

For APEX source modules (apex and override_apex), it's necessary to add
a new field in apexBundle, since the name property gets overridden for
the override variant that override_apex creates.

Cherry-picked from https://r.android.com/1748294.

Test: m nothing
Bug: 191269918
Change-Id: If8612639bffdf91cbcab3387b0603bf5dffef1f5
Merged-In: If8612639bffdf91cbcab3387b0603bf5dffef1f5

3 files changed, 90 insertions, 22 deletions

diff --git a/apex/apex.go b/apex/apex.go
index e731702ff..bea54bccc 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -177,6 +177,12 @@ type apexBundleProperties struct {
 	// used in tests.
 	Test_only_force_compression *bool
 
+	// Canonical name of this APEX bundle. Used to determine the path to the
+	// activated APEX on device (i.e. /apex/<apexVariationName>), and used for the
+	// apex mutator variations. For override_apex modules, this is the name of the
+	// overridden base module.
+	ApexVariationName string `blueprint:"mutated"`
+
 	IsCoverageVariant bool `blueprint:"mutated"`
 
 	// List of sanitizer names that this APEX is enabled for
@@ -808,6 +814,10 @@ var ApexBundleInfoProvider = blueprint.NewMutatorProvider(ApexBundleInfo{}, "ape
 
 var _ ApexInfoMutator = (*apexBundle)(nil)
 
+func (a *apexBundle) ApexVariationName() string {
+	return a.properties.ApexVariationName
+}
+
 // ApexInfoMutator 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
@@ -896,15 +906,15 @@ func (a *apexBundle) ApexInfoMutator(mctx android.TopDownMutatorContext) {
 	// This is the main part of this mutator. Mark the collected dependencies that they need to
 	// be built for this apexBundle.
 
-	// Note that there are many different names.
-	// ApexVariationName: this is the name of the apex variation
+	apexVariationName := proptools.StringDefault(a.properties.Apex_name, mctx.ModuleName()) // could be com.android.foo
+	a.properties.ApexVariationName = apexVariationName
 	apexInfo := android.ApexInfo{
-		ApexVariationName: mctx.ModuleName(), // could be com.android.foo
+		ApexVariationName: apexVariationName,
 		MinSdkVersion:     minSdkVersion,
 		RequiredSdks:      a.RequiredSdks(),
 		Updatable:         a.Updatable(),
-		InApexVariants:    []string{mctx.ModuleName()}, // could be com.android.foo
-		InApexModules:     []string{a.Name()},          // could be com.mycompany.android.foo
+		InApexVariants:    []string{apexVariationName},
+		InApexModules:     []string{a.Name()}, // could be com.mycompany.android.foo
 		ApexContents:      []*android.ApexContents{apexContents},
 	}
 	mctx.WalkDeps(func(child, parent android.Module) bool {
@@ -917,6 +927,10 @@ func (a *apexBundle) ApexInfoMutator(mctx android.TopDownMutatorContext) {
 }
 
 type ApexInfoMutator interface {
+	// ApexVariationName returns the name of the APEX variation to use in the apex
+	// mutator etc. It is the same name as ApexInfo.ApexVariationName.
+	ApexVariationName() string
+
 	// ApexInfoMutator implementations must call BuildForApex(ApexInfo) on any modules that are
 	// depended upon by an apex and which require an apex specific variant.
 	ApexInfoMutator(android.TopDownMutatorContext)
@@ -1042,10 +1056,8 @@ func apexMutator(mctx android.BottomUpMutatorContext) {
 	}
 
 	// 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.
-	unprefixedModuleName := android.RemoveOptionalPrebuiltPrefix(mctx.ModuleName())
-	if apexModuleTypeRequiresVariant(mctx.Module()) {
-		apexBundleName := unprefixedModuleName
+	if ai, ok := mctx.Module().(ApexInfoMutator); ok && apexModuleTypeRequiresVariant(ai) {
+		apexBundleName := ai.ApexVariationName()
 		mctx.CreateVariations(apexBundleName)
 		if strings.HasPrefix(apexBundleName, "com.android.art") {
 			// Create an alias from the platform variant. This is done to make
@@ -1068,6 +1080,7 @@ func apexMutator(mctx android.BottomUpMutatorContext) {
 		// apex variant name. This name matches the name used to create the variations of modules for
 		// which apexModuleTypeRequiresVariant return true.
 		// TODO(b/191269918): Remove this workaround.
+		unprefixedModuleName := android.RemoveOptionalPrebuiltPrefix(mctx.ModuleName())
 		mctx.SetDefaultDependencyVariation(&unprefixedModuleName)
 		mctx.CreateVariations(apexBundleName)
 		if strings.HasPrefix(apexBundleName, "com.android.art") {
@@ -1079,18 +1092,13 @@ func apexMutator(mctx android.BottomUpMutatorContext) {
 
 // apexModuleTypeRequiresVariant determines whether the module supplied requires an apex specific
 // variant.
-func apexModuleTypeRequiresVariant(module android.Module) bool {
+func apexModuleTypeRequiresVariant(module ApexInfoMutator) bool {
 	if a, ok := module.(*apexBundle); ok {
+		// TODO(jiyong): document the reason why the VNDK APEX is an exception here.
 		return !a.vndkApex
 	}
 
-	// Match apex_set and prebuilt_apex. Would also match apexBundle but that is handled specially
-	// above.
-	if _, ok := module.(ApexInfoMutator); ok {
-		return true
-	}
-
-	return false
+	return true
 }
 
 // See android.UpdateDirectlyInAnyApex
diff --git a/apex/apex_test.go b/apex/apex_test.go
index c50c4a94f..4829029b3 100644
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -3670,13 +3670,13 @@ func TestApexName(t *testing.T) {
 		}
 	`)
 
-	module := ctx.ModuleForTests("myapex", "android_common_myapex_image")
+	module := ctx.ModuleForTests("myapex", "android_common_com.android.myapex_image")
 	apexManifestRule := module.Rule("apexManifestRule")
 	ensureContains(t, apexManifestRule.Args["opt"], "-v name com.android.myapex")
 	apexRule := module.Rule("apexRule")
 	ensureContains(t, apexRule.Args["opt_flags"], "--do_not_check_keyname")
 
-	apexBundle := ctx.ModuleForTests("myapex", "android_common_myapex_image").Module().(*apexBundle)
+	apexBundle := module.Module().(*apexBundle)
 	data := android.AndroidMkDataForTest(t, ctx, apexBundle)
 	name := apexBundle.BaseModuleName()
 	prefix := "TARGET_"
@@ -4219,6 +4219,59 @@ func TestPrebuiltOverrides(t *testing.T) {
 	}
 }
 
+func TestPrebuiltApexName(t *testing.T) {
+	testApex(t, `
+		prebuilt_apex {
+			name: "com.company.android.myapex",
+			apex_name: "com.android.myapex",
+			src: "company-myapex-arm.apex",
+		}
+	`).ModuleForTests("com.company.android.myapex", "android_common_com.android.myapex")
+
+	testApex(t, `
+		apex_set {
+			name: "com.company.android.myapex",
+			apex_name: "com.android.myapex",
+			set: "company-myapex.apks",
+		}
+	`).ModuleForTests("com.company.android.myapex", "android_common_com.android.myapex")
+}
+
+func TestPrebuiltApexNameWithPlatformBootclasspath(t *testing.T) {
+	_ = android.GroupFixturePreparers(
+		java.PrepareForTestWithJavaDefaultModules,
+		PrepareForTestWithApexBuildComponents,
+		android.FixtureWithRootAndroidBp(`
+			platform_bootclasspath {
+				name: "platform-bootclasspath",
+				fragments: [
+					{
+						apex: "com.android.art",
+						module: "art-bootclasspath-fragment",
+					},
+				],
+			}
+
+			prebuilt_apex {
+				name: "com.company.android.art",
+				apex_name: "com.android.art",
+				src: "com.company.android.art-arm.apex",
+				exported_bootclasspath_fragments: ["art-bootclasspath-fragment"],
+			}
+
+			prebuilt_bootclasspath_fragment {
+				name: "art-bootclasspath-fragment",
+				contents: ["core-oj"],
+			}
+
+			java_import {
+				name: "core-oj",
+				jars: ["prebuilt.jar"],
+			}
+		`),
+	).RunTest(t)
+}
+
 // These tests verify that the prebuilt_apex/deapexer to java_import wiring allows for the
 // propagation of paths to dex implementation jars from the former to the latter.
 func TestPrebuiltExportDexImplementationJars(t *testing.T) {
diff --git a/apex/prebuilt.go b/apex/prebuilt.go
index 9504b0797..548cf4c89 100644
--- a/apex/prebuilt.go
+++ b/apex/prebuilt.go
@@ -23,7 +23,6 @@ import (
 	"android/soong/android"
 	"android/soong/java"
 	"github.com/google/blueprint"
-
 	"github.com/google/blueprint/proptools"
 )
 
@@ -75,6 +74,10 @@ type sanitizedPrebuilt interface {
 type PrebuiltCommonProperties struct {
 	SelectedApexProperties
 
+	// Canonical name of this APEX. Used to determine the path to the activated APEX on
+	// device (/apex/<apex_name>). If unspecified, follows the name property.
+	Apex_name *string
+
 	ForceDisable bool `blueprint:"mutated"`
 
 	// whether the extracted apex file is installable.
@@ -109,6 +112,10 @@ func (p *prebuiltCommon) initPrebuiltCommon(module android.Module, properties *P
 	android.InitAndroidMultiTargetsArchModule(module, android.DeviceSupported, android.MultilibCommon)
 }
 
+func (p *prebuiltCommon) ApexVariationName() string {
+	return proptools.StringDefault(p.prebuiltCommonProperties.Apex_name, p.ModuleBase.BaseModuleName())
+}
+
 func (p *prebuiltCommon) Prebuilt() *android.Prebuilt {
 	return &p.prebuilt
 }
@@ -389,11 +396,11 @@ func (p *prebuiltCommon) apexInfoMutator(mctx android.TopDownMutatorContext) {
 	})
 
 	// Create an ApexInfo for the prebuilt_apex.
-	apexVariationName := android.RemoveOptionalPrebuiltPrefix(mctx.ModuleName())
+	apexVariationName := p.ApexVariationName()
 	apexInfo := android.ApexInfo{
 		ApexVariationName: apexVariationName,
 		InApexVariants:    []string{apexVariationName},
-		InApexModules:     []string{apexVariationName},
+		InApexModules:     []string{p.ModuleBase.BaseModuleName()}, // BaseModuleName() to avoid the prebuilt_ prefix.
 		ApexContents:      []*android.ApexContents{apexContents},
 		ForPrebuiltApex:   true,
 	}