diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index c1c369c0b7acf4..c925b61d36d2dd 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -34,6 +34,7 @@ java_library( "Retrier.java", "AbstractActionInputPrefetcher.java", "ToplevelArtifactsDownloader.java", + "LeaseService.java", ], ), exports = [ @@ -46,13 +47,13 @@ java_library( ":ReferenceCountedChannel", ":Retrier", ":abstract_action_input_prefetcher", + ":lease_service", ":toplevel_artifacts_downloader", "//src/main/java/com/google/devtools/build/lib:build-request-options", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", - "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", @@ -83,6 +84,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/disk", "//src/main/java/com/google/devtools/build/lib/remote/downloader", "//src/main/java/com/google/devtools/build/lib/remote/grpc", @@ -185,6 +187,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", @@ -220,3 +223,15 @@ java_library( "//third_party:jsr305", ], ) + +java_library( + name = "lease_service", + srcs = ["LeaseService.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/skyframe", + "//third_party:jsr305", + ], +) 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 ad63d6c80aec9e..ce7372a1c18185 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 @@ -34,7 +34,6 @@ import java.io.PushbackInputStream; import java.util.NoSuchElementException; import java.util.Objects; -import java.util.function.Supplier; /** * Splits a data source into one or more {@link Chunk}s of at most {@code chunkSize} bytes. @@ -92,8 +91,7 @@ public boolean equals(Object o) { return false; } Chunk other = (Chunk) o; - return other.offset == offset - && other.data.equals(data); + return other.offset == offset && other.data.equals(data); } @Override @@ -102,7 +100,12 @@ public int hashCode() { } } - private final Supplier dataSupplier; + /** A supplier that provide data as {@link InputStream}. */ + public interface ChunkDataSupplier { + InputStream get() throws IOException; + } + + private final ChunkDataSupplier dataSupplier; private final long size; private final int chunkSize; private final Chunk emptyChunk; @@ -117,7 +120,7 @@ public int hashCode() { // lazily on the first call to next(), as opposed to opening it in the constructor or on reset(). private boolean initialized; - Chunker(Supplier dataSupplier, long size, int chunkSize, boolean compressed) { + Chunker(ChunkDataSupplier dataSupplier, long size, int chunkSize, boolean compressed) { this.dataSupplier = checkNotNull(dataSupplier); this.size = size; this.chunkSize = chunkSize; @@ -287,7 +290,7 @@ public static class Builder { private int chunkSize = getDefaultChunkSize(); protected long size; private boolean compressed; - protected Supplier inputStream; + protected ChunkDataSupplier inputStream; @CanIgnoreReturnValue public Builder setInput(byte[] data) { @@ -310,14 +313,7 @@ public Builder setInput(long size, InputStream in) { public Builder setInput(long size, Path file) { checkState(inputStream == null); this.size = size; - inputStream = - () -> { - try { - return file.getInputStream(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; + inputStream = file::getInputStream; return this; } @@ -326,30 +322,16 @@ public Builder setInput(long size, ActionInput actionInput, Path execRoot) { checkState(inputStream == null); this.size = size; if (actionInput instanceof VirtualActionInput) { - inputStream = - () -> { - try { - return ((VirtualActionInput) actionInput).getBytes().newInput(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; + inputStream = () -> ((VirtualActionInput) actionInput).getBytes().newInput(); } else { - inputStream = - () -> { - try { - return ActionInputHelper.toInputPath(actionInput, execRoot).getInputStream(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; + inputStream = () -> ActionInputHelper.toInputPath(actionInput, execRoot).getInputStream(); } return this; } @CanIgnoreReturnValue @VisibleForTesting - protected final Builder setInputSupplier(Supplier inputStream) { + protected final Builder setInputSupplier(ChunkDataSupplier inputStream) { this.inputStream = inputStream; return this; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java b/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java new file mode 100644 index 00000000000000..8479d787c99dc0 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java @@ -0,0 +1,76 @@ +// Copyright 2023 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 com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionCacheUtils; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.ActionLookupData; +import com.google.devtools.build.lib.actions.ActionLookupValue; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.cache.ActionCache; +import com.google.devtools.build.skyframe.MemoizingEvaluator; +import java.util.HashMap; +import java.util.Set; +import javax.annotation.Nullable; + +/** A lease service that manages the lease of remote blobs. */ +public class LeaseService { + private final MemoizingEvaluator memoizingEvaluator; + @Nullable private final ActionCache actionCache; + + public LeaseService(MemoizingEvaluator memoizingEvaluator, @Nullable ActionCache actionCache) { + this.memoizingEvaluator = memoizingEvaluator; + this.actionCache = actionCache; + } + + /** Clean up internal state when files are evicted from remote CAS. */ + public void handleMissingInputs(Set missingActionInputs) { + if (missingActionInputs.isEmpty()) { + return; + } + + var actions = new HashMap(); + + try { + for (ActionInput actionInput : missingActionInputs) { + if (actionInput instanceof Artifact.DerivedArtifact) { + Artifact.DerivedArtifact output = (Artifact.DerivedArtifact) actionInput; + ActionLookupData actionLookupData = output.getGeneratingActionKey(); + var actionLookupValue = + memoizingEvaluator.getExistingValue(actionLookupData.getActionLookupKey()); + if (actionLookupValue instanceof ActionLookupValue) { + Action action = + ((ActionLookupValue) actionLookupValue) + .getAction(actionLookupData.getActionIndex()); + actions.put(actionLookupData, action); + } + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + + if (!actions.isEmpty()) { + var actionKeys = actions.keySet(); + memoizingEvaluator.delete(key -> key instanceof ActionLookupData && actionKeys.contains(key)); + + if (actionCache != null) { + for (var action : actions.values()) { + ActionCacheUtils.removeCacheEntry(actionCache, action); + } + } + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 2f7099a1730fe5..b576f72d4a528f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -64,6 +64,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionRequirements; @@ -371,10 +372,14 @@ private MerkleTree buildInputMerkleTree( spawn, context, (Object nodeKey, InputWalker walker) -> { - subMerkleTrees.add(buildMerkleTreeVisitor(nodeKey, walker, metadataProvider)); + subMerkleTrees.add( + buildMerkleTreeVisitor( + nodeKey, walker, metadataProvider, context.getPathResolver())); }); if (!outputDirMap.isEmpty()) { - subMerkleTrees.add(MerkleTree.build(outputDirMap, metadataProvider, execRoot, digestUtil)); + subMerkleTrees.add( + MerkleTree.build( + outputDirMap, metadataProvider, execRoot, context.getPathResolver(), digestUtil)); } return MerkleTree.merge(subMerkleTrees, digestUtil); } else { @@ -394,16 +399,20 @@ private MerkleTree buildInputMerkleTree( toolSignature == null ? ImmutableSet.of() : toolSignature.toolInputs, context.getMetadataProvider(), execRoot, + context.getPathResolver(), digestUtil); } } private MerkleTree buildMerkleTreeVisitor( - Object nodeKey, InputWalker walker, MetadataProvider metadataProvider) + Object nodeKey, + InputWalker walker, + MetadataProvider metadataProvider, + ArtifactPathResolver artifactPathResolver) throws IOException, ForbiddenActionInputException { MerkleTree result = merkleTreeCache.getIfPresent(nodeKey); if (result == null) { - result = uncachedBuildMerkleTreeVisitor(walker, metadataProvider); + result = uncachedBuildMerkleTreeVisitor(walker, metadataProvider, artifactPathResolver); merkleTreeCache.put(nodeKey, result); } return result; @@ -411,14 +420,23 @@ private MerkleTree buildMerkleTreeVisitor( @VisibleForTesting public MerkleTree uncachedBuildMerkleTreeVisitor( - InputWalker walker, MetadataProvider metadataProvider) + InputWalker walker, + MetadataProvider metadataProvider, + ArtifactPathResolver artifactPathResolver) throws IOException, ForbiddenActionInputException { ConcurrentLinkedQueue subMerkleTrees = new ConcurrentLinkedQueue<>(); subMerkleTrees.add( - MerkleTree.build(walker.getLeavesInputMapping(), metadataProvider, execRoot, digestUtil)); + MerkleTree.build( + walker.getLeavesInputMapping(), + metadataProvider, + execRoot, + artifactPathResolver, + digestUtil)); walker.visitNonLeaves( (Object subNodeKey, InputWalker subWalker) -> { - subMerkleTrees.add(buildMerkleTreeVisitor(subNodeKey, subWalker, metadataProvider)); + subMerkleTrees.add( + buildMerkleTreeVisitor( + subNodeKey, subWalker, metadataProvider, artifactPathResolver)); }); return MerkleTree.merge(subMerkleTrees, digestUtil); } 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 9d567bf755a2c6..6f1bfbed7ba473 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 @@ -957,9 +957,13 @@ public ActionInput getActionInput(Path path) { }); env.getEventBus().register(toplevelArtifactsDownloader); + var leaseService = + new LeaseService( + env.getSkyframeExecutor().getEvaluator(), + env.getBlazeWorkspace().getPersistentActionCache()); + remoteOutputService.setActionInputFetcher(actionInputFetcher); - remoteOutputService.setMemoizingEvaluator(env.getSkyframeExecutor().getEvaluator()); - remoteOutputService.setActionCache(env.getBlazeWorkspace().getPersistentActionCache()); + remoteOutputService.setLeaseService(leaseService); env.getEventBus().register(remoteOutputService); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index f6c5f138a89dd0..2ea7a0f2aa9219 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -20,15 +20,10 @@ import com.google.common.collect.ImmutableMap; import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.Action; -import com.google.devtools.build.lib.actions.ActionCacheUtils; -import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputMap; -import com.google.devtools.build.lib.actions.ActionLookupData; -import com.google.devtools.build.lib.actions.ActionLookupValue; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; -import com.google.devtools.build.lib.actions.cache.ActionCache; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent; @@ -40,10 +35,8 @@ import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.skyframe.MemoizingEvaluator; import com.google.devtools.build.skyframe.SkyFunction.Environment; import java.io.IOException; -import java.util.HashMap; import java.util.Map; import java.util.UUID; import javax.annotation.Nullable; @@ -52,19 +45,14 @@ public class RemoteOutputService implements OutputService { @Nullable private RemoteActionInputFetcher actionInputFetcher; - @Nullable MemoizingEvaluator memoizingEvaluator; - @Nullable ActionCache actionCache; + @Nullable private LeaseService leaseService; void setActionInputFetcher(RemoteActionInputFetcher actionInputFetcher) { this.actionInputFetcher = Preconditions.checkNotNull(actionInputFetcher, "actionInputFetcher"); } - void setMemoizingEvaluator(MemoizingEvaluator memoizingEvaluator) { - this.memoizingEvaluator = memoizingEvaluator; - } - - void setActionCache(ActionCache actionCache) { - this.actionCache = actionCache; + void setLeaseService(LeaseService leaseService) { + this.leaseService = leaseService; } @Override @@ -128,44 +116,8 @@ public void finalizeBuild(boolean buildSuccessful) { @Subscribe public void onExecutionPhaseCompleteEvent(ExecutionPhaseCompleteEvent event) { - processMissingInputs(); - } - - private void processMissingInputs() { - if (memoizingEvaluator == null || actionInputFetcher == null) { - return; - } - - var actions = new HashMap(); - - try { - for (ActionInput actionInput : actionInputFetcher.getMissingActionInputs()) { - if (actionInput instanceof Artifact.DerivedArtifact) { - Artifact.DerivedArtifact output = (Artifact.DerivedArtifact) actionInput; - ActionLookupData actionLookupData = output.getGeneratingActionKey(); - var actionLookupValue = - memoizingEvaluator.getExistingValue(actionLookupData.getActionLookupKey()); - if (actionLookupValue instanceof ActionLookupValue) { - Action action = - ((ActionLookupValue) actionLookupValue) - .getAction(actionLookupData.getActionIndex()); - actions.put(actionLookupData, action); - } - } - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - - if (!actions.isEmpty()) { - var actionKeys = actions.keySet(); - memoizingEvaluator.delete(key -> key instanceof ActionLookupData && actionKeys.contains(key)); - - if (actionCache != null) { - for (var action : actions.values()) { - ActionCacheUtils.removeCacheEntry(actionCache, action); - } - } + if (leaseService != null && actionInputFetcher != null) { + leaseService.handleMissingInputs(actionInputFetcher.getMissingActionInputs()); } } 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 5b6622fbb0023d..0f7bb077577efa 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 @@ -557,7 +557,11 @@ private SpawnResult handleError( catastrophe = true; } else if (remoteCacheFailed) { status = Status.REMOTE_CACHE_FAILED; - detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_FAILED; + if (remoteOptions.useNewExitCodeForLostInputs) { + detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_EVICTED; + } else { + detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_FAILED; + } catastrophe = false; } else { status = Status.EXECUTION_FAILED; diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD index e6279c38a989ff..36e0a515df1eb5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD @@ -10,14 +10,25 @@ filegroup( visibility = ["//src:__subpackages__"], ) +java_library( + name = "cache_not_found_exception", + srcs = ["CacheNotFoundException.java"], + deps = [ + "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", + ], +) + java_library( name = "common", - srcs = glob(["*.java"]), + srcs = glob( + ["*.java"], + exclude = ["CacheNotFoundException.java"], + ), deps = [ + ":cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", - "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD b/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD index 42872e7ea0d1f1..dc506d1ba2ae81 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD @@ -15,7 +15,7 @@ java_library( srcs = glob(["*.java"]), deps = [ "//src/main/java/com/google/devtools/build/lib/remote/common", - "//src/main/java/com/google/devtools/build/lib/remote/options", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/BUILD b/src/main/java/com/google/devtools/build/lib/remote/http/BUILD index 67e7bd0a4689f4..86b527cd6219f1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/http/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/http/BUILD @@ -21,6 +21,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info", "//src/main/java/com/google/devtools/build/lib/authandtls", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:auth", diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java index 3a731981345ede..c9089c187e9091 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; @@ -61,9 +62,11 @@ static DirectoryTree fromActionInputs( SortedMap inputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { - return fromActionInputs(inputs, ImmutableSet.of(), metadataProvider, execRoot, digestUtil); + return fromActionInputs( + inputs, ImmutableSet.of(), metadataProvider, execRoot, artifactPathResolver, digestUtil); } static DirectoryTree fromActionInputs( @@ -71,11 +74,13 @@ static DirectoryTree fromActionInputs( Set toolInputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { Map tree = new HashMap<>(); int numFiles = - buildFromActionInputs(inputs, toolInputs, metadataProvider, execRoot, digestUtil, tree); + buildFromActionInputs( + inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil, tree); return new DirectoryTree(tree, numFiles); } @@ -135,6 +140,7 @@ private static int buildFromActionInputs( Set toolInputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil, Map tree) throws IOException { @@ -164,7 +170,7 @@ private static int buildFromActionInputs( case REGULAR_FILE: { Digest d = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); - Path inputPath = ActionInputHelper.toInputPath(input, execRoot); + Path inputPath = artifactPathResolver.toPath(input); boolean childAdded = currDir.addChild( FileNode.createExecutable( @@ -176,7 +182,13 @@ private static int buildFromActionInputs( SortedMap directoryInputs = explodeDirectory(input.getExecPath(), execRoot); return buildFromActionInputs( - directoryInputs, toolInputs, metadataProvider, execRoot, digestUtil, tree); + directoryInputs, + toolInputs, + metadataProvider, + execRoot, + artifactPathResolver, + digestUtil, + tree); case SYMLINK: { @@ -185,7 +197,7 @@ private static int buildFromActionInputs( "Encountered symlink input '%s', but all source symlinks should have been" + " resolved by SkyFrame. This is a bug.", path); - Path inputPath = ActionInputHelper.toInputPath(input, execRoot); + Path inputPath = artifactPathResolver.toPath(input); boolean childAdded = currDir.addChild( new SymlinkNode( diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java index 5af8d982ba3a00..1179ceca87462c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java @@ -30,6 +30,7 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -222,11 +223,13 @@ public static MerkleTree build( SortedMap inputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { try (SilentCloseable c = Profiler.instance().profile("MerkleTree.build(ActionInput)")) { DirectoryTree tree = - DirectoryTreeBuilder.fromActionInputs(inputs, metadataProvider, execRoot, digestUtil); + DirectoryTreeBuilder.fromActionInputs( + inputs, metadataProvider, execRoot, artifactPathResolver, digestUtil); return build(tree, digestUtil); } } @@ -247,12 +250,13 @@ public static MerkleTree build( Set toolInputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { try (SilentCloseable c = Profiler.instance().profile("MerkleTree.build(ActionInput)")) { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - inputs, toolInputs, metadataProvider, execRoot, digestUtil); + inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil); return build(tree, digestUtil); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD index 0fc286112da896..e01dc07b7fdf74 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD @@ -24,6 +24,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/remote:ExecutionStatusException", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index c9cc73b57da2b8..fae2d964f9ef04 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java @@ -491,6 +491,20 @@ public void downloadFile_onInterrupt_deletePartialDownloadedFile() throws Except assertThat(tempPathGenerator.getTempDir().getDirectoryEntries()).isEmpty(); } + @Test + public void missingInputs_addedToList() { + Map metadata = new HashMap<>(); + Map cas = new HashMap<>(); + Artifact a = createRemoteArtifact("file", "hello world", metadata, /* cas= */ null); + MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); + AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); + + assertThrows( + Exception.class, () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider))); + + assertThat(prefetcher.getMissingActionInputs()).contains(a); + } + protected static void wait(ListenableFuture future) throws IOException, ExecException, InterruptedException { try { @@ -505,7 +519,7 @@ protected static void wait(ListenableFuture future) } throw new IOException(e); } catch (InterruptedException e) { - future.cancel(/*mayInterruptIfRunning=*/ true); + future.cancel(/* mayInterruptIfRunning= */ true); throw e; } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index 477813978b5f03..be53291e5bebec 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -75,6 +75,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote:abstract_action_input_prefetcher", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/disk", "//src/main/java/com/google/devtools/build/lib/remote/grpc", "//src/main/java/com/google/devtools/build/lib/remote/http", diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index 94ef104b381cef..25b5c3cf33c0bb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java @@ -86,7 +86,8 @@ protected ImmutableList getSpawnModules() { } @Override - protected void assertOutputEquals(String realContent, String expectedContent) throws Exception { + protected void assertOutputEquals(String realContent, String expectedContent, boolean isLocal) + throws Exception { assertThat(realContent).isEqualTo(expectedContent); } @@ -95,6 +96,11 @@ protected void assertOutputContains(String content, String contains) throws Exce assertThat(content).contains(contains); } + @Override + protected void evictAllBlobs() throws Exception { + worker.restart(); + } + @After public void tearDown() throws IOException { if (worker != null) { @@ -418,7 +424,7 @@ public void symlinkToNestedDirectory() throws Exception { } @Test - public void remoteCacheEvictBlobs_exitWithCode39() throws Exception { + public void remoteCacheEvictBlobs_whenPrefetchingInput_exitWithCode39() throws Exception { // Arrange: Prepare workspace and populate remote cache write( "a/BUILD", @@ -453,7 +459,7 @@ public void remoteCacheEvictBlobs_exitWithCode39() throws Exception { assertOutputDoesNotExist("a/foo.out"); // Act: Evict blobs from remote cache and do an incremental build - worker.restart(); + evictAllBlobs(); write("a/bar.in", "updated bar"); var error = assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); @@ -468,7 +474,7 @@ public void remoteCacheEvictBlobs_exitWithCode39() throws Exception { } @Test - public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception { + public void remoteCacheEvictBlobs_whenUploadingInput_exitWithCode39() throws Exception { // Arrange: Prepare workspace and populate remote cache write( "a/BUILD", @@ -483,24 +489,71 @@ public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception " srcs = ['foo.out', 'bar.in'],", " outs = ['bar.out'],", " cmd = 'cat $(SRCS) > $@',", - " tags = ['no-remote-exec'],", ")"); write("a/foo.in", "foo"); write("a/bar.in", "bar"); // Populate remote cache + setDownloadAll(); buildTarget("//a:bar"); + waitDownloads(); + var bytes = FileSystemUtils.readContent(getOutputPath("a/foo.out")); + var hashCode = getDigestHashFunction().getHashFunction().hashBytes(bytes); getOutputPath("a/foo.out").delete(); getOutputPath("a/bar.out").delete(); getOutputBase().getRelative("action_cache").deleteTreesBelow(); restartServer(); + addOptions("--incompatible_remote_use_new_exit_code_for_lost_inputs"); // Clean build, foo.out isn't downloaded buildTarget("//a:bar"); assertOutputDoesNotExist("a/foo.out"); + // Act: Evict blobs from remote cache and do an incremental build + evictAllBlobs(); + write("a/bar.in", "updated bar"); + var error = assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Assert: Exit code is 39 + assertThat(error).hasMessageThat().contains(String.format("%s/%s", hashCode, bytes.length)); + assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39); + } + + @Test + public void remoteCacheEvictBlobs_whenUploadingInputFile_incrementalBuildCanContinue() + throws Exception { + // Arrange: Prepare workspace and populate remote cache + write( + "a/BUILD", + "genrule(", + " name = 'foo',", + " srcs = ['foo.in'],", + " outs = ['foo.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")", + "genrule(", + " name = 'bar',", + " srcs = ['foo.out', 'bar.in'],", + " outs = ['bar.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")"); + write("a/foo.in", "foo"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + getOutputPath("a/foo.out").delete(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + + // Clean build, foo.out isn't downloaded + setDownloadToplevel(); + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out"); + // Evict blobs from remote cache - worker.restart(); + evictAllBlobs(); // trigger build error write("a/bar.in", "updated bar"); @@ -509,13 +562,15 @@ public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception // Act: Do an incremental build without "clean" or "shutdown" buildTarget("//a:bar"); + waitDownloads(); // Assert: target was successfully built assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator()); } @Test - public void remoteCacheEvictBlobs_treeArtifact_incrementalBuildCanContinue() throws Exception { + public void remoteCacheEvictBlobs_whenUploadingInputTree_incrementalBuildCanContinue() + throws Exception { // Arrange: Prepare workspace and populate remote cache write("BUILD"); writeOutputDirRule(); @@ -531,7 +586,6 @@ public void remoteCacheEvictBlobs_treeArtifact_incrementalBuildCanContinue() thr " srcs = ['foo.out', 'bar.in'],", " outs = ['bar.out'],", " cmd = '( ls $(location :foo.out); cat $(location :bar.in) ) > $@',", - " tags = ['no-remote-exec'],", ")"); write("a/manifest", "file-inside"); write("a/bar.in", "bar"); @@ -548,18 +602,22 @@ public void remoteCacheEvictBlobs_treeArtifact_incrementalBuildCanContinue() thr assertOutputDoesNotExist("a/foo.out/file-inside"); // Evict blobs from remote cache - worker.restart(); + evictAllBlobs(); // trigger build error + setDownloadToplevel(); write("a/bar.in", "updated bar"); // Build failed because of remote cache eviction assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); // Act: Do an incremental build without "clean" or "shutdown" buildTarget("//a:bar"); + waitDownloads(); // Assert: target was successfully built assertValidOutputFile( - "a/bar.out", "file-inside" + lineSeparator() + "updated bar" + lineSeparator()); + "a/bar.out", + "file-inside" + lineSeparator() + "updated bar" + lineSeparator(), + /* isLocal= */ true); } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java index 0fd2d2d4639ea8..215df976fd1316 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java @@ -42,11 +42,13 @@ public abstract class BuildWithoutTheBytesIntegrationTestBase extends BuildInteg protected abstract void setDownloadAll(); - protected abstract void assertOutputEquals(String realContent, String expectedContent) - throws Exception; + protected abstract void assertOutputEquals( + String realContent, String expectedContent, boolean isLocal) throws Exception; protected abstract void assertOutputContains(String content, String contains) throws Exception; + protected abstract void evictAllBlobs() throws Exception; + protected void waitDownloads() throws Exception { // Trigger afterCommand of modules so that downloads are waited. runtimeWrapper.newCommand(); @@ -731,6 +733,106 @@ public void incrementalBuild_intermediateOutputModified_rerunGeneratingActions() assertThat(executedAction.getPrimaryOutput().getFilename()).isEqualTo("foo.txt"); } + @Test + public void remoteCacheEvictBlobs_whenPrefetchingInputFile_incrementalBuildCanContinue() + throws Exception { + // Arrange: Prepare workspace and populate remote cache + write( + "a/BUILD", + "genrule(", + " name = 'foo',", + " srcs = ['foo.in'],", + " outs = ['foo.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")", + "genrule(", + " name = 'bar',", + " srcs = ['foo.out', 'bar.in'],", + " outs = ['bar.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")"); + write("a/foo.in", "foo"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + getOutputPath("a/foo.out").delete(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + + // Clean build, foo.out isn't downloaded + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out"); + + // Evict blobs from remote cache + evictAllBlobs(); + + // trigger build error + write("a/bar.in", "updated bar"); + addOptions("--strategy_regexp=.*bar=local"); + // Build failed because of remote cache eviction + assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Act: Do an incremental build without "clean" or "shutdown" + buildTarget("//a:bar"); + + // Assert: target was successfully built + assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator()); + } + + @Test + public void remoteCacheEvictBlobs_whenPrefetchingInputTree_incrementalBuildCanContinue() + throws Exception { + // Arrange: Prepare workspace and populate remote cache + write("BUILD"); + writeOutputDirRule(); + write( + "a/BUILD", + "load('//:output_dir.bzl', 'output_dir')", + "output_dir(", + " name = 'foo.out',", + " manifest = ':manifest',", + ")", + "genrule(", + " name = 'bar',", + " srcs = ['foo.out', 'bar.in'],", + " outs = ['bar.out'],", + " cmd = '( ls $(location :foo.out); cat $(location :bar.in) ) > $@',", + ")"); + write("a/manifest", "file-inside"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + getOutputPath("a/foo.out").deleteTreesBelow(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + + // Clean build, foo.out isn't downloaded + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out/file-inside"); + + // Evict blobs from remote cache + evictAllBlobs(); + + // trigger build error + write("a/bar.in", "updated bar"); + addOptions("--strategy_regexp=.*bar=local"); + // Build failed because of remote cache eviction + assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Act: Do an incremental build without "clean" or "shutdown" + buildTarget("//a:bar"); + + // Assert: target was successfully built + assertValidOutputFile( + "a/bar.out", + "file-inside" + lineSeparator() + "updated bar" + lineSeparator(), + /* isLocal= */ true); + } + protected void assertOutputsDoNotExist(String target) throws Exception { for (Artifact output : getArtifacts(target)) { assertWithMessage( @@ -757,12 +859,17 @@ protected void assertOnlyOutputContent(String target, String filename, String co Artifact output = getOnlyElement(getArtifacts(target)); assertThat(output.getFilename()).isEqualTo(filename); assertThat(output.getPath().exists()).isTrue(); - assertOutputEquals(readContent(output.getPath(), UTF_8), content); + assertOutputEquals(readContent(output.getPath(), UTF_8), content, /* isLocal= */ false); } protected void assertValidOutputFile(String binRelativePath, String content) throws Exception { + assertValidOutputFile(binRelativePath, content, /* isLocal= */ false); + } + + protected void assertValidOutputFile(String binRelativePath, String content, boolean isLocal) + throws Exception { Path output = getOutputPath(binRelativePath); - assertOutputEquals(readContent(output, UTF_8), content); + assertOutputEquals(readContent(output, UTF_8), content, isLocal); assertThat(output.isReadable()).isTrue(); assertThat(output.isWritable()).isFalse(); assertThat(output.isExecutable()).isTrue(); 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 ebf15d53f593c6..f0880d2ea9b6eb 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 @@ -79,7 +79,6 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Supplier; import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; @@ -1637,7 +1636,7 @@ public void queryWriteStatus( /* Custom Chunker used to track number of open files */ private static class TestChunker extends Chunker { - TestChunker(Supplier dataSupplier, long size, int chunkSize, boolean compressed) { + TestChunker(ChunkDataSupplier dataSupplier, long size, int chunkSize, boolean compressed) { super(dataSupplier, size, chunkSize, compressed); } 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 6de8758a7d0466..cd4fa6943034c3 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 @@ -18,6 +18,7 @@ import com.github.luben.zstd.Zstd; import com.google.devtools.build.lib.remote.Chunker.Chunk; +import com.google.devtools.build.lib.remote.Chunker.ChunkDataSupplier; import com.google.protobuf.ByteString; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -26,7 +27,6 @@ import java.util.NoSuchElementException; import java.util.Random; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Supplier; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -126,7 +126,7 @@ public void resourcesShouldBeReleased() throws IOException { byte[] data = new byte[] {1, 2}; final AtomicReference in = new AtomicReference<>(); - Supplier supplier = + ChunkDataSupplier supplier = () -> { in.set(Mockito.spy(new ByteArrayInputStream(data))); return in.get(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index 9f7b55148c6330..c71f7166bff510 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -51,6 +51,7 @@ import com.google.common.collect.Maps; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionInputHelper; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.events.NullEventHandler; @@ -109,6 +110,7 @@ public void testVirtualActionInputSupport() throws Exception { ImmutableSortedMap.of(execPath, virtualActionInput), fakeFileCache, execRoot, + ArtifactPathResolver.forExecRoot(execRoot), DIGEST_UTIL); Digest digest = DIGEST_UTIL.compute(virtualActionInput.getBytes().toByteArray()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java index e919392fda5d1b..e82fcfede54211 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java @@ -152,21 +152,6 @@ public void prefetchFiles_missingFiles_failsWithSpecificMessage() throws Excepti .contains(String.format("%s/%s", digest.getHash(), digest.getSizeBytes())); } - @Test - public void missingInputs_addedToList() { - Map metadata = new HashMap<>(); - Map cas = new HashMap<>(); - Artifact a = createRemoteArtifact("file", "hello world", metadata, /* cas= */ null); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); - AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - - assertThrows( - ExecException.class, - () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider))); - - assertThat(prefetcher.getMissingActionInputs()).contains(a); - } - private RemoteCache newCache( RemoteOptions options, DigestUtil digestUtil, Map cas) { Map cacheEntries = Maps.newHashMapWithExpectedSize(cas.size()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index 5145d78b216e4c..e7bf966362aaab 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -1796,14 +1796,14 @@ public void buildMerkleTree_withMemoization_works() throws Exception { // assert first time // Called for: manifests, runfiles, nodeRoot1, nodeFoo1 and nodeBar. - verify(service, times(5)).uncachedBuildMerkleTreeVisitor(any(), any()); + verify(service, times(5)).uncachedBuildMerkleTreeVisitor(any(), any(), any()); // act second time service.buildRemoteAction(spawn2, context2); // assert second time // Called again for: manifests, runfiles, nodeRoot2 and nodeFoo2 but not nodeBar (cached). - verify(service, times(5 + 4)).uncachedBuildMerkleTreeVisitor(any(), any()); + verify(service, times(5 + 4)).uncachedBuildMerkleTreeVisitor(any(), any(), any()); } @Test 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 05a635dfde09ae..f0a6fefd5eb023 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 @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.MetadataProvider; @@ -151,6 +152,11 @@ public MetadataProvider getMetadataProvider() { return fakeFileCache; } + @Override + public ArtifactPathResolver getPathResolver() { + return ArtifactPathResolver.forExecRoot(execRoot); + } + @Override public ArtifactExpander getArtifactExpander() { throw new UnsupportedOperationException(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java index 6af2ab698c80ff..d70d39a3e1aefe 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; @@ -50,7 +51,11 @@ protected DirectoryTree build(Path... paths) throws IOException { } return DirectoryTreeBuilder.fromActionInputs( - inputFiles, new StaticMetadataProvider(metadata), execRoot, digestUtil); + inputFiles, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); } @Test @@ -63,7 +68,11 @@ public void virtualActionInputShouldWork() throws Exception { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); assertLexicographicalOrder(tree); assertThat(directoriesAtDepth(0, tree)).containsExactly("srcs"); @@ -108,7 +117,11 @@ public void directoryInputShouldBeExpanded() throws Exception { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); assertLexicographicalOrder(tree); assertThat(directoriesAtDepth(0, tree)).containsExactly("srcs"); @@ -152,7 +165,11 @@ public void filesShouldBeDeduplicated() throws Exception { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); assertLexicographicalOrder(tree); assertThat(directoriesAtDepth(0, tree)).containsExactly("srcs"); diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java index af96bb2d46f106..3c01f95354f4b1 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.FileArtifactValue; @@ -72,6 +73,7 @@ public void emptyMerkleTree() throws IOException { Collections.emptySortedMap(), new StaticMetadataProvider(Collections.emptyMap()), execRoot, + ArtifactPathResolver.forExecRoot(execRoot), digestUtil); Digest emptyDigest = digestUtil.compute(new byte[0]); assertThat(tree.getRootDigest()).isEqualTo(emptyDigest); @@ -108,7 +110,12 @@ public void buildMerkleTree() throws IOException { // act MerkleTree tree = - MerkleTree.build(sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree.build( + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); // assert Digest expectedRootDigest = digestUtil.compute(rootDir); @@ -157,14 +164,32 @@ public void mergeMerkleTrees() throws IOException { MerkleTree treeEmpty = MerkleTree.build( - new TreeMap<>(), new StaticMetadataProvider(metadata), execRoot, digestUtil); + new TreeMap<>(), + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); MerkleTree tree1 = - MerkleTree.build(sortedInputs1, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree.build( + sortedInputs1, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); MerkleTree tree2 = - MerkleTree.build(sortedInputs2, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree.build( + sortedInputs2, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); MerkleTree treeAll = MerkleTree.build( - sortedInputsAll, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputsAll, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); // act MerkleTree mergedTreeEmpty = MerkleTree.merge(Arrays.asList(), digestUtil); diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD index d5d0bfaa71d13c..d22fbca83e3ad8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD @@ -24,6 +24,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java index 46dc2572a29ae9..8f9e41e31e7a25 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.Spawn; @@ -122,6 +123,11 @@ public SpawnInputExpander getSpawnInputExpander() { return new SpawnInputExpander(execRoot, /* strict= */ false); } + @Override + public ArtifactPathResolver getPathResolver() { + return ArtifactPathResolver.forExecRoot(execRoot); + } + @Override public Duration getTimeout() { return Duration.ZERO; diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD index b0a8dbcc529645..a6309fb6303701 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD @@ -21,6 +21,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/disk", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/remote/util",