From 315a53c5cb5af5d354a56f5229f546e7ed9ff1c1 Mon Sep 17 00:00:00 2001 From: Yu Liu Date: Wed, 24 Apr 2024 16:41:57 +0000 Subject: [PATCH] Make container mandatory in aconfig_declarations. Bug: 330354107 Test: Unit test and CI. Ignore-AOSP-First: It is easier to detect all the missing ones in internal master. (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e916a2c758d2a95037d1d366e7cd0e10d241d510) Merged-In: I4ab4271c67a35d0fdcc0b57c27260e29fb7dea56 Change-Id: I4ab4271c67a35d0fdcc0b57c27260e29fb7dea56 --- aconfig/aconfig_declarations.go | 5 +- aconfig/aconfig_declarations_test.go | 60 ++++++++++++++----- .../aconfig_declarations_group_test.go | 5 +- aconfig/codegen/cc_aconfig_library_test.go | 4 ++ aconfig/codegen/java_aconfig_library_test.go | 55 ++++++++++++++++- aconfig/codegen/rust_aconfig_library_test.go | 3 + apex/apex_test.go | 1 + java/aar_test.go | 5 +- java/app_test.go | 2 + java/droidstubs_test.go | 2 + java/java_test.go | 1 + java/rro_test.go | 2 + java/sdk_library_test.go | 1 + 13 files changed, 126 insertions(+), 20 deletions(-) diff --git a/aconfig/aconfig_declarations.go b/aconfig/aconfig_declarations.go index d29e3122d..71a64dddc 100644 --- a/aconfig/aconfig_declarations.go +++ b/aconfig/aconfig_declarations.go @@ -73,8 +73,9 @@ func (module *DeclarationsModule) DepsMutator(ctx android.BottomUpMutatorContext if len(module.properties.Package) == 0 { ctx.PropertyErrorf("package", "missing package property") } - // TODO(b/311155208): Add mandatory check for container after all pre-existing - // ones are changed. + if len(module.properties.Container) == 0 { + ctx.PropertyErrorf("container", "missing container property") + } // Add a dependency on the aconfig_value_sets defined in // RELEASE_ACONFIG_VALUE_SETS, and add any aconfig_values that diff --git a/aconfig/aconfig_declarations_test.go b/aconfig/aconfig_declarations_test.go index 5201fedb1..c37274c71 100644 --- a/aconfig/aconfig_declarations_test.go +++ b/aconfig/aconfig_declarations_test.go @@ -88,19 +88,49 @@ func TestAconfigDeclarationsWithContainer(t *testing.T) { android.AssertStringEquals(t, "rule must contain container", rule.Args["container"], "--container com.android.foo") } -func TestAconfigDeclarationsWithoutContainer(t *testing.T) { - bp := ` - aconfig_declarations { - name: "module_name", - package: "com.example.package", - srcs: [ - "foo.aconfig", - ], - } - ` - result := runTest(t, android.FixtureExpectsNoErrors, bp) - - module := result.ModuleForTests("module_name", "") - rule := module.Rule("aconfig") - android.AssertIntEquals(t, "rule must not contain container", len(rule.Args["container"]), 0) +func TestMandatoryProperties(t *testing.T) { + testCases := []struct { + name string + expectedError string + bp string + }{ + { + name: "Srcs missing from aconfig_declarations", + bp: ` + aconfig_declarations { + name: "my_aconfig_declarations_foo", + package: "com.example.package", + container: "otherapex", + }`, + expectedError: `missing source files`, + }, + { + name: "Package missing from aconfig_declarations", + bp: ` + aconfig_declarations { + name: "my_aconfig_declarations_foo", + container: "otherapex", + srcs: ["foo.aconfig"], + }`, + expectedError: `missing package property`, + }, + { + name: "Container missing from aconfig_declarations", + bp: ` + aconfig_declarations { + name: "my_aconfig_declarations_foo", + package: "com.example.package", + srcs: ["foo.aconfig"], + }`, + expectedError: `missing container property`, + }, + } + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + errorHandler := android.FixtureExpectsAtLeastOneErrorMatchingPattern(test.expectedError) + android.GroupFixturePreparers(PrepareForTestWithAconfigBuildComponents). + ExtendWithErrorHandler(errorHandler). + RunTestWithBp(t, test.bp) + }) + } } diff --git a/aconfig/codegen/aconfig_declarations_group_test.go b/aconfig/codegen/aconfig_declarations_group_test.go index ec7cea383..c69d21ffd 100644 --- a/aconfig/codegen/aconfig_declarations_group_test.go +++ b/aconfig/codegen/aconfig_declarations_group_test.go @@ -15,9 +15,10 @@ package codegen import ( + "testing" + "android/soong/android" "android/soong/java" - "testing" ) func TestAconfigDeclarationsGroup(t *testing.T) { @@ -28,6 +29,7 @@ func TestAconfigDeclarationsGroup(t *testing.T) { aconfig_declarations { name: "foo-aconfig", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } @@ -39,6 +41,7 @@ func TestAconfigDeclarationsGroup(t *testing.T) { aconfig_declarations { name: "bar-aconfig", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } diff --git a/aconfig/codegen/cc_aconfig_library_test.go b/aconfig/codegen/cc_aconfig_library_test.go index 2e7fdc2c1..36f8b0e76 100644 --- a/aconfig/codegen/cc_aconfig_library_test.go +++ b/aconfig/codegen/cc_aconfig_library_test.go @@ -50,6 +50,7 @@ func testCCCodegenModeHelper(t *testing.T, bpMode string, ruleMode string) { aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } @@ -112,6 +113,7 @@ func testIncorrectCCCodegenModeHelper(t *testing.T, bpMode string, err string) { aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } @@ -167,6 +169,7 @@ func TestAndroidMkCcLibrary(t *testing.T) { aconfig_declarations { name: "my_aconfig_declarations_bar", package: "com.example.package", + container: "com.android.foo", srcs: ["bar.aconfig"], } @@ -241,6 +244,7 @@ func TestForceReadOnly(t *testing.T) { aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } diff --git a/aconfig/codegen/java_aconfig_library_test.go b/aconfig/codegen/java_aconfig_library_test.go index de45b5cc3..87b54a47f 100644 --- a/aconfig/codegen/java_aconfig_library_test.go +++ b/aconfig/codegen/java_aconfig_library_test.go @@ -35,6 +35,7 @@ func runJavaAndroidMkTest(t *testing.T, bp string) { aconfig_declarations { name: "my_aconfig_declarations_foo", package: "com.example.package.foo", + container: "system", srcs: ["foo.aconfig"], } @@ -46,6 +47,7 @@ func runJavaAndroidMkTest(t *testing.T, bp string) { aconfig_declarations { name: "my_aconfig_declarations_bar", package: "com.example.package.bar", + container: "system", srcs: ["bar.aconfig"], } @@ -60,7 +62,7 @@ func runJavaAndroidMkTest(t *testing.T, bp string) { entry := android.AndroidMkEntriesForTest(t, result.TestContext, module)[0] makeVar := entry.EntryMap["LOCAL_ACONFIG_FILES"] - android.EnsureListContainsSuffix(t, makeVar, "android_common/aconfig_merged.pb") + android.EnsureListContainsSuffix(t, makeVar, "android_common/system/aconfig_merged.pb") } func TestAndroidMkJavaLibrary(t *testing.T) { @@ -175,6 +177,7 @@ func testCodegenMode(t *testing.T, bpMode string, ruleMode string) { aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], exportable: true, } @@ -200,6 +203,7 @@ func testCodegenModeWithError(t *testing.T, bpMode string, err string) { aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } @@ -234,3 +238,52 @@ func TestForceReadOnlyMode(t *testing.T) { func TestUnsupportedMode(t *testing.T) { testCodegenModeWithError(t, "mode: `unsupported`,", "mode: \"unsupported\" is not a supported mode") } + +func TestMkEntriesMatchedContainer(t *testing.T) { + result := android.GroupFixturePreparers( + PrepareForTestWithAconfigBuildComponents, + java.PrepareForTestWithJavaDefaultModules). + ExtendWithErrorHandler(android.FixtureExpectsNoErrors). + RunTestWithBp(t, ` + aconfig_declarations { + name: "my_aconfig_declarations_foo", + package: "com.example.package.foo", + container: "system", + srcs: ["foo.aconfig"], + } + + java_aconfig_library { + name: "my_java_aconfig_library_foo", + aconfig_declarations: "my_aconfig_declarations_foo", + } + + aconfig_declarations { + name: "my_aconfig_declarations_bar", + package: "com.example.package.bar", + container: "system_ext", + srcs: ["bar.aconfig"], + } + + java_aconfig_library { + name: "my_java_aconfig_library_bar", + aconfig_declarations: "my_aconfig_declarations_bar", + } + + java_library { + name: "my_module", + srcs: [ + "src/foo.java", + ], + static_libs: [ + "my_java_aconfig_library_foo", + "my_java_aconfig_library_bar", + ], + platform_apis: true, + } + `) + + module := result.ModuleForTests("my_module", "android_common").Module() + entry := android.AndroidMkEntriesForTest(t, result.TestContext, module)[0] + makeVar := entry.EntryMap["LOCAL_ACONFIG_FILES"] + android.EnsureListContainsSuffix(t, makeVar, "my_aconfig_declarations_foo/intermediate.pb") +} diff --git a/aconfig/codegen/rust_aconfig_library_test.go b/aconfig/codegen/rust_aconfig_library_test.go index fe28f9441..523b464c0 100644 --- a/aconfig/codegen/rust_aconfig_library_test.go +++ b/aconfig/codegen/rust_aconfig_library_test.go @@ -46,6 +46,7 @@ func TestRustAconfigLibrary(t *testing.T) { aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } @@ -131,6 +132,7 @@ func testRustCodegenModeHelper(t *testing.T, bpMode string, ruleMode string) { aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } rust_aconfig_library { @@ -193,6 +195,7 @@ func testIncorrectRustCodegenModeHelper(t *testing.T, bpMode string, err string) aconfig_declarations { name: "my_aconfig_declarations", package: "com.example.package", + container: "com.android.foo", srcs: ["foo.aconfig"], } rust_aconfig_library { diff --git a/apex/apex_test.go b/apex/apex_test.go index 8a3735cbe..9254014e7 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -11425,6 +11425,7 @@ func TestAconfifDeclarationsValidation(t *testing.T) { aconfig_declarations { name: "%[1]s", package: "com.example.package", + container: "system", srcs: [ "%[1]s.aconfig", ], diff --git a/java/aar_test.go b/java/aar_test.go index d6dbe3c25..18efd2067 100644 --- a/java/aar_test.go +++ b/java/aar_test.go @@ -15,8 +15,9 @@ package java import ( - "android/soong/android" "testing" + + "android/soong/android" ) func TestAarImportProducesJniPackages(t *testing.T) { @@ -98,6 +99,7 @@ func TestLibraryFlagsPackages(t *testing.T) { aconfig_declarations { name: "bar", package: "com.example.package.bar", + container: "com.android.foo", srcs: [ "bar.aconfig", ], @@ -105,6 +107,7 @@ func TestLibraryFlagsPackages(t *testing.T) { aconfig_declarations { name: "baz", package: "com.example.package.baz", + container: "com.android.foo", srcs: [ "baz.aconfig", ], diff --git a/java/app_test.go b/java/app_test.go index eab40e7da..a7c48a1ed 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -4382,6 +4382,7 @@ func TestAppFlagsPackages(t *testing.T) { aconfig_declarations { name: "bar", package: "com.example.package.bar", + container: "com.android.foo", srcs: [ "bar.aconfig", ], @@ -4389,6 +4390,7 @@ func TestAppFlagsPackages(t *testing.T) { aconfig_declarations { name: "baz", package: "com.example.package.baz", + container: "com.android.foo", srcs: [ "baz.aconfig", ], diff --git a/java/droidstubs_test.go b/java/droidstubs_test.go index 8da695f08..6a14f3645 100644 --- a/java/droidstubs_test.go +++ b/java/droidstubs_test.go @@ -379,6 +379,7 @@ func TestAconfigDeclarations(t *testing.T) { aconfig_declarations { name: "bar", package: "com.example.package", + container: "com.android.foo", srcs: [ "bar.aconfig", ], @@ -434,6 +435,7 @@ func TestReleaseExportRuntimeApis(t *testing.T) { aconfig_declarations { name: "bar", package: "com.example.package", + container: "com.android.foo", srcs: [ "bar.aconfig", ], diff --git a/java/java_test.go b/java/java_test.go index a1192bb5f..2f2793202 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -2801,6 +2801,7 @@ func TestApiLibraryAconfigDeclarations(t *testing.T) { aconfig_declarations { name: "bar", package: "com.example.package", + container: "com.android.foo", srcs: [ "bar.aconfig", ], diff --git a/java/rro_test.go b/java/rro_test.go index d697ec627..742c83982 100644 --- a/java/rro_test.go +++ b/java/rro_test.go @@ -421,6 +421,7 @@ func TestRuntimeResourceOverlayFlagsPackages(t *testing.T) { aconfig_declarations { name: "bar", package: "com.example.package.bar", + container: "com.android.foo", srcs: [ "bar.aconfig", ], @@ -428,6 +429,7 @@ func TestRuntimeResourceOverlayFlagsPackages(t *testing.T) { aconfig_declarations { name: "baz", package: "com.example.package.baz", + container: "com.android.foo", srcs: [ "baz.aconfig", ], diff --git a/java/sdk_library_test.go b/java/sdk_library_test.go index 0f163e6e0..34c63ac61 100644 --- a/java/sdk_library_test.go +++ b/java/sdk_library_test.go @@ -1715,6 +1715,7 @@ func TestSdkLibraryExportableStubsLibrary(t *testing.T) { aconfig_declarations { name: "bar", package: "com.example.package", + container: "com.android.foo", srcs: [ "bar.aconfig", ],