diff options
author | 2017-11-30 11:09:35 -0800 | |
---|---|---|
committer | 2017-11-30 11:11:06 -0800 | |
commit | 559a07d2dd88a53d6dac8bf2d77a28db44fd7659 (patch) | |
tree | 1d2d6bc6317d6a8fca6d776510f6cb0cceb6e172 /src/main/java/com/google/devtools | |
parent | 1c9b698ede5527ff658b70ee15ef8eed01beb034 (diff) |
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
Diffstat (limited to 'src/main/java/com/google/devtools')
21 files changed, 276 insertions, 135 deletions
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<Class<? extends BlazeModule>> 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<InputStream> 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<InputStream> dataSupplier, Digest digest, int chunkSize) + Chunker(Supplier<InputStream> 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/Digests.java b/src/main/java/com/google/devtools/build/lib/remote/DigestUtil.java index c3b6e0d3a6..2ef23b54fa 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/Digests.java +++ b/src/main/java/com/google/devtools/build/lib/remote/DigestUtil.java @@ -1,4 +1,4 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. +// 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. @@ -11,7 +11,6 @@ // 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; @@ -23,8 +22,7 @@ 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.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.remoteexecution.v1test.Action; import com.google.devtools.remoteexecution.v1test.Digest; @@ -32,26 +30,45 @@ 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() {} +/** 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 static Digest computeDigest(byte[] blob) { - return buildDigest( - FileSystem.getDigestFunction().getHash().hashBytes(blob).toString(), blob.length); + public DigestUtil(HashFunction hashFn) { + this.hashFn = hashFn; } - public static Digest computeDigest(Path file) throws IOException { + 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 static Digest computeDigest(VirtualActionInput input) throws IOException { + public Digest compute(VirtualActionInput input) throws IOException { ByteArrayOutputStream buffer = new ByteArrayOutputStream(); input.writeTo(buffer); - return computeDigest(buffer.toByteArray()); + return compute(buffer.toByteArray()); } /** @@ -59,40 +76,24 @@ public final class Digests { * 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)); + public Digest compute(Message message) { + return compute(message.toByteArray()); } - /** - * 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 Digest computeAsUtf8(String str) { + return compute(str.getBytes(UTF_8)); } - public static ActionKey computeActionKey(Action action) { - return new ActionKey(computeDigest(action)); + 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! + * 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 DigestUtil.ActionKey asActionKey(Digest digest) { + return new DigestUtil.ActionKey(digest); } public static Digest buildDigest(byte[] hash, long size) { @@ -103,7 +104,7 @@ public final class Digests { return Digest.newBuilder().setHash(hexHash).setSizeBytes(size).build(); } - public static Digest getDigestFromInputCache(ActionInput input, MetadataProvider cache) + 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); 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<Digest> 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<Digest> 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<Digest> 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<Digest> 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<PathFragment, ActionInput> 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<PathFragment, ActionInput> 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<TreeNodeRepository.T private final Map<TreeNode, Directory> directoryCache = new HashMap<>(); private final Map<VirtualActionInput, Digest> virtualInputDigestCache = new HashMap<>(); private final Map<Digest, VirtualActionInput> 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 TreeTraverser<TreeNodeRepository.T ActionInput input = child.getActionInput(); if (input instanceof VirtualActionInput) { VirtualActionInput virtualInput = (VirtualActionInput) input; - Digest digest = Digests.computeDigest(virtualInput); + Digest digest = digestUtil.compute(virtualInput); virtualInputDigestCache.put(virtualInput, digest); // There may be multiple inputs with the same digest. In that case, we don't care which // one we get back from the digestVirtualInputCache later. @@ -314,7 +317,7 @@ public final class TreeNodeRepository extends TreeTraverser<TreeNodeRepository.T } else { b.addFilesBuilder() .setName(entry.getSegment()) - .setDigest(Digests.getDigestFromInputCache(input, inputFileCache)) + .setDigest(DigestUtil.getFromInputCache(input, inputFileCache)) .setIsExecutable(execRoot.getRelative(input.getExecPathString()).isExecutable()); } } else { @@ -324,7 +327,7 @@ public final class TreeNodeRepository extends TreeTraverser<TreeNodeRepository.T } directory = b.build(); directoryCache.put(node, directory); - Digest digest = Digests.computeDigest(directory); + Digest digest = digestUtil.compute(directory); treeNodeDigestCache.put(node, digest); digestTreeNodeCache.put(digest, node); } @@ -377,7 +380,7 @@ public final class TreeNodeRepository extends TreeTraverser<TreeNodeRepository.T if (input instanceof VirtualActionInput) { return Preconditions.checkNotNull(virtualInputDigestCache.get(input)); } - return Digests.getDigestFromInputCache(input, inputFileCache); + return DigestUtil.getFromInputCache(input, inputFileCache); } /** diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java new file mode 100644 index 0000000000..d7d6b0a892 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java @@ -0,0 +1,59 @@ +// 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.runtime; + +import com.google.devtools.build.lib.unix.UnixFileSystem; +import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.util.ExitCode; +import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; +import com.google.devtools.build.lib.vfs.JavaIoFileSystem; +import com.google.devtools.build.lib.windows.WindowsFileSystem; +import com.google.devtools.common.options.OptionsParsingException; +import com.google.devtools.common.options.OptionsProvider; + +/** + * Module to provide a {@link FileSystem} instance that uses {@code SHA256} as the default hash + * function, or else what's specified by {@code -Dbazel.DigestFunction}. + * + * <p>For 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; |