Skip to content

Commit

Permalink
Flip --incompatible_macos_set_install_name
Browse files Browse the repository at this point in the history
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
fmeum authored and copybara-github committed Sep 25, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 05dc25f commit 62dc3b5
Showing 2 changed files with 119 additions and 66 deletions.
Original file line number Diff line number Diff line change
@@ -974,9 +974,12 @@ public Label getMemProfProfileLabel() {
help = "If enabled, give distinguishing mnemonic to header processing actions")
public boolean useCppCompileHeaderMnemonic;

// TODO: When moving this flag to the graveyard, also delete
// tools/cpp/osx_cc_wrapper.sh.tpl and make tools/cpp/linux_cc_wrapper.sh.tpl
// the generic wrapper for header parsing on all Unix platforms.
@Option(
name = "incompatible_macos_set_install_name",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
180 changes: 115 additions & 65 deletions tools/cpp/unix_cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
@@ -231,6 +231,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()
@@ -761,69 +763,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",
@@ -1245,7 +1294,6 @@ def _impl(ctx):
],
)

is_linux = ctx.attr.target_libc != "macosx"
libtool_feature = feature(
name = "libtool",
enabled = not is_linux,
@@ -1774,6 +1822,8 @@ def _impl(ctx):
macos_minimum_os_feature,
macos_default_link_flags_feature,
dependency_file_feature,
runtime_library_search_directories_feature,
set_install_name_feature,
libtool_feature,
archiver_flags_feature,
asan_feature,
@@ -1865,6 +1915,6 @@ cc_toolchain_config = rule(
name = "xcode_config_label",
)),
},
fragments = ["apple"],
fragments = ["apple", "cpp"],
provides = [CcToolchainConfigInfo],
)

0 comments on commit 62dc3b5

Please sign in to comment.