From a56e97042c5aa1a3ff1f083a461e2ec730b6aaaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20M=C3=A1rquez=20P=C3=A9rez=20Mu=C3=B1=C3=ADz=20D?= =?UTF-8?q?=C3=ADaz=20P=C3=BAras=20Thaureaux?= Date: Wed, 23 Feb 2022 18:39:59 -0500 Subject: [PATCH] Support `suffix` property in bp2build Support this in cc_{binary,library{,_shared}} Bug: 204811222 Test: Suffix additions to cc_{binary,library{,_shared}}_conversion_test.go Test: mixed_{libc,droid}.sh also builds newly allowlisted Change-Id: I596694794b01b04c542cbcd7d54baeb7d914ba50 --- android/allowlists/allowlists.go | 24 +----- bp2build/cc_binary_conversion_test.go | 63 +++++++++++++- bp2build/cc_library_conversion_test.go | 85 +++++++++++++++++++ bp2build/cc_library_shared_conversion_test.go | 73 ++++++++++++++++ cc/bp2build.go | 9 ++ cc/library.go | 6 ++ 6 files changed, 235 insertions(+), 25 deletions(-) diff --git a/android/allowlists/allowlists.go b/android/allowlists/allowlists.go index 8bef7251b..0cdf792ac 100644 --- a/android/allowlists/allowlists.go +++ b/android/allowlists/allowlists.go @@ -591,30 +591,8 @@ var ( "libadb_pairing_connection_static", "libadb_pairing_server", "libadb_pairing_server_static", - // TODO(b/204811222) support suffix in cc_binary - "acvp_modulewrapper", - "android.hardware.media.c2@1.0-service-v4l2", - "app_process", - "bar_test", - "bench_cxa_atexit", - "bench_noop", - "bench_noop_nostl", - "bench_noop_static", - "boringssl_self_test", - "boringssl_self_test_vendor", - "bssl", - "cavp", - "crash_dump", + // TODO(b/240563612) Needing `stem` selection support for cc_binary "crasher", - "libcxx_test_template", - "linker", - "memory_replay", - "native_bridge_guest_linker", - "native_bridge_stub_library_defaults", - "noop", - "simpleperf_ndk", - "toybox-static", - "zlib_bench", // java_import[_host] issues // tradefed prebuilts depend on libprotobuf diff --git a/bp2build/cc_binary_conversion_test.go b/bp2build/cc_binary_conversion_test.go index 9e449eb10..67d4a1c96 100644 --- a/bp2build/cc_binary_conversion_test.go +++ b/bp2build/cc_binary_conversion_test.go @@ -605,8 +605,67 @@ func TestCcBinaryWithInstructionSet(t *testing.T) { "//conditions:default": [], })`, "local_includes": `["."]`, - }, - }, + }}, + }, + }) +} + +func TestCcBinaryEmptySuffix(t *testing.T) { + runCcBinaryTests(t, ccBinaryBp2buildTestCase{ + description: "binary with empty suffix", + blueprint: ` +{rule_name} { + name: "foo", + suffix: "", +}`, + targets: []testBazelTarget{ + {"cc_binary", "foo", AttrNameToString{ + "local_includes": `["."]`, + "suffix": `""`, + }}, + }, + }) +} + +func TestCcBinarySuffix(t *testing.T) { + runCcBinaryTests(t, ccBinaryBp2buildTestCase{ + description: "binary with suffix", + blueprint: ` +{rule_name} { + name: "foo", + suffix: "-suf", +} +`, + targets: []testBazelTarget{ + {"cc_binary", "foo", AttrNameToString{ + "local_includes": `["."]`, + "suffix": `"-suf"`, + }}, + }, + }) +} + +func TestCcArchVariantBinarySuffix(t *testing.T) { + runCcBinaryTests(t, ccBinaryBp2buildTestCase{ + description: "binary with suffix", + blueprint: ` +{rule_name} { + name: "foo", + arch: { + arm64: { suffix: "-64" }, + arm: { suffix: "-32" }, + }, +} +`, + targets: []testBazelTarget{ + {"cc_binary", "foo", AttrNameToString{ + "local_includes": `["."]`, + "suffix": `select({ + "//build/bazel/platforms/arch:arm": "-32", + "//build/bazel/platforms/arch:arm64": "-64", + "//conditions:default": None, + })`, + }}, }, }) } diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 024d4e0df..c1b4cd097 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -2593,6 +2593,91 @@ func TestCcLibraryWithInstructionSet(t *testing.T) { }) } +func TestCcLibraryEmptySuffix(t *testing.T) { + runCcLibraryTestCase(t, Bp2buildTestCase{ + Description: "cc_library with empty suffix", + ModuleTypeUnderTest: "cc_library", + ModuleTypeUnderTestFactory: cc.LibraryFactory, + Filesystem: map[string]string{ + "foo.c": "", + }, + Blueprint: `cc_library { + name: "foo", + suffix: "", + srcs: ["foo.c"], + include_build_directory: false, +}`, + ExpectedBazelTargets: []string{ + makeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ + "srcs_c": `["foo.c"]`, + }), + makeBazelTarget("cc_library_shared", "foo", AttrNameToString{ + "srcs_c": `["foo.c"]`, + "suffix": `""`, + }), + }, + }) +} + +func TestCcLibrarySuffix(t *testing.T) { + runCcLibraryTestCase(t, Bp2buildTestCase{ + Description: "cc_library with suffix", + ModuleTypeUnderTest: "cc_library", + ModuleTypeUnderTestFactory: cc.LibraryFactory, + Filesystem: map[string]string{ + "foo.c": "", + }, + Blueprint: `cc_library { + name: "foo", + suffix: "-suf", + srcs: ["foo.c"], + include_build_directory: false, +}`, + ExpectedBazelTargets: []string{ + makeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ + "srcs_c": `["foo.c"]`, + }), + makeBazelTarget("cc_library_shared", "foo", AttrNameToString{ + "srcs_c": `["foo.c"]`, + "suffix": `"-suf"`, + }), + }, + }) +} + +func TestCcLibraryArchVariantSuffix(t *testing.T) { + runCcLibraryTestCase(t, Bp2buildTestCase{ + Description: "cc_library with arch-variant suffix", + ModuleTypeUnderTest: "cc_library", + ModuleTypeUnderTestFactory: cc.LibraryFactory, + Filesystem: map[string]string{ + "foo.c": "", + }, + Blueprint: `cc_library { + name: "foo", + arch: { + arm64: { suffix: "-64" }, + arm: { suffix: "-32" }, + }, + srcs: ["foo.c"], + include_build_directory: false, +}`, + ExpectedBazelTargets: []string{ + makeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ + "srcs_c": `["foo.c"]`, + }), + makeBazelTarget("cc_library_shared", "foo", AttrNameToString{ + "srcs_c": `["foo.c"]`, + "suffix": `select({ + "//build/bazel/platforms/arch:arm": "-32", + "//build/bazel/platforms/arch:arm64": "-64", + "//conditions:default": None, + })`, + }), + }, + }) +} + func TestCcLibraryWithAidlSrcs(t *testing.T) { runCcLibraryTestCase(t, Bp2buildTestCase{ Description: "cc_library with aidl srcs", diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go index 7a44f69a2..ed983bf43 100644 --- a/bp2build/cc_library_shared_conversion_test.go +++ b/bp2build/cc_library_shared_conversion_test.go @@ -641,3 +641,76 @@ cc_library_shared { }, }) } + +func TestCcLibrarySharedEmptySuffix(t *testing.T) { + runCcLibrarySharedTestCase(t, Bp2buildTestCase{ + Description: "cc_library_shared with empty suffix", + Filesystem: map[string]string{ + "foo.c": "", + }, + Blueprint: soongCcLibrarySharedPreamble + ` +cc_library_shared { + name: "foo_shared", + suffix: "", + srcs: ["foo.c"], + include_build_directory: false, +}`, + ExpectedBazelTargets: []string{ + makeBazelTarget("cc_library_shared", "foo_shared", AttrNameToString{ + "srcs_c": `["foo.c"]`, + "suffix": `""`, + }), + }, + }) +} + +func TestCcLibrarySharedSuffix(t *testing.T) { + runCcLibrarySharedTestCase(t, Bp2buildTestCase{ + Description: "cc_library_shared with suffix", + Filesystem: map[string]string{ + "foo.c": "", + }, + Blueprint: soongCcLibrarySharedPreamble + ` +cc_library_shared { + name: "foo_shared", + suffix: "-suf", + srcs: ["foo.c"], + include_build_directory: false, +}`, + ExpectedBazelTargets: []string{ + makeBazelTarget("cc_library_shared", "foo_shared", AttrNameToString{ + "srcs_c": `["foo.c"]`, + "suffix": `"-suf"`, + }), + }, + }) +} + +func TestCcLibrarySharedArchVariantSuffix(t *testing.T) { + runCcLibrarySharedTestCase(t, Bp2buildTestCase{ + Description: "cc_library_shared with arch-variant suffix", + Filesystem: map[string]string{ + "foo.c": "", + }, + Blueprint: soongCcLibrarySharedPreamble + ` +cc_library_shared { + name: "foo_shared", + arch: { + arm64: { suffix: "-64" }, + arm: { suffix: "-32" }, + }, + srcs: ["foo.c"], + include_build_directory: false, +}`, + ExpectedBazelTargets: []string{ + makeBazelTarget("cc_library_shared", "foo_shared", AttrNameToString{ + "srcs_c": `["foo.c"]`, + "suffix": `select({ + "//build/bazel/platforms/arch:arm": "-32", + "//build/bazel/platforms/arch:arm64": "-64", + "//conditions:default": None, + })`, + }), + }, + }) +} diff --git a/cc/bp2build.go b/cc/bp2build.go index 625e7ce14..77aaec13d 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -339,6 +339,8 @@ type compilerAttributes struct { stubsVersions bazel.StringListAttribute features bazel.StringListAttribute + + suffix bazel.StringAttribute } type filterOutFn func(string) bool @@ -694,6 +696,9 @@ func bp2BuildParseBaseProps(ctx android.Bp2buildMutatorContext, module *Module) compilerAttrs.stubsSymbolFile = libraryProps.Stubs.Symbol_file compilerAttrs.stubsVersions.SetSelectValue(axis, config, libraryProps.Stubs.Versions) } + if suffix := libraryProps.Suffix; suffix != nil { + compilerAttrs.suffix.SetSelectValue(axis, config, suffix) + } } } } @@ -1201,6 +1206,7 @@ func bazelLabelForSharedDepsExcludes(ctx android.BazelConversionPathContext, mod type binaryLinkerAttrs struct { Linkshared *bool + Suffix bazel.StringAttribute } func bp2buildBinaryLinkerProps(ctx android.BazelConversionPathContext, m *Module) binaryLinkerAttrs { @@ -1217,6 +1223,9 @@ func bp2buildBinaryLinkerProps(ctx android.BazelConversionPathContext, m *Module // nonconfigurable attribute. Only 4 AOSP modules use this feature, defer handling ctx.ModuleErrorf("bp2build cannot migrate a module with arch/target-specific static_executable values") } + if suffix := linkerProps.Suffix; suffix != nil { + attrs.Suffix.SetSelectValue(axis, config, suffix) + } }) return attrs diff --git a/cc/library.go b/cc/library.go index fc03fa2fb..c97970e77 100644 --- a/cc/library.go +++ b/cc/library.go @@ -408,6 +408,8 @@ func libraryBp2Build(ctx android.TopDownMutatorContext, m *Module) { sharedTargetAttrs.Has_stubs.SetValue(&hasStubs) } + sharedTargetAttrs.Suffix = compilerAttrs.suffix + for axis, configToProps := range m.GetArchVariantProperties(ctx, &LibraryProperties{}) { for config, props := range configToProps { if props, ok := props.(*LibraryProperties); ok { @@ -2647,6 +2649,8 @@ func sharedOrStaticLibraryBp2Build(ctx android.TopDownMutatorContext, module *Mo }, Features: baseAttributes.features, + + Suffix: compilerAttrs.suffix, } if compilerAttrs.stubsSymbolFile != nil && len(compilerAttrs.stubsVersions.Value) > 0 { hasStubs := true @@ -2729,6 +2733,8 @@ type bazelCcLibrarySharedAttributes struct { Has_stubs bazel.BoolAttribute Inject_bssl_hash bazel.BoolAttribute + + Suffix bazel.StringAttribute } type bazelCcStubSuiteAttributes struct {