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.
  • Loading branch information
fmeum committed Jul 24, 2024
1 parent 7646d6b commit 9e983c5
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 1 deletion.
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
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 9e983c5

Please sign in to comment.