Merge "Fix override_android_app dependency issues."

am: 105920a792

Change-Id: I288d99d717ff1b048e0677e3777e8061a1c4f063

android/mutator.go

@@ -77,7 +77,6 @@ var preArch = []RegisterMutatorFunc{
 	RegisterNamespaceMutator,
 	RegisterPrebuiltsPreArchMutators,
 	RegisterDefaultsPreArchMutators,
-	RegisterOverridePreArchMutators,
 	registerVisibilityRuleGatherer,
 }
 
@@ -95,6 +94,7 @@ var postDeps = []RegisterMutatorFunc{
 	RegisterPrebuiltsPostDepsMutators,
 	registerVisibilityRuleEnforcer,
 	registerNeverallowMutator,
+	RegisterOverridePostDepsMutators,
 }
 
 func PreArchMutators(f RegisterMutatorFunc) {

android/override_module.go

@@ -84,8 +84,13 @@ type OverridableModule interface {
 	getOverrides() []OverrideModule
 
 	override(ctx BaseModuleContext, o OverrideModule)
+	getOverriddenBy() string
 
 	setOverridesProperty(overridesProperties *[]string)
+
+	// Due to complications with incoming dependencies, overrides are processed after DepsMutator.
+	// So, overridable properties need to be handled in a separate, dedicated deps mutator.
+	OverridablePropertiesDepsMutator(ctx BottomUpMutatorContext)
 }
 
 // Base module struct for overridable module types
@@ -106,6 +111,8 @@ type OverridableModuleBase struct {
 	// set this to a pointer to the property through the InitOverridableModule function, so that
 	// override information is propagated and aggregated correctly.
 	overridesProperty *[]string
+
+	overriddenBy string
 }
 
 func InitOverridableModule(m OverridableModule, overridesProperty *[]string) {
@@ -153,14 +160,23 @@ func (b *OverridableModuleBase) override(ctx BaseModuleContext, o OverrideModule
 			}
 		}
 	}
+	b.overriddenBy = o.Name()
+}
+
+func (b *OverridableModuleBase) getOverriddenBy() string {
+	return b.overriddenBy
+}
+
+func (b *OverridableModuleBase) OverridablePropertiesDepsMutator(ctx BottomUpMutatorContext) {
 }
 
 // Mutators for override/overridable modules. All the fun happens in these functions. It is critical
 // to keep them in this order and not put any order mutators between them.
-func RegisterOverridePreArchMutators(ctx RegisterMutatorsContext) {
+func RegisterOverridePostDepsMutators(ctx RegisterMutatorsContext) {
 	ctx.BottomUp("override_deps", overrideModuleDepsMutator).Parallel()
 	ctx.TopDown("register_override", registerOverrideMutator).Parallel()
 	ctx.BottomUp("perform_override", performOverrideMutator).Parallel()
+	ctx.BottomUp("overridable_deps", overridableModuleDepsMutator).Parallel()
 }
 
 type overrideBaseDependencyTag struct {
@@ -207,5 +223,22 @@ func performOverrideMutator(ctx BottomUpMutatorContext) {
 		for i, o := range overrides {
 			mods[i+1].(OverridableModule).override(ctx, o)
 		}
+	} else if o, ok := ctx.Module().(OverrideModule); ok {
+		// Create a variant of the overriding module with its own name. This matches the above local
+		// variant name rule for overridden modules, and thus allows ReplaceDependencies to match the
+		// two.
+		ctx.CreateLocalVariations(o.Name())
+	}
+}
+
+func overridableModuleDepsMutator(ctx BottomUpMutatorContext) {
+	if b, ok := ctx.Module().(OverridableModule); ok {
+		if o := b.getOverriddenBy(); o != "" {
+			// Redirect dependencies on the overriding module to this overridden module. Overriding
+			// modules are basically pseudo modules, and all build actions are associated to overridden
+			// modules. Therefore, dependencies on overriding modules need to be forwarded there as well.
+			ctx.ReplaceDependencies(o)
+		}
+		b.OverridablePropertiesDepsMutator(ctx)
 	}
 }

java/app.go

@@ -163,7 +163,9 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) {
 		}
 		ctx.AddFarVariationDependencies(variation, tag, a.appProperties.Jni_libs...)
 	}
+}
 
