diff --git a/prelude/apple/apple_binary.bzl b/prelude/apple/apple_binary.bzl index 93649f0a7235..bcd4ca318c38 100644 --- a/prelude/apple/apple_binary.bzl +++ b/prelude/apple/apple_binary.bzl @@ -45,6 +45,8 @@ def apple_binary_impl(ctx: "context") -> ["provider"]: link_postprocessor = get_apple_link_postprocessor(ctx), link_group_info = get_link_group_info(ctx), prefer_stripped_objects = ctx.attrs.prefer_stripped_objects, + # Some apple rules rely on `static` libs *not* following dependents. + link_groups_force_static_follows_dependents = False, ) (cxx_output, _comp_db_info, xcode_data_info) = cxx_executable(ctx, constructor_params) diff --git a/prelude/apple/apple_library.bzl b/prelude/apple/apple_library.bzl index c2d6d07739a4..9ffd6c18b1b2 100644 --- a/prelude/apple/apple_library.bzl +++ b/prelude/apple/apple_library.bzl @@ -166,6 +166,8 @@ def apple_library_rule_constructor_params_and_swift_providers(ctx: "context", pa generate_sub_targets = params.generate_sub_targets, generate_providers = params.generate_providers, link_postprocessor = get_apple_link_postprocessor(ctx), + # Some apple rules rely on `static` libs *not* following dependents. + link_groups_force_static_follows_dependents = False, ), swift_providers, exported_pre def _filter_swift_srcs(ctx: "context") -> (["CxxSrcWithFlags"], ["CxxSrcWithFlags"]): diff --git a/prelude/cxx/cxx_executable.bzl b/prelude/cxx/cxx_executable.bzl index 9c095736b74f..41b0e24971f9 100644 --- a/prelude/cxx/cxx_executable.bzl +++ b/prelude/cxx/cxx_executable.bzl @@ -95,6 +95,7 @@ load( ":link_groups.bzl", "LINK_GROUP_MAP_DATABASE_SUB_TARGET", "create_link_groups", + "find_relevant_roots", "get_filtered_labels_to_links_map", "get_filtered_links", "get_filtered_targets", @@ -267,10 +268,18 @@ def cxx_executable(ctx: "context", impl_params: CxxRuleConstructorParams.type, i for name, lib in link_group_libs.items() }, link_style = link_style, - deps = [d.linkable_graph.nodes.value.label for d in link_deps], - other_roots = link_group_other_roots, + roots = ( + [d.linkable_graph.nodes.value.label for d in link_deps] + + find_relevant_roots( + link_group = link_group, + linkable_graph_node_map = linkable_graph_node_map, + link_group_mappings = link_group_mappings, + roots = link_group_other_roots, + ) + ), is_executable_link = True, prefer_stripped = impl_params.prefer_stripped_objects, + force_static_follows_dependents = impl_params.link_groups_force_static_follows_dependents, ) if is_cxx_test and link_group != None: @@ -282,7 +291,7 @@ def cxx_executable(ctx: "context", impl_params: CxxRuleConstructorParams.type, i link_group_mappings, link_group_preferred_linkage, link_style, - deps = [d.linkable_graph.nodes.value.label for d in link_deps], + roots = [d.linkable_graph.nodes.value.label for d in link_deps], is_executable_link = True, prefer_stripped = impl_params.prefer_stripped_objects, ) diff --git a/prelude/cxx/cxx_library.bzl b/prelude/cxx/cxx_library.bzl index cf448f3cd336..c2476a67468c 100644 --- a/prelude/cxx/cxx_library.bzl +++ b/prelude/cxx/cxx_library.bzl @@ -338,6 +338,7 @@ def cxx_library_parameterized(ctx: "context", impl_params: "CxxRuleConstructorPa non_exported_deps, impl_params.force_link_group_linking, frameworks_linkable, + force_static_follows_dependents = impl_params.link_groups_force_static_follows_dependents, ) if impl_params.generate_sub_targets.link_group_map and link_group_map: sub_targets[LINK_GROUP_MAP_DATABASE_SUB_TARGET] = [link_group_map] @@ -815,7 +816,8 @@ def _get_shared_library_links( exported_deps: ["dependency"], non_exported_deps: ["dependency"], force_link_group_linking, - frameworks_linkable: ["FrameworksLinkable", None]) -> ("LinkArgs", [DefaultInfo.type, None]): + frameworks_linkable: ["FrameworksLinkable", None], + force_static_follows_dependents: bool.type = True) -> ("LinkArgs", [DefaultInfo.type, None]): """ TODO(T110378116): Omnibus linking always creates shared libraries by linking against shared dependencies. This is not true for link groups and possibly @@ -862,8 +864,9 @@ def _get_shared_library_links( for name, lib in link_group_libs.items() }, link_style = link_style, - deps = linkable_deps(non_exported_deps + exported_deps), + roots = linkable_deps(non_exported_deps + exported_deps), prefer_stripped = prefer_stripped, + force_static_follows_dependents = force_static_follows_dependents, ) filtered_links = get_filtered_links(filtered_labels_to_links_map) filtered_targets = get_filtered_targets(filtered_labels_to_links_map) diff --git a/prelude/cxx/cxx_types.bzl b/prelude/cxx/cxx_types.bzl index 3956e1914631..e90f4ed1e97c 100644 --- a/prelude/cxx/cxx_types.bzl +++ b/prelude/cxx/cxx_types.bzl @@ -168,4 +168,7 @@ CxxRuleConstructorParams = record( prefer_stripped_objects = field(bool.type, False), # The category suffix to use for executables actions (e.g. linking). exe_category_suffix = field(str.type, "executable"), + # Whether link groups liking should make `preferred_linkage = "static"` libs + # "follow" their dependents across link group boundaries. + link_groups_force_static_follows_dependents = field(bool.type, True), ) diff --git a/prelude/cxx/link_groups.bzl b/prelude/cxx/link_groups.bzl index c6c5d5206af8..69267efd5bee 100644 --- a/prelude/cxx/link_groups.bzl +++ b/prelude/cxx/link_groups.bzl @@ -221,13 +221,11 @@ def get_filtered_labels_to_links_map( link_group_mappings: [{"label": str.type}, None], link_group_preferred_linkage: {"label": Linkage.type}, link_style: LinkStyle.type, - deps: ["label"], - # Additional roots to search for nodes in the target link group (e.g. - # dlopen-enabled libs or outlined native Python extensions). - other_roots: ["label"] = [], + roots: ["label"], link_group_libs: {str.type: (["label", None], LinkInfos.type)} = {}, prefer_stripped: bool.type = False, - is_executable_link: bool.type = False) -> {"label": LinkGroupLinkInfo.type}: + is_executable_link: bool.type = False, + force_static_follows_dependents: bool.type = True) -> {"label": LinkGroupLinkInfo.type}: """ Given a linkable graph, link style and link group mappings, finds all links to consider for linking traversing the graph as necessary and then @@ -254,28 +252,10 @@ def get_filtered_labels_to_links_map( return node_linkables - # Walk through roots looking for the first node which maps to the current - # link group. - def collect_and_traverse_roots(roots, node_target): - node_link_group = link_group_mappings.get(node_target) - if node_link_group == MATCH_ALL_LABEL: - roots.append(node_target) - return [] - if node_link_group == link_group: - roots.append(node_target) - return [] - return get_traversed_deps(node_target) - # Get all potential linkable targets - other_deps = [] - breadth_first_traversal_by( - linkable_graph_node_map, - other_roots, - partial(collect_and_traverse_roots, other_deps), - ) linkables = breadth_first_traversal_by( linkable_graph_node_map, - deps + other_deps, + roots, get_traversed_deps, ) @@ -349,7 +329,10 @@ def get_filtered_labels_to_links_map( else: # static or static_pic target_link_group = link_group_mappings.get(target) - if not target_link_group and not link_group: + # Always add force-static libs to the link. + if force_static_follows_dependents and node.preferred_linkage == Linkage("static"): + add_link(target, actual_link_style) + elif not target_link_group and not link_group: # Ungrouped linkable targets belong to the unlabeled executable add_link(target, actual_link_style) elif is_executable_link and target_link_group == NO_MATCH_LABEL: @@ -467,6 +450,36 @@ def make_link_group_info(groups: [Group.type], mappings: {"label": str.type}) -> mappings = mappings, ) +def find_relevant_roots( + link_group: [str.type, None] = None, + linkable_graph_node_map: {"label": LinkableNode.type} = {}, + link_group_mappings: {"label": str.type} = {}, + roots: ["label"] = []): + # Walk through roots looking for the first node which maps to the current + # link group. + def collect_and_traverse_roots(roots, node_target): + node = linkable_graph_node_map.get(node_target) + if node.preferred_linkage == Linkage("static"): + return node.deps + node.exported_deps + node_link_group = link_group_mappings.get(node_target) + if node_link_group == MATCH_ALL_LABEL: + roots.append(node_target) + return [] + if node_link_group == link_group: + roots.append(node_target) + return [] + return node.deps + node.exported_deps + + relevant_roots = [] + + breadth_first_traversal_by( + linkable_graph_node_map, + roots, + partial(collect_and_traverse_roots, relevant_roots), + ) + + return relevant_roots + def _create_link_group( ctx: "context", spec: LinkGroupLibSpec.type, @@ -497,7 +510,7 @@ def _create_link_group( # Get roots to begin the linkable search. # TODO(agallagher): We should use the groups "public" nodes as the roots. - deps = [] + roots = [] has_empty_root = False if spec.root != None: # If there's a linkable root attached to the spec, use that to guide @@ -507,7 +520,7 @@ def _create_link_group( spec.root.link_infos, prefer_stripped = prefer_stripped_objects, )) - deps.extend(spec.root.deps) + roots.extend(spec.root.deps) else: for mapping in spec.group.mappings: # If there's no explicit root, this means we need to search the entire @@ -515,9 +528,20 @@ def _create_link_group( if mapping.root == None: has_empty_root = True else: - deps.append(mapping.root.label) + roots.append(mapping.root.label) + + # If this link group has an empty mapping, we need to search everything + # -- even the additional roots -- to find potential nodes in the link + # group. if has_empty_root: - deps.extend(executable_deps) + roots.extend( + find_relevant_roots( + link_group = spec.group.name, + linkable_graph_node_map = linkable_graph_node_map, + link_group_mappings = link_group_mappings, + roots = executable_deps + other_roots, + ), + ) # Add roots... filtered_labels_to_links_map = get_filtered_labels_to_links_map( @@ -527,11 +551,7 @@ def _create_link_group( link_group_preferred_linkage, link_group_libs = link_group_libs, link_style = link_style, - # If this link group has an empty mapping, we need to search everything - # -- even the additional roots -- to find potential nodes in the link - # group. - other_roots = other_roots if has_empty_root else [], - deps = deps, + roots = roots, is_executable_link = False, prefer_stripped = prefer_stripped_objects, ) diff --git a/prelude/python/cxx_python_extension.bzl b/prelude/python/cxx_python_extension.bzl index 001a2b93b927..caa4bbc3e9ce 100644 --- a/prelude/python/cxx_python_extension.bzl +++ b/prelude/python/cxx_python_extension.bzl @@ -37,6 +37,8 @@ load( ) load( "@prelude//linking:link_info.bzl", + "LinkInfo", + "LinkInfos", "LinkStyle", "Linkage", "create_merged_link_info", @@ -163,6 +165,11 @@ def cxx_python_extension_impl(ctx: "context") -> ["provider"]: python_module_names[base_module.replace("/", ".") + module_name] = pyinit_symbol + # Add a dummy shared link info to avoid marking this node as preferred + # linkage being "static", which has a special meaning for various link + # strategies + link_infos[LinkStyle("shared")] = LinkInfos(default = LinkInfo()) + # Create linkable providers for the extension. link_deps = linkables(cxx_deps) linkable_providers = LinkableProviders( @@ -174,7 +181,7 @@ def cxx_python_extension_impl(ctx: "context") -> ["provider"]: linkable_node = create_linkable_node( ctx = ctx, deps = cxx_deps, - preferred_linkage = Linkage("static"), + preferred_linkage = Linkage("any"), link_infos = link_infos, ), ),