Remove the static_deps attribute from cc_shared_library.

The idea behind this attribute was to force a user creating a shared library to
account for every library that was linked statically into it, so that a new
dependency further upstream wouldn't end up being linked without previous
approval from the shared library owner.

However, in the real world this attribute has turned out to be more of a pain
than useful for users. Users will wildcard as much as possible using the
@repo_name//:__subpackages__ syntax and will often get errors due to a new
dependency from a different repository.

At the same time, if any users (haven't seen any so far) have any interest in
this restriction where an allowlist is required for libraries linked
statically, then they can still do so by either writing a Starlark test which
looks at the CcSharedLibraryInfo provider (which is public API) or they can
write a test that reads the targets written to the debug
*_link_once_static_libs.txt files.

With this change the static_deps attribute will still be present but it will be
a no-op as long as you are using the --experimental_cc_shared_library flag.
Using the attribute will be an error without the experimental flag. In a follow
up change we will not require the experimental flag to use the rule. Some time
later the flag will be removed (therefore the static_deps attribute should be
removed from targets).

RELNOTES[inc]: cc_shared_library.static_deps attribute is removed

PiperOrigin-RevId: 505107353
Change-Id: I438a1e47451ac53375dbe7940f238473807a0acc

Author: oquenchil

src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl

@@ -73,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 = {}
 
@@ -97,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(
@@ -142,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():
@@ -251,7 +255,7 @@ def _filter_inputs(
         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 = []
@@ -267,16 +271,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:
@@ -285,11 +298,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
@@ -311,41 +334,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 " +
@@ -356,8 +364,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:
@@ -376,28 +383,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
@@ -421,6 +406,14 @@ def _get_permissions(ctx):
 def _cc_shared_library_impl(ctx):
     semantics.check_experimental_cc_shared_library(ctx)
 
+    if len(ctx.attr.static_deps) and not cc_common.check_experimental_cc_shared_library():
+        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",
+        )
+
     cc_toolchain = cc_helper.find_cpp_toolchain(ctx)
     feature_configuration = cc_common.configure_features(
         ctx = ctx,
@@ -460,7 +453,7 @@ 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,
@@ -544,8 +537,8 @@ def _cc_shared_library_impl(ctx):
     for export in ctx.attr.roots:
         export_label = str(export.label)
         if GraphNodeInfo in export and export[GraphNodeInfo].no_exporting:
-            if export_label in link_once_static_libs:
-                link_once_static_libs.remove(export_label)
+            if export_label in curr_link_once_static_libs_set:
+                curr_link_once_static_libs_set.remove(export_label)
             continue
         exports[export_label] = True
 
@@ -554,13 +547,13 @@ def _cc_shared_library_impl(ctx):
     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:
@@ -589,7 +582,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),

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test

@@ -106,11 +106,6 @@ cc_shared_library(
         "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",
@@ -207,11 +202,6 @@ cc_shared_library(
         ":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)",
@@ -488,20 +478,6 @@ 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",

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test

@@ -22,9 +22,6 @@ cc_shared_library(
     roots = [
         ":intermediate",
     ],
-    static_deps = [
-        "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
-    ],
     tags = TAGS,
 )
 
@@ -103,33 +100,3 @@ cc_shared_library(
     ],
     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"],
-)