From ccecff9ad7b339536b42fab64549175fe68cc4ab Mon Sep 17 00:00:00 2001
From: Cameron Mulhern <csmulhern@gmail.com>
Date: Tue, 4 May 2021 22:05:52 -0400
Subject: [PATCH] Fixes incorrect install names on darwin platforms

#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.
---
 .../lib/rules/cpp/CppLinkActionBuilder.java   |  3 +-
 .../bazel/cpp_darwin_integration_test.sh      | 50 +++++++++++++------
 2 files changed, 35 insertions(+), 18 deletions(-)

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
index 089ae24b37bd3f..d30464ab7d026b 100644
--- 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
index e26d2a902e93fd..4676e6c8f38c2a 100755
--- 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"
-