+func (a *AndroidApp) OverridablePropertiesDepsMutator(ctx android.BottomUpMutatorContext) {
 	cert := android.SrcIsModule(a.getCertString(ctx))
 	if cert != "" {
 		ctx.AddDependency(ctx.Module(), certificateTag, cert)
@@ -632,7 +634,7 @@ func OverrideAndroidAppModuleFactory() android.Module {
 	m := &OverrideAndroidApp{}
 	m.AddProperties(&overridableAppProperties{})
 
-	android.InitAndroidModule(m)
+	android.InitAndroidMultiTargetsArchModule(m, android.DeviceSupported, android.MultilibCommon)
 	android.InitOverrideModule(m)
 	return m
 }

java/app_test.go

@@ -881,7 +881,7 @@ func TestOverrideAndroidApp(t *testing.T) {
 			name: "foo",
 			srcs: ["a.java"],
 			certificate: "expiredkey",
-			overrides: ["baz"],
+			overrides: ["qux"],
 		}
 
 		override_android_app {
@@ -903,6 +903,7 @@ func TestOverrideAndroidApp(t *testing.T) {
 		`)
 
 	expectedVariants := []struct {
+		moduleName  string
 		variantName string
 		apkName     string
 		apkPath     string
@@ -911,24 +912,27 @@ func TestOverrideAndroidApp(t *testing.T) {
 		aaptFlag    string
 	}{
 		{
+			moduleName:  "foo",
 			variantName: "android_common",
 			apkPath:     "/target/product/test_device/system/app/foo/foo.apk",
 			signFlag:    "build/make/target/product/security/expiredkey.x509.pem build/make/target/product/security/expiredkey.pk8",
-			overrides:   []string{"baz"},
+			overrides:   []string{"qux"},
 			aaptFlag:    "",
 		},
 		{
-			variantName: "bar_android_common",
+			moduleName:  "bar",
+			variantName: "android_common_bar",
 			apkPath:     "/target/product/test_device/system/app/bar/bar.apk",
 			signFlag:    "cert/new_cert.x509.pem cert/new_cert.pk8",
-			overrides:   []string{"baz", "foo"},
+			overrides:   []string{"qux", "foo"},
 			aaptFlag:    "",
 		},
 		{
-			variantName: "baz_android_common",
+			moduleName:  "baz",
+			variantName: "android_common_baz",
 			apkPath:     "/target/product/test_device/system/app/baz/baz.apk",
 			signFlag:    "build/make/target/product/security/expiredkey.x509.pem build/make/target/product/security/expiredkey.pk8",
-			overrides:   []string{"baz", "foo"},
+			overrides:   []string{"qux", "foo"},
 			aaptFlag:    "--rename-manifest-package org.dandroid.bp",
 		},
 	}
@@ -972,6 +976,47 @@ func TestOverrideAndroidApp(t *testing.T) {
 	}
 }
 
+func TestOverrideAndroidAppDependency(t *testing.T) {
+	ctx := testJava(t, `
+		android_app {
+			name: "foo",
+			srcs: ["a.java"],
+		}
+
+		override_android_app {
+			name: "bar",
+			base: "foo",
+			package_name: "org.dandroid.bp",
+		}
+
+		android_test {
+			name: "baz",
+			srcs: ["b.java"],
+			instrumentation_for: "foo",
+		}
+
+		android_test {
+			name: "qux",
+			srcs: ["b.java"],
+			instrumentation_for: "bar",
+		}
+		`)
+
+	// Verify baz, which depends on the overridden module foo, has the correct classpath javac arg.
+	javac := ctx.ModuleForTests("baz", "android_common").Rule("javac")
+	fooTurbine := filepath.Join(buildDir, ".intermediates", "foo", "android_common", "turbine-combined", "foo.jar")
+	if !strings.Contains(javac.Args["classpath"], fooTurbine) {
+		t.Errorf("baz classpath %v does not contain %q", javac.Args["classpath"], fooTurbine)
+	}
+
+	// Verify qux, which depends on the overriding module bar, has the correct classpath javac arg.
+	javac = ctx.ModuleForTests("qux", "android_common").Rule("javac")
+	barTurbine := filepath.Join(buildDir, ".intermediates", "foo", "android_common_bar", "turbine-combined", "foo.jar")
+	if !strings.Contains(javac.Args["classpath"], barTurbine) {
+		t.Errorf("qux classpath %v does not contain %q", javac.Args["classpath"], barTurbine)
+	}
+}
+
 func TestAndroidAppImport(t *testing.T) {
 	ctx := testJava(t, `
 		android_app_import {

java/java_test.go

@@ -93,10 +93,10 @@ func testContext(config android.Config, bp string,
 	ctx.PreArchMutators(android.RegisterPrebuiltsPreArchMutators)
 	ctx.PreArchMutators(android.RegisterPrebuiltsPostDepsMutators)
 	ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators)
-	ctx.PreArchMutators(android.RegisterOverridePreArchMutators)
 	ctx.PreArchMutators(func(ctx android.RegisterMutatorsContext) {
 		ctx.TopDown("prebuilt_apis", PrebuiltApisMutator).Parallel()
 	})
+	ctx.PostDepsMutators(android.RegisterOverridePostDepsMutators)
 	ctx.RegisterPreSingletonType("overlay", android.SingletonFactoryAdaptor(OverlaySingletonFactory))
 	ctx.RegisterPreSingletonType("sdk_versions", android.SingletonFactoryAdaptor(sdkPreSingletonFactory))