From 921af32310e740ea0c4f0f83b7349771b3378916 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Wed, 26 Apr 2023 02:56:37 +0000 Subject: [PATCH 1/5] Print default val if all vals in axis match default val To avoid verbosity, we currently dedupe keys in axis if its value matches the value of //conditions:default. For axes where all values might match the default value, we would effectively drop the common value. To fix this, we are now dropping the select statement and not the common value. Test: go test ./bp2build Change-Id: Ic377b93ee2aba971753f6a5e7a62e15d1fcfa2bc --- bp2build/build_conversion_test.go | 15 +++++++++++++++ bp2build/configurability.go | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 1b64055f7..e198a11b8 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -21,6 +21,7 @@ import ( "android/soong/android" "android/soong/android/allowlists" + "android/soong/bazel" "android/soong/python" ) @@ -1931,3 +1932,17 @@ func TestGenerateConfigSetting(t *testing.T) { Description: "Generating API contribution Bazel targets for custom module", }) } + +// If values of all keys in an axis are equal to //conditions:default, drop the axis and print the common value +func TestPrettyPrintSelectMapEqualValues(t *testing.T) { + lla := bazel.LabelListAttribute{ + Value: bazel.LabelList{}, + } + libFooImplLabel := bazel.Label{ + Label: ":libfoo.impl", + } + lla.SetSelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndNonApex, bazel.MakeLabelList([]bazel.Label{libFooImplLabel})) + lla.SetSelectValue(bazel.OsAndInApexAxis, bazel.ConditionsDefaultConfigKey, bazel.MakeLabelList([]bazel.Label{libFooImplLabel})) + actual, _ := prettyPrintAttribute(lla, 0) + android.AssertStringEquals(t, "Print the common value if all keys in an axis have the same value", `[":libfoo.impl"]`, actual) +} diff --git a/bp2build/configurability.go b/bp2build/configurability.go index 8e171031c..3d9f0a274 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -279,6 +279,10 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue *stri } if len(selects) == 0 { + // If there is a default value, and there are no selects for this axis, print that without any selects. + if val, exists := selectMap[bazel.ConditionsDefaultSelectKey]; exists { + return prettyPrint(val, indent, emitZeroValues) + } // No conditions (or all values are empty lists), so no need for a map. return "", nil } From 9cad90f966cb3245accfb1cd788cc0ee1c6048f9 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Thu, 4 May 2023 17:15:44 +0000 Subject: [PATCH 2/5] Broaden the granularity of config_setting from apex_name to api_domain The use case for this is for creating a select statement for stub/impl selection. Since Bazel propagates apex_name from the top-level apex, the source apex and test apex builds a specific library in two different configurations. We need select statements for both these two configurations, but this metadata might not always exist in Android.bp since test apexes are not necessary to be listed in Android.bp files. To overcome this, the select statements will be created per api domain instead. This CL uses a naming convention to infer the api domain of apexes. Test: go test ./bp2build Change-Id: Iad2b45f736bc98a24d2ce1cf2f69aad67973092d --- bazel/properties.go | 2 ++ bp2build/cc_library_conversion_test.go | 5 +-- cc/bp2build.go | 48 +++++++++++++++++++------- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/bazel/properties.go b/bazel/properties.go index 1757bad49..c3b68a47e 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -1434,4 +1434,6 @@ type StringMapAttribute map[string]string type ConfigSettingAttributes struct { // Each key in Flag_values is a label to a custom string_setting Flag_values StringMapAttribute + // Each element in Constraint_values is a label to a constraint_value + Constraint_values LabelListAttribute } diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 776129f58..6d5fd7ec9 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -4473,11 +4473,12 @@ cc_library { ExpectedBazelTargets: []string{ MakeBazelTargetNoRestrictions( "config_setting", - "android-in_myapex", + "myapex", AttrNameToString{ "flag_values": `{ - "//build/bazel/rules/apex:apex_name": "myapex", + "//build/bazel/rules/apex:api_domain": "myapex", }`, + "constraint_values": `["//build/bazel/platforms/os:android"]`, }, ), }, diff --git a/cc/bp2build.go b/cc/bp2build.go index 749fce59a..6e91bffa2 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -1194,16 +1194,30 @@ func availableToSameApexes(a, b []string) bool { } var ( - apexConfigSettingKey = android.NewOnceKey("apexConfigSetting") - apexConfigSettingLock sync.Mutex + apiDomainConfigSettingKey = android.NewOnceKey("apiDomainConfigSettingKey") + apiDomainConfigSettingLock sync.Mutex ) -func getApexConfigSettingMap(config android.Config) *map[string]bool { - return config.Once(apexConfigSettingKey, func() interface{} { +func getApiDomainConfigSettingMap(config android.Config) *map[string]bool { + return config.Once(apiDomainConfigSettingKey, func() interface{} { return &map[string]bool{} }).(*map[string]bool) } +var ( + testApexNameToApiDomain = map[string]string{ + "test_broken_com.android.art": "com.android.art", + } +) + +func getApiDomain(apexName string) string { + if apiDomain, exists := testApexNameToApiDomain[apexName]; exists { + return apiDomain + } + // Remove `test_` prefix + return strings.TrimPrefix(apexName, "test_") +} + // Create a config setting for this apex in build/bazel/rules/apex // The use case for this is stub/impl selection in cc libraries // Long term, these config_setting(s) should be colocated with the respective apex definitions. @@ -1215,23 +1229,32 @@ func createInApexConfigSetting(ctx android.TopDownMutatorContext, apexName strin // These correspond to android-non_apex and android-in_apex return } - apexConfigSettingLock.Lock() - defer apexConfigSettingLock.Unlock() + apiDomainConfigSettingLock.Lock() + defer apiDomainConfigSettingLock.Unlock() // Return if a config_setting has already been created - acsm := getApexConfigSettingMap(ctx.Config()) - if _, exists := (*acsm)[apexName]; exists { + apiDomain := getApiDomain(apexName) + acsm := getApiDomainConfigSettingMap(ctx.Config()) + if _, exists := (*acsm)[apiDomain]; exists { return } - (*acsm)[apexName] = true + (*acsm)[apiDomain] = true csa := bazel.ConfigSettingAttributes{ Flag_values: bazel.StringMapAttribute{ - "//build/bazel/rules/apex:apex_name": apexName, + "//build/bazel/rules/apex:api_domain": apiDomain, }, + // Constraint this to android + Constraint_values: bazel.MakeLabelListAttribute( + bazel.MakeLabelList( + []bazel.Label{ + bazel.Label{Label: "//build/bazel/platforms/os:android"}, + }, + ), + ), } ca := android.CommonAttributes{ - Name: "android-in_" + apexName, + Name: apiDomain, } ctx.CreateBazelConfigSetting( csa, @@ -1247,7 +1270,8 @@ func inApexConfigSetting(apexAvailable string) string { if apexAvailable == android.AvailableToAnyApex { return bazel.AndroidAndInApex } - return "//build/bazel/rules/apex:android-in_" + apexAvailable + apiDomain := getApiDomain(apexAvailable) + return "//build/bazel/rules/apex:" + apiDomain } func setStubsForDynamicDeps(ctx android.BazelConversionPathContext, axis bazel.ConfigurationAxis, From a43ae1366ea312f1865ad672d82ed2480f4109e8 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Mon, 8 May 2023 18:33:16 +0000 Subject: [PATCH 3/5] For test apexes, base_apex_name is the api domain apex_test do not "override" the source apexes that they test. However, similar to override_apexes, test apexes have the same path (/apex/) on device. Instead of creating another property, reuse the existing base_apex_name property. The use case for this will be for creating selection statements for stub/impl selection. Test: go test ./bp2build Change-Id: I4b7548e0e0fc920e407e1c5e5c00e87efc6e70c9 --- apex/apex.go | 2 ++ bp2build/apex_conversion_test.go | 9 +++++---- cc/bp2build.go | 10 +++++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 69547c3b7..e01406bc6 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -3703,6 +3703,8 @@ func convertWithBp2build(a *apexBundle, ctx android.TopDownMutatorContext) (baze commonAttrs := android.CommonAttributes{} if a.testApex { commonAttrs.Testonly = proptools.BoolPtr(true) + // Set the api_domain of the test apex + attrs.Base_apex_name = proptools.StringPtr(cc.GetApiDomain(a.Name())) } return attrs, props, commonAttrs diff --git a/bp2build/apex_conversion_test.go b/bp2build/apex_conversion_test.go index 1cc3f22ed..390cabe1f 100644 --- a/bp2build/apex_conversion_test.go +++ b/bp2build/apex_conversion_test.go @@ -1475,10 +1475,11 @@ apex_test { `, ExpectedBazelTargets: []string{ MakeBazelTarget("apex", "test_com.android.apogee", AttrNameToString{ - "file_contexts": `"file_contexts_file"`, - "manifest": `"apex_manifest.json"`, - "testonly": `True`, - "tests": `[":cc_test_1"]`, + "file_contexts": `"file_contexts_file"`, + "base_apex_name": `"com.android.apogee"`, + "manifest": `"apex_manifest.json"`, + "testonly": `True`, + "tests": `[":cc_test_1"]`, }), }}) } diff --git a/cc/bp2build.go b/cc/bp2build.go index 6e91bffa2..ca8d60902 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -1210,7 +1210,11 @@ var ( } ) -func getApiDomain(apexName string) string { +// GetApiDomain returns the canonical name of the apex. This is synonymous to the apex_name definition. +// https://cs.android.com/android/_/android/platform/build/soong/+/e3f0281b8897da1fe23b2f4f3a05f1dc87bcc902:apex/prebuilt.go;l=81-83;drc=2dc7244af985a6ad701b22f1271e606cabba527f;bpv=1;bpt=0 +// For test apexes, it uses a naming convention heuristic to determine the api domain. +// TODO (b/281548611): Move this build/soong/android +func GetApiDomain(apexName string) string { if apiDomain, exists := testApexNameToApiDomain[apexName]; exists { return apiDomain } @@ -1233,7 +1237,7 @@ func createInApexConfigSetting(ctx android.TopDownMutatorContext, apexName strin defer apiDomainConfigSettingLock.Unlock() // Return if a config_setting has already been created - apiDomain := getApiDomain(apexName) + apiDomain := GetApiDomain(apexName) acsm := getApiDomainConfigSettingMap(ctx.Config()) if _, exists := (*acsm)[apiDomain]; exists { return @@ -1270,7 +1274,7 @@ func inApexConfigSetting(apexAvailable string) string { if apexAvailable == android.AvailableToAnyApex { return bazel.AndroidAndInApex } - apiDomain := getApiDomain(apexAvailable) + apiDomain := GetApiDomain(apexAvailable) return "//build/bazel/rules/apex:" + apiDomain } From 6d4d9da47f8c9d5a947c03554853dec12c02bddd Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 18 Apr 2023 06:20:40 +0000 Subject: [PATCH 4/5] Select stub/impl per apex variant Create a select statement for every api_domain a library could be included in. The stub/impl selection heuristics per apex domain are 1. If dep has stubs and the api domain appears in dep's apex_available, use impl 2. If dep has stubs and the api domain does not appear in dep's apex_available, use stubs (Category 3: If dep does not have stubs and the apex does not appear in dep's apex_available, then a separate apex_available validation in Bazel will emit an error). Platform variants have been special-cased for now to use equality of apex_available for stub/impl selection Test: go test ./bp2build Bug: 272378496 Change-Id: Ibd29efd763c8863c7e6d2a9af0da30bbde07175d --- bazel/configurability.go | 6 +- bp2build/build_conversion_test.go | 2 +- bp2build/cc_library_conversion_test.go | 41 +++--- bp2build/cc_library_shared_conversion_test.go | 94 +++++++++++- bp2build/cc_library_static_conversion_test.go | 4 +- cc/bp2build.go | 137 ++++++++++++------ 6 files changed, 207 insertions(+), 77 deletions(-) diff --git a/bazel/configurability.go b/bazel/configurability.go index d01877dd5..29299e9b9 100644 --- a/bazel/configurability.go +++ b/bazel/configurability.go @@ -69,8 +69,8 @@ const ( productVariableBazelPackage = "//build/bazel/product_variables" - AndroidAndInApex = "android-in_apex" - AndroidAndNonApex = "android-non_apex" + AndroidAndInApex = "android-in_apex" + AndroidPlatform = "system" InApex = "in_apex" NonApex = "non_apex" @@ -202,7 +202,7 @@ var ( osAndInApexMap = map[string]string{ AndroidAndInApex: "//build/bazel/rules/apex:android-in_apex", - AndroidAndNonApex: "//build/bazel/rules/apex:android-non_apex", + AndroidPlatform: "//build/bazel/rules/apex:system", osDarwin: "//build/bazel/platforms/os:darwin", osLinux: "//build/bazel/platforms/os:linux_glibc", osLinuxMusl: "//build/bazel/platforms/os:linux_musl", diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index e198a11b8..e127fd542 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -1941,7 +1941,7 @@ func TestPrettyPrintSelectMapEqualValues(t *testing.T) { libFooImplLabel := bazel.Label{ Label: ":libfoo.impl", } - lla.SetSelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndNonApex, bazel.MakeLabelList([]bazel.Label{libFooImplLabel})) + lla.SetSelectValue(bazel.OsAndInApexAxis, bazel.AndroidPlatform, bazel.MakeLabelList([]bazel.Label{libFooImplLabel})) lla.SetSelectValue(bazel.OsAndInApexAxis, bazel.ConditionsDefaultConfigKey, bazel.MakeLabelList([]bazel.Label{libFooImplLabel})) actual, _ := prettyPrintAttribute(lla, 0) android.AssertStringEquals(t, "Print the common value if all keys in an axis have the same value", `[":libfoo.impl"]`, actual) diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 6d5fd7ec9..1b681efb0 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -3042,7 +3042,8 @@ cc_library { }`, ExpectedBazelTargets: makeCcLibraryTargets("foolib", AttrNameToString{ "implementation_dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:barlib"], + "//build/bazel/rules/apex:foo": ["@api_surfaces//module-libapi/current:barlib"], + "//build/bazel/rules/apex:system": ["@api_surfaces//module-libapi/current:barlib"], "//conditions:default": [":barlib"], })`, "local_includes": `["."]`, @@ -3096,7 +3097,11 @@ cc_library { "//build/bazel/platforms/os:linux_glibc": [":quxlib"], "//build/bazel/platforms/os:linux_musl": [":quxlib"], "//build/bazel/platforms/os:windows": [":quxlib"], - "//build/bazel/rules/apex:android-in_apex": [ + "//build/bazel/rules/apex:foo": [ + "@api_surfaces//module-libapi/current:barlib", + "@api_surfaces//module-libapi/current:quxlib", + ], + "//build/bazel/rules/apex:system": [ "@api_surfaces//module-libapi/current:barlib", "@api_surfaces//module-libapi/current:quxlib", ], @@ -4139,44 +4144,34 @@ cc_library { name: "barlib", stubs: { symbol_file: "bar.map.txt", versions: ["28", "29", "current"] }, bazel_module: { bp2build_available: false }, + apex_available: ["//apex_available:platform",], } cc_library { name: "bazlib", stubs: { symbol_file: "bar.map.txt", versions: ["28", "29", "current"] }, bazel_module: { bp2build_available: false }, + apex_available: ["//apex_available:platform",], } cc_library { name: "foo", shared_libs: ["barlib", "bazlib"], export_shared_lib_headers: ["bazlib"], apex_available: [ - "apex_available:platform", + "//apex_available:platform", ], }`, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ - "implementation_dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:barlib"], - "//conditions:default": [":barlib"], - })`, - "dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:bazlib"], - "//conditions:default": [":bazlib"], - })`, - "local_includes": `["."]`, - "tags": `["apex_available=apex_available:platform"]`, + "implementation_dynamic_deps": `[":barlib"]`, + "dynamic_deps": `[":bazlib"]`, + "local_includes": `["."]`, + "tags": `["apex_available=//apex_available:platform"]`, }), MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ - "implementation_dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:barlib"], - "//conditions:default": [":barlib"], - })`, - "dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:bazlib"], - "//conditions:default": [":bazlib"], - })`, - "local_includes": `["."]`, - "tags": `["apex_available=apex_available:platform"]`, + "implementation_dynamic_deps": `[":barlib"]`, + "dynamic_deps": `[":bazlib"]`, + "local_includes": `["."]`, + "tags": `["apex_available=//apex_available:platform"]`, }), }, }) diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go index 47dff8a57..2ee9c99f9 100644 --- a/bp2build/cc_library_shared_conversion_test.go +++ b/bp2build/cc_library_shared_conversion_test.go @@ -609,7 +609,8 @@ cc_library_shared { ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_shared", "b", AttrNameToString{ "implementation_dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:a"], + "//build/bazel/rules/apex:apex_b": ["@api_surfaces//module-libapi/current:a"], + "//build/bazel/rules/apex:system": ["@api_surfaces//module-libapi/current:a"], "//conditions:default": [":a"], })`, "tags": `["apex_available=apex_b"]`, @@ -618,6 +619,64 @@ cc_library_shared { }) } +// Tests that library in apexfoo links against stubs of platform_lib and otherapex_lib +func TestCcLibrarySharedStubs_UseStubsFromMultipleApiDomains(t *testing.T) { + runCcLibrarySharedTestCase(t, Bp2buildTestCase{ + Description: "cc_library_shared stubs", + ModuleTypeUnderTest: "cc_library_shared", + ModuleTypeUnderTestFactory: cc.LibrarySharedFactory, + Blueprint: soongCcLibrarySharedPreamble + ` +cc_library_shared { + name: "libplatform_stable", + stubs: { symbol_file: "libplatform_stable.map.txt", versions: ["28", "29", "current"] }, + apex_available: ["//apex_available:platform"], + bazel_module: { bp2build_available: false }, + include_build_directory: false, +} +cc_library_shared { + name: "libapexfoo_stable", + stubs: { symbol_file: "libapexfoo_stable.map.txt", versions: ["28", "29", "current"] }, + apex_available: ["apexfoo"], + bazel_module: { bp2build_available: false }, + include_build_directory: false, +} +cc_library_shared { + name: "libutils", + shared_libs: ["libplatform_stable", "libapexfoo_stable",], + apex_available: ["//apex_available:platform", "apexfoo", "apexbar"], + include_build_directory: false, +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget("cc_library_shared", "libutils", AttrNameToString{ + "implementation_dynamic_deps": `select({ + "//build/bazel/rules/apex:apexbar": [ + "@api_surfaces//module-libapi/current:libplatform_stable", + "@api_surfaces//module-libapi/current:libapexfoo_stable", + ], + "//build/bazel/rules/apex:apexfoo": [ + "@api_surfaces//module-libapi/current:libplatform_stable", + ":libapexfoo_stable", + ], + "//build/bazel/rules/apex:system": [ + "@api_surfaces//module-libapi/current:libplatform_stable", + "@api_surfaces//module-libapi/current:libapexfoo_stable", + ], + "//conditions:default": [ + ":libplatform_stable", + ":libapexfoo_stable", + ], + })`, + "tags": `[ + "apex_available=//apex_available:platform", + "apex_available=apexfoo", + "apex_available=apexbar", + ]`, + }), + }, + }) +} + func TestCcLibrarySharedStubs_IgnorePlatformAvailable(t *testing.T) { runCcLibrarySharedTestCase(t, Bp2buildTestCase{ Description: "cc_library_shared stubs", @@ -641,7 +700,8 @@ cc_library_shared { ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_shared", "b", AttrNameToString{ "implementation_dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:a"], + "//build/bazel/rules/apex:apex_b": ["@api_surfaces//module-libapi/current:a"], + "//build/bazel/rules/apex:system": ["@api_surfaces//module-libapi/current:a"], "//conditions:default": [":a"], })`, "tags": `[ @@ -653,6 +713,34 @@ cc_library_shared { }) } +func TestCcLibraryDoesNotDropStubDepIfNoVariationAcrossAxis(t *testing.T) { + runCcLibrarySharedTestCase(t, Bp2buildTestCase{ + Description: "cc_library depeends on impl for all configurations", + ModuleTypeUnderTest: "cc_library_shared", + ModuleTypeUnderTestFactory: cc.LibrarySharedFactory, + Blueprint: soongCcLibrarySharedPreamble + ` +cc_library_shared { + name: "a", + stubs: { symbol_file: "a.map.txt", versions: ["28", "29", "current"] }, + bazel_module: { bp2build_available: false }, + apex_available: ["//apex_available:platform"], +} +cc_library_shared { + name: "b", + shared_libs: [":a"], + include_build_directory: false, + apex_available: ["//apex_available:platform"], +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget("cc_library_shared", "b", AttrNameToString{ + "implementation_dynamic_deps": `[":a"]`, + "tags": `["apex_available=//apex_available:platform"]`, + }), + }, + }) +} + func TestCcLibrarySharedStubs_MultipleApexAvailable(t *testing.T) { runCcLibrarySharedTestCase(t, Bp2buildTestCase{ ModuleTypeUnderTest: "cc_library_shared", @@ -682,7 +770,7 @@ cc_library_shared { ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_shared", "b", AttrNameToString{ "implementation_dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:a"], + "//build/bazel/rules/apex:system": ["@api_surfaces//module-libapi/current:a"], "//conditions:default": [":a"], })`, "tags": `[ diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 948801486..2705aafe5 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -1504,6 +1504,7 @@ cc_library { versions: ["current"], }, bazel_module: { bp2build_available: false }, + apex_available: ["com.android.runtime"], } cc_library_static { @@ -1561,7 +1562,8 @@ cc_library_static { }), MakeBazelTarget("cc_library_static", "keep_with_stubs", AttrNameToString{ "implementation_dynamic_deps": `select({ - "//build/bazel/rules/apex:android-in_apex": ["@api_surfaces//module-libapi/current:libm"], + "//build/bazel/rules/apex:foo": ["@api_surfaces//module-libapi/current:libm"], + "//build/bazel/rules/apex:system": ["@api_surfaces//module-libapi/current:libm"], "//conditions:default": [":libm"], })`, "system_dynamic_deps": `[]`, diff --git a/cc/bp2build.go b/cc/bp2build.go index ca8d60902..c42a7e8ba 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -1269,7 +1269,7 @@ func createInApexConfigSetting(ctx android.TopDownMutatorContext, apexName strin func inApexConfigSetting(apexAvailable string) string { if apexAvailable == android.AvailableToPlatform { - return bazel.AndroidAndNonApex + return bazel.AndroidPlatform } if apexAvailable == android.AvailableToAnyApex { return bazel.AndroidAndInApex @@ -1278,58 +1278,99 @@ func inApexConfigSetting(apexAvailable string) string { return "//build/bazel/rules/apex:" + apiDomain } +// Inputs to stub vs impl selection. +type stubSelectionInfo struct { + // Label of the implementation library (e.g. //bionic/libc:libc) + impl bazel.Label + // Axis containing the implementation library + axis bazel.ConfigurationAxis + // Axis key containing the implementation library + config string + // API domain of the apex + // For test apexes (test_com.android.foo), this will be the source apex (com.android.foo) + apiDomain string + // List of dep labels + dynamicDeps *bazel.LabelListAttribute + // Boolean value for determining if the dep is in the same api domain + // If false, the label will be rewritten to to the stub label + sameApiDomain bool +} + +func useStubOrImplInApexWithName(ssi stubSelectionInfo) { + lib := ssi.impl + if !ssi.sameApiDomain { + lib = bazel.Label{ + Label: apiSurfaceModuleLibCurrentPackage + strings.TrimPrefix(lib.OriginalModuleName, ":"), + } + } + // Create a select statement specific to this apex + inApexSelectValue := ssi.dynamicDeps.SelectValue(bazel.OsAndInApexAxis, inApexConfigSetting(ssi.apiDomain)) + (&inApexSelectValue).Append(bazel.MakeLabelList([]bazel.Label{lib})) + ssi.dynamicDeps.SetSelectValue(bazel.OsAndInApexAxis, inApexConfigSetting(ssi.apiDomain), bazel.FirstUniqueBazelLabelList(inApexSelectValue)) + // Delete the library from the common config for this apex + implDynamicDeps := ssi.dynamicDeps.SelectValue(ssi.axis, ssi.config) + implDynamicDeps = bazel.SubtractBazelLabelList(implDynamicDeps, bazel.MakeLabelList([]bazel.Label{ssi.impl})) + ssi.dynamicDeps.SetSelectValue(ssi.axis, ssi.config, implDynamicDeps) + if ssi.axis == bazel.NoConfigAxis { + // Set defaults. Defaults (i.e. host) should use impl and not stubs. + defaultSelectValue := ssi.dynamicDeps.SelectValue(bazel.OsAndInApexAxis, bazel.ConditionsDefaultConfigKey) + (&defaultSelectValue).Append(bazel.MakeLabelList([]bazel.Label{ssi.impl})) + ssi.dynamicDeps.SetSelectValue(bazel.OsAndInApexAxis, bazel.ConditionsDefaultConfigKey, bazel.FirstUniqueBazelLabelList(defaultSelectValue)) + } +} + func setStubsForDynamicDeps(ctx android.BazelConversionPathContext, axis bazel.ConfigurationAxis, config string, apexAvailable []string, dynamicLibs bazel.LabelList, dynamicDeps *bazel.LabelListAttribute, ind int, buildNonApexWithStubs bool) { - depsWithStubs := []bazel.Label{} - for _, l := range dynamicLibs.Includes { - dep, _ := ctx.ModuleFromName(l.OriginalModuleName) - if d, ok := dep.(*Module); ok && d.HasStubsVariants() { - depApexAvailable := d.ApexAvailable() - if !availableToSameApexes(apexAvailable, depApexAvailable) { - depsWithStubs = append(depsWithStubs, l) - } - } - } - if len(depsWithStubs) > 0 { - implDynamicDeps := bazel.SubtractBazelLabelList(dynamicLibs, bazel.MakeLabelList(depsWithStubs)) - dynamicDeps.SetSelectValue(axis, config, implDynamicDeps) - - stubLibLabels := []bazel.Label{} - for _, l := range depsWithStubs { - stubLabelInApiSurfaces := bazel.Label{ - Label: apiSurfaceModuleLibCurrentPackage + strings.TrimPrefix(l.OriginalModuleName, ":"), - } - stubLibLabels = append(stubLibLabels, stubLabelInApiSurfaces) - } - inApexSelectValue := dynamicDeps.SelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndInApex) - nonApexSelectValue := dynamicDeps.SelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndNonApex) - defaultSelectValue := dynamicDeps.SelectValue(bazel.OsAndInApexAxis, bazel.ConditionsDefaultConfigKey) - nonApexDeps := depsWithStubs - if buildNonApexWithStubs { - nonApexDeps = stubLibLabels - } - if axis == bazel.NoConfigAxis { - (&inApexSelectValue).Append(bazel.MakeLabelList(stubLibLabels)) - (&nonApexSelectValue).Append(bazel.MakeLabelList(nonApexDeps)) - (&defaultSelectValue).Append(bazel.MakeLabelList(depsWithStubs)) - dynamicDeps.SetSelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndInApex, bazel.FirstUniqueBazelLabelList(inApexSelectValue)) - dynamicDeps.SetSelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndNonApex, bazel.FirstUniqueBazelLabelList(nonApexSelectValue)) - dynamicDeps.SetSelectValue(bazel.OsAndInApexAxis, bazel.ConditionsDefaultConfigKey, bazel.FirstUniqueBazelLabelList(defaultSelectValue)) - } else if config == bazel.OsAndroid { - (&inApexSelectValue).Append(bazel.MakeLabelList(stubLibLabels)) - (&nonApexSelectValue).Append(bazel.MakeLabelList(nonApexDeps)) - dynamicDeps.SetSelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndInApex, bazel.FirstUniqueBazelLabelList(inApexSelectValue)) - dynamicDeps.SetSelectValue(bazel.OsAndInApexAxis, bazel.AndroidAndNonApex, bazel.FirstUniqueBazelLabelList(nonApexSelectValue)) - } - } - // Create a config_setting for each apex_available. // This will be used to select impl of a dep if dep is available to the same apex. for _, aa := range apexAvailable { createInApexConfigSetting(ctx.(android.TopDownMutatorContext), aa) } + apiDomainForSelects := []string{} + for _, apex := range apexAvailable { + apiDomainForSelects = append(apiDomainForSelects, GetApiDomain(apex)) + } + // Always emit a select statement for the platform variant. + // This ensures that b build //foo --config=android works + // Soong always creates a platform variant even when the library might not be available to platform. + if !android.InList(android.AvailableToPlatform, apiDomainForSelects) { + apiDomainForSelects = append(apiDomainForSelects, android.AvailableToPlatform) + } + apiDomainForSelects = android.SortedUniqueStrings(apiDomainForSelects) + + // Create a select for each apex this library could be included in. + for _, l := range dynamicLibs.Includes { + dep, _ := ctx.ModuleFromName(l.OriginalModuleName) + if c, ok := dep.(*Module); !ok || !c.HasStubsVariants() { + continue + } + // TODO (b/280339069): Decrease the verbosity of the generated BUILD files + for _, apiDomain := range apiDomainForSelects { + var sameApiDomain bool + if apiDomain == android.AvailableToPlatform { + // Platform variants in Soong use equality of apex_available for stub/impl selection. + // https://cs.android.com/android/_/android/platform/build/soong/+/316b0158fe57ee7764235923e7c6f3d530da39c6:cc/cc.go;l=3393-3404;drc=176271a426496fa2688efe2b40d5c74340c63375;bpv=1;bpt=0 + // One of the factors behind this design choice is cc_test + // Tests only have a platform variant, and using equality of apex_available ensures + // that tests of an apex library gets its implementation and not stubs. + // TODO (b/280343104): Discuss if we can drop this special handling for platform variants. + sameApiDomain = availableToSameApexes(apexAvailable, dep.(*Module).ApexAvailable()) + } else { + sameApiDomain = android.InList(apiDomain, dep.(*Module).ApexAvailable()) + } + ssi := stubSelectionInfo{ + impl: l, + axis: axis, + config: config, + apiDomain: apiDomain, + dynamicDeps: dynamicDeps, + sameApiDomain: sameApiDomain, + } + useStubOrImplInApexWithName(ssi) + } + } } func (la *linkerAttributes) convertStripProps(ctx android.BazelConversionPathContext, module *Module) { @@ -1424,7 +1465,6 @@ func (la *linkerAttributes) finalize(ctx android.BazelConversionPathContext) { la.implementationDynamicDeps.Exclude(bazel.OsConfigurationAxis, "linux_bionic", toRemove) la.implementationDynamicDeps.Exclude(bazel.OsAndInApexAxis, bazel.ConditionsDefaultConfigKey, toRemove) - la.implementationDynamicDeps.Exclude(bazel.OsAndInApexAxis, bazel.AndroidAndNonApex, toRemove) stubsToRemove := make([]bazel.Label, 0, len(la.usedSystemDynamicDepAsDynamicDep)) for _, lib := range toRemove.Includes { stubLabelInApiSurfaces := bazel.Label{ @@ -1432,7 +1472,12 @@ func (la *linkerAttributes) finalize(ctx android.BazelConversionPathContext) { } stubsToRemove = append(stubsToRemove, stubLabelInApiSurfaces) } - la.implementationDynamicDeps.Exclude(bazel.OsAndInApexAxis, bazel.AndroidAndInApex, bazel.MakeLabelList(stubsToRemove)) + // system libraries (e.g. libc, libm, libdl) belong the com.android.runtime api domain + // dedupe the stubs of these libraries from the other api domains (platform, other_apexes...) + for _, aa := range ctx.Module().(*Module).ApexAvailable() { + la.implementationDynamicDeps.Exclude(bazel.OsAndInApexAxis, inApexConfigSetting(aa), bazel.MakeLabelList(stubsToRemove)) + } + la.implementationDynamicDeps.Exclude(bazel.OsAndInApexAxis, bazel.AndroidPlatform, bazel.MakeLabelList(stubsToRemove)) } la.deps.ResolveExcludes() From 6aaab9d2aa4f529d04f8bcbb4f87e3f1841e314e Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Thu, 4 May 2023 17:27:30 +0000 Subject: [PATCH 5/5] Special case platform variant of bootstrap libs Platform variants of libraries that set `bootstrap: true` in their Android.bp file gets impl in Soong. This CL ports this behavior to bp2build. Note that even after this CL, there will be still be some other cases where stub/impl logic does not match Soong perfectly (most notably the platform_apis property which is propagated top-down from the parent apex). Test: bp2build.sh Change-Id: I3da284ab42631d6de1c0d52e56ccbfd4e4a09f1d --- cc/bp2build.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cc/bp2build.go b/cc/bp2build.go index c42a7e8ba..33eafe3e7 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -1357,6 +1357,9 @@ func setStubsForDynamicDeps(ctx android.BazelConversionPathContext, axis bazel.C // that tests of an apex library gets its implementation and not stubs. // TODO (b/280343104): Discuss if we can drop this special handling for platform variants. sameApiDomain = availableToSameApexes(apexAvailable, dep.(*Module).ApexAvailable()) + if linkable, ok := ctx.Module().(LinkableInterface); ok && linkable.Bootstrap() { + sameApiDomain = true + } } else { sameApiDomain = android.InList(apiDomain, dep.(*Module).ApexAvailable()) }