From 3d6de3c5f7609dc1a80f9b96266a9a7c3b4a9f72 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 25 Sep 2024 11:28:25 -0700 Subject: [PATCH 1/2] Flip `--incompatible_macos_set_install_name` This requires adding the `set_install_name` feature to the Unix toolchain on macOS only. The legacy install name patcher is kept around for as long as the flag can be unflipped. RELNOTES[INC]: With the default Unix toolchain on macOS, binaries now use `@rpath` to find their `.dylib` dependencies. This is required to fix issues where tools run during the build couldn't find their dynamic dependencies. Closes #12370 Closes #23090. PiperOrigin-RevId: 678780800 Change-Id: I13a1bb993c13fb2f9d7b358c2881a7ccdafe66cc --- tools/cpp/unix_cc_toolchain_config.bzl | 180 ++++++++++++++++--------- 1 file changed, 115 insertions(+), 65 deletions(-) diff --git a/tools/cpp/unix_cc_toolchain_config.bzl b/tools/cpp/unix_cc_toolchain_config.bzl index 45fd0c6e1e8dd8..d228a20cc342b2 100644 --- a/tools/cpp/unix_cc_toolchain_config.bzl +++ b/tools/cpp/unix_cc_toolchain_config.bzl @@ -222,6 +222,8 @@ def _sanitizer_feature(name = "", specific_compile_flags = [], specific_link_fla ) def _impl(ctx): + is_linux = ctx.attr.target_libc != "macosx" + tool_paths = [ tool_path(name = name, path = path) for name, path in ctx.attr.tool_paths.items() @@ -612,69 +614,116 @@ def _impl(ctx): provides = ["profile"], ) - runtime_library_search_directories_feature = feature( - name = "runtime_library_search_directories", - flag_sets = [ - flag_set( - actions = all_link_actions + lto_index_actions, - flag_groups = [ - flag_group( - iterate_over = "runtime_library_search_directories", - flag_groups = [ - flag_group( - flags = [ - "-Xlinker", - "-rpath", - "-Xlinker", - "$EXEC_ORIGIN/%{runtime_library_search_directories}", - ], - expand_if_true = "is_cc_test", - ), - flag_group( - flags = [ - "-Xlinker", - "-rpath", - "-Xlinker", - "$ORIGIN/%{runtime_library_search_directories}", - ], - expand_if_false = "is_cc_test", - ), - ], - expand_if_available = - "runtime_library_search_directories", - ), - ], - with_features = [ - with_feature_set(features = ["static_link_cpp_runtimes"]), - ], - ), - flag_set( - actions = all_link_actions + lto_index_actions, - flag_groups = [ - flag_group( - iterate_over = "runtime_library_search_directories", - flag_groups = [ - flag_group( - flags = [ - "-Xlinker", - "-rpath", - "-Xlinker", - "$ORIGIN/%{runtime_library_search_directories}", - ], - ), - ], - expand_if_available = - "runtime_library_search_directories", - ), - ], - with_features = [ - with_feature_set( - not_features = ["static_link_cpp_runtimes"], - ), - ], - ), - ], - ) + if is_linux: + runtime_library_search_directories_feature = feature( + name = "runtime_library_search_directories", + flag_sets = [ + flag_set( + actions = all_link_actions + lto_index_actions, + flag_groups = [ + flag_group( + iterate_over = "runtime_library_search_directories", + flag_groups = [ + flag_group( + flags = [ + "-Xlinker", + "-rpath", + "-Xlinker", + "$EXEC_ORIGIN/%{runtime_library_search_directories}", + ], + expand_if_true = "is_cc_test", + ), + flag_group( + flags = [ + "-Xlinker", + "-rpath", + "-Xlinker", + "$ORIGIN/%{runtime_library_search_directories}", + ], + expand_if_false = "is_cc_test", + ), + ], + expand_if_available = + "runtime_library_search_directories", + ), + ], + with_features = [ + with_feature_set(features = ["static_link_cpp_runtimes"]), + ], + ), + flag_set( + actions = all_link_actions + lto_index_actions, + flag_groups = [ + flag_group( + iterate_over = "runtime_library_search_directories", + flag_groups = [ + flag_group( + flags = [ + "-Xlinker", + "-rpath", + "-Xlinker", + "$ORIGIN/%{runtime_library_search_directories}", + ], + ), + ], + expand_if_available = + "runtime_library_search_directories", + ), + ], + with_features = [ + with_feature_set( + not_features = ["static_link_cpp_runtimes"], + ), + ], + ), + ], + ) + set_install_name_feature = None + else: + runtime_library_search_directories_feature = feature( + name = "runtime_library_search_directories", + flag_sets = [ + flag_set( + actions = all_link_actions + lto_index_actions, + flag_groups = [ + flag_group( + iterate_over = "runtime_library_search_directories", + flag_groups = [ + flag_group( + flags = [ + "-Xlinker", + "-rpath", + "-Xlinker", + "@loader_path/%{runtime_library_search_directories}", + ], + ), + ], + expand_if_available = "runtime_library_search_directories", + ), + ], + ), + ], + ) + set_install_name_feature = feature( + name = "set_install_name", + enabled = ctx.fragments.cpp.do_not_use_macos_set_install_name, + flag_sets = [ + flag_set( + actions = [ + ACTION_NAMES.cpp_link_dynamic_library, + ACTION_NAMES.cpp_link_nodeps_dynamic_library, + ], + flag_groups = [ + flag_group( + flags = [ + "-Wl,-install_name,@rpath/%{runtime_solib_name}", + ], + expand_if_available = "runtime_solib_name", + ), + ], + ), + ], + ) fission_support_feature = feature( name = "fission_support", @@ -1086,7 +1135,6 @@ def _impl(ctx): ], ) - is_linux = ctx.attr.target_libc != "macosx" libtool_feature = feature( name = "libtool", enabled = not is_linux, @@ -1542,6 +1590,8 @@ def _impl(ctx): features = [ macos_minimum_os_feature, macos_default_link_flags_feature, + runtime_library_search_directories_feature, + set_install_name_feature, libtool_feature, archiver_flags_feature, asan_feature, @@ -1631,6 +1681,6 @@ cc_toolchain_config = rule( name = "xcode_config_label", )), }, - fragments = ["apple"], + fragments = ["apple", "cpp"], provides = [CcToolchainConfigInfo], ) From 5ca2c1cbe3e778310db95108fccba67b7985abff Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Thu, 3 Oct 2024 06:09:52 -0700 Subject: [PATCH 2/2] Add opt-in set_soname feature This allows correctly setting the `soname` for shared libraries on Linux. This is useful for similar reasons to the macOS `set_install_name` feature. Fixes https://github.com/bazelbuild/bazel/issues/18798 Closes #23839. PiperOrigin-RevId: 681847016 Change-Id: I2d3584c9d91564b41c2501e6664e2956f9760e41 --- tools/cpp/unix_cc_toolchain_config.bzl | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tools/cpp/unix_cc_toolchain_config.bzl b/tools/cpp/unix_cc_toolchain_config.bzl index d228a20cc342b2..1dd0e7a6e7ad0b 100644 --- a/tools/cpp/unix_cc_toolchain_config.bzl +++ b/tools/cpp/unix_cc_toolchain_config.bzl @@ -678,7 +678,25 @@ def _impl(ctx): ), ], ) - set_install_name_feature = None + set_install_name_feature = feature( + name = "set_soname", + flag_sets = [ + flag_set( + actions = [ + ACTION_NAMES.cpp_link_dynamic_library, + ACTION_NAMES.cpp_link_nodeps_dynamic_library, + ], + flag_groups = [ + flag_group( + flags = [ + "-Wl,-soname,%{runtime_solib_name}", + ], + expand_if_available = "runtime_solib_name", + ), + ], + ), + ], + ) else: runtime_library_search_directories_feature = feature( name = "runtime_library_search_directories", @@ -1576,6 +1594,7 @@ def _impl(ctx): unfiltered_compile_flags_feature, treat_warnings_as_errors_feature, archive_param_file_feature, + set_install_name_feature, ] + layering_check_features(ctx.attr.compiler, ctx.attr.extra_flags_per_feature, is_macos = False) else: # macOS artifact name patterns differ from the defaults only for dynamic