From e4bc370b226eb0cc536b55641640266345a214ec Mon Sep 17 00:00:00 2001 From: Chenchu K Date: Mon, 27 Jun 2022 22:15:43 -0500 Subject: [PATCH] Ck/cherry pick cc shared library (#15754) * 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 --- .../cc/experimental_cc_shared_library.bzl | 21 +++++++ .../test_cc_shared_library/starlark_tests.bzl | 55 +++++++++++++++++++ 2 files changed, 76 insertions(+) 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 1d536883d8cf4f..76a1bcb813ead5 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 @@ -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 index 9e099061b80b0c..d3ca91df23044e 100644 --- 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(), + }, +)