From 559a07d2dd88a53d6dac8bf2d77a28db44fd7659 Mon Sep 17 00:00:00 2001 From: buchgr Date: Thu, 30 Nov 2017 11:09:35 -0800 Subject: Refactor the FileSystem API to allow for different hash functions. Refactor the FileSystem class to include the hash function as an instance field. This allows us to have a different hash function per FileSystem and removes technical debt, as currently that's somewhat accomplished by a horrible hack that has a static method to set the hash function for all FileSystem instances. The FileSystem's default hash function remains MD5. RELNOTES: None PiperOrigin-RevId: 177479772 --- .../google/devtools/build/lib/bazel/BazelMain.java | 1 + .../google/devtools/build/lib/remote/Chunker.java | 68 +++++++----- .../devtools/build/lib/remote/DigestUtil.java | 117 +++++++++++++++++++++ .../google/devtools/build/lib/remote/Digests.java | 116 -------------------- .../devtools/build/lib/remote/GrpcRemoteCache.java | 36 ++++--- .../build/lib/remote/RemoteActionCache.java | 2 +- .../lib/remote/RemoteActionContextProvider.java | 15 ++- .../devtools/build/lib/remote/RemoteModule.java | 17 ++- .../build/lib/remote/RemoteSpawnCache.java | 14 ++- .../build/lib/remote/RemoteSpawnRunner.java | 13 ++- .../lib/remote/SimpleBlobStoreActionCache.java | 14 +-- .../build/lib/remote/TracingMetadataUtils.java | 2 +- .../build/lib/remote/TreeNodeRepository.java | 13 ++- .../build/lib/runtime/BazelFileSystemModule.java | 59 +++++++++++ .../devtools/build/lib/runtime/BlazeModule.java | 2 +- .../devtools/build/lib/unix/UnixFileSystem.java | 5 + .../devtools/build/lib/vfs/AbstractFileSystem.java | 6 ++ .../lib/vfs/AbstractFileSystemWithCustomStat.java | 7 ++ .../google/devtools/build/lib/vfs/FileSystem.java | 23 ++-- .../devtools/build/lib/vfs/JavaIoFileSystem.java | 5 + .../lib/vfs/inmemoryfs/InMemoryFileSystem.java | 20 +++- .../build/lib/windows/WindowsFileSystem.java | 6 ++ src/test/java/com/google/devtools/build/lib/BUILD | 1 + .../build/lib/actions/DigestUtilsTest.java | 61 +++++------ .../build/lib/remote/ByteStreamUploaderTest.java | 27 ++--- .../build/lib/remote/CasPathConverterTest.java | 11 ++ .../devtools/build/lib/remote/ChunkerTest.java | 17 +-- .../build/lib/remote/FakeActionInputFileCache.java | 4 +- .../build/lib/remote/GrpcRemoteCacheTest.java | 46 ++++---- .../lib/remote/GrpcRemoteExecutionClientTest.java | 30 +++--- .../build/lib/remote/RemoteSpawnCacheTest.java | 17 ++- .../build/lib/remote/RemoteSpawnRunnerTest.java | 51 ++++++--- .../build/lib/remote/TreeNodeRepositoryTest.java | 10 +- .../devtools/build/remote/ActionCacheServer.java | 12 ++- .../devtools/build/remote/ByteStreamServer.java | 12 ++- .../devtools/build/remote/ExecutionServer.java | 11 +- .../google/devtools/build/remote/RemoteWorker.java | 36 +++++-- 37 files changed, 569 insertions(+), 338 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/DigestUtil.java delete mode 100644 src/main/java/com/google/devtools/build/lib/remote/Digests.java create mode 100644 src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelMain.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelMain.java index 4c52695f10..96a52be8e8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelMain.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelMain.java @@ -37,6 +37,7 @@ public final class BazelMain { */ public static final ImmutableList> BAZEL_MODULES = ImmutableList.of( + com.google.devtools.build.lib.runtime.BazelFileSystemModule.class, com.google.devtools.build.lib.runtime.mobileinstall.MobileInstallModule.class, com.google.devtools.build.lib.bazel.BazelWorkspaceStatusModule.class, com.google.devtools.build.lib.bazel.BazelDiffAwarenessModule.class, diff --git a/src/main/java/com/google/devtools/build/lib/remote/Chunker.java b/src/main/java/com/google/devtools/build/lib/remote/Chunker.java index abf894e2d9..9f14fa3f0b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/Chunker.java +++ b/src/main/java/com/google/devtools/build/lib/remote/Chunker.java @@ -43,9 +43,6 @@ import java.util.function.Supplier; */ public final class Chunker { - private static final Chunk EMPTY_CHUNK = - new Chunk(Digests.computeDigest(new byte[0]), ByteString.EMPTY, 0); - private static int defaultChunkSize = 1024 * 16; /** This method must only be called in tests! */ @@ -106,6 +103,7 @@ public final class Chunker { private final Supplier dataSupplier; private final Digest digest; private final int chunkSize; + private final Chunk emptyChunk; private InputStream data; private long offset; @@ -115,12 +113,12 @@ public final class Chunker { // lazily on the first call to next(), as opposed to opening it in the constructor or on reset(). private boolean initialized; - public Chunker(byte[] data) throws IOException { - this(data, getDefaultChunkSize()); + public Chunker(byte[] data, DigestUtil digestUtil) throws IOException { + this(data, getDefaultChunkSize(), digestUtil); } - public Chunker(byte[] data, int chunkSize) throws IOException { - this(() -> new ByteArrayInputStream(data), Digests.computeDigest(data), chunkSize); + public Chunker(byte[] data, int chunkSize, DigestUtil digestUtil) throws IOException { + this(() -> new ByteArrayInputStream(data), digestUtil.compute(data), chunkSize, digestUtil); } public Chunker(Path file) throws IOException { @@ -128,38 +126,52 @@ public final class Chunker { } public Chunker(Path file, int chunkSize) throws IOException { - this(() -> { - try { - return file.getInputStream(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }, Digests.computeDigest(file), chunkSize); + this( + () -> { + try { + return file.getInputStream(); + } catch (IOException e) { + throw new RuntimeException(e); + } + }, + new DigestUtil(file.getFileSystem().getDigestFunction()).compute(file), + chunkSize, + new DigestUtil(file.getFileSystem().getDigestFunction())); } - public Chunker(ActionInput actionInput, MetadataProvider inputCache, Path execRoot) throws - IOException{ - this(actionInput, inputCache, execRoot, getDefaultChunkSize()); + public Chunker( + ActionInput actionInput, MetadataProvider inputCache, Path execRoot, DigestUtil digestUtil) + throws IOException { + this(actionInput, inputCache, execRoot, getDefaultChunkSize(), digestUtil); } - public Chunker(ActionInput actionInput, MetadataProvider inputCache, Path execRoot, - int chunkSize) + public Chunker( + ActionInput actionInput, + MetadataProvider inputCache, + Path execRoot, + int chunkSize, + DigestUtil digestUtil) throws IOException { - this(() -> { - try { - return execRoot.getRelative(actionInput.getExecPathString()).getInputStream(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }, Digests.getDigestFromInputCache(actionInput, inputCache), chunkSize); + this( + () -> { + try { + return execRoot.getRelative(actionInput.getExecPathString()).getInputStream(); + } catch (IOException e) { + throw new RuntimeException(e); + } + }, + DigestUtil.getFromInputCache(actionInput, inputCache), + chunkSize, + digestUtil); } @VisibleForTesting - Chunker(Supplier dataSupplier, Digest digest, int chunkSize) + Chunker(Supplier dataSupplier, Digest digest, int chunkSize, DigestUtil digestUtil) throws IOException { this.dataSupplier = checkNotNull(dataSupplier); this.digest = checkNotNull(digest); this.chunkSize = chunkSize; + this.emptyChunk = new Chunk(digestUtil.compute(new byte[0]), ByteString.EMPTY, 0); } public Digest digest() { @@ -206,7 +218,7 @@ public final class Chunker { if (digest.getSizeBytes() == 0) { data = null; - return EMPTY_CHUNK; + return emptyChunk; } // The cast to int is safe, because the return value is capped at chunkSize. diff --git a/src/main/java/com/google/devtools/build/lib/remote/DigestUtil.java b/src/main/java/com/google/devtools/build/lib/remote/DigestUtil.java new file mode 100644 index 0000000000..2ef23b54fa --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/DigestUtil.java @@ -0,0 +1,117 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.base.Preconditions; +import com.google.common.hash.HashCode; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.MetadataProvider; +import com.google.devtools.build.lib.actions.cache.DigestUtils; +import com.google.devtools.build.lib.actions.cache.Metadata; +import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.remoteexecution.v1test.Action; +import com.google.devtools.remoteexecution.v1test.Digest; +import com.google.protobuf.Message; +import java.io.ByteArrayOutputStream; +import java.io.IOException; + +/** Utility methods to work with {@link Digest}. */ +public class DigestUtil { + + /** + * A special type of Digest that is used only as a remote action cache key. This is a separate + * type in order to prevent accidentally using other Digests as action keys. + */ + public static final class ActionKey { + private final Digest digest; + + public Digest getDigest() { + return digest; + } + + private ActionKey(Digest digest) { + this.digest = digest; + } + } + + private final HashFunction hashFn; + + public DigestUtil(HashFunction hashFn) { + this.hashFn = hashFn; + } + + public Digest compute(byte[] blob) { + return buildDigest(hashFn.getHash().hashBytes(blob).toString(), blob.length); + } + + public Digest compute(Path file) throws IOException { + long fileSize = file.getFileSize(); + byte[] digest = DigestUtils.getDigestOrFail(file, fileSize); + return buildDigest(digest, fileSize); + } + + public Digest compute(VirtualActionInput input) throws IOException { + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + input.writeTo(buffer); + return compute(buffer.toByteArray()); + } + + /** + * Computes a digest of the given proto message. Currently, we simply rely on message output as + * bytes, but this implementation relies on the stability of the proto encoding, in particular + * between different platforms and languages. TODO(olaola): upgrade to a better implementation! + */ + public Digest compute(Message message) { + return compute(message.toByteArray()); + } + + public Digest computeAsUtf8(String str) { + return compute(str.getBytes(UTF_8)); + } + + public DigestUtil.ActionKey computeActionKey(Action action) { + return new DigestUtil.ActionKey(compute(action)); + } + + /** + * Assumes that the given Digest is a valid digest of an Action, and creates an ActionKey wrapper. + * This should not be called on the client side! + */ + public DigestUtil.ActionKey asActionKey(Digest digest) { + return new DigestUtil.ActionKey(digest); + } + + public static Digest buildDigest(byte[] hash, long size) { + return buildDigest(HashCode.fromBytes(hash).toString(), size); + } + + public static Digest buildDigest(String hexHash, long size) { + return Digest.newBuilder().setHash(hexHash).setSizeBytes(size).build(); + } + + public static Digest getFromInputCache(ActionInput input, MetadataProvider cache) + throws IOException { + Metadata metadata = cache.getMetadata(input); + Preconditions.checkNotNull(metadata, "Input cache %s returned no value for %s", cache, input); + Preconditions.checkNotNull( + metadata.getDigest(), + "Null digest for %s, possible directory. Data dependencies on directories are not allowed.", + input); + return buildDigest(metadata.getDigest(), metadata.getSize()); + } +} 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 deleted file mode 100644 index c3b6e0d3a6..0000000000 --- a/src/main/java/com/google/devtools/build/lib/remote/Digests.java +++ /dev/null @@ -1,116 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.remote; - -import static java.nio.charset.StandardCharsets.UTF_8; - -import com.google.common.base.Preconditions; -import com.google.common.hash.HashCode; -import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.MetadataProvider; -import com.google.devtools.build.lib.actions.cache.DigestUtils; -import com.google.devtools.build.lib.actions.cache.Metadata; -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; -import com.google.protobuf.Message; -import java.io.ByteArrayOutputStream; -import java.io.IOException; - -/** Helper methods relating to computing Digest messages for remote execution. */ -@ThreadSafe -public final class Digests { - private Digests() {} - - public static Digest computeDigest(byte[] blob) { - return buildDigest( - FileSystem.getDigestFunction().getHash().hashBytes(blob).toString(), blob.length); - } - - public static Digest computeDigest(Path file) throws IOException { - long fileSize = file.getFileSize(); - byte[] digest = DigestUtils.getDigestOrFail(file, fileSize); - return buildDigest(digest, fileSize); - } - - public static Digest computeDigest(VirtualActionInput input) throws IOException { - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - input.writeTo(buffer); - return computeDigest(buffer.toByteArray()); - } - - /** - * Computes a digest of the given proto message. Currently, we simply rely on message output as - * bytes, but this implementation relies on the stability of the proto encoding, in particular - * between different platforms and languages. TODO(olaola): upgrade to a better implementation! - */ - public static Digest computeDigest(Message message) { - return computeDigest(message.toByteArray()); - } - - public static Digest computeDigestUtf8(String str) { - return computeDigest(str.getBytes(UTF_8)); - } - - /** - * A special type of Digest that is used only as a remote action cache key. This is a - * separate type in order to prevent accidentally using other Digests as action keys. - */ - public static final class ActionKey { - private final Digest digest; - - public Digest getDigest() { - return digest; - } - - private ActionKey(Digest digest) { - this.digest = digest; - } - } - - public static ActionKey computeActionKey(Action action) { - return new ActionKey(computeDigest(action)); - } - - /** - * Assumes that the given Digest is a valid digest of an Action, and creates an ActionKey - * wrapper. This should not be called on the client side! - */ - public static ActionKey unsafeActionKeyFromDigest(Digest digest) { - return new ActionKey(digest); - } - - public static Digest buildDigest(byte[] hash, long size) { - return buildDigest(HashCode.fromBytes(hash).toString(), size); - } - - public static Digest buildDigest(String hexHash, long size) { - return Digest.newBuilder().setHash(hexHash).setSizeBytes(size).build(); - } - - public static Digest getDigestFromInputCache(ActionInput input, MetadataProvider cache) - throws IOException { - Metadata metadata = cache.getMetadata(input); - Preconditions.checkNotNull(metadata, "Input cache %s returned no value for %s", cache, input); - Preconditions.checkNotNull( - metadata.getDigest(), - "Null digest for %s, possible directory. Data dependencies on directories are not allowed.", - input); - return buildDigest(metadata.getDigest(), metadata.getSize()); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java index 28c04bffb9..5c3722e727 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java @@ -29,7 +29,7 @@ import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -71,19 +71,23 @@ public class GrpcRemoteCache implements RemoteActionCache { private final CallCredentials credentials; private final Channel channel; private final Retrier retrier; - private final ByteStreamUploader uploader; - + private final DigestUtil digestUtil; private final ListeningScheduledExecutorService retryScheduler = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); @VisibleForTesting - public GrpcRemoteCache(Channel channel, CallCredentials credentials, RemoteOptions options, - Retrier retrier) { + public GrpcRemoteCache( + Channel channel, + CallCredentials credentials, + RemoteOptions options, + Retrier retrier, + DigestUtil digestUtil) { this.options = options; this.credentials = credentials; this.channel = channel; this.retrier = retrier; + this.digestUtil = digestUtil; uploader = new ByteStreamUploader(options.remoteInstanceName, channel, credentials, options.remoteTimeout, retrier, retryScheduler); @@ -143,7 +147,7 @@ public class GrpcRemoteCache implements RemoteActionCache { TreeNodeRepository repository, Path execRoot, TreeNode root, Command command) throws IOException, InterruptedException { repository.computeMerkleDigests(root); - Digest commandDigest = Digests.computeDigest(command); + Digest commandDigest = digestUtil.compute(command); // TODO(olaola): avoid querying all the digests, only ask for novel subtrees. ImmutableSet missingDigests = getMissingDigests( @@ -158,17 +162,17 @@ public class GrpcRemoteCache implements RemoteActionCache { repository.getDataFromDigests(missingTreeDigests, missingActionInputs, missingTreeNodes); if (missingDigests.contains(commandDigest)) { - toUpload.add(new Chunker(command.toByteArray())); + toUpload.add(new Chunker(command.toByteArray(), digestUtil)); } if (!missingTreeNodes.isEmpty()) { for (Directory d : missingTreeNodes) { - toUpload.add(new Chunker(d.toByteArray())); + toUpload.add(new Chunker(d.toByteArray(), digestUtil)); } } if (!missingActionInputs.isEmpty()) { MetadataProvider inputFileCache = repository.getInputFileCache(); for (ActionInput actionInput : missingActionInputs) { - toUpload.add(new Chunker(actionInput, inputFileCache, execRoot)); + toUpload.add(new Chunker(actionInput, inputFileCache, execRoot, digestUtil)); } } uploader.uploadBlobs(toUpload); @@ -202,7 +206,7 @@ public class GrpcRemoteCache implements RemoteActionCache { } return null; }); - Digest receivedDigest = Digests.computeDigest(path); + Digest receivedDigest = digestUtil.compute(path); if (!receivedDigest.equals(digest)) { throw new IOException( "Digest does not match " + receivedDigest + " != " + digest); @@ -336,7 +340,7 @@ public class GrpcRemoteCache implements RemoteActionCache { throw new UnsupportedOperationException("Storing a directory is not yet supported."); } - Digest digest = Digests.computeDigest(file); + Digest digest = digestUtil.compute(file); // TODO(olaola): inline small results here. result .addOutputFilesBuilder() @@ -378,7 +382,7 @@ public class GrpcRemoteCache implements RemoteActionCache { * @return The key for fetching the file contents blob from cache. */ private Digest uploadFileContents(Path file) throws IOException, InterruptedException { - Digest digest = Digests.computeDigest(file); + Digest digest = digestUtil.compute(file); ImmutableSet missing = getMissingDigests(ImmutableList.of(digest)); if (!missing.isEmpty()) { uploader.uploadBlob(new Chunker(file)); @@ -394,19 +398,19 @@ public class GrpcRemoteCache implements RemoteActionCache { */ Digest uploadFileContents(ActionInput input, Path execRoot, MetadataProvider inputCache) throws IOException, InterruptedException { - Digest digest = Digests.getDigestFromInputCache(input, inputCache); + Digest digest = DigestUtil.getFromInputCache(input, inputCache); ImmutableSet missing = getMissingDigests(ImmutableList.of(digest)); if (!missing.isEmpty()) { - uploader.uploadBlob(new Chunker(input, inputCache, execRoot)); + uploader.uploadBlob(new Chunker(input, inputCache, execRoot, digestUtil)); } return digest; } Digest uploadBlob(byte[] blob) throws IOException, InterruptedException { - Digest digest = Digests.computeDigest(blob); + Digest digest = digestUtil.compute(blob); ImmutableSet missing = getMissingDigests(ImmutableList.of(digest)); if (!missing.isEmpty()) { - uploader.uploadBlob(new Chunker(blob)); + uploader.uploadBlob(new Chunker(blob, digestUtil)); } return digest; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java index d480a93b38..6bc53aadd6 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java @@ -16,7 +16,7 @@ package com.google.devtools.build.lib.remote; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index 46ef89faff..c66015a1ed 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java @@ -36,12 +36,17 @@ final class RemoteActionContextProvider extends ActionContextProvider { private final CommandEnvironment env; private final RemoteActionCache cache; private final GrpcRemoteExecutor executor; + private final DigestUtil digestUtil; - RemoteActionContextProvider(CommandEnvironment env, @Nullable RemoteActionCache cache, - @Nullable GrpcRemoteExecutor executor) { + RemoteActionContextProvider( + CommandEnvironment env, + @Nullable RemoteActionCache cache, + @Nullable GrpcRemoteExecutor executor, + DigestUtil digestUtil) { this.env = env; this.executor = executor; this.cache = cache; + this.digestUtil = digestUtil; } @Override @@ -61,7 +66,8 @@ final class RemoteActionContextProvider extends ActionContextProvider { buildRequestId, commandId, executionOptions.verboseFailures, - env.getReporter()); + env.getReporter(), + digestUtil); return ImmutableList.of(spawnCache); } else { RemoteSpawnRunner spawnRunner = @@ -74,7 +80,8 @@ final class RemoteActionContextProvider extends ActionContextProvider { buildRequestId, commandId, cache, - executor); + executor, + digestUtil); return ImmutableList.of(new RemoteSpawnStrategy(spawnRunner)); } } 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 702d7a6b5d..c923a4d811 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 @@ -28,6 +28,7 @@ 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.HashFunction; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsProvider; @@ -50,16 +51,17 @@ public final class RemoteModule extends BlazeModule { // TODO(ulfjack): Change the Bazel startup process to make the options available when we create // the PathConverter. RemoteOptions options; + DigestUtil digestUtil; @Override public String apply(Path path) { - if (options == null || !remoteEnabled(options)) { + if (options == null || digestUtil == null || !remoteEnabled(options)) { return null; } String server = options.remoteCache; String remoteInstanceName = options.remoteInstanceName; try { - Digest digest = Digests.computeDigest(path); + Digest digest = digestUtil.compute(path); return remoteInstanceName.isEmpty() ? String.format( "bytestream://%s/blobs/%s/%d", @@ -97,13 +99,17 @@ public final class RemoteModule extends BlazeModule { logger.info("Command: buildRequestId = " + buildRequestId + ", commandId = " + commandId); RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class); AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class); + HashFunction hashFn = env.getRuntime().getFileSystem().getDigestFunction(); + DigestUtil digestUtil = new DigestUtil(hashFn); converter.options = remoteOptions; + converter.digestUtil = digestUtil; // Quit if no remote options specified. if (remoteOptions == null) { return; } + try { boolean remoteOrLocalCache = SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions); boolean grpcCache = GrpcRemoteCache.isRemoteCacheOptions(remoteOptions); @@ -116,12 +122,13 @@ public final class RemoteModule extends BlazeModule { if (remoteOrLocalCache) { cache = new SimpleBlobStoreActionCache( - SimpleBlobStoreFactory.create(remoteOptions, env.getWorkingDirectory())); + SimpleBlobStoreFactory.create(remoteOptions, env.getWorkingDirectory()), + digestUtil); } else if (grpcCache || remoteOptions.remoteExecutor != null) { // If a remote executor but no remote cache is specified, assume both at the same target. String target = grpcCache ? remoteOptions.remoteCache : remoteOptions.remoteExecutor; Channel ch = GrpcUtils.newChannel(target, authAndTlsOptions); - cache = new GrpcRemoteCache(ch, creds, remoteOptions, retrier); + cache = new GrpcRemoteCache(ch, creds, remoteOptions, retrier, digestUtil); } else { cache = null; } @@ -137,7 +144,7 @@ public final class RemoteModule extends BlazeModule { executor = null; } - actionContextProvider = new RemoteActionContextProvider(env, cache, executor); + actionContextProvider = new RemoteActionContextProvider(env, cache, executor, digestUtil); } catch (IOException e) { env.getReporter().handle(Event.error(e.getMessage())); env.getBlazeModuleEnvironment().exit(new AbruptExitException(ExitCode.COMMAND_LINE_ERROR)); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 4a6ba9c61e..57f19fa258 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -24,7 +24,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.SpawnCache; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -64,6 +64,8 @@ final class RemoteSpawnCache implements SpawnCache { // Used to ensure that a warning is reported only once. private final AtomicBoolean warningReported = new AtomicBoolean(); + private final DigestUtil digestUtil; + RemoteSpawnCache( Path execRoot, RemoteOptions options, @@ -71,7 +73,8 @@ final class RemoteSpawnCache implements SpawnCache { String buildRequestId, String commandId, boolean verboseFailures, - @Nullable Reporter cmdlineReporter) { + @Nullable Reporter cmdlineReporter, + DigestUtil digestUtil) { this.execRoot = execRoot; this.options = options; this.platform = options.parseRemotePlatformOverride(); @@ -80,6 +83,7 @@ final class RemoteSpawnCache implements SpawnCache { this.cmdlineReporter = cmdlineReporter; this.buildRequestId = buildRequestId; this.commandId = commandId; + this.digestUtil = digestUtil; } @Override @@ -87,7 +91,7 @@ final class RemoteSpawnCache implements SpawnCache { throws InterruptedException, IOException, ExecException { // Temporary hack: the TreeNodeRepository should be created and maintained upstream! TreeNodeRepository repository = - new TreeNodeRepository(execRoot, policy.getActionInputFileCache()); + new TreeNodeRepository(execRoot, policy.getActionInputFileCache(), digestUtil); SortedMap inputMap = policy.getInputMapping(); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); @@ -95,13 +99,13 @@ final class RemoteSpawnCache implements SpawnCache { Action action = RemoteSpawnRunner.buildAction( spawn.getOutputFiles(), - Digests.computeDigest(command), + digestUtil.compute(command), repository.getMerkleDigest(inputRoot), platform, policy.getTimeout()); // Look up action cache, and reuse the action output if it is found. - final ActionKey actionKey = Digests.computeActionKey(action); + final ActionKey actionKey = digestUtil.computeActionKey(action); Context withMetadata = TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey); // Metadata will be available in context.current() until we detach. diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 7672539a6a..3a1b474000 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -32,7 +32,7 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.io.FileOutErr; @@ -79,6 +79,7 @@ class RemoteSpawnRunner implements SpawnRunner { @Nullable private final GrpcRemoteExecutor remoteExecutor; private final String buildRequestId; private final String commandId; + private final DigestUtil digestUtil; // Used to ensure that a warning is reported only once. private final AtomicBoolean warningReported = new AtomicBoolean(); @@ -92,7 +93,8 @@ class RemoteSpawnRunner implements SpawnRunner { String buildRequestId, String commandId, @Nullable RemoteActionCache remoteCache, - @Nullable GrpcRemoteExecutor remoteExecutor) { + @Nullable GrpcRemoteExecutor remoteExecutor, + DigestUtil digestUtil) { this.execRoot = execRoot; this.options = options; this.platform = options.parseRemotePlatformOverride(); @@ -103,6 +105,7 @@ class RemoteSpawnRunner implements SpawnRunner { this.cmdlineReporter = cmdlineReporter; this.buildRequestId = buildRequestId; this.commandId = commandId; + this.digestUtil = digestUtil; } @Override @@ -115,7 +118,7 @@ class RemoteSpawnRunner implements SpawnRunner { policy.report(ProgressStatus.EXECUTING, "remote"); // Temporary hack: the TreeNodeRepository should be created and maintained upstream! ActionInputFileCache inputFileCache = policy.getActionInputFileCache(); - TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache); + TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil); SortedMap inputMap = policy.getInputMapping(); TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); @@ -123,13 +126,13 @@ class RemoteSpawnRunner implements SpawnRunner { Action action = buildAction( spawn.getOutputFiles(), - Digests.computeDigest(command), + digestUtil.compute(command), repository.getMerkleDigest(inputRoot), platform, policy.getTimeout()); // Look up action cache, and reuse the action output if it is found. - ActionKey actionKey = Digests.computeActionKey(action); + ActionKey actionKey = digestUtil.computeActionKey(action); Context withMetadata = TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey); Context previous = withMetadata.attach(); diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java index af7920ed67..e5d32e4d86 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java @@ -20,7 +20,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.remote.blobstore.SimpleBlobStore; import com.google.devtools.build.lib.util.io.FileOutErr; @@ -55,9 +55,11 @@ public final class SimpleBlobStoreActionCache implements RemoteActionCache { private static final int MAX_BLOB_SIZE_FOR_INLINE = 10 * 1024; private final SimpleBlobStore blobStore; + private final DigestUtil digestUtil; - public SimpleBlobStoreActionCache(SimpleBlobStore blobStore) { + public SimpleBlobStoreActionCache(SimpleBlobStore blobStore, DigestUtil digestUtil) { this.blobStore = blobStore; + this.digestUtil = digestUtil; } @Override @@ -88,7 +90,7 @@ public final class SimpleBlobStoreActionCache implements RemoteActionCache { } private Digest uploadFileContents(Path file) throws IOException, InterruptedException { - Digest digest = Digests.computeDigest(file); + Digest digest = digestUtil.compute(file); try (InputStream in = file.getInputStream()) { return uploadStream(digest, in); } @@ -99,10 +101,10 @@ public final class SimpleBlobStoreActionCache implements RemoteActionCache { throws IOException, InterruptedException { if (input instanceof VirtualActionInput) { byte[] blob = ((VirtualActionInput) input).getBytes().toByteArray(); - return uploadBlob(blob, Digests.computeDigest(blob)); + return uploadBlob(blob, digestUtil.compute(blob)); } try (InputStream in = execRoot.getRelative(input.getExecPathString()).getInputStream()) { - return uploadStream(Digests.getDigestFromInputCache(input, inputCache), in); + return uploadStream(DigestUtil.getFromInputCache(input, inputCache), in); } } @@ -243,7 +245,7 @@ public final class SimpleBlobStoreActionCache implements RemoteActionCache { } public Digest uploadBlob(byte[] blob) throws IOException, InterruptedException { - return uploadBlob(blob, Digests.computeDigest(blob)); + return uploadBlob(blob, digestUtil.compute(blob)); } private Digest uploadBlob(byte[] blob, Digest digest) throws IOException, InterruptedException { diff --git a/src/main/java/com/google/devtools/build/lib/remote/TracingMetadataUtils.java b/src/main/java/com/google/devtools/build/lib/remote/TracingMetadataUtils.java index 18da62205c..a7958617af 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/TracingMetadataUtils.java +++ b/src/main/java/com/google/devtools/build/lib/remote/TracingMetadataUtils.java @@ -15,7 +15,7 @@ package com.google.devtools.build.lib.remote; import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.remoteexecution.v1test.RequestMetadata; import com.google.devtools.remoteexecution.v1test.ToolDetails; import io.grpc.ClientInterceptor; diff --git a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java index 4eac48ef89..0554682723 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java +++ b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java @@ -189,10 +189,13 @@ public final class TreeNodeRepository extends TreeTraverser directoryCache = new HashMap<>(); private final Map virtualInputDigestCache = new HashMap<>(); private final Map digestVirtualInputCache = new HashMap<>(); + private final DigestUtil digestUtil; - public TreeNodeRepository(Path execRoot, ActionInputFileCache inputFileCache) { + public TreeNodeRepository( + Path execRoot, ActionInputFileCache inputFileCache, DigestUtil digestUtil) { this.execRoot = execRoot; this.inputFileCache = inputFileCache; + this.digestUtil = digestUtil; } public ActionInputFileCache getInputFileCache() { @@ -302,7 +305,7 @@ public final class TreeNodeRepository extends TreeTraverserFor legacy reasons we can't make the {@link FileSystem} class use {@code SHA256} by default. + */ +public class BazelFileSystemModule extends BlazeModule { + + @Override + public FileSystem getFileSystem(OptionsProvider startupOptions) throws AbruptExitException { + final HashFunction hashFunction; + String value = null; + try { + value = System.getProperty("bazel.DigestFunction", "MD5"); + hashFunction = new HashFunction.Converter().convert(value); + } catch (OptionsParsingException e) { + throw new AbruptExitException( + "The specified hash function '" + value + "' is not supported.", + ExitCode.COMMAND_LINE_ERROR, + e); + } + if ("0".equals(System.getProperty("io.bazel.EnableJni"))) { + // Ignore UnixFileSystem, to be used for bootstrapping. + return OS.getCurrent() == OS.WINDOWS + ? new WindowsFileSystem(hashFunction) + : new JavaIoFileSystem(hashFunction); + } + // The JNI-based UnixFileSystem is faster, but on Windows it is not available. + return OS.getCurrent() == OS.WINDOWS + ? new WindowsFileSystem(hashFunction) + : new UnixFileSystem(hashFunction); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java index 65dccd9847..4eb1908598 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java @@ -80,7 +80,7 @@ public abstract class BlazeModule { * * @param startupOptions the server's startup options */ - public FileSystem getFileSystem(OptionsProvider startupOptions) { + public FileSystem getFileSystem(OptionsProvider startupOptions) throws AbruptExitException { return null; } 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 72476805bc..1a13b461fc 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 @@ -36,9 +36,14 @@ import java.util.List; */ @ThreadSafe public class UnixFileSystem extends AbstractFileSystemWithCustomStat { + public UnixFileSystem() { } + public UnixFileSystem(HashFunction hashFunction) { + super(hashFunction); + } + /** * Eager implementation of FileStatus for file systems that have an atomic * stat(2) syscall. A proxy for {@link com.google.devtools.build.lib.unix.FileStatus}. diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java index a03ec02fc5..ef1946bb8e 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java @@ -30,6 +30,12 @@ abstract class AbstractFileSystem extends FileSystem { protected static final String ERR_PERMISSION_DENIED = " (Permission denied)"; protected static final Profiler profiler = Profiler.instance(); + public AbstractFileSystem() {} + + public AbstractFileSystem(HashFunction digestFunction) { + super(digestFunction); + } + @Override protected InputStream getInputStream(Path path) throws IOException { // This loop is a workaround for an apparent bug in FileInputStream.open, which delegates diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java index db4fca94d5..875df98bab 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java @@ -21,6 +21,13 @@ import java.io.IOException; * {@link FileSystem} does). */ public abstract class AbstractFileSystemWithCustomStat extends AbstractFileSystem { + + public AbstractFileSystemWithCustomStat() {} + + public AbstractFileSystemWithCustomStat(HashFunction hashFunction) { + super(hashFunction); + } + @Override protected boolean isFile(Path path, boolean followSymlinks) { FileStatus stat = statNullable(path, followSymlinks); 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 fe707ea6c3..6386b738b1 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 @@ -16,7 +16,7 @@ package com.google.devtools.build.lib.vfs; import static java.nio.charset.StandardCharsets.ISO_8859_1; -import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.hash.Hashing; import com.google.common.io.ByteSource; @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.vfs.Dirent.Type; import com.google.devtools.build.lib.vfs.Path.PathFactory; import com.google.devtools.common.options.EnumConverter; -import com.google.devtools.common.options.OptionsParsingException; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -71,23 +70,17 @@ public abstract class FileSystem { } } - // This is effectively final, should be changed only in unit-tests! - private static HashFunction digestFunction; - static { - try { - digestFunction = new HashFunction.Converter().convert( - System.getProperty("bazel.DigestFunction", "MD5")); - } catch (OptionsParsingException e) { - throw new IllegalStateException(e); - } + private final HashFunction digestFunction; + + public FileSystem() { + this(HashFunction.MD5); } - @VisibleForTesting - public static void setDigestFunctionForTesting(HashFunction value) { - digestFunction = value; + public FileSystem(HashFunction digestFunction) { + this.digestFunction = Preconditions.checkNotNull(digestFunction); } - public static HashFunction getDigestFunction() { + public HashFunction getDigestFunction() { return 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 73074b2b90..e5ca35d3f4 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 @@ -56,6 +56,11 @@ public class JavaIoFileSystem extends AbstractFileSystemWithCustomStat { this(new JavaClock()); } + public JavaIoFileSystem(HashFunction hashFunction) { + super(hashFunction); + this.clock = new JavaClock(); + } + @VisibleForTesting JavaIoFileSystem(Clock clock) { this.clock = clock; diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java index d7b0960429..7e4d6601bc 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java @@ -70,7 +70,18 @@ public class InMemoryFileSystem extends FileSystem { * paths are considered to be within scope). */ public InMemoryFileSystem(Clock clock) { - this(clock, null); + this(clock, (PathFragment) null); + } + + /** + * Creates a new InMemoryFileSystem with scope checking disabled (all paths are considered to be + * within scope). + */ + public InMemoryFileSystem(Clock clock, HashFunction hashFunction) { + super(hashFunction); + this.clock = clock; + this.rootInode = newRootInode(clock); + this.scopeRoot = null; } /** @@ -80,9 +91,14 @@ public class InMemoryFileSystem extends FileSystem { public InMemoryFileSystem(Clock clock, PathFragment scopeRoot) { this.scopeRoot = scopeRoot; this.clock = clock; - this.rootInode = new InMemoryDirectoryInfo(clock); + this.rootInode = newRootInode(clock); + } + + private static InMemoryDirectoryInfo newRootInode(Clock clock) { + InMemoryDirectoryInfo rootInode = new InMemoryDirectoryInfo(clock); rootInode.addChild(".", rootInode); rootInode.addChild("..", rootInode); + return rootInode; } /** diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java index dfbb8ca651..21de058e07 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java @@ -308,6 +308,12 @@ public class WindowsFileSystem extends JavaIoFileSystem { public static final LinkOption[] NO_OPTIONS = new LinkOption[0]; public static final LinkOption[] NO_FOLLOW = new LinkOption[] {LinkOption.NOFOLLOW_LINKS}; + public WindowsFileSystem() {} + + public WindowsFileSystem(HashFunction hashFunction) { + super(hashFunction); + } + @Override protected PathFactory getPathFactory() { return WindowsPathFactory.INSTANCE; diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 90cb70680a..4cb175a461 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1067,6 +1067,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:io", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/authandtls", + "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", diff --git a/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java b/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java index fddab806b9..259e0f2088 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java @@ -55,28 +55,28 @@ public class DigestUtilsTest { final CountDownLatch barrierLatch = new CountDownLatch(2); // Used to block test threads. final CountDownLatch readyLatch = new CountDownLatch(1); // Used to block main thread. - FileSystem myfs = new InMemoryFileSystem(BlazeClock.instance()) { - @Override - protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException { - try { - barrierLatch.countDown(); - readyLatch.countDown(); - // Either both threads will be inside getDigest at the same time or they - // both will be blocked. - barrierLatch.await(); - } catch (Exception e) { - throw new IOException(e); + FileSystem myfs = + new InMemoryFileSystem(BlazeClock.instance(), hf) { + @Override + protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException { + try { + barrierLatch.countDown(); + readyLatch.countDown(); + // Either both threads will be inside getDigest at the same time or they + // both will be blocked. + barrierLatch.await(); + } catch (Exception e) { + throw new IOException(e); + } + return super.getDigest(path, hashFunction); } - return super.getDigest(path, hashFunction); - } - @Override - protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { - return fastDigest ? super.getDigest(path, hashFunction) : null; - } - }; + @Override + protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { + return fastDigest ? super.getDigest(path, hashFunction) : null; + } + }; - FileSystem.setDigestFunctionForTesting(hf); final Path myFile1 = myfs.getPath("/f1.dat"); final Path myFile2 = myfs.getPath("/f2.dat"); FileSystemUtils.writeContentAsLatin1(myFile1, Strings.repeat("a", fileSize1)); @@ -126,18 +126,19 @@ public class DigestUtilsTest { } public void assertRecoverFromMalformedDigest(HashFunction... hashFunctions) throws Exception { - final byte[] malformed = {0, 0, 0}; - FileSystem myFS = new InMemoryFileSystem(BlazeClock.instance()) { - @Override - protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { - // Digest functions have more than 3 bytes, usually at least 16. - return malformed; - } - }; - Path path = myFS.getPath("/file"); - FileSystemUtils.writeContentAsLatin1(path, "a"); for (HashFunction hf : hashFunctions) { - FileSystem.setDigestFunctionForTesting(hf); + final byte[] malformed = {0, 0, 0}; + FileSystem myFS = + new InMemoryFileSystem(BlazeClock.instance(), hf) { + @Override + protected byte[] getFastDigest(Path path, HashFunction hashFunction) + throws IOException { + // Digest functions have more than 3 bytes, usually at least 16. + return malformed; + } + }; + Path path = myFS.getPath("/file"); + FileSystemUtils.writeContentAsLatin1(path, "a"); byte[] result = DigestUtils.getDigestOrFail(path, 1); assertThat(result).isEqualTo(path.getDigest()); assertThat(result).isNotSameAs(malformed); diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java index e809b74b7e..0b70f36623 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java @@ -26,6 +26,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.RequestMetadata; import com.google.protobuf.ByteString; @@ -75,6 +76,8 @@ import org.mockito.MockitoAnnotations; @RunWith(JUnit4.class) public class ByteStreamUploaderTest { + private static final DigestUtil DIGEST_UTIL = new DigestUtil(HashFunction.SHA256); + private static final int CHUNK_SIZE = 10; private static final String INSTANCE_NAME = "foo"; @@ -99,7 +102,7 @@ public class ByteStreamUploaderTest { channel = InProcessChannelBuilder.forName(serverName).build(); withEmptyMetadata = TracingMetadataUtils.contextWithMetadata( - "none", "none", Digests.unsafeActionKeyFromDigest(Digest.getDefaultInstance())); + "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance())); // Needs to be repeated in every test that uses the timeout setting, since the tests run // on different threads than the setUp. withEmptyMetadata.attach(); @@ -121,7 +124,7 @@ public class ByteStreamUploaderTest { byte[] blob = new byte[CHUNK_SIZE * 2 + 1]; new Random().nextBytes(blob); - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); serviceRegistry.addService(new ByteStreamImplBase() { @Override @@ -195,7 +198,7 @@ public class ByteStreamUploaderTest { int blobSize = rand.nextInt(CHUNK_SIZE * 10) + CHUNK_SIZE; byte[] blob = new byte[blobSize]; rand.nextBytes(blob); - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); builders.add(chunker); blobsByHash.put(chunker.digest().getHash(), blob); } @@ -281,7 +284,7 @@ public class ByteStreamUploaderTest { List builders = new ArrayList<>(toUpload.size()); Map uploadsFailed = new HashMap<>(); for (String s : toUpload) { - Chunker chunker = new Chunker(s.getBytes(UTF_8), /* chunkSize=*/ 3); + Chunker chunker = new Chunker(s.getBytes(UTF_8), /* chunkSize=*/ 3, DIGEST_UTIL); builders.add(chunker); uploadsFailed.put(chunker.digest().getHash(), 0); } @@ -342,7 +345,7 @@ public class ByteStreamUploaderTest { for (Chunker chunker : builders) { Context ctx = TracingMetadataUtils.contextWithMetadata( - "build-req-id", "command-id", Digests.unsafeActionKeyFromDigest(chunker.digest())); + "build-req-id", "command-id", DIGEST_UTIL.asActionKey(chunker.digest())); ctx.call( () -> { uploads.add(uploader.uploadBlobAsync(chunker)); @@ -367,7 +370,7 @@ public class ByteStreamUploaderTest { new ByteStreamUploader(INSTANCE_NAME, channel, null, 3, retrier, retryService); byte[] blob = new byte[CHUNK_SIZE * 10]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); AtomicInteger numWriteCalls = new AtomicInteger(); CountDownLatch blocker = new CountDownLatch(1); @@ -426,7 +429,7 @@ public class ByteStreamUploaderTest { new ByteStreamUploader(INSTANCE_NAME, channel, null, 3, retrier, retryService); byte[] blob = new byte[CHUNK_SIZE]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); serviceRegistry.addService(new ByteStreamImplBase() { @Override @@ -477,10 +480,10 @@ public class ByteStreamUploaderTest { serviceRegistry.addService(service); byte[] blob1 = new byte[CHUNK_SIZE]; - Chunker chunker1 = new Chunker(blob1, CHUNK_SIZE); + Chunker chunker1 = new Chunker(blob1, CHUNK_SIZE, DIGEST_UTIL); byte[] blob2 = new byte[CHUNK_SIZE + 1]; - Chunker chunker2 = new Chunker(blob2, CHUNK_SIZE); + Chunker chunker2 = new Chunker(blob2, CHUNK_SIZE, DIGEST_UTIL); ListenableFuture f1 = uploader.uploadBlobAsync(chunker1); ListenableFuture f2 = uploader.uploadBlobAsync(chunker2); @@ -519,7 +522,7 @@ public class ByteStreamUploaderTest { assertThat(retryService.isShutdown()).isTrue(); byte[] blob = new byte[1]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); try { uploader.uploadBlob(chunker); fail("Should have thrown an exception."); @@ -560,7 +563,7 @@ public class ByteStreamUploaderTest { }); byte[] blob = new byte[1]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); uploader.uploadBlob(chunker); } @@ -585,7 +588,7 @@ public class ByteStreamUploaderTest { }); byte[] blob = new byte[1]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); try { uploader.uploadBlob(chunker); diff --git a/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java b/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java index 5ba4ef0435..493041b5dd 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.devtools.build.lib.remote.RemoteModule.CasPathConverter; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; @@ -34,12 +35,20 @@ public class CasPathConverterTest { @Test public void noOptionsShouldntCrash() { + converter.digestUtil = new DigestUtil(HashFunction.SHA256); + assertThat(converter.apply(fs.getPath("/foo"))).isNull(); + } + + @Test + public void noDigestUtilShouldntCrash() { + converter.options = Options.getDefaults(RemoteOptions.class); assertThat(converter.apply(fs.getPath("/foo"))).isNull(); } @Test public void disabledRemote() { converter.options = Options.getDefaults(RemoteOptions.class); + converter.digestUtil = new DigestUtil(HashFunction.SHA256); assertThat(converter.apply(fs.getPath("/foo"))).isNull(); } @@ -48,6 +57,7 @@ public class CasPathConverterTest { OptionsParser parser = OptionsParser.newOptionsParser(RemoteOptions.class); parser.parse("--remote_cache=machine"); converter.options = parser.getOptions(RemoteOptions.class); + converter.digestUtil = new DigestUtil(HashFunction.SHA256); Path path = fs.getPath("/foo"); FileSystemUtils.writeContentAsLatin1(path, "foobar"); assertThat(converter.apply(fs.getPath("/foo"))) @@ -59,6 +69,7 @@ public class CasPathConverterTest { OptionsParser parser = OptionsParser.newOptionsParser(RemoteOptions.class); parser.parse("--remote_cache=machine", "--remote_instance_name=projects/bazel"); converter.options = parser.getOptions(RemoteOptions.class); + converter.digestUtil = new DigestUtil(HashFunction.SHA256); Path path = fs.getPath("/foo"); FileSystemUtils.writeContentAsLatin1(path, "foobar"); assertThat(converter.apply(fs.getPath("/foo"))) diff --git a/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java b/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java index fd0cbade33..ed1ccf6ed2 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static junit.framework.TestCase.fail; import com.google.devtools.build.lib.remote.Chunker.Chunk; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.protobuf.ByteString; import java.io.ByteArrayInputStream; @@ -36,14 +37,16 @@ import org.mockito.Mockito; @RunWith(JUnit4.class) public class ChunkerTest { + private final DigestUtil digestUtil = new DigestUtil(HashFunction.SHA256); + @Test public void chunkingShouldWork() throws IOException { Random rand = new Random(); byte[] expectedData = new byte[21]; rand.nextBytes(expectedData); - Digest expectedDigest = Digests.computeDigest(expectedData); + Digest expectedDigest = digestUtil.compute(expectedData); - Chunker chunker = new Chunker(expectedData, 10); + Chunker chunker = new Chunker(expectedData, 10, digestUtil); ByteArrayOutputStream actualData = new ByteArrayOutputStream(); @@ -76,7 +79,7 @@ public class ChunkerTest { @Test public void nextShouldThrowIfNoMoreData() throws IOException { byte[] data = new byte[10]; - Chunker chunker = new Chunker(data, 10); + Chunker chunker = new Chunker(data, 10, digestUtil); assertThat(chunker.hasNext()).isTrue(); assertThat(chunker.next()).isNotNull(); @@ -94,7 +97,7 @@ public class ChunkerTest { @Test public void emptyData() throws Exception { byte[] data = new byte[0]; - Chunker chunker = new Chunker(data); + Chunker chunker = new Chunker(data, digestUtil); assertThat(chunker.hasNext()).isTrue(); @@ -117,7 +120,7 @@ public class ChunkerTest { @Test public void reset() throws Exception { byte[] data = new byte[]{1, 2, 3}; - Chunker chunker = new Chunker(data, 1); + Chunker chunker = new Chunker(data, 1, digestUtil); assertNextEquals(chunker, (byte) 1); assertNextEquals(chunker, (byte) 2); @@ -144,9 +147,9 @@ public class ChunkerTest { in.set(Mockito.spy(new ByteArrayInputStream(data))); return in.get(); }; - Digest digest = Digests.computeDigest(data); + Digest digest = digestUtil.compute(data); - Chunker chunker = new Chunker(supplier, digest, 1); + Chunker chunker = new Chunker(supplier, digest, 1, digestUtil); assertThat(in.get()).isNull(); assertNextEquals(chunker, (byte) 1); Mockito.verify(in.get(), Mockito.never()).close(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java index 565e5e927d..3f509684f2 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java +++ b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java @@ -32,9 +32,11 @@ import javax.annotation.Nullable; final class FakeActionInputFileCache implements ActionInputFileCache { private final Path execRoot; private final BiMap cas = HashBiMap.create(); + private final DigestUtil digestUtil; FakeActionInputFileCache(Path execRoot) { this.execRoot = execRoot; + this.digestUtil = new DigestUtil(execRoot.getFileSystem().getDigestFunction()); } @Override @@ -70,7 +72,7 @@ final class FakeActionInputFileCache implements ActionInputFileCache { Path inputFile = execRoot.getRelative(input.getExecPath()); FileSystemUtils.createDirectoryAndParents(inputFile.getParentDirectory()); FileSystemUtils.writeContentAsLatin1(inputFile, content); - Digest digest = Digests.computeDigest(inputFile); + Digest digest = digestUtil.compute(inputFile); setDigest(input, digest.getHash()); return digest; } diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java index 5cc03be824..5664ea37cc 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java @@ -29,7 +29,8 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.authandtls.GrpcUtils; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.clock.JavaClock; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; @@ -75,6 +76,9 @@ import org.mockito.stubbing.Answer; /** Tests for {@link GrpcRemoteCache}. */ @RunWith(JUnit4.class) public class GrpcRemoteCacheTest { + + private static final DigestUtil DIGEST_UTIL = new DigestUtil(HashFunction.SHA256); + private FileSystem fs; private Path execRoot; private FileOutErr outErr; @@ -85,7 +89,6 @@ public class GrpcRemoteCacheTest { @Before public final void setUp() throws Exception { - FileSystem.setDigestFunctionForTesting(HashFunction.SHA1); // Use a mutable service registry for later registering the service impl for each test case. fakeServer = InProcessServerBuilder.forName(fakeServerName) @@ -94,7 +97,7 @@ public class GrpcRemoteCacheTest { .build() .start(); Chunker.setDefaultChunkSizeForTesting(1000); // Enough for everything to be one chunk. - fs = new InMemoryFileSystem(); + fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -106,7 +109,7 @@ public class GrpcRemoteCacheTest { outErr = new FileOutErr(stdout, stderr); Context withEmptyMetadata = TracingMetadataUtils.contextWithMetadata( - "none", "none", Digests.unsafeActionKeyFromDigest(Digest.getDefaultInstance())); + "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance())); withEmptyMetadata.attach(); } @@ -156,13 +159,14 @@ public class GrpcRemoteCacheTest { ImmutableList.of(new CallCredentialsInterceptor(creds))), creds, remoteOptions, - retrier); + retrier, + DIGEST_UTIL); } @Test public void testDownloadEmptyBlob() throws Exception { GrpcRemoteCache client = newClient(); - Digest emptyDigest = Digests.computeDigest(new byte[0]); + Digest emptyDigest = DIGEST_UTIL.compute(new byte[0]); // Will not call the mock Bytestream interface at all. assertThat(client.downloadBlob(emptyDigest)).isEmpty(); } @@ -170,7 +174,7 @@ public class GrpcRemoteCacheTest { @Test public void testDownloadBlobSingleChunk() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ByteStreamImplBase() { @Override @@ -187,7 +191,7 @@ public class GrpcRemoteCacheTest { @Test public void testDownloadBlobMultipleChunks() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ByteStreamImplBase() { @Override @@ -208,9 +212,9 @@ public class GrpcRemoteCacheTest { @Test public void testDownloadAllResults() throws Exception { GrpcRemoteCache client = newClient(); - Digest fooDigest = Digests.computeDigestUtf8("foo-contents"); - Digest barDigest = Digests.computeDigestUtf8("bar-contents"); - Digest emptyDigest = Digests.computeDigest(new byte[0]); + Digest fooDigest = DIGEST_UTIL.computeAsUtf8("foo-contents"); + Digest barDigest = DIGEST_UTIL.computeAsUtf8("bar-contents"); + Digest emptyDigest = DIGEST_UTIL.compute(new byte[0]); serviceRegistry.addService( new FakeImmutableCacheByteStreamImpl(fooDigest, "foo-contents", barDigest, "bar-contents")); @@ -219,16 +223,16 @@ public class GrpcRemoteCacheTest { result.addOutputFilesBuilder().setPath("b/empty").setDigest(emptyDigest); result.addOutputFilesBuilder().setPath("a/bar").setDigest(barDigest).setIsExecutable(true); client.download(result.build(), execRoot, null); - assertThat(Digests.computeDigest(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); - assertThat(Digests.computeDigest(execRoot.getRelative("b/empty"))).isEqualTo(emptyDigest); - assertThat(Digests.computeDigest(execRoot.getRelative("a/bar"))).isEqualTo(barDigest); + assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); + assertThat(DIGEST_UTIL.compute(execRoot.getRelative("b/empty"))).isEqualTo(emptyDigest); + assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/bar"))).isEqualTo(barDigest); assertThat(execRoot.getRelative("a/bar").isExecutable()).isTrue(); } @Test public void testUploadBlobCacheHitWithRetries() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ContentAddressableStorageImplBase() { private int numErrors = 4; @@ -251,7 +255,7 @@ public class GrpcRemoteCacheTest { @Test public void testUploadBlobSingleChunk() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ContentAddressableStorageImplBase() { @Override @@ -302,7 +306,7 @@ public class GrpcRemoteCacheTest { this.responseObserver = responseObserver; this.contents = contents; try { - chunker = new Chunker(contents.getBytes(UTF_8), chunkSizeBytes); + chunker = new Chunker(contents.getBytes(UTF_8), chunkSizeBytes, DIGEST_UTIL); } catch (IOException e) { fail("An error occurred:" + e); } @@ -357,7 +361,7 @@ public class GrpcRemoteCacheTest { @Test public void testUploadBlobMultipleChunks() throws Exception { - final Digest digest = Digests.computeDigestUtf8("abcdef"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdef"); serviceRegistry.addService( new ContentAddressableStorageImplBase() { @Override @@ -446,7 +450,7 @@ public class GrpcRemoteCacheTest { } }); - ActionKey emptyKey = Digests.computeActionKey(Action.getDefaultInstance()); + ActionKey emptyKey = DIGEST_UTIL.computeActionKey(Action.getDefaultInstance()); Path fooFile = execRoot.getRelative("a/foo"); Path barFile = execRoot.getRelative("bar"); client.upload(emptyKey, execRoot, ImmutableList.of(fooFile, barFile), outErr, false); @@ -464,7 +468,7 @@ public class GrpcRemoteCacheTest { final Path fooFile = execRoot.getRelative("a/foo"); final Path barFile = execRoot.getRelative("bar"); final Path bazFile = execRoot.getRelative("baz"); - ActionKey actionKey = Digests.unsafeActionKeyFromDigest(fooDigest); // Could be any key. + ActionKey actionKey = DIGEST_UTIL.asActionKey(fooDigest); // Could be any key. barFile.setExecutable(true); serviceRegistry.addService( new ContentAddressableStorageImplBase() { @@ -576,7 +580,7 @@ public class GrpcRemoteCacheTest { @Test public void testGetCachedActionResultWithRetries() throws Exception { final GrpcRemoteCache client = newClient(); - ActionKey actionKey = Digests.unsafeActionKeyFromDigest(Digests.computeDigestUtf8("key")); + ActionKey actionKey = DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")); serviceRegistry.addService( new ActionCacheImplBase() { private int numErrors = 4; diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java index 35bd1950e3..2e06805539 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.authandtls.GrpcUtils; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; @@ -102,6 +103,9 @@ import org.mockito.stubbing.Answer; /** Tests for {@link RemoteSpawnRunner} in combination with {@link GrpcRemoteExecutor}. */ @RunWith(JUnit4.class) public class GrpcRemoteExecutionClientTest { + + private static final DigestUtil DIGEST_UTIL = new DigestUtil(HashFunction.SHA256); + private static final ArtifactExpander SIMPLE_ARTIFACT_EXPANDER = new ArtifactExpander() { @Override @@ -176,7 +180,6 @@ public class GrpcRemoteExecutionClientTest { @Before public final void setUp() throws Exception { - FileSystem.setDigestFunctionForTesting(HashFunction.SHA1); String fakeServerName = "fake server for " + getClass(); // Use a mutable service registry for later registering the service impl for each test case. fakeServer = @@ -187,7 +190,7 @@ public class GrpcRemoteExecutionClientTest { .start(); Chunker.setDefaultChunkSizeForTesting(1000); // Enough for everything to be one chunk. - fs = new InMemoryFileSystem(); + fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -236,7 +239,7 @@ public class GrpcRemoteExecutionClientTest { CallCredentials creds = GrpcUtils.newCallCredentials(Options.getDefaults(AuthAndTLSOptions.class)); GrpcRemoteCache remoteCache = - new GrpcRemoteCache(channel, creds, options, retrier); + new GrpcRemoteCache(channel, creds, options, retrier, DIGEST_UTIL); client = new RemoteSpawnRunner( execRoot, @@ -247,7 +250,8 @@ public class GrpcRemoteExecutionClientTest { "build-req-id", "command-id", remoteCache, - executor); + executor, + DIGEST_UTIL); inputDigest = fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz"); } @@ -277,8 +281,8 @@ public class GrpcRemoteExecutionClientTest { @Test public void cacheHitWithOutput() throws Exception { - final Digest stdOutDigest = Digests.computeDigestUtf8("stdout"); - final Digest stdErrDigest = Digests.computeDigestUtf8("stderr"); + final Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("stdout"); + final Digest stdErrDigest = DIGEST_UTIL.computeAsUtf8("stderr"); serviceRegistry.addService( new ActionCacheImplBase() { @Override @@ -326,7 +330,7 @@ public class GrpcRemoteExecutionClientTest { } private Answer> blobWriteAnswer(final byte[] data) { - final Digest digest = Digests.computeDigest(data); + final Digest digest = DIGEST_UTIL.compute(data); return new Answer>() { @Override public StreamObserver answer(InvocationOnMock invocation) { @@ -444,7 +448,7 @@ public class GrpcRemoteExecutionClientTest { .setValue("value") .build()) .build(); - final Digest cmdDigest = Digests.computeDigest(command); + final Digest cmdDigest = DIGEST_UTIL.compute(command); BindableService cas = new ContentAddressableStorageImplBase() { @Override @@ -633,7 +637,7 @@ public class GrpcRemoteExecutionClientTest { .setValue("value") .build()) .build(); - final Digest cmdDigest = Digests.computeDigest(command); + final Digest cmdDigest = DIGEST_UTIL.compute(command); serviceRegistry.addService( new ContentAddressableStorageImplBase() { private int numErrors = 4; @@ -738,7 +742,7 @@ public class GrpcRemoteExecutionClientTest { responseObserver.onError(Status.NOT_FOUND.asRuntimeException()); } }); - Digest stdOutDigest = Digests.computeDigestUtf8("bla"); + Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("bla"); final ActionResult actionResult = ActionResult.newBuilder().setStdoutDigest(stdOutDigest).build(); serviceRegistry.addService( @@ -788,7 +792,7 @@ public class GrpcRemoteExecutionClientTest { @Test public void remotelyReExecuteOrphanedCachedActions() throws Exception { - final Digest stdOutDigest = Digests.computeDigestUtf8("stdout"); + final Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("stdout"); final ActionResult actionResult = ActionResult.newBuilder().setStdoutDigest(stdOutDigest).build(); serviceRegistry.addService( @@ -858,7 +862,9 @@ public class GrpcRemoteExecutionClientTest { fail("Expected an exception"); } catch (ExecException expected) { assertThat(expected).hasMessageThat().contains("Missing digest"); - assertThat(expected).hasMessageThat().contains("476d9ec701e2de6a6c37ab5211117a7cb8333a27"); + assertThat(expected) + .hasMessageThat() + .contains(DIGEST_UTIL.computeAsUtf8("stdout").toString()); } } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 3295df658c..7ac5cfa190 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; @@ -42,10 +43,11 @@ import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; import com.google.devtools.build.lib.exec.util.FakeOwner; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -80,6 +82,7 @@ public class RemoteSpawnCacheTest { }; private FileSystem fs; + private DigestUtil digestUtil; private Path execRoot; private SimpleSpawn simpleSpawn; private FakeActionInputFileCache fakeFileCache; @@ -147,7 +150,8 @@ public class RemoteSpawnCacheTest { @Before public final void setUp() throws Exception { MockitoAnnotations.initMocks(this); - fs = new InMemoryFileSystem(); + fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); + digestUtil = new DigestUtil(HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -172,7 +176,14 @@ public class RemoteSpawnCacheTest { reporter.addHandler(eventHandler); cache = new RemoteSpawnCache( - execRoot, options, remoteCache, "build-req-id", "command-id", false, reporter); + execRoot, + options, + remoteCache, + "build-req-id", + "command-id", + false, + reporter, + digestUtil); fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz"); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index 5fec618cc8..31681906d8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; @@ -48,10 +49,11 @@ import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; import com.google.devtools.build.lib.exec.util.FakeOwner; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -81,6 +83,7 @@ public class RemoteSpawnRunnerTest { ImmutableMap.of(ExecutionRequirements.NO_CACHE, ""); private Path execRoot; + private DigestUtil digestUtil; private FakeActionInputFileCache fakeFileCache; private FileOutErr outErr; @@ -96,8 +99,8 @@ public class RemoteSpawnRunnerTest { @Before public final void setUp() throws Exception { MockitoAnnotations.initMocks(this); - - FileSystem fs = new InMemoryFileSystem(); + digestUtil = new DigestUtil(HashFunction.SHA256); + FileSystem fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -130,7 +133,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ExecuteResponse succeeded = ExecuteResponse.newBuilder().setResult( ActionResult.newBuilder().setExitCode(0).build()).build(); @@ -186,7 +190,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); // Throw an IOException to trigger the local fallback. when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(IOException.class); @@ -237,7 +242,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null)); + null, + digestUtil)); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -284,7 +290,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null)); + null, + digestUtil)); try { runner.exec(spawn, policy); @@ -317,7 +324,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -367,7 +375,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -405,7 +414,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -440,7 +450,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException()); @@ -477,7 +488,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(cachedResult); @@ -517,7 +529,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -552,7 +565,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -585,7 +599,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -624,7 +639,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException()); @@ -659,7 +675,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); when(cache.getCachedActionResult(any(ActionKey.class))).thenThrow(new IOException()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java index 94374f6364..9c0f43afc8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java @@ -21,13 +21,14 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; +import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.exec.SingleBuildFileCache; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.testutil.Scratch; -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.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.Directory; import java.util.ArrayList; @@ -42,13 +43,14 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class TreeNodeRepositoryTest { private Scratch scratch; + private DigestUtil digestUtil; private Root rootDir; private Path rootPath; @Before public final void setRootDir() throws Exception { - FileSystem.setDigestFunctionForTesting(HashFunction.SHA1); - scratch = new Scratch(); + digestUtil = new DigestUtil(HashFunction.SHA256); + scratch = new Scratch(new InMemoryFileSystem(BlazeClock.instance(), HashFunction.SHA256)); rootDir = Root.asDerivedRoot(scratch.dir("/exec/root")); rootPath = rootDir.getPath(); } @@ -56,7 +58,7 @@ public class TreeNodeRepositoryTest { private TreeNodeRepository createTestTreeNodeRepository() { ActionInputFileCache inputFileCache = new SingleBuildFileCache(rootPath.getPathString(), scratch.getFileSystem()); - return new TreeNodeRepository(rootPath, inputFileCache); + return new TreeNodeRepository(rootPath, inputFileCache, digestUtil); } @Test diff --git a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ActionCacheServer.java b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ActionCacheServer.java index 008e826e7d..8e9ba2fafb 100644 --- a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ActionCacheServer.java +++ b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ActionCacheServer.java @@ -16,8 +16,8 @@ package com.google.devtools.build.remote; import static java.util.logging.Level.WARNING; -import com.google.devtools.build.lib.remote.Digests; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.SimpleBlobStoreActionCache; import com.google.devtools.remoteexecution.v1test.ActionCacheGrpc.ActionCacheImplBase; import com.google.devtools.remoteexecution.v1test.ActionResult; @@ -31,16 +31,18 @@ final class ActionCacheServer extends ActionCacheImplBase { private static final Logger logger = Logger.getLogger(ActionCacheImplBase.class.getName()); private final SimpleBlobStoreActionCache cache; + private final DigestUtil digestUtil; - public ActionCacheServer(SimpleBlobStoreActionCache cache) { + public ActionCacheServer(SimpleBlobStoreActionCache cache, DigestUtil digestUtil) { this.cache = cache; + this.digestUtil = digestUtil; } @Override public void getActionResult( GetActionResultRequest request, StreamObserver responseObserver) { try { - ActionKey actionKey = Digests.unsafeActionKeyFromDigest(request.getActionDigest()); + ActionKey actionKey = digestUtil.asActionKey(request.getActionDigest()); ActionResult result = cache.getCachedActionResult(actionKey); if (result == null) { @@ -60,7 +62,7 @@ final class ActionCacheServer extends ActionCacheImplBase { public void updateActionResult( UpdateActionResultRequest request, StreamObserver responseObserver) { try { - ActionKey actionKey = Digests.unsafeActionKeyFromDigest(request.getActionDigest()); + ActionKey actionKey = digestUtil.asActionKey(request.getActionDigest()); cache.setCachedActionResult(actionKey, request.getActionResult()); responseObserver.onNext(request.getActionResult()); responseObserver.onCompleted(); diff --git a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ByteStreamServer.java b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ByteStreamServer.java index 869d8cf908..778d74335a 100644 --- a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ByteStreamServer.java +++ b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ByteStreamServer.java @@ -24,7 +24,7 @@ import com.google.bytestream.ByteStreamProto.WriteRequest; import com.google.bytestream.ByteStreamProto.WriteResponse; import com.google.devtools.build.lib.remote.CacheNotFoundException; import com.google.devtools.build.lib.remote.Chunker; -import com.google.devtools.build.lib.remote.Digests; +import com.google.devtools.build.lib.remote.DigestUtil; import com.google.devtools.build.lib.remote.SimpleBlobStoreActionCache; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -44,6 +44,7 @@ final class ByteStreamServer extends ByteStreamImplBase { private static final Logger logger = Logger.getLogger(ByteStreamServer.class.getName()); private final SimpleBlobStoreActionCache cache; private final Path workPath; + private final DigestUtil digestUtil; static @Nullable Digest parseDigestFromResourceName(String resourceName) { try { @@ -53,15 +54,16 @@ final class ByteStreamServer extends ByteStreamImplBase { } String hash = tokens[tokens.length - 2]; long size = Long.parseLong(tokens[tokens.length - 1]); - return Digests.buildDigest(hash, size); + return DigestUtil.buildDigest(hash, size); } catch (NumberFormatException e) { return null; } } - public ByteStreamServer(SimpleBlobStoreActionCache cache, Path workPath) { + public ByteStreamServer(SimpleBlobStoreActionCache cache, Path workPath, DigestUtil digestUtil) { this.cache = cache; this.workPath = workPath; + this.digestUtil = digestUtil; } @Override @@ -78,7 +80,7 @@ final class ByteStreamServer extends ByteStreamImplBase { try { // This still relies on the blob size to be small enough to fit in memory. // TODO(olaola): refactor to fix this if the need arises. - Chunker c = new Chunker(cache.downloadBlob(digest)); + Chunker c = new Chunker(cache.downloadBlob(digest), digestUtil); while (c.hasNext()) { responseObserver.onNext( ReadResponse.newBuilder().setData(c.next().getData()).build()); @@ -227,7 +229,7 @@ final class ByteStreamServer extends ByteStreamImplBase { } try { - Digest d = Digests.computeDigest(temp); + Digest d = digestUtil.compute(temp); try (InputStream in = temp.getInputStream()) { cache.uploadStream(d, in); } diff --git a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ExecutionServer.java b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ExecutionServer.java index 7dd5f95de4..0b3f2780bf 100644 --- a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ExecutionServer.java +++ b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/ExecutionServer.java @@ -25,8 +25,8 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.devtools.build.lib.remote.CacheNotFoundException; -import com.google.devtools.build.lib.remote.Digests; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.SimpleBlobStoreActionCache; import com.google.devtools.build.lib.remote.TracingMetadataUtils; import com.google.devtools.build.lib.shell.AbnormalTerminationException; @@ -93,18 +93,21 @@ final class ExecutionServer extends ExecutionImplBase { private final SimpleBlobStoreActionCache cache; private final ConcurrentHashMap> operationsCache; private final ListeningExecutorService executorService; + private final DigestUtil digestUtil; public ExecutionServer( Path workPath, Path sandboxPath, RemoteWorkerOptions workerOptions, SimpleBlobStoreActionCache cache, - ConcurrentHashMap> operationsCache) { + ConcurrentHashMap> operationsCache, + DigestUtil digestUtil) { this.workPath = workPath; this.sandboxPath = sandboxPath; this.workerOptions = workerOptions; this.cache = cache; this.operationsCache = operationsCache; + this.digestUtil = digestUtil; ThreadPoolExecutor realExecutor = new ThreadPoolExecutor( // This is actually the max number of concurrent jobs. workerOptions.jobs, @@ -265,7 +268,7 @@ final class ExecutionServer extends ExecutionImplBase { cache.uploadOutErr(result, stdout, stderr); ActionResult finalResult = result.setExitCode(exitCode).build(); if (exitCode == 0) { - ActionKey actionKey = Digests.computeActionKey(action); + ActionKey actionKey = digestUtil.computeActionKey(action); cache.setCachedActionResult(actionKey, finalResult); } return finalResult; diff --git a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/RemoteWorker.java b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/RemoteWorker.java index 2cff8f860e..097029440d 100644 --- a/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/RemoteWorker.java +++ b/src/tools/remote_worker/src/main/java/com/google/devtools/build/remote/RemoteWorker.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.io.ByteStreams; import com.google.common.util.concurrent.ListenableFuture; +import com.google.devtools.build.lib.remote.DigestUtil; import com.google.devtools.build.lib.remote.RemoteOptions; import com.google.devtools.build.lib.remote.SimpleBlobStoreActionCache; import com.google.devtools.build.lib.remote.SimpleBlobStoreFactory; @@ -39,11 +40,13 @@ import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.ProcessUtils; import com.google.devtools.build.lib.util.SingleLineFormatter; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.JavaIoFileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.OptionsParser; +import com.google.devtools.common.options.OptionsParsingException; import com.google.devtools.remoteexecution.v1test.ActionCacheGrpc.ActionCacheImplBase; import com.google.devtools.remoteexecution.v1test.ActionResult; import com.google.devtools.remoteexecution.v1test.ContentAddressableStorageGrpc.ContentAddressableStorageImplBase; @@ -85,15 +88,28 @@ public final class RemoteWorker { private final ExecutionImplBase execServer; static FileSystem getFileSystem() { - return OS.getCurrent() == OS.WINDOWS ? new JavaIoFileSystem() : new UnixFileSystem(); + final HashFunction hashFunction; + String value = null; + try { + value = System.getProperty("bazel.DigestFunction", "MD5"); + hashFunction = new HashFunction.Converter().convert(value); + } catch (OptionsParsingException e) { + throw new Error("The specified hash function '" + value + "' is not supported."); + } + return OS.getCurrent() == OS.WINDOWS + ? new JavaIoFileSystem(hashFunction) + : new UnixFileSystem(hashFunction); } public RemoteWorker( - FileSystem fs, RemoteWorkerOptions workerOptions, SimpleBlobStoreActionCache cache, - Path sandboxPath) + FileSystem fs, + RemoteWorkerOptions workerOptions, + SimpleBlobStoreActionCache cache, + Path sandboxPath, + DigestUtil digestUtil) throws IOException { this.workerOptions = workerOptions; - this.actionCacheServer = new ActionCacheServer(cache); + this.actionCacheServer = new ActionCacheServer(cache, digestUtil); Path workPath; if (workerOptions.workPath != null) { workPath = fs.getPath(workerOptions.workPath); @@ -110,7 +126,7 @@ public final class RemoteWorker { // For now, we use a temporary path if no work path was provided. workPath = fs.getPath("/tmp/remote-worker"); } - this.bsServer = new ByteStreamServer(cache, workPath); + this.bsServer = new ByteStreamServer(cache, workPath, digestUtil); this.casServer = new CasServer(cache); if (workerOptions.workPath != null) { @@ -119,7 +135,8 @@ public final class RemoteWorker { FileSystemUtils.createDirectoryAndParents(workPath); watchServer = new WatcherServer(operationsCache); execServer = - new ExecutionServer(workPath, sandboxPath, workerOptions, cache, operationsCache); + new ExecutionServer( + workPath, sandboxPath, workerOptions, cache, operationsCache, digestUtil); } else { watchServer = null; execServer = null; @@ -252,9 +269,14 @@ public final class RemoteWorker { blobStore = new ConcurrentMapBlobStore(new ConcurrentHashMap()); } + DigestUtil digestUtil = new DigestUtil(HashFunction.SHA256); RemoteWorker worker = new RemoteWorker( - fs, remoteWorkerOptions, new SimpleBlobStoreActionCache(blobStore), sandboxPath); + fs, + remoteWorkerOptions, + new SimpleBlobStoreActionCache(blobStore, digestUtil), + sandboxPath, + digestUtil); final Server server = worker.startServer(); worker.createPidFile(); -- cgit v1.2.3