From 97494b197e8f59a60baf693e5053b3cb72b2e359 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Fri, 12 Jan 2024 14:02:47 -0800 Subject: [PATCH] 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 | 3 + android/soong_config_modules.go | 21 +++++ java/app_import.go | 8 +- java/app_import_test.go | 136 +++++++++++++++++++++++++++----- 4 files changed, 147 insertions(+), 21 deletions(-) diff --git a/android/prebuilt.go b/android/prebuilt.go index d2b8fa13f..a94f5b75a 100644 --- a/android/prebuilt.go +++ b/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 { diff --git a/android/soong_config_modules.go b/android/soong_config_modules.go index ec0edfd73..90b49eb19 100644 --- a/android/soong_config_modules.go +++ b/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. diff --git a/java/app_import.go b/java/app_import.go index 997274c93..5f20fdd05 100644 --- a/java/app_import.go +++ b/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) }) diff --git a/java/app_import_test.go b/java/app_import_test.go index ef4626e26..44f8f1644 100644 --- a/java/app_import_test.go +++ b/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") - if test.expected == "" { - if variant.Module().Enabled() { - t.Error("module should have been disabled, but wasn't") + 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 } - rule := variant.MaybeRule("genProvenanceMetaData") - android.AssertDeepEquals(t, "Provenance metadata is not empty", android.TestingBuildParams{}, rule) - continue - } - 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"]) + 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"]) + }) + } +} + +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"]) + }) } }