C++: Add compound error linked statically but not exported (bazelbuild#16113)

oquenchil · ShreeM01 · web-flow · commit c62496f7b76d · 2022-08-16T10:04:04.000-07:00
Co-authored-by: kshyanashree &lt;109167932+kshyanashree@users.noreply.github.com&gt;
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
@@ -267,6 +267,7 @@ def _filter_inputs(
     unaccounted_for_libs = []
     exports = {}
     owners_seen = {}
+    linked_statically_but_not_exported = {}
     for linker_input in dependency_linker_inputs:
         owner = str(linker_input.owner)
         if owner in owners_seen:
@@ -277,8 +278,7 @@ def _filter_inputs(
             linker_inputs.append(dynamic_linker_input)
         elif owner in link_statically_labels:
             if owner in link_once_static_libs_map:
-                fail(owner + " is already linked statically in " +
-                     link_once_static_libs_map[owner] + " but not exported")
+                linked_statically_but_not_exported.setdefault(link_once_static_libs_map[owner], []).append(owner)
 
             is_direct_export = owner in direct_exports
 
@@ -337,9 +337,27 @@ def _filter_inputs(
                 else:
                     unaccounted_for_libs.append(linker_input.owner)
 
+    _throw_linked_but_not_exported_errors(linked_statically_but_not_exported)
     _throw_error_if_unaccounted_libs(unaccounted_for_libs)
     return (exports, linker_inputs, link_once_static_libs, precompiled_only_dynamic_libraries)
 
+def _throw_linked_but_not_exported_errors(error_libs_dict):
+    if not error_libs_dict:
+        return
+
+    error_builder = ["The following libraries were linked statically by different cc_shared_libraries but not exported:\n"]
+    for cc_shared_library_target, error_libs in error_libs_dict.items():
+        error_builder.append("cc_shared_library %s:\n" % str(cc_shared_library_target))
+        for error_lib in error_libs:
+            error_builder.append("  \"%s\",\n" % str(error_lib))
+
+    error_builder.append("If you are sure that the previous libraries are exported by the cc_shared_libraries because:\n")
+    error_builder.append("  1. You have visibility declarations in the source code\n")
+    error_builder.append("  2. Or you are passing a visibility script to the linker to export symbols from them\n")
+    error_builder.append("then add those libraries to roots or exports_filter for each cc_shared_library.\n")
+
+    fail("".join(error_builder))
+
 def _throw_error_if_unaccounted_libs(unaccounted_for_libs):
     if not unaccounted_for_libs:
         return
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
@@ -282,6 +282,14 @@ build_failure_test(
     target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:should_fail_binary",
 )
 
+build_failure_test(
+    name = "link_once_repeated_test_shared_lib",
+    messages = [
+        "cc_shared_library/test_cc_shared_library:barX\",",
+    ],
+    target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:should_fail_shared_lib",
+)
+
 paths_test(
     name = "path_matching_test",
 )
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
@@ -17,6 +17,25 @@ 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 = [
+        ":intermediate",
+    ],
+    static_deps = [
+        "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
+    ],
+    tags = TAGS,
+)
+
+cc_library(
+    name = "intermediate",
+    deps = [
+        "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
+    ],
+)
+
 cc_shared_library(
     name = "permissions_fail_so",
     roots = [

Original file line number	Diff line number	Diff line change
`@@ -282,6 +282,14 @@ build_failure_test(`
`282`	`282`	`target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:should_fail_binary",`
`283`	`283`	`)`
`284`	`284`
	`285`	`+build_failure_test(`
	`286`	`+ name = "link_once_repeated_test_shared_lib",`
	`287`	`+ messages = [`
	`288`	`+ "cc_shared_library/test_cc_shared_library:barX\",",`
	`289`	`+ ],`
	`290`	`+ target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:should_fail_shared_lib",`
	`291`	`+)`
	`292`	`+`
`285`	`293`	`paths_test(`
`286`	`294`	`name = "path_matching_test",`
`287`	`295`	`)`