diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/CcRules.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/CcRules.java index 467f953c149d50..5e8f3d8e5b2273 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/CcRules.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/CcRules.java @@ -65,6 +65,7 @@ public void init(ConfiguredRuleClassProvider.Builder builder) { BazelCcModule bazelCcModule = new BazelCcModule(); builder.addConfigurationFragment(CppConfiguration.class); builder.addStarlarkAccessibleTopLevels("CcSharedLibraryInfo", Starlark.NONE); + builder.addStarlarkAccessibleTopLevels("CcSharedLibraryHintInfo", Starlark.NONE); builder.addBuildInfoFactory(new CppBuildInfo()); builder.addNativeAspectClass(graphNodeAspect); 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 adc20e04e66f54..618a4530cfe667 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl @@ -15,7 +15,7 @@ """cc_binary Starlark implementation replacing native""" load(":common/cc/semantics.bzl", "semantics") -load(":common/cc/experimental_cc_shared_library.bzl", "GraphNodeInfo", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "merge_cc_shared_library_infos", "throw_linked_but_not_exported_errors") +load(":common/cc/experimental_cc_shared_library.bzl", "GraphNodeInfo", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "merge_cc_shared_library_infos", "separate_static_and_dynamic_link_libraries", "throw_linked_but_not_exported_errors") load(":common/cc/cc_helper.bzl", "cc_helper", "linker_mode") load(":common/cc/cc_info.bzl", "CcInfo") load(":common/cc/cc_common.bzl", "cc_common") @@ -329,29 +329,6 @@ def _collect_transitive_dwo_artifacts(cc_compilation_outputs, cc_debug_context, transitive_dwo_files = cc_debug_context.files return depset(dwo_files, transitive = [transitive_dwo_files]) -def _separate_static_and_dynamic_link_libraries(direct_children, can_be_linked_dynamically): - link_statically_labels = {} - link_dynamically_labels = {} - all_children = list(direct_children) - seen_labels = {} - - # Some of the logic here is a duplicate from cc_shared_library. - # But some parts are different hence rewriting. - for i in range(2147483647): - if i == len(all_children): - break - node = all_children[i] - node_label = str(node.label) - if node_label in seen_labels: - continue - seen_labels[node_label] = True - if node_label in can_be_linked_dynamically: - link_dynamically_labels[node_label] = True - else: - link_statically_labels[node_label] = True - all_children.extend(node.children) - return (link_statically_labels, link_dynamically_labels) - def _filter_libraries_that_are_linked_dynamically(ctx, cc_linking_context, cpp_config): merged_cc_shared_library_infos = merge_cc_shared_library_infos(ctx) link_once_static_libs_map = build_link_once_static_libs_map(merged_cc_shared_library_infos) @@ -368,7 +345,7 @@ def _filter_libraries_that_are_linked_dynamically(ctx, cc_linking_context, cpp_c if owner in exports_map: can_be_linked_dynamically[owner] = True - (link_statically_labels, link_dynamically_labels) = _separate_static_and_dynamic_link_libraries(graph_structure_aspect_nodes, can_be_linked_dynamically) + (link_statically_labels, link_dynamically_labels) = separate_static_and_dynamic_link_libraries(graph_structure_aspect_nodes, can_be_linked_dynamically) linker_inputs_seen = {} linked_statically_but_not_exported = {} 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 8ad4bc5b8b75d5..fb843b25253d12 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 @@ -37,7 +37,7 @@ GraphNodeInfo = provider( "Nodes in the graph of shared libraries.", fields = { "children": "Other GraphNodeInfo from dependencies of this target", - "label": "Label of the target visited", + "owners": "Owners of the linker inputs in the targets visited", "linkable_more_than_once": "Linkable into more than a single cc_shared_library", }, ) @@ -54,6 +54,42 @@ CcSharedLibraryInfo = provider( }, ) +CcSharedLibraryHintInfo = provider( + doc = """ + This provider should be used by rules that provide C++ linker inputs and + want to guide what the cc_shared_library uses. The reason for this may be + for example because the rule is not providing a standard provider like + CcInfo or ProtoInfo or because the rule does not want certain attributes + to be used for linking into shared libraries. It may also be needed if the + rule is using non-standard linker_input.owner names. + + Propagation of the cc_shared_library aspect will always happen via all + attributes that provide either CcInfo, ProtoInfo or + CcSharedLibraryHintInfo, the hints control whether the result of that + propagation actually gets used. + """, + fields = { + "attributes": ("[String] - If not set, the aspect will use the result of every " + + "dependency that provides CcInfo, ProtoInfo or CcSharedLibraryHintInfo. " + + "If empty list, the aspect will not use the result of any dependency. If " + + "the list contains a list of attribute names, the aspect will only use the " + + "dependencies corresponding to those attributes as long as they provide CcInfo, " + + "ProtoInfo or CcSharedLibraryHintInfo"), + "owners": ("[Label] - cc_shared_library will know which linker_inputs to link based on the owners " + + "field of each linker_input. Most rules will simply use the ctx.label but certain " + + "APIs like cc_common.create_linker_input(owner=) accept any label. " + + "cc_common.create_linking_context_from_compilation_outputs() accepts a `name` which " + + "will then be used to create the owner of the linker_input together with ctx.package." + + "For these cases, since the cc_shared_library cannot guess, the rule author should " + + "provide a hint with the owners of the linker inputs. If the value of owners is not set, then " + + "ctx.label will be used. If the rule author passes a list and they want ctx.label plus some other " + + "label then they will have to add ctx.label explicitly. If you want to use custom owners from C++ " + + "rules keep as close to the original ctx.label as possible, to avoid conflicts with linker_inputs " + + "created by other targets keep the original repository name, the original package name and re-use " + + "the original name as part of your new name, limiting your custom addition to a prefix or suffix."), + }, +) + # For each target, find out whether it should be linked statically or # dynamically. def _separate_static_and_dynamic_link_libraries( @@ -72,16 +108,46 @@ def _separate_static_and_dynamic_link_libraries( break node = all_children[i] - node_label = str(node.label) - if node_label in seen_labels: - continue - seen_labels[node_label] = True - - if node_label in can_be_linked_dynamically: - targets_to_be_linked_dynamically_set[node_label] = True - else: - targets_to_be_linked_statically_map[node_label] = node.linkable_more_than_once + must_add_children = False + + # The *_seen variables are used to track a programmatic error and fail + # if it happens. Every value in node.owners presumably corresponds to + # a linker_input in the same exact target. Therefore if we have seen + # any of the owners already, then we must have also seen all the other + # owners in the same node. Viceversa when we haven't seen them yet. If + # both of these values are non-zero after the loop, the most likely + # reason would be a bug in the implementation. It could potentially be + # triggered by users if they use owner labels that do not keep most of + # the ctx.label.package and ctx.label.name which then clash with other + # target's owners (unlikely). For now though if the error is + # triggered, it's reasonable to require manual revision by + # the cc_shared_library implementation owners. + has_owners_seen = False + has_owners_not_seen = False + for owner in node.owners: + # TODO(bazel-team): Do not convert Labels to string to save on + # garbage string allocations. + owner_str = str(owner) + + if owner_str in seen_labels: + has_owners_seen = True + continue + + has_owners_not_seen = True + seen_labels[owner_str] = True + + if owner_str in can_be_linked_dynamically: + targets_to_be_linked_dynamically_set[owner_str] = True + else: + targets_to_be_linked_statically_map[owner_str] = node.linkable_more_than_once + must_add_children = True + + if has_owners_seen and has_owners_not_seen: + fail("Your build has triggered a programmatic error in the cc_shared_library rule. " + + "Please file an issue in https://github.com/bazelbuild/bazel") + + if must_add_children: all_children.extend(node.children) return (targets_to_be_linked_statically_map, targets_to_be_linked_dynamically_set) @@ -209,20 +275,22 @@ def _find_top_level_linker_input_labels( break node = nodes_to_check[i] - node_label = str(node.label) - if node_label in linker_inputs_to_be_linked_statically_map: - has_code_to_link = False - for linker_input in linker_inputs_to_be_linked_statically_map[node_label]: - if _contains_code_to_link(linker_input): - top_level_linker_input_labels_set[node_label] = True - has_code_to_link = True - break - - if not has_code_to_link: - nodes_to_check.extend(node.children) - elif node_label not in targets_to_be_linked_dynamically_set: - # This can happen when there was a target in the graph that exported other libraries' - # linker_inputs but didn't contribute any linker_input of its own. + must_add_children = False + for owner in node.owners: + owner_str = str(owner) + if owner_str in linker_inputs_to_be_linked_statically_map: + must_add_children = True + for linker_input in linker_inputs_to_be_linked_statically_map[owner_str]: + if _contains_code_to_link(linker_input): + top_level_linker_input_labels_set[owner_str] = True + must_add_children = False + break + elif owner_str not in targets_to_be_linked_dynamically_set: + # This can happen when there was a target in the graph that exported other libraries' + # linker_inputs but didn't contribute any linker_input of its own. + must_add_children = True + + if must_add_children: nodes_to_check.extend(node.children) return top_level_linker_input_labels_set @@ -590,10 +658,16 @@ def _cc_shared_library_impl(ctx): def _graph_structure_aspect_impl(target, ctx): children = [] + attributes = dir(ctx.rule.attr) + owners = [ctx.label] + if CcSharedLibraryHintInfo in target: + attributes = getattr(target[CcSharedLibraryHintInfo], "attributes", dir(ctx.rule.attr)) + owners = getattr(target[CcSharedLibraryHintInfo], "owners", [ctx.label]) + # Collect graph structure info from any possible deplike attribute. The aspect # itself applies across every deplike attribute (attr_aspects is *), so enumerate # over all attributes and consume GraphNodeInfo if available. - for fieldname in dir(ctx.rule.attr): + for fieldname in attributes: deps = getattr(ctx.rule.attr, fieldname, None) if type(deps) == "list": for dep in deps: @@ -609,17 +683,16 @@ def _graph_structure_aspect_impl(target, ctx): for tag in ctx.rule.attr.tags: if tag == LINKABLE_MORE_THAN_ONCE: linkable_more_than_once = True - return [GraphNodeInfo( - label = ctx.label, + owners = owners, children = children, linkable_more_than_once = linkable_more_than_once, )] graph_structure_aspect = aspect( attr_aspects = ["*"], - required_providers = [[CcInfo], [ProtoInfo]], - required_aspect_providers = [[CcInfo]], + required_providers = [[CcInfo], [ProtoInfo], [CcSharedLibraryHintInfo]], + required_aspect_providers = [[CcInfo], [CcSharedLibraryHintInfo]], implementation = _graph_structure_aspect_impl, ) @@ -654,3 +727,4 @@ merge_cc_shared_library_infos = _merge_cc_shared_library_infos build_link_once_static_libs_map = _build_link_once_static_libs_map build_exports_map_from_only_dynamic_deps = _build_exports_map_from_only_dynamic_deps throw_linked_but_not_exported_errors = _throw_linked_but_not_exported_errors +separate_static_and_dynamic_link_libraries = _separate_static_and_dynamic_link_libraries diff --git a/src/main/starlark/builtins_bzl/common/exports.bzl b/src/main/starlark/builtins_bzl/common/exports.bzl index 16336d8698e340..2a07fbaf5d62ff 100755 --- a/src/main/starlark/builtins_bzl/common/exports.bzl +++ b/src/main/starlark/builtins_bzl/common/exports.bzl @@ -17,7 +17,7 @@ load("@_builtins//:common/cc/cc_import.bzl", "cc_import") load("@_builtins//:common/cc/cc_binary_wrapper.bzl", "cc_binary") load("@_builtins//:common/cc/cc_test_wrapper.bzl", cc_test = "cc_test_wrapper") -load("@_builtins//:common/cc/experimental_cc_shared_library.bzl", "CcSharedLibraryInfo", "cc_shared_library") +load("@_builtins//:common/cc/experimental_cc_shared_library.bzl", "CcSharedLibraryHintInfo", "CcSharedLibraryInfo", "cc_shared_library") load("@_builtins//:common/objc/objc_import.bzl", "objc_import") load("@_builtins//:common/objc/objc_library.bzl", "objc_library") load("@_builtins//:common/objc/compilation_support.bzl", "compilation_support") @@ -40,6 +40,7 @@ exported_toplevels = { # "original value". "_builtins_dummy": "overridden value", "CcSharedLibraryInfo": CcSharedLibraryInfo, + "CcSharedLibraryHintInfo": CcSharedLibraryHintInfo, "proto_common_do_not_use": proto_common_do_not_use, "PyRuntimeInfo": PyRuntimeInfo, "PyInfo": PyInfo, 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 343fee87acb42d..fbd1166302d5a4 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 @@ -10,6 +10,7 @@ load( "check_linking_action_lib_parameters_test", "forwarding_cc_lib", "nocode_cc_lib", + "wrapped_cc_lib", ) LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE" @@ -121,6 +122,7 @@ cc_shared_library( "foo", "cc_lib_with_no_srcs", "nocode_cc_lib", + "should_not_be_linked_cc_lib", "a_suffix", ], user_link_flags = select({ @@ -160,10 +162,31 @@ cc_library( }), ) +wrapped_cc_lib( + name = "wrapped_cc_lib", + deps = [ + "indirect_dep", + ], +) + forwarding_cc_lib( name = "cc_lib_with_no_srcs", deps = [ - "indirect_dep", + "wrapped_cc_lib", + ], +) + +wrapped_cc_lib( + name = "should_not_be_linked_wrapped", + deps = [ + "indirect_dep3", + ], +) + +forwarding_cc_lib( + name = "should_not_be_linked_cc_lib", + do_not_follow_deps = [ + "should_not_be_linked_wrapped", ], ) @@ -187,6 +210,11 @@ cc_library( srcs = ["indirect_dep2.cc"], ) +cc_library( + name = "indirect_dep3", + srcs = ["indirect_dep3.cc"], +) + cc_library( name = "a_suffix", srcs = ["a_suffix.cc"], diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh index 61d36400e29c50..35d55769e652ed 100755 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh @@ -46,6 +46,7 @@ function test_shared_library_symbols() { check_symbol_present "$symbols" "t _Z3quxv" check_symbol_present "$symbols" "t _Z12indirect_depv" check_symbol_present "$symbols" "t _Z13indirect_dep2v" + check_symbol_absent "$symbols" "_Z13indirect_dep3v" check_symbol_absent "$symbols" "_Z4bar3v" } diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep3.cc b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep3.cc new file mode 100644 index 00000000000000..3f5fba1c2bc644 --- /dev/null +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep3.cc @@ -0,0 +1,15 @@ +// 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. + +int indirect_dep3() { return 0; } 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 2426a3f272e87c..96e6337181c43f 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 @@ -247,15 +247,65 @@ check_linking_action_lib_parameters_test = analysistest.make( }, ) +AspectCcInfo = provider("Takes a cc_info.", fields = {"cc_info": "cc_info"}) +WrappedCcInfo = provider("Takes a cc_info.", fields = {"cc_info": "cc_info"}) + +def _forwarding_cc_lib_aspect_impl(target, ctx): + cc_info = target[WrappedCcInfo].cc_info + linker_inputs = [] + owner = ctx.label.relative(ctx.label.name + ".custom") + for linker_input in cc_info.linking_context.linker_inputs.to_list(): + if linker_input.owner == ctx.label.relative("indirect_dep"): + linker_inputs.append(cc_common.create_linker_input( + owner = owner, + libraries = depset(linker_input.libraries), + )) + else: + linker_inputs.append(linker_input) + cc_info = CcInfo( + compilation_context = cc_info.compilation_context, + linking_context = cc_common.create_linking_context( + linker_inputs = depset(linker_inputs), + ), + ) + return [ + AspectCcInfo(cc_info = cc_info), + CcSharedLibraryHintInfo( + owners = [owner], + ), + ] + +forwarding_cc_lib_aspect = aspect( + implementation = _forwarding_cc_lib_aspect_impl, + required_providers = [WrappedCcInfo], + provides = [AspectCcInfo, CcSharedLibraryHintInfo], +) + +def _wrapped_cc_lib_impl(ctx): + return [WrappedCcInfo(cc_info = ctx.attr.deps[0][CcInfo]), ProtoInfo()] + +wrapped_cc_lib = rule( + implementation = _wrapped_cc_lib_impl, + attrs = { + "deps": attr.label_list(providers = [CcInfo]), + }, + provides = [WrappedCcInfo, ProtoInfo], +) + def _forwarding_cc_lib_impl(ctx): - return [ctx.attr.deps[0][CcInfo]] + hints = CcSharedLibraryHintInfo(attributes = ["deps"]) + if ctx.attr.deps: + return [ctx.attr.deps[0][AspectCcInfo].cc_info, hints] + else: + return [ctx.attr.do_not_follow_deps[0][AspectCcInfo].cc_info, hints] forwarding_cc_lib = rule( implementation = _forwarding_cc_lib_impl, attrs = { - "deps": attr.label_list(), + "deps": attr.label_list(providers = [WrappedCcInfo], aspects = [forwarding_cc_lib_aspect]), + "do_not_follow_deps": attr.label_list(providers = [WrappedCcInfo], aspects = [forwarding_cc_lib_aspect]), }, - provides = [CcInfo], + provides = [CcInfo, CcSharedLibraryHintInfo], ) def _nocode_cc_lib_impl(ctx):