Ck/cherry pick cc shared library (bazelbuild#15754)

ckolli5 · oquenchil · web-flow · commit e4bc370b226e · 2022-06-27T22:15:43.000-05:00
* Add regular library in interface_library group of cc_shared_library

The regular library was already accessible via the main_shared_library_output group but making it also available in interface_library when there is no actual interface library available is inline with the current behavior of cc_binary.

RELNOTES:none
PiperOrigin-RevId: 455497044
Change-Id: Ia80fed253e0c79584f23f08eb25b930362e3ff02

* Remove unnecessary file from runfiles of cc_shared_library

When a precompilied library appeared in the transitive closure of a
cc_shared_library, we would be adding the library itself to runfiles as well as
the symlink created in the solib directory. Although harmless, it is
unnecessary to add the original library since the symlink in the solib
directory will be followed anyway by the Bazel runfiles logic. The behavior now
matches that of cc_binary(linkshared=1)

The same reasoning can be applied for the main output of a cc_shared_library or
a cc_binary(linkshared=1). However, for the latter the logic is written the
opposite way, it only adds the library and not the symlink in solib. Unlike
cc_binary(linkshared=1), for cc_shared_library we must add the symlink in
cc_shared_library because this is what its dependents are linked against. We
could just add the symlink in solib and be done since it will in turn pull the
original library. However, to keep it more similar to cc_binary(linkshared=1)
we add also add the original library, this is documented in this CL with a
comment.

RELNOTES:none
PiperOrigin-RevId: 456074285
Change-Id: Ic383c04569b3d780ca09fc09713bce95c3304636

Co-authored-by: Googler &lt;plf@google.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
@@ -480,6 +480,11 @@ def _cc_shared_library_impl(ctx):
     runfiles_files = []
     if linking_outputs.library_to_link.resolved_symlink_dynamic_library != None:
         runfiles_files.append(linking_outputs.library_to_link.resolved_symlink_dynamic_library)
+
+    # This is different to cc_binary(linkshared=1). Bazel never handles the
+    # linking implicitly for a cc_binary(linkshared=1) but it does so for a cc_shared_library,
+    # for which it will use the symlink in the solib directory. If we don't add it, a dependent
+    # linked against it would fail.
     runfiles_files.append(linking_outputs.library_to_link.dynamic_library)
     runfiles = ctx.runfiles(
         files = runfiles_files,
@@ -489,6 +494,15 @@ def _cc_shared_library_impl(ctx):
         runfiles = runfiles.merge(dep[DefaultInfo].data_runfiles)
         transitive_debug_files.append(dep[OutputGroupInfo].rule_impl_debug_files)
 
+    precompiled_only_dynamic_libraries_runfiles = []
+    for precompiled_dynamic_library in precompiled_only_dynamic_libraries:
+        # precompiled_dynamic_library.dynamic_library could be None if the library to link just contains
+        # an interface library which is valid if the actual library is obtained from the system.
+        if precompiled_dynamic_library.dynamic_library != None:
+            precompiled_only_dynamic_libraries_runfiles.append(precompiled_dynamic_library.dynamic_library)
+
+    runfiles = runfiles.merge(ctx.runfiles(files = precompiled_only_dynamic_libraries_runfiles))
+
     for export in ctx.attr.roots:
         exports[str(export.label)] = True
 
@@ -511,6 +525,13 @@ def _cc_shared_library_impl(ctx):
             precompiled_only_dynamic_libraries_runfiles.append(precompiled_dynamic_library.resolved_symlink_dynamic_library)
 
     runfiles = runfiles.merge(ctx.runfiles(files = precompiled_only_dynamic_libraries_runfiles))
+    interface_library = []
+    if linking_outputs.library_to_link.resolved_symlink_interface_library != None:
+        interface_library.append(linking_outputs.library_to_link.resolved_symlink_interface_library)
+    elif linking_outputs.library_to_link.interface_library != None:
+        interface_library.append(linking_outputs.library_to_link.interface_library)
+    else:
+        interface_library = library
 
     return [
         DefaultInfo(
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
@@ -146,3 +146,58 @@ def _debug_files_test_impl(ctx):
     return analysistest.end(env)
 
 debug_files_test = analysistest.make(_debug_files_test_impl)
+
+def _runfiles_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    if not ctx.attr.is_linux:
+        return analysistest.end(env)
+
+    target_under_test = analysistest.target_under_test(env)
+    expected_suffixes = [
+        "libfoo_so.so",
+        "libbar_so.so",
+        "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibfoo_Uso.so",
+        "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibbar_Uso.so",
+        "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/renamed_so_file_copy.so",
+        "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/libdirect_so_file.so",
+    ]
+    for runfile in target_under_test[DefaultInfo].default_runfiles.files.to_list():
+        # Ignore Python runfiles
+        if "python" in runfile.path:
+            continue
+        found_suffix = False
+        for expected_suffix in expected_suffixes:
+            if runfile.path.endswith(expected_suffix):
+                found_suffix = True
+                break
+        asserts.true(env, found_suffix, runfile.path + " not found in expected suffixes:\n" + "\n".join(expected_suffixes))
+
+    return analysistest.end(env)
+
+runfiles_test = analysistest.make(
+    _runfiles_test_impl,
+    attrs = {
+        "is_linux": attr.bool(),
+    },
+)
+
+def _interface_library_output_group_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    if not ctx.attr.is_windows:
+        return analysistest.end(env)
+
+    target_under_test = analysistest.target_under_test(env)
+    actual_files = []
+    for interface_library in target_under_test[OutputGroupInfo].interface_library.to_list():
+        actual_files.append(interface_library.basename)
+    expected_files = ["foo_so.if.lib"]
+    asserts.equals(env, expected_files, actual_files)
+
+    return analysistest.end(env)
+
+interface_library_output_group_test = analysistest.make(
+    _interface_library_output_group_test_impl,
+    attrs = {
+        "is_windows": attr.bool(),
+    },
+)
