Skip to content

Commit

Permalink
Remove DigestHashFunction#getDefaultUnchecked in favor of explicit in…
Browse files Browse the repository at this point in the history
…jection.

PiperOrigin-RevId: 330584154
  • Loading branch information
janakdr authored and copybara-github committed Sep 8, 2020
1 parent 3af783a commit c39fb00
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.hash.HashFunction;
import com.google.common.io.BaseEncoding;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
Expand Down Expand Up @@ -697,6 +698,10 @@ private InlineFileArtifactValue(byte[] data, byte[] digest) {
this.digest = Preconditions.checkNotNull(digest);
}

private InlineFileArtifactValue(byte[] bytes, HashFunction hashFunction) {
this(bytes, hashFunction.hashBytes(bytes).asBytes());
}

@Override
public boolean equals(Object o) {
if (!(o instanceof InlineFileArtifactValue)) {
Expand All @@ -712,16 +717,11 @@ public int hashCode() {
return Objects.hash(Arrays.hashCode(digest), dataIsShareable());
}

private InlineFileArtifactValue(byte[] bytes) {
this(
bytes,
DigestHashFunction.getDefaultUnchecked().getHashFunction().hashBytes(bytes).asBytes());
}

public static InlineFileArtifactValue create(byte[] bytes, boolean shareable) {
public static InlineFileArtifactValue create(
byte[] bytes, boolean shareable, HashFunction hashFunction) {
return shareable
? new InlineFileArtifactValue(bytes)
: new UnshareableInlineFileArtifactValue(bytes);
? new InlineFileArtifactValue(bytes, hashFunction)
: new UnshareableInlineFileArtifactValue(bytes, hashFunction);
}

public ByteArrayInputStream getInputStream() {
Expand Down Expand Up @@ -760,8 +760,8 @@ public boolean wasModifiedSinceDigest(Path path) {
}

private static final class UnshareableInlineFileArtifactValue extends InlineFileArtifactValue {
UnshareableInlineFileArtifactValue(byte[] bytes) {
super(bytes);
UnshareableInlineFileArtifactValue(byte[] bytes, HashFunction hashFunction) {
super(bytes, hashFunction);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -114,7 +115,8 @@ private static void fullyResolveRelativeSymlinks(
// in-memory Filesystem. This allows us to then crawl the filesystem for files. Any readable
// file is a valid part of the FilesetManifest. Dangling internal links or symlink cycles will
// be discovered by the in-memory filesystem.
InMemoryFileSystem fs = new InMemoryFileSystem();
// (Choice of digest function is irrelevant).
InMemoryFileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256);
Path root = fs.getPath("/");
for (Map.Entry<PathFragment, String> e : entries.entrySet()) {
PathFragment location = e.getKey();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,16 @@

import static com.google.common.base.Preconditions.checkArgument;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.hash.HashFunction;
import com.google.common.hash.Hashing;
import com.google.devtools.build.lib.util.TestType;
import com.google.devtools.build.lib.vfs.DigestHashFunction.DigestLength.DigestLengthImpl;
import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.OptionsParsingException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map.Entry;

Expand Down Expand Up @@ -151,20 +149,6 @@ public static synchronized DigestHashFunction getDefault()
return hash;
}

/**
* Returns the default DigestHashFunction, or the testing default if unset.
*/
public static DigestHashFunction getDefaultUnchecked() {
try {
return getDefault();
} catch (DefaultHashFunctionNotSetException e) {
// Some tests use this class without calling GoogleUnixFileSystemModule.globalInit().
Preconditions.checkState(TestType.isInTest(), "Default hash function has not been set");
return DigestHashFunction.SHA256;
}
}


/** Indicates that the default has not been initialized. */
public static final class DefaultHashFunctionNotSetException extends Exception {
DefaultHashFunctionNotSetException(String message) {
Expand Down Expand Up @@ -266,8 +250,7 @@ private static boolean supportsClone(MessageDigest toCheck) {
}
}

@VisibleForTesting
static Collection<DigestHashFunction> getPossibleHashFunctions() {
return hashFunctionRegistry.values();
public static ImmutableSet<DigestHashFunction> getPossibleHashFunctions() {
return ImmutableSet.copyOf(hashFunctionRegistry.values());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.devtools.build.lib.vfs.inmemoryfs;

import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.clock.Clock;
Expand Down Expand Up @@ -80,22 +79,6 @@ public InMemoryFileSystem(Clock clock, DigestHashFunction hashFunction) {
this.rootInode = newRootInode(clock);
}

/**
* Creates a new InMemoryFileSystem with default clock and hash function.
*/
@VisibleForTesting
public InMemoryFileSystem() {
this(new JavaClock());
}

/**
* Creates a new InMemoryFileSystem.
*/
@VisibleForTesting
public InMemoryFileSystem(Clock clock) {
this(clock, DigestHashFunction.getDefaultUnchecked());
}

private static InMemoryDirectoryInfo newRootInode(Clock clock) {
InMemoryDirectoryInfo rootInode = new InMemoryDirectoryInfo(clock);
rootInode.addChild(".", rootInode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ java_library(
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
"//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
"//src/test/java/com/google/devtools/build/lib/vfs/util",
"//third_party:error_prone_annotations",
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,15 @@
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.util.io.OutErr;
import com.google.devtools.build.lib.util.io.RecordingOutErr;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.util.FileSystems;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
import com.google.errorprone.annotations.ForOverride;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -288,7 +290,12 @@ protected boolean realFileSystem() {
}

protected FileSystem createFileSystem() throws Exception {
return FileSystems.getNativeFileSystem();
return FileSystems.getNativeFileSystem(getDigestHashFunction());
}

@ForOverride
protected DigestHashFunction getDigestHashFunction() {
return DigestHashFunction.SHA256;
}

protected Path createTestRoot(FileSystem fileSystem) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,25 @@ public final class FileSystems {
private FileSystems() {}

/** Constructs a platform native (Unix or Windows) file system. */
public static FileSystem getNativeFileSystem() {
public static FileSystem getNativeFileSystem(DigestHashFunction digestHashFunction) {
if (OS.getCurrent() == OS.WINDOWS) {
return new WindowsFileSystem(
DigestHashFunction.getDefaultUnchecked(), /*createSymbolicLinks=*/ false);
return new WindowsFileSystem(digestHashFunction, /*createSymbolicLinks=*/ false);
}
try {
return Class.forName(TestConstants.TEST_REAL_UNIX_FILE_SYSTEM)
.asSubclass(FileSystem.class)
.getDeclaredConstructor(DigestHashFunction.class)
.newInstance(DigestHashFunction.getDefaultUnchecked());
.newInstance(digestHashFunction);
} catch (Exception e) {
throw new IllegalStateException(e);
}
}

/** Constructs a platform native (Unix or Windows) file system with SHA256 digests. */
public static FileSystem getNativeFileSystem() {
return getNativeFileSystem(DigestHashFunction.SHA256);
}

/** Constructs a java.io.File file system. */
public static FileSystem getJavaIoFileSystem() {
return new JavaIoFileSystem(DigestHashFunction.SHA256);
Expand Down

0 comments on commit c39fb00

Please sign in to comment.