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

9 files changed, 41 insertions, 110 deletions

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 6fe3f73446..57df2cf8b0 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 @@ public enum ProfilerTask {
   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 1c70db306e..cc14598ef1 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 @@ package com.google.devtools.build.lib.remote;
 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 @@ public final class Digests {
   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 2525240b93..e8487ec8b1 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=Javac=remote
 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 6e5f7909cb..8da0e9c477 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 @@ package com.google.devtools.build.lib.remote;
 
 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 @@ public final class RemoteModule extends BlazeModule {
   }
 
   private final CasPathConverter converter = new CasPathConverter();
-  private CommandEnvironment env;
 
   @Override
   public void serverInit(OptionsProvider startupOptions, ServerBuilder builder)
@@ -87,7 +81,6 @@ public final class RemoteModule extends BlazeModule {
 
   @Override
   public void beforeCommand(CommandEnvironment env) {
-    this.env = env;
     env.getEventBus().register(this);
   }
 
@@ -97,30 +90,10 @@ public final class RemoteModule extends BlazeModule {
   }
 
   @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 c70420ba7d..91268fe90d 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.util.Preconditions;
 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 @@ public class UnixFileSystem extends AbstractFileSystemWithCustomStat {
   }
 
   @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 03a0afc391..13a7360532 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 @@ import java.util.List;
 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 abstract class FileSystem {
       }
     }
 
+    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,55 +343,26 @@ public abstract class FileSystem {
    *
    * @return a new byte array containing the file's digest
    * @throws IOException if the digest could not be computed for any reason
-   */
-  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);
-    }
-  }
-
-  /**
-   * Returns the digest of the file denoted by the path, following symbolic links.
    *
-   * @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 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.
+  protected byte[] getDigest(final Path path, HashFunction hashFunction) throws IOException {
     return new ByteSource() {
       @Override
       public InputStream openStream() throws IOException {
         return getInputStream(path);
       }
-    }.hash(Hashing.md5()).asBytes();
+    }.hash(hashFunction.getHash()).asBytes();
   }
 
   /**
-   * Returns the MD5 digest of the file denoted by {@code path}. See
-   * {@link Path#getMD5Digest} for specification.
+   * Returns the digest of the file denoted by the path, following symbolic links.
+   *
+   * @return a new byte array containing the file's digest
+   * @throws IOException if the digest could not be computed for any reason
    */
-  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();
+  protected final byte[] getDigest(final Path path) throws IOException {
+    return getDigest(path, digestFunction);
   }
 
   /**
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 2512308808..aeccb3bf4b 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 @@ public class JavaIoFileSystem extends AbstractFileSystemWithCustomStat {
   }
 
   @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 bf8c612d66..d5aa5eef85 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 @@ public class Path implements Comparable<Path>, Serializable {
   }
 
   /**
-   * 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 b246aa68fd..d1d82d9361 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.concurrent.ThreadSafety;
 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 @@ public class UnionFileSystem extends FileSystem {
   }
 
   @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