C++: Fix versioned shared libraries for macOS

The toolchain was passing -l:name. This mechanism doesn' t exist on macOS, instead the full path to the shared library should be passed. To add a proper test like in #20847, this change must first be checked in, the Apple toolchain needs to be updated and then the test can be added. RELNOTES:none PiperOrigin-RevId: 597366229 Change-Id: Ib7cf237cb6c58a69e781197408d3f703010909da
bazelbuild · Jan 10, 2024 · f0ade80 · f0ade80
1 parent 6272705
commit f0ade80
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 8 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java
@@ -661,6 +661,7 @@ public abstract static class LibraryToLinkValue extends VariableValueAdapter {
 
     public static final String OBJECT_FILES_FIELD_NAME = "object_files";
     public static final String NAME_FIELD_NAME = "name";
+    public static final String PATH_FIELD_NAME = "path";
     public static final String TYPE_FIELD_NAME = "type";
     public static final String IS_WHOLE_ARCHIVE_FIELD_NAME = "is_whole_archive";
 
@@ -670,8 +671,8 @@ public static LibraryToLinkValue forDynamicLibrary(String name) {
       return interner.intern(new ForDynamicLibrary(name));
     }
 
-    public static LibraryToLinkValue forVersionedDynamicLibrary(String name) {
-      return interner.intern(new ForVersionedDynamicLibrary(name));
+    public static LibraryToLinkValue forVersionedDynamicLibrary(String name, String path) {
+      return interner.intern(new ForVersionedDynamicLibrary(name, path));
     }
 
     public static LibraryToLinkValue forInterfaceLibrary(String name) {
@@ -797,8 +798,40 @@ protected String getTypeName() {
     }
 
     private static final class ForVersionedDynamicLibrary extends LibraryToLinkValueWithName {
-      private ForVersionedDynamicLibrary(String name) {
+      private final String path;
+
+      private ForVersionedDynamicLibrary(String name, String path) {
         super(name);
+        this.path = path;
+      }
+
+      @Override
+      public VariableValue getFieldValue(
+          String variableName,
+          String field,
+          @Nullable ArtifactExpander expander,
+          boolean throwOnMissingVariable) {
+        if (PATH_FIELD_NAME.equals(field)) {
+          return new StringValue(path);
+        }
+        return super.getFieldValue(variableName, field, expander, throwOnMissingVariable);
+      }
+
+      @Override
+      public boolean equals(Object obj) {
+        if (!(obj instanceof ForVersionedDynamicLibrary)) {
+          return false;
+        }
+        if (this == obj) {
+          return true;
+        }
+        ForVersionedDynamicLibrary other = (ForVersionedDynamicLibrary) obj;
+        return this.path.equals(other.path) && super.equals(other);
+      }
+
+      @Override
+      public int hashCode() {
+        return 31 * super.hashCode() + path.hashCode();
       }
 
       @Override

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java
@@ -613,7 +613,8 @@ private void addDynamicInputLinkOptions(
       librariesToLink.addValue(LibraryToLinkValue.forDynamicLibrary(libName));
     } else if (CppFileTypes.SHARED_LIBRARY.matches(name)
         || CppFileTypes.VERSIONED_SHARED_LIBRARY.matches(name)) {
-      librariesToLink.addValue(LibraryToLinkValue.forVersionedDynamicLibrary(name));
+      librariesToLink.addValue(
+          LibraryToLinkValue.forVersionedDynamicLibrary(name, inputArtifact.getExecPathString()));
     } else {
       // Interface shared objects have a non-standard extension
       // that the linker won't be able to find.  So use the

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryToLinkValueTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryToLinkValueTest.java
@@ -45,9 +45,10 @@ public void equalsAndHashCode() {
 
     // #forVersionedDynamicLibrary
     equalsTester.addEqualityGroup(
-        LibraryToLinkValue.forVersionedDynamicLibrary("foo"),
-        LibraryToLinkValue.forVersionedDynamicLibrary("foo"));
-    equalsTester.addEqualityGroup(LibraryToLinkValue.forVersionedDynamicLibrary("bar"));
+        LibraryToLinkValue.forVersionedDynamicLibrary("foo", /* path= */ ""),
+        LibraryToLinkValue.forVersionedDynamicLibrary("foo", /* path= */ ""));
+    equalsTester.addEqualityGroup(
+        LibraryToLinkValue.forVersionedDynamicLibrary("bar", /* path= */ ""));
 
     // #forInterfaceLibrary
     equalsTester.addEqualityGroup(
@@ -139,7 +140,8 @@ public void getFieldValue_forDynamicLibrary() throws Exception {
 
   @Test
   public void getFieldValue_forVersionedDynamicLibrary() throws Exception {
-    LibraryToLinkValue libraryToLinkValue = LibraryToLinkValue.forVersionedDynamicLibrary("foo");
+    LibraryToLinkValue libraryToLinkValue =
+        LibraryToLinkValue.forVersionedDynamicLibrary("foo", "foo/bar.so");
     assertThat(
             libraryToLinkValue
                 .getFieldValue(
@@ -149,6 +151,15 @@ public void getFieldValue_forVersionedDynamicLibrary() throws Exception {
                     /* throwOnMissingVariable= */ false)
                 .getStringValue("variable name doesn't matter"))
         .isEqualTo("versioned_dynamic_library");
+    assertThat(
+            libraryToLinkValue
+                .getFieldValue(
+                    /* variableName= */ "variable name doesn't matter",
+                    /* field= */ "path",
+                    /* expander= */ null,
+                    /* throwOnMissingVariable= */ false)
+                .getStringValue("variable name doesn't matter"))
+        .isEqualTo("foo/bar.so");
     assertThat(
             libraryToLinkValue
                 .getFieldValue(