Adding support for SHA256 for remote execution. Switch remote execution to use
the currently defined hash function for blobs. Some refactoring. Adding an option to set the hash function in the remote worker, defaulting to the current behavior (unfortunately it is a build option, have not found a clean way to specify it at runtime).
BUG=62622420
TESTED=remote worker
RELNOTES: none
PiperOrigin-RevId: 159473116
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java b/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java
index 6fe3f73..57df2cf 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.profiler;
import com.google.common.base.Predicate;
-
import java.util.EnumSet;
/**
@@ -57,6 +56,7 @@
VFS_STAT("VFS stat", 10000000, 0x9999FF, 30, true),
VFS_DIR("VFS readdir", 10000000, 0x0066CC, 30, true),
VFS_READLINK("VFS readlink", 10000000, 0x99CCCC, 30, true),
+ // TODO(olaola): rename to VFS_DIGEST. This refers to all digest function computations.
VFS_MD5("VFS md5", 10000000, 0x999999, 30, true),
VFS_XATTR("VFS xattr", 10000000, 0x9999DD, 30, true),
VFS_DELETE("VFS delete", 10000000, 0xFFCC00, 0, true),
diff --git a/src/main/java/com/google/devtools/build/lib/remote/Digests.java b/src/main/java/com/google/devtools/build/lib/remote/Digests.java
index 1c70db3..cc14598 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/Digests.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/Digests.java
@@ -17,12 +17,12 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.hash.HashCode;
-import com.google.common.hash.Hashing;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputFileCache;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.remoteexecution.v1test.Action;
import com.google.devtools.remoteexecution.v1test.Digest;
@@ -36,7 +36,8 @@
private Digests() {}
public static Digest computeDigest(byte[] blob) {
- return buildDigest(Hashing.sha1().hashBytes(blob).toString(), blob.length);
+ return buildDigest(
+ FileSystem.getDigestFunction().getHash().hashBytes(blob).toString(), blob.length);
}
public static Digest computeDigest(Path file) throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/README.md b/src/main/java/com/google/devtools/build/lib/remote/README.md
index 2525240..e8487ec 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/README.md
+++ b/src/main/java/com/google/devtools/build/lib/remote/README.md
@@ -37,6 +37,11 @@
build --strategy=Closure=remote
```
+#### Customizing The Digest Function
+
+Bazel currently supports the following digest functions with the remote worker: SHA1, SHA256, and MD5. The digest function is passed via the `--host_jvm_args=-Dbazel.DigestFunction=###` startup option. In the example above, SHA1 is used, but you can use any one of SHA1, SHA256, and MD5, provided that your remote execution server supports it and is configured to use the same one. For example, the provided remote worker (`//src/tools/remote_worker`) is configured to use SHA1 by default in the binary build rule. You can customize it there by modifying the `jvm_flags` attribute to use, for example, `"-Dbazel.DigestFunction=SHA256"` instead.
+
+
### Hazelcast with REST interface
[Hazelcast](https://hazelcast.org/) is a distributed in-memory cache which can be used by Bazel as a remote cache.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 6e5f790..8da0e9c 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -16,20 +16,15 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
-import com.google.common.eventbus.Subscribe;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.buildeventstream.PathConverter;
import com.google.devtools.build.lib.buildtool.BuildRequest;
-import com.google.devtools.build.lib.buildtool.buildevent.BuildStartingEvent;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.ServerBuilder;
import com.google.devtools.build.lib.util.AbruptExitException;
-import com.google.devtools.build.lib.util.ExitCode;
-import com.google.devtools.build.lib.vfs.FileSystem;
-import com.google.devtools.build.lib.vfs.FileSystem.HashFunction;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsProvider;
@@ -77,7 +72,6 @@
}
private final CasPathConverter converter = new CasPathConverter();
- private CommandEnvironment env;
@Override
public void serverInit(OptionsProvider startupOptions, ServerBuilder builder)
@@ -87,7 +81,6 @@
@Override
public void beforeCommand(CommandEnvironment env) {
- this.env = env;
env.getEventBus().register(this);
}
@@ -97,30 +90,10 @@
}
@Override
- public void afterCommand() {
- this.env = null;
- }
-
- @Override
public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
builder.addActionContextProvider(new RemoteActionContextProvider(env));
}
- @Subscribe
- public void buildStarting(BuildStartingEvent event) {
- RemoteOptions options = event.getRequest().getOptions(RemoteOptions.class);
-
- if (remoteEnabled(options)) {
- HashFunction hf = FileSystem.getDigestFunction();
- if (hf != HashFunction.SHA1) {
- env.getBlazeModuleEnvironment().exit(new AbruptExitException(
- "Remote cache/execution requires SHA1 digests, got " + hf
- + ", run with --host_jvm_args=-Dbazel.DigestFunction=SHA1",
- ExitCode.COMMAND_LINE_ERROR));
- }
- }
- }
-
@Override
public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
return "build".equals(command.name())
diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
index c70420b..91268fe 100644
--- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.vfs.AbstractFileSystemWithCustomStat;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileStatus;
+import com.google.devtools.build.lib.vfs.FileSystem.HashFunction;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
@@ -402,11 +403,14 @@
}
@Override
- protected byte[] getMD5Digest(Path path) throws IOException {
+ protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException {
String name = path.toString();
long startTime = Profiler.nanoTimeMaybe();
try {
- return NativePosixFiles.md5sum(name).asBytes();
+ if (hashFunction == HashFunction.MD5) {
+ return NativePosixFiles.md5sum(name).asBytes();
+ }
+ return super.getDigest(path, hashFunction);
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_MD5, name);
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
index 03a0afc..13a7360 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
@@ -42,14 +42,17 @@
public abstract class FileSystem {
/** Type of hash function to use for digesting files. */
+ // The underlying HashFunctions are immutable and thread safe.
+ @SuppressWarnings("ImmutableEnumChecker")
public enum HashFunction {
- MD5(16),
- SHA1(20);
+ MD5(Hashing.md5()),
+ SHA1(Hashing.sha1()),
+ SHA256(Hashing.sha256());
- private final int digestSize;
+ private final com.google.common.hash.HashFunction hash;
- HashFunction(int digestSize) {
- this.digestSize = digestSize;
+ HashFunction(com.google.common.hash.HashFunction hash) {
+ this.hash = hash;
}
/** Converts to {@link HashFunction}. */
@@ -59,8 +62,12 @@
}
}
+ public com.google.common.hash.HashFunction getHash() {
+ return hash;
+ }
+
public boolean isValidDigest(byte[] digest) {
- return digest != null && digest.length == digestSize;
+ return digest != null && digest.length * 8 == hash.bits();
}
}
@@ -336,16 +343,16 @@
*
* @return a new byte array containing the file's digest
* @throws IOException if the digest could not be computed for any reason
+ *
+ * Subclasses may (and do) optimize this computation for particular digest functions.
*/
- protected final byte[] getDigest(final Path path, HashFunction hashFunction) throws IOException {
- switch(hashFunction) {
- case MD5:
- return getMD5Digest(path);
- case SHA1:
- return getSHA1Digest(path);
- default:
- throw new IOException("Unsupported hash function: " + hashFunction);
- }
+ protected byte[] getDigest(final Path path, HashFunction hashFunction) throws IOException {
+ return new ByteSource() {
+ @Override
+ public InputStream openStream() throws IOException {
+ return getInputStream(path);
+ }
+ }.hash(hashFunction.getHash()).asBytes();
}
/**
@@ -354,40 +361,11 @@
* @return a new byte array containing the file's digest
* @throws IOException if the digest could not be computed for any reason
*/
- protected byte[] getDigest(final Path path) throws IOException {
+ protected final byte[] getDigest(final Path path) throws IOException {
return getDigest(path, digestFunction);
}
/**
- * Returns the MD5 digest of the file denoted by {@code path}. See
- * {@link Path#getMD5Digest} for specification.
- */
- protected byte[] getMD5Digest(final Path path) throws IOException {
- // Naive I/O implementation. Subclasses may (and do) optimize.
- // This code is only used by the InMemory or Zip or other weird FSs.
- return new ByteSource() {
- @Override
- public InputStream openStream() throws IOException {
- return getInputStream(path);
- }
- }.hash(Hashing.md5()).asBytes();
- }
-
- /**
- * Returns the MD5 digest of the file denoted by {@code path}. See
- * {@link Path#getMD5Digest} for specification.
- */
- protected byte[] getSHA1Digest(final Path path) throws IOException {
- // Naive I/O implementation. TODO(olaola): optimize!
- return new ByteSource() {
- @Override
- public InputStream openStream() throws IOException {
- return getInputStream(path);
- }
- }.hash(Hashing.sha1()).asBytes();
- }
-
- /**
* Returns true if "path" denotes an existing symbolic link. See
* {@link Path#isSymbolicLink} for specification.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
index 2512308..aeccb3b 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
@@ -378,11 +378,11 @@
}
@Override
- protected byte[] getMD5Digest(Path path) throws IOException {
+ protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException {
String name = path.toString();
long startTime = Profiler.nanoTimeMaybe();
try {
- return super.getMD5Digest(path);
+ return super.getDigest(path, hashFunction);
} finally {
profiler.logSimpleTask(startTime, ProfilerTask.VFS_MD5, name);
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
index bf8c612..d5aa5ee 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
@@ -1046,34 +1046,6 @@
}
/**
- * Returns the MD5 digest of the file denoted by the current path, following
- * symbolic links.
- *
- * <p>This method runs in O(n) time where n is the length of the file, but
- * certain implementations may be much faster than the worst case.
- *
- * @return a new 16-byte array containing the file's MD5 digest
- * @throws IOException if the MD5 digest could not be computed for any reason
- */
- public byte[] getMD5Digest() throws IOException {
- return fileSystem.getMD5Digest(this);
- }
-
- /**
- * Returns the SHA1 digest of the file denoted by the current path, following
- * symbolic links.
- *
- * <p>This method runs in O(n) time where n is the length of the file, but
- * certain implementations may be much faster than the worst case.
- *
- * @return a new 20-byte array containing the file's SHA1 digest
- * @throws IOException if the SHA1 digest could not be computed for any reason
- */
- public byte[] getSHA1Digest() throws IOException {
- return fileSystem.getSHA1Digest(this);
- }
-
- /**
* Returns the digest of the file denoted by the current path,
* following symbolic links.
*
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java
index b246aa6..d1d82d9 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/UnionFileSystem.java
@@ -19,13 +19,11 @@
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.StringTrie;
import com.google.devtools.build.lib.vfs.FileSystem.HashFunction;
-
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.Collection;
import java.util.Map;
-
import javax.annotation.Nullable;
/**
@@ -190,9 +188,9 @@
}
@Override
- protected byte[] getMD5Digest(Path path) throws IOException {
+ protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException {
FileSystem delegate = getDelegate(path);
- return delegate.getMD5Digest(adjustPath(path, delegate));
+ return delegate.getDigest(adjustPath(path, delegate), hashFunction);
}
@Override
