From dde13c89068051abdfd5f808e248cd70b35c50f9 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 10 May 2024 15:51:02 -0700 Subject: [PATCH] Apply repo mapping to `cc_shared_library`'s `exports_filter` This is required for `exports_filter` to match any external repositories with Bzlmod enabled. `native.package_relative_label` is used to convert the user-specified filter strings, which are always valid labels, to canonical label literals that are then passed to the actual `cc_shared_library` rule. Fixes #21872 Closes #21622. PiperOrigin-RevId: 632624776 Change-Id: I2f80563d8c434b2726f4facb0551b316b2cd2e1c --- .../builtins_bzl/common/cc/cc_binary.bzl | 5 ++-- .../common/cc/cc_shared_library.bzl | 25 +++++++++++++++- .../builtins_bzl/common/cc/cc_test.bzl | 4 +-- src/main/starlark/tests/builtins_bzl/BUILD | 3 ++ .../test_cc_shared_library/BUILD.builtin_test | 30 +++++++++++++++++-- .../test_cc_shared_library/starlark_tests.bzl | 17 +++++++++-- .../test_cc_shared_library2/MODULE.bazel | 1 + .../test_cc_shared_library2/WORKSPACE | 1 - .../tests/builtins_bzl/cc_builtin_tests.sh | 7 +++-- 9 files changed, 78 insertions(+), 15 deletions(-) create mode 100644 src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/MODULE.bazel diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl index fb8e289e489b05..211b59c4d402ae 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl @@ -15,11 +15,10 @@ """cc_binary Starlark implementation replacing native""" load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs") -load(":common/cc/cc_shared_library.bzl", "cc_shared_library_initializer") load(":common/cc/cc_common.bzl", "cc_common") load(":common/cc/cc_helper.bzl", "cc_helper", "linker_mode") load(":common/cc/cc_info.bzl", "CcInfo") -load(":common/cc/cc_shared_library.bzl", "GraphNodeInfo", "add_unused_dynamic_deps", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "merge_cc_shared_library_infos", "separate_static_and_dynamic_link_libraries", "sort_linker_inputs", "throw_linked_but_not_exported_errors") +load(":common/cc/cc_shared_library.bzl", "GraphNodeInfo", "add_unused_dynamic_deps", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "dynamic_deps_initializer", "merge_cc_shared_library_infos", "separate_static_and_dynamic_link_libraries", "sort_linker_inputs", "throw_linked_but_not_exported_errors") load(":common/cc/semantics.bzl", "semantics") DebugPackageInfo = _builtins.toplevel.DebugPackageInfo @@ -924,7 +923,7 @@ def _impl(ctx): cc_binary = rule( implementation = _impl, - initializer = cc_shared_library_initializer, + initializer = dynamic_deps_initializer, attrs = cc_binary_attrs, outputs = { "stripped_binary": "%{name}.stripped", diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl index 7c52aa46117481..e0039cb1ae3d84 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl @@ -838,8 +838,31 @@ graph_structure_aspect = aspect( implementation = _graph_structure_aspect_impl, ) +def _cc_shared_library_initializer(**kwargs): + """Converts labels in exports_filter into canonical form relative to the current repository. + + This conversion can only be done in a macro as it requires access to the repository mapping of + the repository containing the cc_shared_library target. This mapping is automatically + applied to label attributes, but exports_filter is a list of strings attribute. + """ + if "exports_filter" not in kwargs: + return kwargs + + raw_exports_filter = kwargs["exports_filter"] + if type(raw_exports_filter) != type([]): + # TODO: Also canonicalize labels in selects once macros can operate on them. + # https://github.com/bazelbuild/bazel/issues/14157 + return kwargs + + canonical_exports_filter = [ + str(_builtins.native.package_relative_label(s)) + for s in raw_exports_filter + ] + return kwargs | {"exports_filter": canonical_exports_filter} + cc_shared_library = rule( implementation = _cc_shared_library_impl, + initializer = _cc_shared_library_initializer, attrs = { "additional_linker_inputs": attr.label_list(allow_files = True), "shared_lib_name": attr.string(), @@ -858,7 +881,7 @@ cc_shared_library = rule( fragments = ["cpp"] + semantics.additional_fragments(), ) -def cc_shared_library_initializer(**kwargs): +def dynamic_deps_initializer(**kwargs): """Initializes dynamic_deps_attrs""" if "dynamic_deps" in kwargs and cc_helper.is_non_empty_list_or_select(kwargs["dynamic_deps"], "dynamic_deps"): # Propagate an aspect if dynamic_deps attribute is specified. diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl index a23ecb3e232382..8272136dba792f 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_test.bzl @@ -16,8 +16,8 @@ load(":common/cc/cc_binary.bzl", "cc_binary_impl") load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs") -load(":common/cc/cc_shared_library.bzl", "cc_shared_library_initializer") load(":common/cc/cc_helper.bzl", "cc_helper") +load(":common/cc/cc_shared_library.bzl", "dynamic_deps_initializer") load(":common/cc/semantics.bzl", "semantics") load(":common/paths.bzl", "paths") @@ -100,7 +100,7 @@ _cc_test_attrs.update(semantics.get_test_malloc_attr()) _cc_test_attrs.update(semantics.get_coverage_attrs()) cc_test = rule( - initializer = cc_shared_library_initializer, + initializer = dynamic_deps_initializer, implementation = _impl, attrs = _cc_test_attrs, outputs = { diff --git a/src/main/starlark/tests/builtins_bzl/BUILD b/src/main/starlark/tests/builtins_bzl/BUILD index 804dc1133ad10c..c84c5126a7b93a 100644 --- a/src/main/starlark/tests/builtins_bzl/BUILD +++ b/src/main/starlark/tests/builtins_bzl/BUILD @@ -26,6 +26,9 @@ sh_test( "@rules_testing//lib:truth_bzl", "@rules_testing//lib:util_bzl", ], + tags = [ + "requires-network", # for Bzlmod + ], ) sh_library( diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test index a65044d8e107f9..43e5b16cfe72ab 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test @@ -286,7 +286,7 @@ cc_shared_library( "bar", "bar2", ] + select({ - ":is_bazel": ["@test_repo//:bar"], + ":is_bazel": ["@my_test_repo//:bar"], "//conditions:default": [], }), ) @@ -296,7 +296,7 @@ cc_library( srcs = ["barX.cc"], hdrs = ["barX.h"], deps = select({ - ":is_bazel": ["@test_repo//:bar"], + ":is_bazel": ["@my_test_repo//:bar"], "//conditions:default": [], }), ) @@ -473,6 +473,22 @@ cc_library( hdrs = [":hdr_only_hdr"], ) +cc_library( + name = "external_export", + deps = select({ + ":is_bazel": ["@my_test_repo//:bar"], + "//conditions:default": [], + }), +) + +cc_shared_library( + name = "external_export_so", + exports_filter = ["@my_test_repo//:__pkg__"], + deps = [ + ":external_export", + ], +) + build_failure_test( name = "two_dynamic_deps_same_export_in_so_test", message = "Two shared libraries in dependencies export the same symbols", @@ -536,6 +552,16 @@ exports_test( ], ) +exports_test( + name = "external_export_exports_test", + target = "external_export_so", + bazel_only = True, + targets_that_should_be_claimed_to_be_exported = [ + "@@test_repo~//:bar", + "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:external_export", + ], +) + pdb_test( name = "pdb_test", target = ":foo_so", diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl index d2b096fef647c0..cb84a7408e1ded 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl @@ -351,25 +351,36 @@ nocode_cc_lib = rule( ) def _exports_test_impl(env, target): + if not env.ctx.attr.is_bazel and env.ctx.attr._bazel_only: + return + actual = list(target[CcSharedLibraryInfo].exports) - # Remove the @@ prefix on Bazel + # Remove the @@ prefix for main repo labels on Bazel for i in range(len(actual)): - if actual[i].startswith("@@"): + if actual[i].startswith("@@//"): actual[i] = actual[i][2:] expected = env.ctx.attr._targets_that_should_be_claimed_to_be_exported env.expect.where( detail = "Exports lists do not match.", ).that_collection(actual).contains_exactly(expected).in_order() -def _exports_test_macro(name, target, targets_that_should_be_claimed_to_be_exported): +def _exports_test_macro(name, target, targets_that_should_be_claimed_to_be_exported, bazel_only = False): analysis_test( name = name, impl = _exports_test_impl, target = target, attrs = { + "is_bazel": attr.bool(), + "_bazel_only": attr.bool(default = bazel_only), "_targets_that_should_be_claimed_to_be_exported": attr.string_list(default = targets_that_should_be_claimed_to_be_exported), }, + attr_values = { + "is_bazel": select({ + ":is_bazel": True, + "//conditions:default": False, + }), + }, ) exports_test = _exports_test_macro diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/MODULE.bazel b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/MODULE.bazel new file mode 100644 index 00000000000000..f757b3a0a14fb1 --- /dev/null +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/MODULE.bazel @@ -0,0 +1 @@ +module(name = "test_repo") diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/WORKSPACE b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/WORKSPACE index 838ce120624db3..e69de29bb2d1d6 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/WORKSPACE +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library2/WORKSPACE @@ -1 +0,0 @@ -workspace(name = "test_repo") diff --git a/src/main/starlark/tests/builtins_bzl/cc_builtin_tests.sh b/src/main/starlark/tests/builtins_bzl/cc_builtin_tests.sh index 7397961ba4bb04..3f721f2b6fb5b5 100755 --- a/src/main/starlark/tests/builtins_bzl/cc_builtin_tests.sh +++ b/src/main/starlark/tests/builtins_bzl/cc_builtin_tests.sh @@ -56,9 +56,10 @@ function test_starlark_cc() { mkdir -p "src/conditions" cp "$(rlocation "io_bazel/src/conditions/BUILD")" "src/conditions/BUILD" - cat >> WORKSPACE<> MODULE.bazel<