From aabfb5dc4778a587ff2df8ec3e81f424f658f5ae Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 8 Dec 2021 15:25:06 -0500 Subject: [PATCH] bp2build: Expand check if filegroup contains proto Previously we looked for proto as suffix, however, some filegroups use "-proto-srcs" or "-proto-sources", instead we look for proto as a distinct word in a filegroup name. Test: go test soong tests Change-Id: Icf916a84304a02617efff9768e5b82d5ffe658bd --- android/bazel.go | 14 +-- bp2build/cc_library_conversion_test.go | 118 +++++++++++++++++++++++++ bp2build/testing.go | 4 +- cc/bp2build.go | 17 ++-- 4 files changed, 140 insertions(+), 13 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index 8c63204b3..4114f37a3 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -333,7 +333,7 @@ var ( "packages/services/Car/tests/SampleRearViewCamera": Bp2BuildDefaultTrue, "prebuilts/clang/host/linux-x86": Bp2BuildDefaultTrueRecursively, "system/apex": Bp2BuildDefaultFalse, // TODO(b/207466993): flaky failures - "system/core/debuggerd": Bp2BuildDefaultTrue, + "system/core/debuggerd": Bp2BuildDefaultTrueRecursively, "system/core/diagnose_usb": Bp2BuildDefaultTrueRecursively, "system/core/libasyncio": Bp2BuildDefaultTrue, "system/core/libcrypto_utils": Bp2BuildDefaultTrueRecursively, @@ -394,9 +394,13 @@ var ( "libbacktrace", // depends on unconverted module libunwindstack "libdebuggerd_handler", // depends on unconverted module libdebuggerd_handler_core "libdebuggerd_handler_core", "libdebuggerd_handler_fallback", // depends on unconverted module libdebuggerd - "unwind_for_offline", // depends on unconverted module libunwindstack_utils - "libdebuggerd", // depends on unconverted modules libdexfile_support, libunwindstack, gwp_asan_crash_handler, libtombstone_proto, libprotobuf-cpp-lite - "libdexfile_static", // depends on libartpalette, libartbase, libdexfile, which are of unsupported type: art_cc_library. + "unwind_for_offline", // depends on unconverted module libunwindstack_utils + "libdebuggerd", // depends on unconverted modules libdexfile_support, libunwindstack, gwp_asan_crash_handler, libtombstone_proto, libprotobuf-cpp-lite + "libdexfile_static", // depends on libartpalette, libartbase, libdexfile, which are of unsupported type: art_cc_library. + + "crasher", // depends on unconverted modules: libseccomp_policy + "static_crasher", // depends on unconverted modules: libdebuggerd_handler, libseccomp_policy + "host_bionic_linker_asm", // depends on extract_linker, a go binary. "host_bionic_linker_script", // depends on extract_linker, a go binary. @@ -409,8 +413,6 @@ var ( "libbase_ndk", // http://b/186826477, fails to link libctscamera2_jni for device (required for CtsCameraTestCases) - "lib_linker_config_proto_lite", // contains .proto sources - "libprotobuf-python", // contains .proto sources "libprotobuf-internal-protos", // b/210751803, we don't handle path property for filegroups "libprotobuf-internal-python-srcs", // b/210751803, we don't handle path property for filegroups diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index b4eb28f00..eaceea969 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -2041,3 +2041,121 @@ func TestCcLibraryProtoExportHeaders(t *testing.T) { }, }) } + +func TestCcLibraryProtoFilegroups(t *testing.T) { + runCcLibraryTestCase(t, bp2buildTestCase{ + moduleTypeUnderTest: "cc_library", + moduleTypeUnderTestFactory: cc.LibraryFactory, + blueprint: soongCcProtoPreamble + + simpleModuleDoNotConvertBp2build("filegroup", "a_fg_proto") + + simpleModuleDoNotConvertBp2build("filegroup", "b_protos") + + simpleModuleDoNotConvertBp2build("filegroup", "c-proto-srcs") + + simpleModuleDoNotConvertBp2build("filegroup", "proto-srcs-d") + ` +cc_library { + name: "a", + srcs: [":a_fg_proto"], + proto: { + canonical_path_from_root: false, + export_proto_headers: true, + }, + include_build_directory: false, +} + +cc_library { + name: "b", + srcs: [":b_protos"], + proto: { + canonical_path_from_root: false, + export_proto_headers: true, + }, + include_build_directory: false, +} + +cc_library { + name: "c", + srcs: [":c-proto-srcs"], + proto: { + canonical_path_from_root: false, + export_proto_headers: true, + }, + include_build_directory: false, +} + +cc_library { + name: "d", + srcs: [":proto-srcs-d"], + proto: { + canonical_path_from_root: false, + export_proto_headers: true, + }, + include_build_directory: false, +}`, + expectedBazelTargets: []string{ + makeBazelTarget("proto_library", "a_proto", attrNameToString{ + "srcs": `[":a_fg_proto"]`, + }), makeBazelTarget("cc_lite_proto_library", "a_cc_proto_lite", attrNameToString{ + "deps": `[":a_proto"]`, + }), makeBazelTarget("cc_library_static", "a_bp2build_cc_library_static", attrNameToString{ + "deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":a_cc_proto_lite"]`, + "srcs": `[":a_fg_proto_cpp_srcs"]`, + "srcs_as": `[":a_fg_proto_as_srcs"]`, + "srcs_c": `[":a_fg_proto_c_srcs"]`, + }), makeBazelTarget("cc_library_shared", "a", attrNameToString{ + "dynamic_deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":a_cc_proto_lite"]`, + "srcs": `[":a_fg_proto_cpp_srcs"]`, + "srcs_as": `[":a_fg_proto_as_srcs"]`, + "srcs_c": `[":a_fg_proto_c_srcs"]`, + }), makeBazelTarget("proto_library", "b_proto", attrNameToString{ + "srcs": `[":b_protos"]`, + }), makeBazelTarget("cc_lite_proto_library", "b_cc_proto_lite", attrNameToString{ + "deps": `[":b_proto"]`, + }), makeBazelTarget("cc_library_static", "b_bp2build_cc_library_static", attrNameToString{ + "deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":b_cc_proto_lite"]`, + "srcs": `[":b_protos_cpp_srcs"]`, + "srcs_as": `[":b_protos_as_srcs"]`, + "srcs_c": `[":b_protos_c_srcs"]`, + }), makeBazelTarget("cc_library_shared", "b", attrNameToString{ + "dynamic_deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":b_cc_proto_lite"]`, + "srcs": `[":b_protos_cpp_srcs"]`, + "srcs_as": `[":b_protos_as_srcs"]`, + "srcs_c": `[":b_protos_c_srcs"]`, + }), makeBazelTarget("proto_library", "c_proto", attrNameToString{ + "srcs": `[":c-proto-srcs"]`, + }), makeBazelTarget("cc_lite_proto_library", "c_cc_proto_lite", attrNameToString{ + "deps": `[":c_proto"]`, + }), makeBazelTarget("cc_library_static", "c_bp2build_cc_library_static", attrNameToString{ + "deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":c_cc_proto_lite"]`, + "srcs": `[":c-proto-srcs_cpp_srcs"]`, + "srcs_as": `[":c-proto-srcs_as_srcs"]`, + "srcs_c": `[":c-proto-srcs_c_srcs"]`, + }), makeBazelTarget("cc_library_shared", "c", attrNameToString{ + "dynamic_deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":c_cc_proto_lite"]`, + "srcs": `[":c-proto-srcs_cpp_srcs"]`, + "srcs_as": `[":c-proto-srcs_as_srcs"]`, + "srcs_c": `[":c-proto-srcs_c_srcs"]`, + }), makeBazelTarget("proto_library", "d_proto", attrNameToString{ + "srcs": `[":proto-srcs-d"]`, + }), makeBazelTarget("cc_lite_proto_library", "d_cc_proto_lite", attrNameToString{ + "deps": `[":d_proto"]`, + }), makeBazelTarget("cc_library_static", "d_bp2build_cc_library_static", attrNameToString{ + "deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":d_cc_proto_lite"]`, + "srcs": `[":proto-srcs-d_cpp_srcs"]`, + "srcs_as": `[":proto-srcs-d_as_srcs"]`, + "srcs_c": `[":proto-srcs-d_c_srcs"]`, + }), makeBazelTarget("cc_library_shared", "d", attrNameToString{ + "dynamic_deps": `[":libprotobuf-cpp-lite"]`, + "whole_archive_deps": `[":d_cc_proto_lite"]`, + "srcs": `[":proto-srcs-d_cpp_srcs"]`, + "srcs_as": `[":proto-srcs-d_as_srcs"]`, + "srcs_c": `[":proto-srcs-d_c_srcs"]`, + }), + }, + }) +} diff --git a/bp2build/testing.go b/bp2build/testing.go index 15cf48689..8ae1a38ba 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -133,8 +133,8 @@ func runBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi android.FailIfErrored(t, errs) } if actualCount, expectedCount := len(bazelTargets), len(tc.expectedBazelTargets); actualCount != expectedCount { - t.Errorf("%s: Expected %d bazel target, got `%d``", - tc.description, expectedCount, actualCount) + t.Errorf("%s: Expected %d bazel target (%s), got `%d`` (%s)", + tc.description, expectedCount, tc.expectedBazelTargets, actualCount, bazelTargets) } else { for i, target := range bazelTargets { if w, g := tc.expectedBazelTargets[i], target.content; w != g { diff --git a/cc/bp2build.go b/cc/bp2build.go index 6c1646cbb..fad40beea 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -16,6 +16,7 @@ package cc import ( "fmt" "path/filepath" + "regexp" "strings" "android/soong/android" @@ -33,6 +34,12 @@ const ( protoSrcPartition = "proto" ) +var ( + // ignoring case, checks for proto or protos as an independent word in the name, whether at the + // beginning, end, or middle. e.g. "proto.foo", "bar-protos", "baz_proto_srcs" would all match + filegroupLikelyProtoPattern = regexp.MustCompile("(?i)(^|[^a-z])proto(s)?([^a-z]|$)") +) + // staticOrSharedAttributes are the Bazel-ified versions of StaticOrSharedProperties -- // properties which apply to either the shared or static version of a cc_library module. type staticOrSharedAttributes struct { @@ -77,18 +84,18 @@ func groupSrcsByExtension(ctx android.BazelConversionPathContext, srcs bazel.Lab if !exists || !isFilegroup(m) { return labelStr, false } - likelyProtos := strings.HasSuffix(labelStr, "proto") || strings.HasSuffix(labelStr, "protos") + likelyProtos := filegroupLikelyProtoPattern.MatchString(label.OriginalModuleName) return labelStr, likelyProtos } // TODO(b/190006308): Handle language detection of sources in a Bazel rule. partitioned := bazel.PartitionLabelListAttribute(ctx, &srcs, bazel.LabelPartitions{ - cSrcPartition: bazel.LabelPartition{Extensions: []string{".c"}, LabelMapper: addSuffixForFilegroup("_c_srcs")}, - asSrcPartition: bazel.LabelPartition{Extensions: []string{".s", ".S"}, LabelMapper: addSuffixForFilegroup("_as_srcs")}, + protoSrcPartition: bazel.LabelPartition{Extensions: []string{".proto"}, LabelMapper: isProtoFilegroup}, + cSrcPartition: bazel.LabelPartition{Extensions: []string{".c"}, LabelMapper: addSuffixForFilegroup("_c_srcs")}, + asSrcPartition: bazel.LabelPartition{Extensions: []string{".s", ".S"}, LabelMapper: addSuffixForFilegroup("_as_srcs")}, // C++ is the "catch-all" group, and comprises generated sources because we don't // know the language of these sources until the genrule is executed. - cppSrcPartition: bazel.LabelPartition{Extensions: []string{".cpp", ".cc", ".cxx", ".mm"}, LabelMapper: addSuffixForFilegroup("_cpp_srcs"), Keep_remainder: true}, - protoSrcPartition: bazel.LabelPartition{Extensions: []string{".proto"}, LabelMapper: isProtoFilegroup}, + cppSrcPartition: bazel.LabelPartition{Extensions: []string{".cpp", ".cc", ".cxx", ".mm"}, LabelMapper: addSuffixForFilegroup("_cpp_srcs"), Keep_remainder: true}, }) return partitioned