From c62496f7b76da473cb1102798373f552ba2f434d Mon Sep 17 00:00:00 2001
From: oquenchil <23365806+oquenchil@users.noreply.github.com>
Date: Tue, 16 Aug 2022 19:04:04 +0200
Subject: [PATCH] C++: Add compound error linked statically but not exported
 (#16113)

Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com>
---
 .../cc/experimental_cc_shared_library.bzl     | 22 +++++++++++++++++--
 .../test_cc_shared_library/BUILD.builtin_test |  8 +++++++
 .../failing_targets/BUILD.builtin_test        | 19 ++++++++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

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 5a605d1299039a..176e43416f3898 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
@@ -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
index 3fe911db5f9263..6d43896a772a08 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
@@ -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
index 72354b7a340ba4..dafa0667344692 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
@@ -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 = [