Skip to content

Commit

Permalink
Fixes incorrect install names on darwin platforms
Browse files Browse the repository at this point in the history
#12304 added support to bazel for setting install names for dynamic
libraries on darwin platforms. This would set LC_ID_DYLIB to
@rpath/{library_name}, so that RPATH would be used to locate these
libraries at runtime. However, the code was using a utility method that
assumed the library name was mangled, which is often not the case. Given
that the output path should already have been determined with the
mangled or unmangled name, we should be able to just use the base name
of the artifact. The test that was added in #12304 has been updated to
actually use dynamic libaries, and passes with the changes made in this
commit.

Closes #13427.

PiperOrigin-RevId: 377504015
  • Loading branch information
csmulhern authored and copybara-github committed Jun 4, 2021
1 parent cf8ec29 commit b06f495
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -872,8 +872,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
getLinkType().linkerOrArchiver().equals(LinkerOrArchiver.LINKER),
configuration.getBinDirectory(repositoryName).getExecPath(),
output.getExecPathString(),
SolibSymlinkAction.getDynamicLibrarySoname(
output.getRootRelativePath(), /* preserveName= */ false),
output.getRootRelativePath().getBaseName(),
linkType.equals(LinkTargetType.DYNAMIC_LIBRARY),
paramFile != null ? paramFile.getExecPathString() : null,
thinltoParamFile != null ? thinltoParamFile.getExecPathString() : null,
Expand Down
50 changes: 34 additions & 16 deletions src/test/shell/bazel/cpp_darwin_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -124,41 +124,59 @@ EOF
}

function test_cc_test_with_explicit_install_name() {
mkdir -p cpp
cat > cpp/BUILD <<EOF
mkdir -p cpp/install_name
cat > cpp/install_name/BUILD <<EOF
cc_library(
name = "foo",
srcs = ["foo.cc"],
hdrs = ["foo.h"],
)
cc_binary(
name = "libbar.so",
srcs = ["bar.cc"],
linkshared = 1,
)
cc_binary(
name = "libbaz.dylib",
srcs = ["baz.cc"],
linkshared = 1,
)
cc_test(
name = "test",
srcs = ["test.cc"],
srcs = ["test.cc", ":libbar.so", ":libbaz.dylib"],
deps = [":foo"],
)
EOF
cat > cpp/foo.h <<EOF
int foo();
cat > cpp/install_name/foo.cc <<EOF
int foo() { return 2; }
EOF
cat > cpp/foo.cc <<EOF
int foo() { return 0; }
cat > cpp/install_name/bar.cc <<EOF
int bar() { return 12; }
EOF
cat > cpp/test.cc <<EOF
#include "cpp/foo.h"
cat > cpp/install_name/baz.cc <<EOF
int baz() { return 42; }
EOF
cat > cpp/install_name/test.cc <<EOF
int foo();
int bar();
int baz();
int main() {
return foo();
int result = foo() + bar() + baz();
if (result == 56) {
return 0;
} else {
return result;
}
}
EOF

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

run_suite "Tests for Bazel's C++ rules on Darwin"

0 comments on commit b06f495

Please sign in to comment.