Skip to content

Commit

Permalink
Fix runtime_solib_name for --incompatible_macos_set_install_name
Browse files Browse the repository at this point in the history
The `runtime_solib_name` link variable had an incorrect value in the case of a transitive dynamic library. Since non-transitive ("nodeps") dynamic libraries are no longer used on macOS, the `--incompatible_macos_set_install_name` flag didn't have any positive effect.

This PR is intentionally limited to the fix so that it can be cherry-picked into Bazel 7, where it can make the incompatible flag work with the `apple_support` toolchain. A follow-up PR will add the feature to the Unix toolchain and flip the incompatible flag for Bazel 8.

Work towards bazelbuild#12370

Closes bazelbuild#23089.

PiperOrigin-RevId: 668228562
Change-Id: I7524679bfe8c6b8b28c0cb04f46c0d22d0adbe99
  • Loading branch information
fmeum authored and copybara-github committed Aug 28, 2024
1 parent e811565 commit 623c24a
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ public CppLinkAction build() throws EvalException, InterruptedException {
output.getExecPathString(),
SolibSymlinkAction.getDynamicLibrarySoname(
output.getRootRelativePath(),
/* preserveName= */ false,
/* preserveName= */ linkType != LinkTargetType.NODEPS_DYNAMIC_LIBRARY,
linkActionConstruction.getContext().getConfiguration().getMnemonic()),
thinltoParamFile != null ? thinltoParamFile.getExecPathString() : null,
toolchain,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def link_action(
cc_internal.dynamic_library_soname(
actions,
output.short_path,
False,
link_type != NODEPS_DYNAMIC_LIBRARY,
),
interface_output.path if interface_output else None,
thinlto_param_file.path if thinlto_param_file else None,
Expand Down
1 change: 1 addition & 0 deletions src/test/shell/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ sh_test(
],
tags = [
"no_windows", # darwin-specific test
"requires-network", # For Bzlmod
],
)

Expand Down
53 changes: 53 additions & 0 deletions src/test/shell/bazel/cpp_darwin_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ EOF
return 0
}

# TODO: This test passes vacuously as the default Unix toolchain doesn't use
# the set_install_name feature yet.
function test_cc_test_with_explicit_install_name() {
mkdir -p cpp
cat > cpp/BUILD <<EOF
Expand All @@ -147,10 +149,61 @@ cc_library(
srcs = ["foo.cc"],
hdrs = ["foo.h"],
)
cc_shared_library(
name = "foo_shared",
deps = [":foo"],
)
cc_test(
name = "test",
srcs = ["test.cc"],
deps = [":foo"],
dynamic_deps = [":foo_shared"],
)
EOF
cat > cpp/foo.h <<EOF
int foo();
EOF
cat > cpp/foo.cc <<EOF
int foo() { return 0; }
EOF
cat > cpp/test.cc <<EOF
#include "cpp/foo.h"
int main() {
return foo();
}
EOF

bazel test --incompatible_macos_set_install_name //cpp:test || \
fail "bazel test //cpp:test failed"
# Ensure @rpath is correctly set in the binary.
./bazel-bin/cpp/test || \
fail "//cpp:test workspace execution failed, expected return 0, got $?"
cd bazel-bin
./cpp/test || \
fail "//cpp:test execution failed, expected 0, but $?"
}

function test_cc_test_with_explicit_install_name_apple_support() {
cat > MODULE.bazel <<EOF
bazel_dep(name = "apple_support", version = "1.16.0")
EOF

mkdir -p cpp
cat > cpp/BUILD <<EOF
cc_library(
name = "foo",
srcs = ["foo.cc"],
hdrs = ["foo.h"],
)
cc_shared_library(
name = "foo_shared",
deps = [":foo"],
)
cc_test(
name = "test",
srcs = ["test.cc"],
deps = [":foo"],
dynamic_deps = [":foo_shared"],
)
EOF
cat > cpp/foo.h <<EOF
Expand Down

0 comments on commit 623c24a

Please sign in to comment.