From 11cbeb10494040ae8576d85c326ccd602d9d9f0e Mon Sep 17 00:00:00 2001 From: Pedro Date: Wed, 8 Feb 2023 12:45:10 +0100 Subject: [PATCH 1/2] Cherrypicks 9815b76, 68aad18 and 4ed6327 from HEAD --- .../builtins_bzl/common/cc/cc_binary.bzl | 4 +- .../cc/experimental_cc_shared_library.bzl | 183 +++++++++++------- .../test_cc_shared_library/BUILD.builtin_test | 104 +++++++--- .../failing_targets/BUILD.builtin_test | 43 +--- .../test_cc_shared_library/foo.cc | 2 + .../test_cc_shared_library/starlark_tests.bzl | 31 ++- .../BUILD.builtin_test | 14 ++ .../test_cc_shared_library3/diff_pkg.cc | 16 ++ .../test_cc_shared_library3/diff_pkg.h | 19 ++ 9 files changed, 271 insertions(+), 145 deletions(-) create mode 100644 src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.cc create mode 100644 src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h 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 23982806a59d4d..baaaaf9e6a4f1c 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl @@ -526,7 +526,7 @@ def _create_transitive_linking_actions( win_def_file = win_def_file, ) cc_launcher_info = cc_internal.create_cc_launcher_info(cc_info = cc_info_without_extra_link_time_libraries, compilation_outputs = cc_compilation_outputs_with_only_objects) - return (cc_linking_outputs, cc_launcher_info, rule_impl_debug_files) + return (cc_linking_outputs, cc_launcher_info, rule_impl_debug_files, cc_linking_context) def _use_pic(ctx, cc_toolchain, cpp_config, feature_configuration): if _is_link_shared(ctx): @@ -735,7 +735,7 @@ def cc_binary_impl(ctx, additional_linkopts): if extra_link_time_libraries != None: linker_inputs_extra, runtime_libraries_extra = extra_link_time_libraries.build_libraries(ctx = ctx, static_mode = linking_mode != _LINKING_DYNAMIC, for_dynamic_library = _is_link_shared(ctx)) - cc_linking_outputs_binary, cc_launcher_info, rule_impl_debug_files = _create_transitive_linking_actions( + cc_linking_outputs_binary, cc_launcher_info, rule_impl_debug_files, deps_cc_linking_context = _create_transitive_linking_actions( ctx, cc_toolchain, feature_configuration, diff --git a/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl b/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl index 32483582fbfb09..5972527b79876e 100644 --- a/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl @@ -33,6 +33,15 @@ cc_common = _builtins.toplevel.cc_common # used sparingly after making sure it's safe to use. LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE" +# Add this as a tag to any static lib target that doesn't export any symbols, +# thus can be statically linked more than once. This is useful in some cases, +# for example, a static lib has a constructor that needs to be run during +# loading time of the shared lib that has it linked into, which is how the +# code gets called by the OS. This static lib might need to be linked as a +# whole archive dep for multiple shared libs, otherwise this static lib will +# be dropped by the linker since there are no incoming symbol references. +NO_EXPORTING = "NO_EXPORTING" + CcSharedLibraryPermissionsInfo = provider( "Permissions for a cc shared library.", fields = { @@ -45,6 +54,7 @@ GraphNodeInfo = provider( "children": "Other GraphNodeInfo from dependencies of this target", "label": "Label of the target visited", "linkable_more_than_once": "Linkable into more than a single cc_shared_library", + "no_exporting": "The static lib doesn't export any symbols so don't export it", }, ) CcSharedLibraryInfo = provider( @@ -63,14 +73,16 @@ CcSharedLibraryInfo = provider( }, ) +# For each target, find out whether it should be linked statically or +# dynamically. def _separate_static_and_dynamic_link_libraries( direct_children, can_be_linked_dynamically, preloaded_deps_direct_labels): node = None all_children = list(direct_children) - link_statically_labels = {} - link_dynamically_labels = {} + targets_to_be_linked_statically_map = {} + targets_to_be_linked_dynamically_set = {} seen_labels = {} @@ -87,12 +99,12 @@ def _separate_static_and_dynamic_link_libraries( seen_labels[node_label] = True if node_label in can_be_linked_dynamically: - link_dynamically_labels[node_label] = True + targets_to_be_linked_dynamically_set[node_label] = True elif node_label not in preloaded_deps_direct_labels: - link_statically_labels[node_label] = node.linkable_more_than_once + targets_to_be_linked_statically_map[node_label] = node.linkable_more_than_once all_children.extend(node.children) - return (link_statically_labels, link_dynamically_labels) + return (targets_to_be_linked_statically_map, targets_to_be_linked_dynamically_set) def _create_linker_context(ctx, linker_inputs): return cc_common.create_linking_context( @@ -132,6 +144,8 @@ def _build_exports_map_from_only_dynamic_deps(merged_shared_library_infos): exports_map[export] = linker_input return exports_map +# The map points from the target that can only be linked once to the +# cc_shared_library target that already links it. def _build_link_once_static_libs_map(merged_shared_library_infos): link_once_static_libs_map = {} for entry in merged_shared_library_infos.to_list(): @@ -237,16 +251,17 @@ def _filter_inputs( ctx, feature_configuration, cc_toolchain, + deps, transitive_exports, preloaded_deps_direct_labels, link_once_static_libs_map): linker_inputs = [] - curr_link_once_static_libs_map = {} + curr_link_once_static_libs_set = {} graph_structure_aspect_nodes = [] dependency_linker_inputs = [] direct_exports = {} - for export in ctx.attr.roots: + for export in deps: direct_exports[str(export.label)] = True dependency_linker_inputs.extend(export[CcInfo].linking_context.linker_inputs.to_list()) graph_structure_aspect_nodes.append(export[GraphNodeInfo]) @@ -257,16 +272,25 @@ def _filter_inputs( if owner in transitive_exports: can_be_linked_dynamically[owner] = True - (link_statically_labels, link_dynamically_labels) = _separate_static_and_dynamic_link_libraries( + # The targets_to_be_linked_statically_map points to whether the target to + # be linked statically can be linked more than once. + (targets_to_be_linked_statically_map, targets_to_be_linked_dynamically_set) = _separate_static_and_dynamic_link_libraries( graph_structure_aspect_nodes, can_be_linked_dynamically, preloaded_deps_direct_labels, ) + # We keep track of precompiled_only_dynamic_libraries, so that we can add + # them to runfiles. precompiled_only_dynamic_libraries = [] - unaccounted_for_libs = [] exports = {} linker_inputs_seen = {} + + # We use this dictionary to give an error if a target containing only + # precompiled dynamic libraries is placed directly in roots. If such a + # precompiled dynamic library is needed it would be because a target in the + # parallel cc_library graph actually needs it. Therefore the precompiled + # dynamic library should be made a dependency of that cc_library instead. dynamic_only_roots = {} linked_statically_but_not_exported = {} for linker_input in dependency_linker_inputs: @@ -275,11 +299,21 @@ def _filter_inputs( continue linker_inputs_seen[stringified_linker_input] = True owner = str(linker_input.owner) - if owner in link_dynamically_labels: - dynamic_linker_input = transitive_exports[owner] - linker_inputs.append(dynamic_linker_input) - elif owner in link_statically_labels: + if owner in targets_to_be_linked_dynamically_set: + # Link the library in this iteration dynamically, + # transitive_exports contains the artifacts produced by a + # cc_shared_library + linker_inputs.append(transitive_exports[owner]) + elif owner in targets_to_be_linked_statically_map: if owner in link_once_static_libs_map: + # We are building a dictionary that will allow us to give + # proper errors for libraries that have been linked multiple + # times elsewhere but haven't been exported. The values in the + # link_once_static_libs_map dictionary are the + # cc_shared_library targets. In this iteration we know of at + # least one target (i.e. owner) which is being linked + # statically by the cc_shared_library + # link_once_static_libs_map[owner] but is not being exported linked_statically_but_not_exported.setdefault(link_once_static_libs_map[owner], []).append(owner) is_direct_export = owner in direct_exports @@ -301,41 +335,26 @@ def _filter_inputs( if len(static_libraries) and owner in dynamic_only_roots: dynamic_only_roots.pop(owner) + linker_input_to_be_linked_statically = linker_input if is_direct_export: - wrapped_library = _wrap_static_library_with_alwayslink( + linker_input_to_be_linked_statically = _wrap_static_library_with_alwayslink( ctx, feature_configuration, cc_toolchain, linker_input, ) + elif _check_if_target_should_be_exported_with_filter( + linker_input.owner, + ctx.label, + ctx.attr.exports_filter, + _get_permissions(ctx), + ): + exports[owner] = True - if not link_statically_labels[owner]: - curr_link_once_static_libs_map[owner] = True - linker_inputs.append(wrapped_library) - else: - can_be_linked_statically = False - - for static_dep_path in ctx.attr.static_deps: - static_dep_path_label = ctx.label.relative(static_dep_path) - if _check_if_target_under_path(linker_input.owner, static_dep_path_label): - can_be_linked_statically = True - break - - if _check_if_target_should_be_exported_with_filter( - linker_input.owner, - ctx.label, - ctx.attr.exports_filter, - _get_permissions(ctx), - ): - exports[owner] = True - can_be_linked_statically = True - - if can_be_linked_statically: - if not link_statically_labels[owner]: - curr_link_once_static_libs_map[owner] = True - linker_inputs.append(linker_input) - else: - unaccounted_for_libs.append(linker_input.owner) + linker_inputs.append(linker_input_to_be_linked_statically) + + if not targets_to_be_linked_statically_map[owner]: + curr_link_once_static_libs_set[owner] = True if dynamic_only_roots: message = ("Do not place libraries which only contain a " + @@ -346,8 +365,7 @@ def _filter_inputs( fail(message) _throw_linked_but_not_exported_errors(linked_statically_but_not_exported) - _throw_error_if_unaccounted_libs(unaccounted_for_libs) - return (exports, linker_inputs, curr_link_once_static_libs_map.keys(), precompiled_only_dynamic_libraries) + return (exports, linker_inputs, curr_link_once_static_libs_set.keys(), precompiled_only_dynamic_libraries) def _throw_linked_but_not_exported_errors(error_libs_dict): if not error_libs_dict: @@ -366,28 +384,6 @@ def _throw_linked_but_not_exported_errors(error_libs_dict): fail("".join(error_builder)) -def _throw_error_if_unaccounted_libs(unaccounted_for_libs): - if not unaccounted_for_libs: - return - libs_message = [] - different_repos = {} - for unaccounted_lib in unaccounted_for_libs: - different_repos[unaccounted_lib.workspace_name] = True - if len(libs_message) < 10: - libs_message.append(str(unaccounted_lib)) - - if len(unaccounted_for_libs) > 10: - libs_message.append("(and " + str(len(unaccounted_for_libs) - 10) + " others)\n") - - static_deps_message = [] - for repo in different_repos: - static_deps_message.append(" \"@" + repo + "//:__subpackages__\",") - - fail("The following libraries cannot be linked either statically or dynamically:\n" + - "\n".join(libs_message) + "\nTo ignore which libraries get linked" + - " statically for now, add the following to 'static_deps':\n" + - "\n".join(static_deps_message)) - def _same_package_or_above(label_a, label_b): if label_a.workspace_name != label_b.workspace_name: return False @@ -408,9 +404,41 @@ def _get_permissions(ctx): return ctx.attr.permissions return None +def _get_deps(ctx): + if len(ctx.attr.deps) and len(ctx.attr.roots): + fail( + "You are using the attribute 'roots' and 'deps'. 'deps' is the " + + "new name for the attribute 'roots'. The attribute 'roots' will be" + + "removed in the future", + attr = "roots", + ) + + deps = ctx.attr.deps + if not len(deps): + deps = ctx.attr.roots + + return deps + def _cc_shared_library_impl(ctx): semantics.check_experimental_cc_shared_library(ctx) + if not cc_common.check_experimental_cc_shared_library(): + if len(ctx.attr.static_deps): + fail( + "This attribute is a no-op and its usage" + + " is forbidden after cc_shared_library is no longer experimental. " + + "Remove it from every cc_shared_library target", + attr = "static_deps", + ) + if len(ctx.attr.roots): + fail( + "This attribute has been renamed to 'deps'. Simply rename the" + + " attribute on the target.", + attr = "roots", + ) + + deps = _get_deps(ctx) + cc_toolchain = cc_helper.find_cpp_toolchain(ctx) feature_configuration = cc_common.configure_features( ctx = ctx, @@ -421,7 +449,7 @@ def _cc_shared_library_impl(ctx): merged_cc_shared_library_info = _merge_cc_shared_library_infos(ctx) exports_map = _build_exports_map_from_only_dynamic_deps(merged_cc_shared_library_info) - for export in ctx.attr.roots: + for export in deps: # Do not check for overlap between targets matched by the current # rule's exports_filter and what is in exports_map. A library in roots # will have to be linked in statically into the current rule with 100% @@ -450,10 +478,11 @@ def _cc_shared_library_impl(ctx): link_once_static_libs_map = _build_link_once_static_libs_map(merged_cc_shared_library_info) - (exports, linker_inputs, link_once_static_libs, precompiled_only_dynamic_libraries) = _filter_inputs( + (exports, linker_inputs, curr_link_once_static_libs_set, precompiled_only_dynamic_libraries) = _filter_inputs( ctx, feature_configuration, cc_toolchain, + deps, exports_map, preloaded_deps_direct_labels, link_once_static_libs_map, @@ -531,21 +560,26 @@ def _cc_shared_library_impl(ctx): runfiles = runfiles.merge(ctx.runfiles(files = precompiled_only_dynamic_libraries_runfiles)) - for export in ctx.attr.roots: - exports[str(export.label)] = True + for export in deps: + export_label = str(export.label) + if GraphNodeInfo in export and export[GraphNodeInfo].no_exporting: + if export_label in curr_link_once_static_libs_set: + curr_link_once_static_libs_set.remove(export_label) + continue + exports[export_label] = True debug_files = [] exports_debug_file = ctx.actions.declare_file(ctx.label.name + "_exports.txt") ctx.actions.write(content = "\n".join(["Owner:" + str(ctx.label)] + exports.keys()), output = exports_debug_file) link_once_static_libs_debug_file = ctx.actions.declare_file(ctx.label.name + "_link_once_static_libs.txt") - ctx.actions.write(content = "\n".join(["Owner:" + str(ctx.label)] + link_once_static_libs), output = link_once_static_libs_debug_file) + ctx.actions.write(content = "\n".join(["Owner:" + str(ctx.label)] + curr_link_once_static_libs_set), output = link_once_static_libs_debug_file) debug_files.append(exports_debug_file) debug_files.append(link_once_static_libs_debug_file) if not ctx.fragments.cpp.experimental_link_static_libraries_once(): - link_once_static_libs = [] + curr_link_once_static_libs_set = {} library = [] if linking_outputs.library_to_link.resolved_symlink_dynamic_library != None: @@ -574,7 +608,7 @@ def _cc_shared_library_impl(ctx): CcSharedLibraryInfo( dynamic_deps = merged_cc_shared_library_info, exports = exports.keys(), - link_once_static_libs = link_once_static_libs, + link_once_static_libs = curr_link_once_static_libs_set, linker_input = cc_common.create_linker_input( owner = ctx.label, libraries = depset([linking_outputs.library_to_link] + precompiled_only_dynamic_libraries), @@ -595,15 +629,19 @@ def _graph_structure_aspect_impl(target, ctx): # TODO(bazel-team): Add flag to Bazel that can toggle the initialization of # linkable_more_than_once. linkable_more_than_once = False + no_exporting = False if hasattr(ctx.rule.attr, "tags"): for tag in ctx.rule.attr.tags: if tag == LINKABLE_MORE_THAN_ONCE: linkable_more_than_once = True + elif tag == NO_EXPORTING: + no_exporting = True return [GraphNodeInfo( label = ctx.label, children = children, linkable_more_than_once = linkable_more_than_once, + no_exporting = no_exporting, )] def _cc_shared_library_permissions_impl(ctx): @@ -642,6 +680,7 @@ cc_shared_library = rule( "preloaded_deps": attr.label_list(providers = [CcInfo]), "win_def_file": attr.label(allow_single_file = [".def"]), "roots": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]), + "deps": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]), "static_deps": attr.string_list(), "user_link_flags": attr.string_list(), "_def_parser": semantics.get_def_parser(), 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 3789ab4ae39aef..8b75e171d342aa 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 @@ -1,4 +1,14 @@ -load(":starlark_tests.bzl", "additional_inputs_test", "build_failure_test", "debug_files_test", "interface_library_output_group_test", "linking_suffix_test", "paths_test", "runfiles_test") +load( + ":starlark_tests.bzl", + "additional_inputs_test", + "build_failure_test", + "debug_files_test", + "interface_library_output_group_test", + "linking_suffix_test", + "paths_test", + "runfiles_test", + "no_exporting_static_lib_test", +) LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE" @@ -42,28 +52,28 @@ cc_binary( cc_shared_library( name = "python_module", features = ["windows_export_all_symbols"], - roots = [":a_suffix"], + deps = [":a_suffix"], shared_lib_name = "python_module.pyd", ) cc_shared_library( name = "a_so", features = ["windows_export_all_symbols"], - roots = [":a_suffix"], + deps = [":a_suffix"], ) cc_shared_library( name = "diamond_so", dynamic_deps = [":a_so"], features = ["windows_export_all_symbols"], - roots = [":qux"], + deps = [":qux"], ) cc_shared_library( name = "diamond2_so", dynamic_deps = [":a_so"], features = ["windows_export_all_symbols"], - roots = [":qux2"], + deps = [":qux2"], ) cc_binary( @@ -88,19 +98,17 @@ cc_shared_library( ], "//conditions:default": [], }), - dynamic_deps = ["bar_so"], + dynamic_deps = [ + "bar_so" + "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so" + ], features = ["windows_export_all_symbols"], preloaded_deps = ["preloaded_dep"], - roots = [ + deps = [ "baz", "foo", "a_suffix", ], - static_deps = [ - "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux", - "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux2", - "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:prebuilt", - ], user_link_flags = select({ "//src/conditions:linux": [ "-Wl,-rpath,kittens", @@ -139,6 +147,7 @@ cc_library( "qux", "qux2", "prebuilt", + "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg" ], ) @@ -190,18 +199,13 @@ cc_shared_library( permissions = [ "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:permissions", ], - roots = [ + deps = [ "bar", "bar2", ] + select({ ":is_bazel": ["@test_repo//:bar"], "//conditions:default": [], }), - static_deps = [ - "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX", - "@test_repo//:bar", - "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux2", - ], user_link_flags = select({ "//src/conditions:linux": [ "-Wl,--version-script=$(location :bar.lds)", @@ -346,7 +350,7 @@ filegroup( cc_shared_library( name = "direct_so_file", features = ["windows_export_all_symbols"], - roots = [ + deps = [ ":direct_so_file_cc_lib", ], ) @@ -367,7 +371,7 @@ filegroup( cc_shared_library( name = "renamed_so_file", features = ["windows_export_all_symbols"], - roots = [ + deps = [ ":direct_so_file_cc_lib2", ], shared_lib_name = "renamed_so_file.so", @@ -398,6 +402,51 @@ cc_shared_library_permissions( ], ) +cc_library( + name = "static_lib_no_exporting", + srcs = [ + "bar.cc", + "bar.h", + ], + tags = ["NO_EXPORTING"], +) + +cc_library( + name = "static_lib_exporting", + srcs = [ + "bar2.cc", + "bar2.h", + ], +) + +cc_shared_library( + name = "lib_with_no_exporting_roots_1", + deps = [":static_lib_no_exporting"], +) + +cc_shared_library( + name = "lib_with_no_exporting_roots_2", + deps = [":static_lib_no_exporting"], + dynamic_deps = [":lib_with_no_exporting_roots_3"], +) + +cc_shared_library( + name = "lib_with_no_exporting_roots_3", + deps = [":static_lib_no_exporting"], +) + +cc_shared_library( + name = "lib_with_no_exporting_roots", + deps = [ + ":static_lib_no_exporting", + ":static_lib_exporting", + ], + dynamic_deps = [ + ":lib_with_no_exporting_roots_1", + ":lib_with_no_exporting_roots_2", + ], +) + build_failure_test( name = "two_dynamic_deps_same_export_in_so_test", message = "Two shared libraries in dependencies export the same symbols", @@ -433,16 +482,7 @@ runfiles_test( target_under_test = ":python_test", ) -build_failure_test( - name = "static_deps_error_test", - messages = select({ - ":is_bazel": [ - "@//:__subpackages__", - "@test_repo//:__subpackages__", - ], - "//conditions:default": [ - "@//:__subpackages__", - ], - }), - target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:unaccounted_for_libs_so", +no_exporting_static_lib_test( + name = "no_exporting_static_lib_test", + target_under_test = ":lib_with_no_exporting_roots", ) diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test index 82d01bd38879ec..6ab7018d6e2f0b 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test @@ -19,12 +19,9 @@ cc_binary( cc_shared_library( name = "should_fail_shared_lib", dynamic_deps = ["//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:bar_so"], - roots = [ + deps = [ ":intermediate", ], - static_deps = [ - "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX", - ], tags = TAGS, ) @@ -37,7 +34,7 @@ cc_library( cc_shared_library( name = "permissions_fail_so", - roots = [ + deps = [ "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar", ], tags = TAGS, @@ -72,7 +69,7 @@ cc_shared_library( ":b_so", ":b2_so", ], - roots = [ + deps = [ ":a", ], tags = TAGS, @@ -90,7 +87,7 @@ cc_binary( cc_shared_library( name = "b_so", - roots = [ + deps = [ ":b", ], tags = TAGS, @@ -98,38 +95,8 @@ cc_shared_library( cc_shared_library( name = "b2_so", - roots = [ + deps = [ ":b", ], tags = TAGS, ) - -cc_shared_library( - name = "unaccounted_for_libs_so", - roots = [ - ":d1", - ], - tags = TAGS, -) - -cc_library( - name = "d1", - srcs = ["empty.cc"], - deps = ["d2"], -) - -cc_library( - name = "d2", - srcs = ["empty.cc"], - deps = [ - "d3", - ] + select({ - "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:is_bazel": ["@test_repo//:bar"], - "//conditions:default": [], - }), -) - -cc_library( - name = "d3", - srcs = ["empty.cc"], -) diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/foo.cc b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/foo.cc index cc9c8da410e740..0559c0a3b9da9c 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/foo.cc +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/foo.cc @@ -17,8 +17,10 @@ #include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/direct_so_file_cc_lib2.h" #include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/preloaded_dep.h" #include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/qux.h" +#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h" int foo() { + diff_pkg(); bar(); baz(); qux(); 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 121bb1d41ad6fd..c5de4978aab962 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 @@ -150,7 +150,16 @@ def _debug_files_test_impl(ctx): actual_files = [] for debug_file in target_under_test[OutputGroupInfo].rule_impl_debug_files.to_list(): actual_files.append(debug_file.basename) - expected_files = ["bar_so_exports.txt", "bar_so_link_once_static_libs.txt", "foo_so_exports.txt", "foo_so_link_once_static_libs.txt", "binary_link_once_static_libs.txt"] + + expected_files = [ + "bar_so_exports.txt", + "bar_so_link_once_static_libs.txt", + "diff_pkg_so_exports.txt", + "diff_pkg_so_link_once_static_libs.txt", + "foo_so_exports.txt", + "foo_so_link_once_static_libs.txt", + "binary_link_once_static_libs.txt", + ] asserts.equals(env, expected_files, actual_files) return analysistest.end(env) @@ -166,8 +175,10 @@ def _runfiles_test_impl(ctx): expected_suffixes = [ "libfoo_so.so", "libbar_so.so", + "libdiff_pkg_so.so", "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibfoo_Uso.so", "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibbar_Uso.so", + "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary3_Slibdiff_Upkg_Uso.so", "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/renamed_so_file_copy.so", "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/libdirect_so_file.so", ] @@ -211,3 +222,21 @@ interface_library_output_group_test = analysistest.make( "is_windows": attr.bool(), }, ) + +def _no_exporting_static_lib_test_impl(ctx): + env = analysistest.begin(ctx) + + target_under_test = analysistest.target_under_test(env) + + # There should be only one exported file + actual_file = target_under_test[CcSharedLibraryInfo].exports[0] + + # Sometimes "@" is prefixed in some test environments + expected = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:static_lib_exporting" + asserts.true(env, actual_file.endswith(expected)) + + return analysistest.end(env) + +no_exporting_static_lib_test = analysistest.make( + _no_exporting_static_lib_test_impl, +) diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test index 6a5b93da89d103..364e7f84367aa6 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test @@ -8,6 +8,20 @@ cc_library( hdrs = ["bar.h"], ) +cc_library( + name = "diff_pkg", + srcs = ["diff_pkg.cc"], + hdrs = ["diff_pkg.h"], +) + +cc_shared_library( + name = "diff_pkg_so", + features = ["windows_export_all_symbols"], + deps = [ + ":diff_pkg", + ], +) + cc_shared_library_permissions( name = "permissions", targets = [ diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.cc b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.cc new file mode 100644 index 00000000000000..70e9e53e73ef83 --- /dev/null +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.cc @@ -0,0 +1,16 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h" + +int diff_pkg() { return 42; } diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h new file mode 100644 index 00000000000000..8c39aa01d0a4f5 --- /dev/null +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h @@ -0,0 +1,19 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef EXAMPLES_TEST_CC_SHARED_LIBRARY_DIFF_PKG_H_ +#define EXAMPLES_TEST_CC_SHARED_LIBRARY_DIFF_PKG_H_ + +int diff_pkg(); + +#endif // EXAMPLES_TEST_CC_SHARED_LIBRARY_DIFF_PKG_H_ From 1efed24d8ec13cc7ad5739db73c9a4677e74ede7 Mon Sep 17 00:00:00 2001 From: Pedro Date: Wed, 8 Feb 2023 13:29:09 +0100 Subject: [PATCH 2/2] Add missing in comma in test BUILD file --- .../cc_shared_library/test_cc_shared_library/BUILD.builtin_test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8b75e171d342aa..de112ad08e23f4 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 @@ -99,7 +99,7 @@ cc_shared_library( "//conditions:default": [], }), dynamic_deps = [ - "bar_so" + "bar_so", "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so" ], features = ["windows_export_all_symbols"],