Disable prebuilt apps without an apk later

Currently, android_app_import modules get disabled during their load
hook if they don't have a set `apk` property. This causes them to be
disabled before soong config variables can be applied, which would've
set the apk property.

Move the disabling into a DefaultableHook, which will run after the
soong config variables.

Bug: 319897584
Test: m nothing --no-skip-soong-tests
Change-Id: Ia0f6a39c35b3b11249bfa74ad532858189be24b1

android/prebuilt.go

@@ -277,6 +277,9 @@ func InitSingleSourcePrebuiltModule(module PrebuiltInterface, srcProps interface
 		}
 		value := srcPropsValue.FieldByIndex(srcFieldIndex)
 		if value.Kind() == reflect.Ptr {
+			if value.IsNil() {
+				return nil
+			}
 			value = value.Elem()
 		}
 		if value.Kind() != reflect.String {

android/soong_config_modules.go

@@ -41,6 +41,7 @@ func RegisterSoongConfigModuleBuildComponents(ctx RegistrationContext) {
 	ctx.RegisterModuleType("soong_config_module_type", SoongConfigModuleTypeFactory)
 	ctx.RegisterModuleType("soong_config_string_variable", SoongConfigStringVariableDummyFactory)
 	ctx.RegisterModuleType("soong_config_bool_variable", SoongConfigBoolVariableDummyFactory)
+	ctx.RegisterModuleType("soong_config_value_variable", SoongConfigValueVariableDummyFactory)
 }
 
 var PrepareForTestWithSoongConfigModuleBuildComponents = FixtureRegisterWithContext(RegisterSoongConfigModuleBuildComponents)
@@ -303,6 +304,11 @@ type soongConfigBoolVariableDummyModule struct {
 	properties soongconfig.VariableProperties
 }
 
+type soongConfigValueVariableDummyModule struct {
+	ModuleBase
+	properties soongconfig.VariableProperties
+}
+
 // soong_config_string_variable defines a variable and a set of possible string values for use
 // in a soong_config_module_type definition.
 func SoongConfigStringVariableDummyFactory() Module {
@@ -321,6 +327,15 @@ func SoongConfigBoolVariableDummyFactory() Module {
 	return module
 }
 
+// soong_config_value_variable defines a variable whose value can be expanded into
+// the value of a string property.
+func SoongConfigValueVariableDummyFactory() Module {
+	module := &soongConfigValueVariableDummyModule{}
+	module.AddProperties(&module.properties)
+	initAndroidModuleBase(module)
+	return module
+}
+
 func (m *soongConfigStringVariableDummyModule) Name() string {
 	return m.properties.Name + fmt.Sprintf("%p", m)
 }
@@ -333,6 +348,12 @@ func (m *soongConfigBoolVariableDummyModule) Name() string {
 func (*soongConfigBoolVariableDummyModule) Namespaceless()                                {}
 func (*soongConfigBoolVariableDummyModule) GenerateAndroidBuildActions(ctx ModuleContext) {}
 
+func (m *soongConfigValueVariableDummyModule) Name() string {
+	return m.properties.Name + fmt.Sprintf("%p", m)
+}
+func (*soongConfigValueVariableDummyModule) Namespaceless()                                {}
+func (*soongConfigValueVariableDummyModule) GenerateAndroidBuildActions(ctx ModuleContext) {}
+
 // importModuleTypes registers the module factories for a list of module types defined
 // in an Android.bp file. These module factories are scoped for the current Android.bp
 // file only.

java/app_import.go

@@ -151,7 +151,9 @@ func (a *AndroidAppImport) IsInstallable() bool {
 }
 
 // Updates properties with variant-specific values.
-func (a *AndroidAppImport) processVariants(ctx android.LoadHookContext) {
+// This happens as a DefaultableHook instead of a LoadHook because we want to run it after
+// soong config variables are applied.
+func (a *AndroidAppImport) processVariants(ctx android.DefaultableHookContext) {
 	config := ctx.Config()
 
 	dpiProps := reflect.ValueOf(a.dpiVariants).Elem().FieldByName("Dpi_variants")
@@ -543,7 +545,7 @@ func AndroidAppImportFactory() android.Module {
 	module.AddProperties(&module.dexpreoptProperties)
 	module.AddProperties(&module.usesLibrary.usesLibraryProperties)
 	module.populateAllVariantStructs()
-	android.AddLoadHook(module, func(ctx android.LoadHookContext) {
+	module.SetDefaultableHook(func(ctx android.DefaultableHookContext) {
 		module.processVariants(ctx)
 	})
 
@@ -594,7 +596,7 @@ func AndroidTestImportFactory() android.Module {
 	module.AddProperties(&module.dexpreoptProperties)
 	module.AddProperties(&module.testProperties)
 	module.populateAllVariantStructs()
-	android.AddLoadHook(module, func(ctx android.LoadHookContext) {
+	module.SetDefaultableHook(func(ctx android.DefaultableHookContext) {
 		module.processVariants(ctx)
 	})
 

java/app_import_test.go

@@ -410,6 +410,27 @@ func TestAndroidAppImport_ArchVariants(t *testing.T) {
 			artifactPath: "prebuilts/apk/app_arm64.apk",
 			installPath:  "/system/app/foo/foo.apk",
 		},
+		{
+			name: "matching arch without default",
+			bp: `
+				android_app_import {
+					name: "foo",
+					apk: "prebuilts/apk/app.apk",
+					arch: {
+						arm64: {
+							apk: "prebuilts/apk/app_arm64.apk",
+						},
+					},
+					presigned: true,
+					dex_preopt: {
+						enabled: true,
+					},
+				}
+			`,
+			expected:     "verify_uses_libraries/apk/app_arm64.apk",
+			artifactPath: "prebuilts/apk/app_arm64.apk",
+			installPath:  "/system/app/foo/foo.apk",
+		},
 		{
 			name: "no matching arch",
 			bp: `
@@ -454,26 +475,105 @@ func TestAndroidAppImport_ArchVariants(t *testing.T) {
 	}
 
 	for _, test := range testCases {
-		ctx, _ := testJava(t, test.bp)
+		t.Run(test.name, func(t *testing.T) {
+			ctx, _ := testJava(t, test.bp)
 
-		variant := ctx.ModuleForTests("foo", "android_common")
+			variant := ctx.ModuleForTests("foo", "android_common")
-		if test.expected == "" {
+			if test.expected == "" {
-			if variant.Module().Enabled() {
+				if variant.Module().Enabled() {
-				t.Error("module should have been disabled, but wasn't")
+					t.Error("module should have been disabled, but wasn't")
+				}
+				rule := variant.MaybeRule("genProvenanceMetaData")
+				android.AssertDeepEquals(t, "Provenance metadata is not empty", android.TestingBuildParams{}, rule)
+				return
 			}
-			rule := variant.MaybeRule("genProvenanceMetaData")
+			input := variant.Output("jnis-uncompressed/foo.apk").Input.String()
-			android.AssertDeepEquals(t, "Provenance metadata is not empty", android.TestingBuildParams{}, rule)
+			if strings.HasSuffix(input, test.expected) {
-			continue
+				t.Errorf("wrong src apk, expected: %q got: %q", test.expected, input)
-		}
+			}
-		input := variant.Output("jnis-uncompressed/foo.apk").Input.String()
+			rule := variant.Rule("genProvenanceMetaData")
-		if strings.HasSuffix(input, test.expected) {
+			android.AssertStringEquals(t, "Invalid input", test.artifactPath, rule.Inputs[0].String())
-			t.Errorf("wrong src apk, expected: %q got: %q", test.expected, input)
+			android.AssertStringEquals(t, "Invalid output", "out/soong/.intermediates/provenance_metadata/foo/provenance_metadata.textproto", rule.Output.String())
-		}
+			android.AssertStringEquals(t, "Invalid args", "foo", rule.Args["module_name"])
-		rule := variant.Rule("genProvenanceMetaData")
+			android.AssertStringEquals(t, "Invalid args", test.installPath, rule.Args["install_path"])
-		android.AssertStringEquals(t, "Invalid input", test.artifactPath, rule.Inputs[0].String())
+		})
-		android.AssertStringEquals(t, "Invalid output", "out/soong/.intermediates/provenance_metadata/foo/provenance_metadata.textproto", rule.Output.String())
+	}
-		android.AssertStringEquals(t, "Invalid args", "foo", rule.Args["module_name"])
+}
-		android.AssertStringEquals(t, "Invalid args", test.installPath, rule.Args["install_path"])
+
+func TestAndroidAppImport_SoongConfigVariables(t *testing.T) {
+	testCases := []struct {
+		name         string
+		bp           string
+		expected     string
+		artifactPath string
+		metaDataPath string
+		installPath  string
+	}{
+		{
+			name: "matching arch",
+			bp: `
+				soong_config_module_type {
+					name: "my_android_app_import",
+					module_type: "android_app_import",
+					config_namespace: "my_namespace",
+					value_variables: ["my_apk_var"],
+					properties: ["apk"],
+				}
+				soong_config_value_variable {
+					name: "my_apk_var",
+				}
+				my_android_app_import {
+					name: "foo",
+					soong_config_variables: {
+						my_apk_var: {
+							apk: "prebuilts/apk/%s.apk",
+						},
+					},
+					presigned: true,
+					dex_preopt: {
+						enabled: true,
+					},
+				}
+			`,
+			expected:     "verify_uses_libraries/apk/name_from_soong_config.apk",
+			artifactPath: "prebuilts/apk/name_from_soong_config.apk",
+			installPath:  "/system/app/foo/foo.apk",
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := android.GroupFixturePreparers(
+				prepareForJavaTest,
+				android.PrepareForTestWithSoongConfigModuleBuildComponents,
+				android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) {
+					variables.VendorVars = map[string]map[string]string{
+						"my_namespace": {
+							"my_apk_var": "name_from_soong_config",
+						},
+					}
+				}),
+			).RunTestWithBp(t, test.bp).TestContext
+
+			variant := ctx.ModuleForTests("foo", "android_common")
+			if test.expected == "" {
+				if variant.Module().Enabled() {
+					t.Error("module should have been disabled, but wasn't")
+				}
+				rule := variant.MaybeRule("genProvenanceMetaData")
+				android.AssertDeepEquals(t, "Provenance metadata is not empty", android.TestingBuildParams{}, rule)
+				return
+			}
+			input := variant.Output("jnis-uncompressed/foo.apk").Input.String()
+			if strings.HasSuffix(input, test.expected) {
+				t.Errorf("wrong src apk, expected: %q got: %q", test.expected, input)
+			}
+			rule := variant.Rule("genProvenanceMetaData")
+			android.AssertStringEquals(t, "Invalid input", test.artifactPath, rule.Inputs[0].String())
+			android.AssertStringEquals(t, "Invalid output", "out/soong/.intermediates/provenance_metadata/foo/provenance_metadata.textproto", rule.Output.String())
+			android.AssertStringEquals(t, "Invalid args", "foo", rule.Args["module_name"])
+			android.AssertStringEquals(t, "Invalid args", test.installPath, rule.Args["install_path"])
+		})
 	}
 }