Handle excessively long dynamic library symlink path

In order to migrate user-generated Apple frameworks to CcInfo, we need to handle dynamic library symlink paths whose length exceed the OS limit (255 on most systems). Hash them into shorter strings in those cases. PiperOrigin-RevId: 480339350 Change-Id: Icabf61cd384f03be82165b3423916ff37df24626
bazelbuild · Oct 11, 2022 · 5b546af · 5b546af
1 parent b8298ac
commit 5b546af
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 3 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SolibSymlinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SolibSymlinkAction.java
@@ -14,9 +14,14 @@
 
 package com.google.devtools.build.lib.rules.cpp;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.hash.Hashing;
+import com.google.common.io.Files;
 import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -220,6 +225,22 @@ private static Artifact getDynamicLibrarySymlinkInternal(
     return symlink;
   }
 
+  @VisibleForTesting public static final int MAX_FILENAME_LENGTH = 255;
+
+  private static String maybeHashPreserveExtension(String filename) {
+    if (filename.length() <= MAX_FILENAME_LENGTH) {
+      return filename;
+    } else {
+      String hashedName = Hashing.sha256().hashString(filename, UTF_8).toString();
+      String extension = Files.getFileExtension(filename);
+      if (extension.isEmpty()) {
+        return hashedName;
+      } else {
+        return hashedName + "." + extension;
+      }
+    }
+  }
+
   /**
    * Returns the name of the symlink that will be created for a library, given its name.
    *
@@ -243,12 +264,14 @@ private static PathFragment getMangledName(
     if (preserveName) {
       String escapedLibraryPath =
           Actions.escapedPath("_" + libraryPath.getParentDirectory().getPathString());
+      String escapedFullPath =
+          prefixConsumer ? escapedRulePath + "__" + escapedLibraryPath : escapedLibraryPath;
       PathFragment mangledDir =
-          solibDirPath.getRelative(
-              prefixConsumer ? escapedRulePath + "__" + escapedLibraryPath : escapedLibraryPath);
+          solibDirPath.getRelative(maybeHashPreserveExtension(escapedFullPath));
       return mangledDir.getRelative(soname);
     } else {
-      return solibDirPath.getRelative(prefixConsumer ? escapedRulePath + "__" + soname : soname);
+      String filename = prefixConsumer ? escapedRulePath + "__" + soname : soname;
+      return solibDirPath.getRelative(maybeHashPreserveExtension(filename));
     }
   }
 

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
@@ -17,6 +17,7 @@
 
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.rules.cpp.SolibSymlinkAction.MAX_FILENAME_LENGTH;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
@@ -2267,4 +2268,35 @@ public void testWindowsCcLibrariesNoDepsDynamicLibrariesDoNotLinkstamp() throws
     CppLinkAction action = (CppLinkAction) getGeneratingAction(sharedObject);
     assertThat(action.getLinkstampObjects()).isEmpty();
   }
+
+  @Test
+  public void testReallyLongSolibLink() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCcToolchainConfig(
+            mockToolsConfig,
+            CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));
+
+    String longpath =
+        "this/is/a/really/really/really/really/really/really/really/really/really/really/"
+            + "really/really/really/really/really/really/really/really/really/really/really/"
+            + "really/really/long/path/that/generates/really/long/solib/link/file";
+    scratch.file(
+        longpath + "/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = ['lib.cc'],",
+        "    linkstatic = 0,",
+        ")");
+
+    ConfiguredTarget lib = getConfiguredTarget("//" + longpath + ":lib");
+    List<Artifact> libraries =
+        lib.get(CcInfo.PROVIDER)
+            .getCcLinkingContext()
+            .getDynamicLibrariesForRuntime(/* linkingStatically= */ false);
+    List<String> libraryBaseNames = ActionsTestUtil.baseArtifactNames(libraries);
+    for (String baseName : libraryBaseNames) {
+      assertThat(baseName.length()).isLessThan(MAX_FILENAME_LENGTH + 1);
+    }
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
@@ -17,6 +17,7 @@
 import static com.google.common.truth.Truth8.assertThat;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseArtifactNames;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames;
+import static com.google.devtools.build.lib.rules.cpp.SolibSymlinkAction.MAX_FILENAME_LENGTH;
 import static org.junit.Assert.assertThrows;
 
 import com.google.common.base.Joiner;
@@ -1527,6 +1528,35 @@ public void testSolibLinkCustom() throws Exception {
         .containsExactly("libcustom.ifso");
   }
 
+  @Test
+  public void testReallyLongSolibLink() throws Exception {
+    setUpCcLinkingContextTest(false);
+
+    String longpath =
+        "this/is/a/really/really/really/really/really/really/really/really/really/really/"
+            + "really/really/really/really/really/really/really/really/really/really/really/"
+            + "really/really/long/path/that/generates/really/long/solib/link/path";
+    scratch.file(
+        longpath + "/BUILD",
+        "load('//tools/build_defs/cc:rule.bzl', 'crule')",
+        "crule(name='a',",
+        "   dynamic_library = 'a.so',",
+        ")");
+
+    ConfiguredTarget a = getConfiguredTarget("//" + longpath + ":a");
+    StructImpl info = ((StructImpl) getMyInfoFromTarget(a).getValue("info"));
+    Depset librariesToLink = info.getValue("libraries_to_link", Depset.class);
+    ImmutableList<String> dynamicLibraryParentDirectories =
+        librariesToLink.toList(LibraryToLink.class).stream()
+            .filter(x -> x.getDynamicLibrary() != null)
+            .map(
+                x -> x.getDynamicLibrary().getRootRelativePath().getParentDirectory().getBaseName())
+            .collect(ImmutableList.toImmutableList());
+    for (String dynamicLibraryParentDirectory : dynamicLibraryParentDirectories) {
+      assertThat(dynamicLibraryParentDirectory.length()).isLessThan(MAX_FILENAME_LENGTH + 1);
+    }
+  }
+
   private void doTestCcLinkingContext(
       List<String> staticLibraryList,
       List<String> picStaticLibraryList,