Fixes incorrect install names on darwin platforms

csmulhern · copybara-github · commit b06f49507e02 · 2021-06-04T07:02:21.000-07:00
#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
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -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,
diff --git a/src/test/shell/bazel/cpp_darwin_integration_test.sh b/src/test/shell/bazel/cpp_darwin_integration_test.sh
@@ -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"
-
