From 0f82844b75680862f867c57b8f6d315a43c88fad Mon Sep 17 00:00:00 2001 From: Zi Wang Date: Wed, 28 Dec 2022 11:18:11 -0800 Subject: [PATCH] Add variant_prepend test for cc_library The comments in aosp/2336916 are also addressed here Test: TH and TestCcLibraryVariantPrependPropOrder Change-Id: If1472658fcd4b5544dec2e2691049a180520c84e --- android/arch_test.go | 2 + bazel/properties.go | 3 - bp2build/cc_library_conversion_test.go | 135 +++++++++++++++++++++++++ bp2build/configurability.go | 2 +- 4 files changed, 138 insertions(+), 4 deletions(-) diff --git a/android/arch_test.go b/android/arch_test.go index 46c018a0a..e445ec66f 100644 --- a/android/arch_test.go +++ b/android/arch_test.go @@ -584,6 +584,8 @@ type testArchPropertiesModule struct { func (testArchPropertiesModule) GenerateAndroidBuildActions(ctx ModuleContext) {} +// Module property "a" does not have "variant_prepend" tag. +// Expected variant property orders are based on this fact. func TestArchProperties(t *testing.T) { bp := ` module { diff --git a/bazel/properties.go b/bazel/properties.go index 692198402..f9cabf2ce 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -1278,9 +1278,6 @@ func (sla StringListAttribute) HasConfigurableValues() bool { // Append appends all values, including os and arch specific ones, from another // StringListAttribute to this StringListAttribute func (sla *StringListAttribute) Append(other StringListAttribute) *StringListAttribute { - if sla.Prepend != other.Prepend { - panic(fmt.Errorf("StringListAttribute could not be appended because it has different prepend value")) - } sla.Value = append(sla.Value, other.Value...) if sla.ConfigurableValues == nil { sla.ConfigurableValues = make(configurableStringLists) diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index cfd1a8cf7..595ee637a 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -3773,3 +3773,138 @@ cc_library { }, ) } + +// Export_include_dirs and Export_system_include_dirs have "variant_prepend" tag. +// In bp2build output, variant info(select) should go before general info. +// Internal order of the property should be unchanged. (e.g. ["eid1", "eid2"]) +func TestCcLibraryVariantPrependPropOrder(t *testing.T) { + runCcLibraryTestCase(t, Bp2buildTestCase{ + Description: "cc_library variant prepend properties order", + ModuleTypeUnderTest: "cc_library", + ModuleTypeUnderTestFactory: cc.LibraryFactory, + Blueprint: soongCcLibraryPreamble + ` +cc_library { + name: "a", + srcs: ["a.cpp"], + export_include_dirs: ["eid1", "eid2"], + export_system_include_dirs: ["esid1", "esid2"], + target: { + android: { + export_include_dirs: ["android_eid1", "android_eid2"], + export_system_include_dirs: ["android_esid1", "android_esid2"], + }, + android_arm: { + export_include_dirs: ["android_arm_eid1", "android_arm_eid2"], + export_system_include_dirs: ["android_arm_esid1", "android_arm_esid2"], + }, + linux: { + export_include_dirs: ["linux_eid1", "linux_eid2"], + export_system_include_dirs: ["linux_esid1", "linux_esid2"], + }, + }, + multilib: { + lib32: { + export_include_dirs: ["lib32_eid1", "lib32_eid2"], + export_system_include_dirs: ["lib32_esid1", "lib32_esid2"], + }, + }, + arch: { + arm: { + export_include_dirs: ["arm_eid1", "arm_eid2"], + export_system_include_dirs: ["arm_esid1", "arm_esid2"], + }, + } +} +`, + ExpectedBazelTargets: makeCcLibraryTargets("a", AttrNameToString{ + "export_includes": `select({ + "//build/bazel/platforms/os_arch:android_arm": [ + "android_arm_eid1", + "android_arm_eid2", + ], + "//conditions:default": [], + }) + select({ + "//build/bazel/platforms/os:android": [ + "android_eid1", + "android_eid2", + "linux_eid1", + "linux_eid2", + ], + "//build/bazel/platforms/os:linux_bionic": [ + "linux_eid1", + "linux_eid2", + ], + "//build/bazel/platforms/os:linux_glibc": [ + "linux_eid1", + "linux_eid2", + ], + "//build/bazel/platforms/os:linux_musl": [ + "linux_eid1", + "linux_eid2", + ], + "//conditions:default": [], + }) + select({ + "//build/bazel/platforms/arch:arm": [ + "lib32_eid1", + "lib32_eid2", + "arm_eid1", + "arm_eid2", + ], + "//build/bazel/platforms/arch:x86": [ + "lib32_eid1", + "lib32_eid2", + ], + "//conditions:default": [], + }) + [ + "eid1", + "eid2", + ]`, + "export_system_includes": `select({ + "//build/bazel/platforms/os_arch:android_arm": [ + "android_arm_esid1", + "android_arm_esid2", + ], + "//conditions:default": [], + }) + select({ + "//build/bazel/platforms/os:android": [ + "android_esid1", + "android_esid2", + "linux_esid1", + "linux_esid2", + ], + "//build/bazel/platforms/os:linux_bionic": [ + "linux_esid1", + "linux_esid2", + ], + "//build/bazel/platforms/os:linux_glibc": [ + "linux_esid1", + "linux_esid2", + ], + "//build/bazel/platforms/os:linux_musl": [ + "linux_esid1", + "linux_esid2", + ], + "//conditions:default": [], + }) + select({ + "//build/bazel/platforms/arch:arm": [ + "lib32_esid1", + "lib32_esid2", + "arm_esid1", + "arm_esid2", + ], + "//build/bazel/platforms/arch:x86": [ + "lib32_esid1", + "lib32_esid2", + ], + "//conditions:default": [], + }) + [ + "esid1", + "esid2", + ]`, + "srcs": `["a.cpp"]`, + "local_includes": `["."]`, + "target_compatible_with": `["//build/bazel/platforms/os:android"]`, + }), + }, + ) +} diff --git a/bp2build/configurability.go b/bp2build/configurability.go index 112755b53..c6309655d 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -39,7 +39,7 @@ func getStringValue(str bazel.StringAttribute) (reflect.Value, []selects) { func getStringListValues(list bazel.StringListAttribute) (reflect.Value, []selects, bool) { value := reflect.ValueOf(list.Value) - prepend := reflect.ValueOf(list.Prepend).Bool() + prepend := list.Prepend if !list.HasConfigurableValues() { return value, []selects{}, prepend }