From 275f654a2e21577bbe743e14ec873cae609631f3 Mon Sep 17 00:00:00 2001 From: Zi Wang Date: Thu, 9 Nov 2023 14:59:31 -0800 Subject: [PATCH] Add "exported" mode to xx_aconfig_library build rules Test: added unit tests and CI Bug: 309990433 Change-Id: Iae1b85265d9780bde7d41ec2ec6e8e441c2b3814 --- aconfig/cc_aconfig_library.go | 18 ++++- aconfig/cc_aconfig_library_test.go | 49 +++++++++++-- aconfig/java_aconfig_library.go | 28 ++++++-- aconfig/java_aconfig_library_test.go | 38 +++++++++- aconfig/rust_aconfig_library.go | 24 ++++++- aconfig/rust_aconfig_library_test.go | 100 ++++++++++++++++++++++++++- 6 files changed, 238 insertions(+), 19 deletions(-) diff --git a/aconfig/cc_aconfig_library.go b/aconfig/cc_aconfig_library.go index 5b0fb4a82..8aa696bf1 100644 --- a/aconfig/cc_aconfig_library.go +++ b/aconfig/cc_aconfig_library.go @@ -39,7 +39,14 @@ type CcAconfigLibraryProperties struct { Aconfig_declarations string // whether to generate test mode version of the library + // TODO: remove "Test" property when "Mode" can be used in all the branches Test *bool + + // default mode is "production", the other accepted modes are: + // "test": to generate test mode version of the library + // "exported": to generate exported mode version of the library + // an error will be thrown if the mode is not supported + Mode *string } type CcAconfigLibraryCallbacks struct { @@ -121,11 +128,16 @@ func (this *CcAconfigLibraryCallbacks) GeneratorBuildActions(ctx cc.ModuleContex } declarations := ctx.OtherModuleProvider(declarationsModules[0], declarationsProviderKey).(declarationsProviderData) - var mode string + if this.properties.Mode != nil && this.properties.Test != nil { + ctx.PropertyErrorf("test", "test prop should not be specified when mode prop is set") + } + mode := proptools.StringDefault(this.properties.Mode, "production") + if !isModeSupported(mode) { + ctx.PropertyErrorf("mode", "%q is not a supported mode", mode) + } + // TODO: remove "Test" property if proptools.Bool(this.properties.Test) { mode = "test" - } else { - mode = "production" } ctx.Build(pctx, android.BuildParams{ Rule: cppRule, diff --git a/aconfig/cc_aconfig_library_test.go b/aconfig/cc_aconfig_library_test.go index 6f17c7594..9a819e59e 100644 --- a/aconfig/cc_aconfig_library_test.go +++ b/aconfig/cc_aconfig_library_test.go @@ -22,16 +22,17 @@ import ( "android/soong/cc" ) -var codegenModeTestData = []struct { +var ccCodegenModeTestData = []struct { setting, expected string }{ {"", "production"}, - {"test: false,", "production"}, - {"test: true,", "test"}, + {"mode: `production`,", "production"}, + {"mode: `test`,", "test"}, + {"mode: `exported`,", "exported"}, } func TestCCCodegenMode(t *testing.T) { - for _, testData := range codegenModeTestData { + for _, testData := range ccCodegenModeTestData { testCCCodegenModeHelper(t, testData.setting, testData.expected) } } @@ -65,3 +66,43 @@ func testCCCodegenModeHelper(t *testing.T, bpMode string, ruleMode string) { rule := module.Rule("cc_aconfig_library") android.AssertStringEquals(t, "rule must contain test mode", rule.Args["mode"], ruleMode) } + +var incorrectCCCodegenModeTestData = []struct { + setting, expectedErr string +}{ + {"mode: `unsupported`,", "mode: \"unsupported\" is not a supported mode"}, + // TODO: remove this test case when test prop is removed + {"mode: `test`, test: true", "test prop should not be specified when mode prop is set"}, +} + +func TestIncorrectCCCodegenMode(t *testing.T) { + for _, testData := range incorrectCCCodegenModeTestData { + testIncorrectCCCodegenModeHelper(t, testData.setting, testData.expectedErr) + } +} + +func testIncorrectCCCodegenModeHelper(t *testing.T, bpMode string, err string) { + t.Helper() + android.GroupFixturePreparers( + PrepareForTestWithAconfigBuildComponents, + cc.PrepareForTestWithCcDefaultModules). + ExtendWithErrorHandler(android.FixtureExpectsOneErrorPattern(err)). + RunTestWithBp(t, fmt.Sprintf(` + aconfig_declarations { + name: "my_aconfig_declarations", + package: "com.example.package", + srcs: ["foo.aconfig"], + } + + cc_library { + name: "server_configurable_flags", + srcs: ["server_configurable_flags.cc"], + } + + cc_aconfig_library { + name: "my_cc_aconfig_library", + aconfig_declarations: "my_aconfig_declarations", + %s + } + `, bpMode)) +} diff --git a/aconfig/java_aconfig_library.go b/aconfig/java_aconfig_library.go index f7f8db85d..b6c90fc5f 100644 --- a/aconfig/java_aconfig_library.go +++ b/aconfig/java_aconfig_library.go @@ -30,12 +30,21 @@ type declarationsTagType struct { var declarationsTag = declarationsTagType{} +var aconfigSupportedModes = []string{"production", "test", "exported"} + type JavaAconfigDeclarationsLibraryProperties struct { // name of the aconfig_declarations module to generate a library for Aconfig_declarations string // whether to generate test mode version of the library + // TODO: remove "Test" property when "Mode" can be used in all the branches Test *bool + + // default mode is "production", the other accepted modes are: + // "test": to generate test mode version of the library + // "exported": to generate exported mode version of the library + // an error will be thrown if the mode is not supported + Mode *string } type JavaAconfigDeclarationsLibraryCallbacks struct { @@ -72,12 +81,19 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuild // Generate the action to build the srcjar srcJarPath := android.PathForModuleGen(ctx, ctx.ModuleName()+".srcjar") - var mode string + + if callbacks.properties.Mode != nil && callbacks.properties.Test != nil { + ctx.PropertyErrorf("test", "test prop should not be specified when mode prop is set") + } + mode := proptools.StringDefault(callbacks.properties.Mode, "production") + if !isModeSupported(mode) { + ctx.PropertyErrorf("mode", "%q is not a supported mode", mode) + } + // TODO: remove "Test" property if proptools.Bool(callbacks.properties.Test) { mode = "test" - } else { - mode = "production" } + ctx.Build(pctx, android.BuildParams{ Rule: javaRule, Input: declarations.IntermediatePath, @@ -95,9 +111,12 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuild return srcJarPath } +func isModeSupported(mode string) bool { + return android.InList(mode, aconfigSupportedModes) +} + type bazelJavaAconfigLibraryAttributes struct { Aconfig_declarations bazel.LabelAttribute - Test *bool Sdk_version *string Libs bazel.LabelListAttribute } @@ -138,7 +157,6 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) Bp2build(ctx android.B attrs := bazelJavaAconfigLibraryAttributes{ Aconfig_declarations: *bazel.MakeLabelAttribute(android.BazelLabelForModuleDepSingle(ctx, callbacks.properties.Aconfig_declarations).Label), - Test: callbacks.properties.Test, Sdk_version: &sdkVersion, Libs: libs, } diff --git a/aconfig/java_aconfig_library_test.go b/aconfig/java_aconfig_library_test.go index af50848f1..05532e701 100644 --- a/aconfig/java_aconfig_library_test.go +++ b/aconfig/java_aconfig_library_test.go @@ -178,14 +178,48 @@ func testCodegenMode(t *testing.T, bpMode string, ruleMode string) { android.AssertStringEquals(t, "rule must contain test mode", rule.Args["mode"], ruleMode) } +func testCodegenModeWithError(t *testing.T, bpMode string, err string) { + android.GroupFixturePreparers( + PrepareForTestWithAconfigBuildComponents, + java.PrepareForTestWithJavaDefaultModules). + ExtendWithErrorHandler(android.FixtureExpectsOneErrorPattern(err)). + RunTestWithBp(t, fmt.Sprintf(` + aconfig_declarations { + name: "my_aconfig_declarations", + package: "com.example.package", + srcs: ["foo.aconfig"], + } + + java_aconfig_library { + name: "my_java_aconfig_library", + aconfig_declarations: "my_aconfig_declarations", + %s + } + `, bpMode)) +} + func TestDefaultProdMode(t *testing.T) { testCodegenMode(t, "", "production") } func TestProdMode(t *testing.T) { - testCodegenMode(t, "test: false,", "production") + testCodegenMode(t, "mode: `production`,", "production") } func TestTestMode(t *testing.T) { - testCodegenMode(t, "test: true,", "test") + testCodegenMode(t, "mode: `test`,", "test") +} + +func TestExportedMode(t *testing.T) { + testCodegenMode(t, "mode: `exported`,", "exported") +} + +func TestUnsupportedMode(t *testing.T) { + testCodegenModeWithError(t, "mode: `unsupported`,", "mode: \"unsupported\" is not a supported mode") +} + +// TODO: remove this test case when test prop is removed +func TestBothModeAndTestAreSet(t *testing.T) { + testCodegenModeWithError(t, "mode: `test`, test: true", + "test prop should not be specified when mode prop is set") } diff --git a/aconfig/rust_aconfig_library.go b/aconfig/rust_aconfig_library.go index 71918dd21..43078b651 100644 --- a/aconfig/rust_aconfig_library.go +++ b/aconfig/rust_aconfig_library.go @@ -1,9 +1,10 @@ package aconfig import ( + "fmt" + "android/soong/android" "android/soong/rust" - "fmt" "github.com/google/blueprint" "github.com/google/blueprint/proptools" @@ -18,7 +19,16 @@ var rustDeclarationsTag = rustDeclarationsTagType{} type RustAconfigLibraryProperties struct { // name of the aconfig_declarations module to generate a library for Aconfig_declarations string - Test *bool + + // whether to generate test mode version of the library + // TODO: remove "Test" property when "Mode" can be used in all the branches + Test *bool + + // default mode is "production", the other accepted modes are: + // "test": to generate test mode version of the library + // "exported": to generate exported mode version of the library + // an error will be thrown if the mode is not supported + Mode *string } type aconfigDecorator struct { @@ -60,7 +70,15 @@ func (a *aconfigDecorator) GenerateSource(ctx rust.ModuleContext, deps rust.Path } declarations := ctx.OtherModuleProvider(declarationsModules[0], declarationsProviderKey).(declarationsProviderData) - mode := "production" + if a.Properties.Mode != nil && a.Properties.Test != nil { + ctx.PropertyErrorf("test", "test prop should not be specified when mode prop is set") + } + mode := proptools.StringDefault(a.Properties.Mode, "production") + if !isModeSupported(mode) { + ctx.PropertyErrorf("mode", "%q is not a supported mode", mode) + } + + // TODO: remove "Test" property if proptools.Bool(a.Properties.Test) { mode = "test" } diff --git a/aconfig/rust_aconfig_library_test.go b/aconfig/rust_aconfig_library_test.go index a10da2dde..5e630b517 100644 --- a/aconfig/rust_aconfig_library_test.go +++ b/aconfig/rust_aconfig_library_test.go @@ -1,10 +1,11 @@ package aconfig import ( - "android/soong/android" - "android/soong/rust" "fmt" "testing" + + "android/soong/android" + "android/soong/rust" ) func TestRustAconfigLibrary(t *testing.T) { @@ -63,3 +64,98 @@ func TestRustAconfigLibrary(t *testing.T) { ) } } + +var rustCodegenModeTestData = []struct { + setting, expected string +}{ + {"", "production"}, + {"mode: `production`,", "production"}, + {"mode: `test`,", "test"}, + {"mode: `exported`,", "exported"}, +} + +func TestRustCodegenMode(t *testing.T) { + for _, testData := range rustCodegenModeTestData { + testRustCodegenModeHelper(t, testData.setting, testData.expected) + } +} + +func testRustCodegenModeHelper(t *testing.T, bpMode string, ruleMode string) { + t.Helper() + result := android.GroupFixturePreparers( + PrepareForTestWithAconfigBuildComponents, + rust.PrepareForTestWithRustIncludeVndk). + ExtendWithErrorHandler(android.FixtureExpectsNoErrors). + RunTestWithBp(t, fmt.Sprintf(` + rust_library { + name: "libflags_rust", // test mock + crate_name: "flags_rust", + srcs: ["lib.rs"], + } + rust_library { + name: "liblazy_static", // test mock + crate_name: "lazy_static", + srcs: ["src/lib.rs"], + } + aconfig_declarations { + name: "my_aconfig_declarations", + package: "com.example.package", + srcs: ["foo.aconfig"], + } + rust_aconfig_library { + name: "libmy_rust_aconfig_library", + crate_name: "my_rust_aconfig_library", + aconfig_declarations: "my_aconfig_declarations", + %s + } + `, bpMode)) + + module := result.ModuleForTests("libmy_rust_aconfig_library", "android_arm64_armv8-a_source") + rule := module.Rule("rust_aconfig_library") + android.AssertStringEquals(t, "rule must contain test mode", rule.Args["mode"], ruleMode) +} + +var incorrectRustCodegenModeTestData = []struct { + setting, expectedErr string +}{ + {"mode: `unsupported`,", "mode: \"unsupported\" is not a supported mode"}, + // TODO: remove this test case when test prop is removed + {"mode: `test`, test: true", "test prop should not be specified when mode prop is set"}, +} + +func TestIncorrectRustCodegenMode(t *testing.T) { + for _, testData := range incorrectRustCodegenModeTestData { + testIncorrectRustCodegenModeHelper(t, testData.setting, testData.expectedErr) + } +} + +func testIncorrectRustCodegenModeHelper(t *testing.T, bpMode string, err string) { + t.Helper() + android.GroupFixturePreparers( + PrepareForTestWithAconfigBuildComponents, + rust.PrepareForTestWithRustIncludeVndk). + ExtendWithErrorHandler(android.FixtureExpectsOneErrorPattern(err)). + RunTestWithBp(t, fmt.Sprintf(` + rust_library { + name: "libflags_rust", // test mock + crate_name: "flags_rust", + srcs: ["lib.rs"], + } + rust_library { + name: "liblazy_static", // test mock + crate_name: "lazy_static", + srcs: ["src/lib.rs"], + } + aconfig_declarations { + name: "my_aconfig_declarations", + package: "com.example.package", + srcs: ["foo.aconfig"], + } + rust_aconfig_library { + name: "libmy_rust_aconfig_library", + crate_name: "my_rust_aconfig_library", + aconfig_declarations: "my_aconfig_declarations", + %s + } + `, bpMode)) +}