From 45f84e92618655c9173c4240fb55b4a3603ac553 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 11 Oct 2022 06:27:56 -0700 Subject: [PATCH] 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 --- .../lib/rules/cpp/SolibSymlinkAction.java | 29 +++++++++++++++-- .../cpp/CcLibraryConfiguredTargetTest.java | 32 +++++++++++++++++++ .../lib/rules/cpp/StarlarkCcCommonTest.java | 30 +++++++++++++++++ 3 files changed, 88 insertions(+), 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 index 8891dc5f610343..be2fac037d8d5a 100644 --- 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 index e70c09c4356795..e3e0967de42fa7 100644 --- 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 libraries = + lib.get(CcInfo.PROVIDER) + .getCcLinkingContext() + .getDynamicLibrariesForRuntime(/* linkingStatically= */ false); + List 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 index a76ffe3ff8ad52..8e0c549227f008 100755 --- 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 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 staticLibraryList, List picStaticLibraryList,