diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java index 2240ee58b5d11a..62cb163d05be9a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java @@ -17,10 +17,13 @@ import static com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition.COMMAND_LINE_OPTION_PREFIX; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.io.BaseEncoding; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.PlatformOptions; @@ -50,10 +53,6 @@ /** A builder for {@link BuildConfigurationValue} instances. */ public final class BuildConfigurationFunction implements SkyFunction { - // The length of the hash of the config tacked onto the end of the output path. - // Limited for ergonomics and MAX_PATH reasons. - private static final int HASH_LENGTH = 12; - private final BlazeDirectories directories; private final ConfiguredRuleClassProvider ruleClassProvider; private final FragmentFactory fragmentFactory = new FragmentFactory(); @@ -269,14 +268,18 @@ private static String hashChosenOptions( @VisibleForTesting public static String transitionDirectoryNameFragment(Iterable opts) { + Preconditions.checkArgument(!Iterables.isEmpty(opts)); Fingerprint fp = new Fingerprint(); for (String opt : opts) { fp.addString(opt); } - // Shorten the hash to 48 bits. This should provide sufficient collision avoidance - // (that is, we don't expect anyone to experience a collision ever). - // Shortening the hash is important for Windows paths that tend to be short. - String suffix = fp.hexDigestAndReset().substring(0, HASH_LENGTH); + byte[] digest = fp.digestAndReset(); + // Shorten the hash to 45 bits (nine Base32 digits). This should provide sufficient collision + // avoidance (that is, we don't expect anyone to experience a collision ever). More efficient + // encoding schemes such as Base64 can't be used as some file systems are not case sensitive. + // Shortening the hash is important for paths on Windows where MAX_PATH is very low. + String suffix = BaseEncoding.base32Hex().lowerCase().omitPadding().encode(digest) + .substring(0, 9); return "ST-" + suffix; } } 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 1fdf0ba0899d5e..acf2d81b5a3717 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 @@ -2237,7 +2237,7 @@ public void testRpathRootIsAddedEvenWithTransitionedDepsOnly() throws Exception .contains("-Xlinker -rpath -Xlinker $ORIGIN/../../../k8-fastbuild-ST-"); assertThat(Joiner.on(" ").join(linkArgv)) .contains("-L" + TestConstants.PRODUCT_NAME + "-out/k8-fastbuild-ST-"); - assertThat(Joiner.on(" ").join(linkArgv)).containsMatch("-lST-[0-9a-f]+_transition_Slibdep2"); + assertThat(Joiner.on(" ").join(linkArgv)).containsMatch("-lST-[0-9a-z]+_transition_Slibdep2"); assertThat(Joiner.on(" ").join(linkArgv)) .doesNotContain("-L" + TestConstants.PRODUCT_NAME + "-out/k8-fastbuild/bin/_solib_k8"); assertThat(Joiner.on(" ").join(linkArgv)).doesNotContain("-ltransition_Slibdep2");