Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate shorter transition output directory suffixes #17474

Closed
wants to merge 1 commit into from
Closed

Generate shorter transition output directory suffixes #17474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -269,14 +268,18 @@ private static String hashChosenOptions(

@VisibleForTesting
public static String transitionDirectoryNameFragment(Iterable<String> 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;
}
}
2 changes: 1 addition & 1 deletion src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down