From 8f5310f90cd1981c412c85a5011a1ff945a3c8c8 Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 7 Oct 2022 18:16:47 -0400 Subject: [PATCH] Default apex's compile_multilib to "first" in bp2build In Soong, decodeMultilib, used to get multilib to determine the dep variations, return "first" if defaultMultilib is set to "common". apex sets defaultMultilib to "common" which means equivalent compileMultilib in bp2build for apex should be "first" (See new Soong unit tests for more context). This CL fixes bp2build for apex to be more correct. Bug: 251559512 Test: go tests Change-Id: Id1cb4407980fc1fab91822c81326f37fb4adfa0a --- apex/apex.go | 7 +- apex/apex_test.go | 70 ++++++++++++++++++ bp2build/apex_conversion_test.go | 117 +++++++++++++++++++++---------- 3 files changed, 156 insertions(+), 38 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 4247db47a..c49a4ffba 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -3420,7 +3420,12 @@ func convertWithBp2build(a *apexBundle, ctx android.TopDownMutatorContext) (baze Native_shared_libs_32: bazel.LabelListAttribute{}, Native_shared_libs_64: bazel.LabelListAttribute{}, } - compileMultilib := "both" + + // https://cs.android.com/android/platform/superproject/+/master:build/soong/android/arch.go;l=698;drc=f05b0d35d2fbe51be9961ce8ce8031f840295c68 + // https://cs.android.com/android/platform/superproject/+/master:build/soong/apex/apex.go;l=2549;drc=ec731a83e3e2d80a1254e32fd4ad7ef85e262669 + // In Soong, decodeMultilib, used to get multilib, return "first" if defaultMultilib is set to "common". + // Since apex sets defaultMultilib to be "common", equivalent compileMultilib in bp2build for apex should be "first" + compileMultilib := "first" if a.CompileMultilib() != nil { compileMultilib = *a.CompileMultilib() } diff --git a/apex/apex_test.go b/apex/apex_test.go index e130fccc8..0ca8f95ea 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -4098,6 +4098,76 @@ func TestApexName(t *testing.T) { ensureNotContains(t, androidMk, "LOCAL_MODULE := mylib.com.android.myapex\n") } +func TestCompileMultilibProp(t *testing.T) { + testCases := []struct { + compileMultiLibProp string + containedLibs []string + notContainedLibs []string + }{ + { + containedLibs: []string{ + "image.apex/lib64/mylib.so", + "image.apex/lib/mylib.so", + }, + compileMultiLibProp: `compile_multilib: "both",`, + }, + { + containedLibs: []string{"image.apex/lib64/mylib.so"}, + notContainedLibs: []string{"image.apex/lib/mylib.so"}, + compileMultiLibProp: `compile_multilib: "first",`, + }, + { + containedLibs: []string{"image.apex/lib64/mylib.so"}, + notContainedLibs: []string{"image.apex/lib/mylib.so"}, + // compile_multilib, when unset, should result to the same output as when compile_multilib is "first" + }, + { + containedLibs: []string{"image.apex/lib64/mylib.so"}, + notContainedLibs: []string{"image.apex/lib/mylib.so"}, + compileMultiLibProp: `compile_multilib: "64",`, + }, + { + containedLibs: []string{"image.apex/lib/mylib.so"}, + notContainedLibs: []string{"image.apex/lib64/mylib.so"}, + compileMultiLibProp: `compile_multilib: "32",`, + }, + } + for _, testCase := range testCases { + ctx := testApex(t, fmt.Sprintf(` + apex { + name: "myapex", + key: "myapex.key", + %s + native_shared_libs: ["mylib"], + updatable: false, + } + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + cc_library { + name: "mylib", + srcs: ["mylib.cpp"], + apex_available: [ + "//apex_available:platform", + "myapex", + ], + } + `, testCase.compileMultiLibProp), + ) + module := ctx.ModuleForTests("myapex", "android_common_myapex_image") + apexRule := module.Rule("apexRule") + copyCmds := apexRule.Args["copy_commands"] + for _, containedLib := range testCase.containedLibs { + ensureContains(t, copyCmds, containedLib) + } + for _, notContainedLib := range testCase.notContainedLibs { + ensureNotContains(t, copyCmds, notContainedLib) + } + } +} + func TestNonTestApex(t *testing.T) { ctx := testApex(t, ` apex { diff --git a/bp2build/apex_conversion_test.go b/bp2build/apex_conversion_test.go index 4fd6e43cf..a666e4994 100644 --- a/bp2build/apex_conversion_test.go +++ b/bp2build/apex_conversion_test.go @@ -22,6 +22,7 @@ import ( "android/soong/java" "android/soong/sh" + "fmt" "testing" ) @@ -153,10 +154,17 @@ apex { "key": `":com.android.apogee.key"`, "manifest": `"apogee_manifest.json"`, "min_sdk_version": `"29"`, - "native_shared_libs_32": `[ - ":native_shared_lib_1", - ":native_shared_lib_2", - ]`, + "native_shared_libs_32": `select({ + "//build/bazel/platforms/arch:arm": [ + ":native_shared_lib_1", + ":native_shared_lib_2", + ], + "//build/bazel/platforms/arch:x86": [ + ":native_shared_lib_1", + ":native_shared_lib_2", + ], + "//conditions:default": [], + })`, "native_shared_libs_64": `select({ "//build/bazel/platforms/arch:arm64": [ ":native_shared_lib_1", @@ -273,10 +281,11 @@ filegroup { } `, }, - Blueprint: createMultilibBlueprint("both"), + Blueprint: createMultilibBlueprint(`compile_multilib: "both",`), ExpectedBazelTargets: []string{ MakeBazelTarget("apex", "com.android.apogee", AttrNameToString{ "native_shared_libs_32": `[ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib32", ] + select({ @@ -286,11 +295,13 @@ filegroup { })`, "native_shared_libs_64": `select({ "//build/bazel/platforms/arch:arm64": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib64", ":native_shared_lib_for_first", ], "//build/bazel/platforms/arch:x86_64": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib64", ":native_shared_lib_for_first", @@ -303,53 +314,69 @@ filegroup { }}) } -func TestApexBundleCompileMultilibFirst(t *testing.T) { - runApexTestCase(t, Bp2buildTestCase{ - Description: "apex - example with compile_multilib=first", - ModuleTypeUnderTest: "apex", - ModuleTypeUnderTestFactory: apex.BundleFactory, - Filesystem: map[string]string{ - "system/sepolicy/apex/Android.bp": ` -filegroup { - name: "com.android.apogee-file_contexts", - srcs: [ "apogee-file_contexts", ], - bazel_module: { bp2build_available: false }, -} -`, - }, - Blueprint: createMultilibBlueprint("first"), - ExpectedBazelTargets: []string{ - MakeBazelTarget("apex", "com.android.apogee", AttrNameToString{ - "native_shared_libs_32": `select({ +func TestApexBundleCompileMultilibFirstAndDefaultValue(t *testing.T) { + expectedBazelTargets := []string{ + MakeBazelTarget("apex", "com.android.apogee", AttrNameToString{ + "native_shared_libs_32": `select({ "//build/bazel/platforms/arch:arm": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib32", ":native_shared_lib_for_first", ], "//build/bazel/platforms/arch:x86": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib32", ":native_shared_lib_for_first", ], "//conditions:default": [], })`, - "native_shared_libs_64": `select({ + "native_shared_libs_64": `select({ "//build/bazel/platforms/arch:arm64": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib64", ":native_shared_lib_for_first", ], "//build/bazel/platforms/arch:x86_64": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib64", ":native_shared_lib_for_first", ], "//conditions:default": [], })`, - "file_contexts": `"//system/sepolicy/apex:com.android.apogee-file_contexts"`, - "manifest": `"apex_manifest.json"`, - }), - }}) + "file_contexts": `"//system/sepolicy/apex:com.android.apogee-file_contexts"`, + "manifest": `"apex_manifest.json"`, + }), + } + + // "first" is the default value of compile_multilib prop so `compile_multilib_: "first"` and unset compile_multilib + // should result to the same bp2build output + compileMultiLibPropValues := []string{`compile_multilib: "first",`, ""} + for _, compileMultiLibProp := range compileMultiLibPropValues { + descriptionSuffix := compileMultiLibProp + if descriptionSuffix == "" { + descriptionSuffix = "compile_multilib unset" + } + runApexTestCase(t, Bp2buildTestCase{ + Description: "apex - example with " + compileMultiLibProp, + ModuleTypeUnderTest: "apex", + ModuleTypeUnderTestFactory: apex.BundleFactory, + Filesystem: map[string]string{ + "system/sepolicy/apex/Android.bp": ` + filegroup { + name: "com.android.apogee-file_contexts", + srcs: [ "apogee-file_contexts", ], + bazel_module: { bp2build_available: false }, + } + `, + }, + Blueprint: createMultilibBlueprint(compileMultiLibProp), + ExpectedBazelTargets: expectedBazelTargets, + }) + } } func TestApexBundleCompileMultilib32(t *testing.T) { @@ -366,10 +393,11 @@ filegroup { } `, }, - Blueprint: createMultilibBlueprint("32"), + Blueprint: createMultilibBlueprint(`compile_multilib: "32",`), ExpectedBazelTargets: []string{ MakeBazelTarget("apex", "com.android.apogee", AttrNameToString{ "native_shared_libs_32": `[ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib32", ] + select({ @@ -397,16 +425,18 @@ filegroup { } `, }, - Blueprint: createMultilibBlueprint("64"), + Blueprint: createMultilibBlueprint(`compile_multilib: "64",`), ExpectedBazelTargets: []string{ MakeBazelTarget("apex", "com.android.apogee", AttrNameToString{ "native_shared_libs_64": `select({ "//build/bazel/platforms/arch:arm64": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib64", ":native_shared_lib_for_first", ], "//build/bazel/platforms/arch:x86_64": [ + ":unnested_native_shared_lib", ":native_shared_lib_for_both", ":native_shared_lib_for_lib64", ":native_shared_lib_for_first", @@ -420,7 +450,7 @@ filegroup { } func createMultilibBlueprint(compile_multilib string) string { - return ` + return fmt.Sprintf(` cc_library { name: "native_shared_lib_for_both", bazel_module: { bp2build_available: false }, @@ -441,9 +471,15 @@ cc_library { bazel_module: { bp2build_available: false }, } +cc_library { + name: "unnested_native_shared_lib", + bazel_module: { bp2build_available: false }, +} + apex { name: "com.android.apogee", - compile_multilib: "` + compile_multilib + `", + %s + native_shared_libs: ["unnested_native_shared_lib"], multilib: { both: { native_shared_libs: [ @@ -466,7 +502,7 @@ apex { ], }, }, -}` +}`, compile_multilib) } func TestApexBundleDefaultPropertyValues(t *testing.T) { @@ -636,10 +672,17 @@ override_apex { "key": `":com.google.android.apogee.key"`, "manifest": `"apogee_manifest.json"`, "min_sdk_version": `"29"`, - "native_shared_libs_32": `[ - ":native_shared_lib_1", - ":native_shared_lib_2", - ]`, + "native_shared_libs_32": `select({ + "//build/bazel/platforms/arch:arm": [ + ":native_shared_lib_1", + ":native_shared_lib_2", + ], + "//build/bazel/platforms/arch:x86": [ + ":native_shared_lib_1", + ":native_shared_lib_2", + ], + "//conditions:default": [], + })`, "native_shared_libs_64": `select({ "//build/bazel/platforms/arch:arm64": [ ":native_shared_lib_1",