From a5dde12b0d945b54967969fafa4d5e40692f9a44 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 4 May 2023 08:59:07 -0700 Subject: [PATCH] Download toplevel artifacts within spawn execution so that after action execution, the outputs are already downloaded. We have three types of downloads: 1. Download within spawn execution: the download is part of the spawn execution. Similar to normal build that, during the spawn execution, outputs are written to local file system. Metadata are not injected to skyframe, but instead, are calculated, e.g RegularFileArtifactValue. 2. For action that doesn't have spawn, e.g. SymlinkAction, the download is part of action finlization before injecting metadata into skyframe. 3. Download afterwards: the download is not part of the spawn execution. During spawn execution, metadata of outputs are injected into skyframe as RemoteFileArtifactValue. Dynamic execution, for example, can then request the downloads later. PiperOrigin-RevId: 529423585 Change-Id: If78663e22f0d36d621ffb35331c7ae08dc79fccd --- .../lib/actions/RemoteArtifactChecker.java | 2 +- .../remote/AbstractActionInputPrefetcher.java | 52 +++-------- .../remote/RemoteActionContextProvider.java | 35 +++++--- .../lib/remote/RemoteActionFileSystem.java | 29 ++++-- .../lib/remote/RemoteExecutionService.java | 18 +++- .../build/lib/remote/RemoteModule.java | 12 ++- .../build/lib/remote/RemoteOutputChecker.java | 70 ++++++++++++--- .../build/lib/remote/RemoteOutputService.java | 3 +- .../lib/skyframe/SkyframeActionExecutor.java | 2 +- .../devtools/build/lib/vfs/OutputService.java | 3 +- .../lib/actions/ActionCacheCheckerTest.java | 6 +- .../google/devtools/build/lib/remote/BUILD | 1 + .../BuildWithoutTheBytesIntegrationTest.java | 90 ++++++++++++++++++- ...ildWithoutTheBytesIntegrationTestBase.java | 85 +++++++++++++----- .../remote/RemoteActionFileSystemTest.java | 16 ++-- .../remote/RemoteExecutionServiceTest.java | 6 +- .../lib/remote/RemoteSpawnCacheTest.java | 8 +- .../lib/remote/RemoteSpawnRunnerTest.java | 15 ++-- ...SpawnRunnerWithGrpcRemoteExecutorTest.java | 8 +- 19 files changed, 344 insertions(+), 117 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/RemoteArtifactChecker.java b/src/main/java/com/google/devtools/build/lib/actions/RemoteArtifactChecker.java index 196ce11371d6dc..af13fedf81f624 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/RemoteArtifactChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/RemoteArtifactChecker.java @@ -24,5 +24,5 @@ public interface RemoteArtifactChecker { * Returns true if Bazel should trust (and not verify) build artifacts that were last seen * remotely and do not exist locally. */ - boolean shouldTrustRemoteArtifact(Artifact file, RemoteFileArtifactValue metadata); + boolean shouldTrustRemoteArtifact(ActionInput file, RemoteFileArtifactValue metadata); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java index b165c42f6baef7..20d8a331cfda5d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java @@ -76,7 +76,6 @@ public abstract class AbstractActionInputPrefetcher implements ActionInputPrefet private final AsyncTaskCache.NoResult downloadCache = AsyncTaskCache.NoResult.create(); private final TempPathGenerator tempPathGenerator; private final OutputPermissions outputPermissions; - protected final Set outputsAreInputs = Sets.newConcurrentHashSet(); protected final Path execRoot; protected final RemoteOutputChecker remoteOutputChecker; @@ -587,61 +586,34 @@ public List getArtifacts() { } } - @SuppressWarnings({"CheckReturnValue", "FutureReturnValueIgnored"}) public void finalizeAction(Action action, OutputMetadataStore outputMetadataStore) throws IOException, InterruptedException { - List inputsToDownload = new ArrayList<>(); List outputsToDownload = new ArrayList<>(); for (Artifact output : action.getOutputs()) { + if (outputMetadataStore.artifactOmitted(output)) { + continue; + } + var metadata = outputMetadataStore.getOutputMetadata(output); if (!metadata.isRemote()) { continue; } - if (outputsAreInputs.remove(output)) { - if (output.isTreeArtifact()) { - var children = outputMetadataStore.getTreeArtifactChildren((SpecialArtifact) output); - inputsToDownload.addAll(children); - } else { - inputsToDownload.add(output); - } - } else if (output.isTreeArtifact()) { + if (output.isTreeArtifact()) { var children = outputMetadataStore.getTreeArtifactChildren((SpecialArtifact) output); for (var file : children) { - if (remoteOutputChecker.shouldDownloadFile(file)) { + if (remoteOutputChecker.shouldDownloadFileAfterActionExecution(file)) { outputsToDownload.add(file); } } - } else if (remoteOutputChecker.shouldDownloadFile(output)) { - outputsToDownload.add(output); + } else { + if (remoteOutputChecker.shouldDownloadFileAfterActionExecution(output)) { + outputsToDownload.add(output); + } } } - if (!inputsToDownload.isEmpty()) { - // "input" here means "input to another action" (but an output of this one), so - // getOutputMetadata() is the right method to pass to prefetchFiles() - var future = - prefetchFiles(inputsToDownload, outputMetadataStore::getOutputMetadata, Priority.HIGH); - addCallback( - future, - new FutureCallback() { - @Override - public void onSuccess(Void unused) { - reporter.post(new InputsEagerlyPrefetched(inputsToDownload)); - } - - @Override - public void onFailure(Throwable throwable) { - reporter.handle( - Event.warn( - String.format( - "Failed to eagerly prefetch inputs: %s", throwable.getMessage()))); - } - }, - directExecutor()); - } - if (!outputsToDownload.isEmpty()) { var future = prefetchFiles(outputsToDownload, outputMetadataStore::getOutputMetadata, Priority.LOW); @@ -669,4 +641,8 @@ public void flushOutputTree() throws InterruptedException { public ImmutableSet getMissingActionInputs() { return ImmutableSet.copyOf(missingActionInputs); } + + public RemoteOutputChecker getRemoteOutputChecker() { + return remoteOutputChecker; + } } 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 61d722400073c0..df57599c3a0837 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 @@ -48,6 +48,7 @@ final class RemoteActionContextProvider { private TempPathGenerator tempPathGenerator; private RemoteExecutionService remoteExecutionService; @Nullable private RemoteActionInputFetcher actionInputFetcher; + @Nullable private final RemoteOutputChecker remoteOutputChecker; private RemoteActionContextProvider( Executor executor, @@ -56,7 +57,8 @@ private RemoteActionContextProvider( @Nullable RemoteExecutionClient remoteExecutor, @Nullable ListeningScheduledExecutorService retryScheduler, DigestUtil digestUtil, - @Nullable Path logDir) { + @Nullable Path logDir, + @Nullable RemoteOutputChecker remoteOutputChecker) { this.executor = executor; this.env = Preconditions.checkNotNull(env, "env"); this.remoteCache = remoteCache; @@ -64,6 +66,7 @@ private RemoteActionContextProvider( this.retryScheduler = retryScheduler; this.digestUtil = digestUtil; this.logDir = logDir; + this.remoteOutputChecker = remoteOutputChecker; } public static RemoteActionContextProvider createForPlaceholder( @@ -73,11 +76,12 @@ public static RemoteActionContextProvider createForPlaceholder( return new RemoteActionContextProvider( directExecutor(), env, - /*remoteCache=*/ null, - /*remoteExecutor=*/ null, + /* remoteCache= */ null, + /* remoteExecutor= */ null, retryScheduler, digestUtil, - /*logDir=*/ null); + /* logDir= */ null, + /* remoteOutputChecker= */ null); } public static RemoteActionContextProvider createForRemoteCaching( @@ -85,15 +89,17 @@ public static RemoteActionContextProvider createForRemoteCaching( CommandEnvironment env, RemoteCache remoteCache, ListeningScheduledExecutorService retryScheduler, - DigestUtil digestUtil) { + DigestUtil digestUtil, + @Nullable RemoteOutputChecker remoteOutputChecker) { return new RemoteActionContextProvider( executor, env, remoteCache, - /*remoteExecutor=*/ null, + /* remoteExecutor= */ null, retryScheduler, digestUtil, - /*logDir=*/ null); + /* logDir= */ null, + remoteOutputChecker); } public static RemoteActionContextProvider createForRemoteExecution( @@ -103,9 +109,17 @@ public static RemoteActionContextProvider createForRemoteExecution( RemoteExecutionClient remoteExecutor, ListeningScheduledExecutorService retryScheduler, DigestUtil digestUtil, - Path logDir) { + Path logDir, + @Nullable RemoteOutputChecker remoteOutputChecker) { return new RemoteActionContextProvider( - executor, env, remoteCache, remoteExecutor, retryScheduler, digestUtil, logDir); + executor, + env, + remoteCache, + remoteExecutor, + retryScheduler, + digestUtil, + logDir, + remoteOutputChecker); } private RemotePathResolver createRemotePathResolver() { @@ -155,7 +169,8 @@ private RemoteExecutionService getRemoteExecutionService() { remoteCache, remoteExecutor, tempPathGenerator, - captureCorruptedOutputsDir); + captureCorruptedOutputsDir, + remoteOutputChecker); env.getEventBus().register(remoteExecutionService); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index e3ab7382e96f70..295c70ec7771f8 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -133,20 +133,20 @@ void injectRemoteFile(PathFragment path, byte[] digest, long size, long expireAt remoteOutputTree.injectRemoteFile(path, digest, size, expireAtEpochMilli); } - void flush() throws IOException { + void flush() throws IOException, InterruptedException { checkNotNull(metadataInjector, "metadataInjector is null"); for (Map.Entry entry : outputMapping.entrySet()) { PathFragment path = execRoot.getRelative(entry.getKey()); Artifact output = entry.getValue(); - maybeInjectMetadataForSymlink(path, output); + maybeInjectMetadataForSymlinkOrDownload(path, output); } } /** * Inject metadata for non-symlink outputs that were materialized as a symlink to a remote - * artifact. + * artifact, and download the target artifact if required by the remote output mode. * *

If a non-symlink output is materialized as a symlink, the symlink has "copy" semantics, * i.e., the output metadata is identical to that of the symlink target. For these artifacts, we @@ -162,8 +162,8 @@ void flush() throws IOException { * fetching multiple copies when multiple symlinks to the same artifact are created in the * same build. */ - private void maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact output) - throws IOException { + private void maybeInjectMetadataForSymlinkOrDownload(PathFragment linkPath, Artifact output) + throws IOException, InterruptedException { if (output.isSymlink()) { return; } @@ -185,6 +185,25 @@ private void maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact outpu targetPath.isAbsolute(), "non-symlink artifact materialized as symlink must point to absolute path"); + if (inputFetcher.getRemoteOutputChecker().shouldDownloadOutputDuringActionExecution(output)) { + var targetActionInput = getInput(targetPath.relativeTo(execRoot).getPathString()); + if (targetActionInput != null) { + if (output.isTreeArtifact()) { + var metadata = getRemoteTreeMetadata(targetPath); + if (metadata != null) { + getFromFuture( + inputFetcher.prefetchFiles( + metadata.getChildren(), this::getInputMetadata, Priority.LOW)); + } + } else { + getFromFuture( + inputFetcher.prefetchFiles( + ImmutableList.of(targetActionInput), this::getInputMetadata, Priority.LOW)); + } + } + return; + } + if (output.isTreeArtifact()) { TreeArtifactValue metadata = getRemoteTreeMetadata(targetPath); if (metadata == null) { 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 8f8874e5c416b6..0923429a757d3b 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 @@ -178,6 +178,8 @@ public class RemoteExecutionService { private final AtomicBoolean shutdown = new AtomicBoolean(false); private final AtomicBoolean buildInterrupted = new AtomicBoolean(false); + @Nullable private final RemoteOutputChecker remoteOutputChecker; + public RemoteExecutionService( Executor executor, Reporter reporter, @@ -191,7 +193,8 @@ public RemoteExecutionService( @Nullable RemoteCache remoteCache, @Nullable RemoteExecutionClient remoteExecutor, TempPathGenerator tempPathGenerator, - @Nullable Path captureCorruptedOutputsDir) { + @Nullable Path captureCorruptedOutputsDir, + @Nullable RemoteOutputChecker remoteOutputChecker) { this.reporter = reporter; this.verboseFailures = verboseFailures; this.execRoot = execRoot; @@ -214,6 +217,7 @@ public RemoteExecutionService( this.captureCorruptedOutputsDir = captureCorruptedOutputsDir; this.scheduler = Schedulers.from(executor, /* interruptibleWorker= */ true); + this.remoteOutputChecker = remoteOutputChecker; } static Command buildCommand( @@ -1158,7 +1162,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re ImmutableList.Builder> downloadsBuilder = ImmutableList.builder(); - boolean downloadOutputs = shouldDownloadOutputsFor(result, metadata); + boolean downloadOutputs = shouldDownloadOutputsFor(action, result, metadata); // Download into temporary paths, then move everything at the end. // This avoids holding the output lock while downloading, which would prevent the local branch @@ -1322,7 +1326,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re } private boolean shouldDownloadOutputsFor( - RemoteActionResult result, ActionResultMetadata metadata) { + RemoteAction action, RemoteActionResult result, ActionResultMetadata metadata) { if (remoteOptions.remoteOutputsMode.downloadAllOutputs()) { return true; } @@ -1341,6 +1345,14 @@ private boolean shouldDownloadOutputsFor( Iterables.get(metadata.symlinks(), 0).path()))); return true; } + + checkNotNull(remoteOutputChecker, "remoteOutputChecker must not be null"); + for (var output : action.getSpawn().getOutputFiles()) { + if (remoteOutputChecker.shouldDownloadOutputDuringActionExecution(output)) { + return true; + } + } + return false; } 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 6180167256d5c9..60a7c3550b7a56 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 @@ -268,7 +268,12 @@ private void initHttpAndDiskCache( new RemoteCache(HTTP_AND_DISK_CACHE_CAPABILITIES, cacheClient, remoteOptions, digestUtil); actionContextProvider = RemoteActionContextProvider.createForRemoteCaching( - executorService, env, remoteCache, /* retryScheduler= */ null, digestUtil); + executorService, + env, + remoteCache, + /* retryScheduler= */ null, + digestUtil, + remoteOutputChecker); } @Override @@ -657,7 +662,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { remoteExecutor, retryScheduler, digestUtil, - logDir); + logDir, + remoteOutputChecker); repositoryRemoteExecutorFactoryDelegate.init( new RemoteRepositoryRemoteExecutorFactory( remoteCache, @@ -689,7 +695,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { cacheCapabilities.getCacheCapabilities(), cacheClient, remoteOptions, digestUtil); actionContextProvider = RemoteActionContextProvider.createForRemoteCaching( - executorService, env, remoteCache, retryScheduler, digestUtil); + executorService, env, remoteCache, retryScheduler, digestUtil, remoteOutputChecker); } buildEventArtifactUploaderFactoryDelegate.init( diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java index 8418237e9697b5..3ac336ca321b92 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; +import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; @@ -47,7 +48,8 @@ private enum CommandMode { private final CommandMode commandMode; private final boolean downloadToplevel; private final ImmutableList patternsToDownload; - private final Set toplevelArtifactsToDownload = Sets.newConcurrentHashSet(); + private final Set toplevelArtifactsToDownload = Sets.newConcurrentHashSet(); + private final Set inputsToDownload = Sets.newConcurrentHashSet(); public RemoteOutputChecker( Clock clock, @@ -125,6 +127,10 @@ private void addRunfiles(ConfiguredTarget buildTarget) { } } + public void addInputToDownload(ActionInput file) { + inputsToDownload.add(file); + } + private boolean shouldDownloadToplevelOutputs(ConfiguredTarget configuredTarget) { switch (commandMode) { case RUN: @@ -144,32 +150,70 @@ private boolean shouldDownloadToplevelOutputs(ConfiguredTarget configuredTarget) } } - public boolean shouldDownloadFile(Artifact file) { - checkArgument(!file.isTreeArtifact(), "file must not be a tree."); + private boolean shouldDownloadOutputForToplevel(ActionInput output) { + return shouldDownloadOutputFor(output, toplevelArtifactsToDownload); + } + + private boolean shouldDownloadOutputForLocalAction(ActionInput output) { + return shouldDownloadOutputFor(output, inputsToDownload); + } + + private boolean shouldDownloadFileForRegex(ActionInput file) { + checkArgument( + !(file instanceof Artifact && ((Artifact) file).isTreeArtifact()), + "file must not be a tree."); - if (file instanceof TreeFileArtifact) { - if (toplevelArtifactsToDownload.contains(file.getParent())) { + for (var pattern : patternsToDownload) { + if (pattern.matcher(file.getExecPathString()).matches()) { return true; } - } else if (toplevelArtifactsToDownload.contains(file)) { - return true; } - return outputMatchesPattern(file); + return false; } - private boolean outputMatchesPattern(Artifact output) { - for (var pattern : patternsToDownload) { - if (pattern.matcher(output.getExecPathString()).matches()) { + private static boolean shouldDownloadOutputFor( + ActionInput output, Set artifactCollection) { + if (output instanceof TreeFileArtifact) { + if (artifactCollection.contains(((Artifact) output).getParent())) { return true; } + } else if (artifactCollection.contains(output)) { + return true; } + return false; } + /** + * Returns {@code true} if Bazel should download this {@link ActionInput} during spawn execution. + * + * @param output output of the spawn. Tree is accepted since we can't know the content of tree + * before executing the spawn. + */ + public boolean shouldDownloadOutputDuringActionExecution(ActionInput output) { + // Download toplevel artifacts within action execution so that when the event TargetComplete is + // emitted, related toplevel artifacts are downloaded. + // + // Download outputs that are inputs to local actions within action execution so that the local + // actions don't need to wait for background downloads. + return shouldDownloadOutputForToplevel(output) || shouldDownloadOutputForLocalAction(output); + } + + /** + * Returns {@code true} if Bazel should download this {@link ActionInput} after action execution. + * + * @param file file output of the action. Tree must be expanded to tree file. + */ + public boolean shouldDownloadFileAfterActionExecution(ActionInput file) { + // Download user requested blobs in background to finish action execution sooner so that other + // actions can start sooner. + return shouldDownloadFileForRegex(file); + } + @Override - public boolean shouldTrustRemoteArtifact(Artifact file, RemoteFileArtifactValue metadata) { - if (shouldDownloadFile(file)) { + public boolean shouldTrustRemoteArtifact(ActionInput file, RemoteFileArtifactValue metadata) { + if (shouldDownloadOutputForToplevel(file) || shouldDownloadFileForRegex(file)) { // If Bazel should download this file, but it does not exist locally, returns false to rerun // the generating action to trigger the download (just like in the normal build, when local // outputs are missing). 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 03048b8defd623..996d1ca65616bc 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 @@ -137,7 +137,8 @@ public void onExecutionPhaseCompleteEvent(ExecutionPhaseCompleteEvent event) { } @Override - public void flushActionFileSystem(FileSystem actionFileSystem) throws IOException { + public void flushActionFileSystem(FileSystem actionFileSystem) + throws InterruptedException, IOException { ((RemoteActionFileSystem) actionFileSystem).flush(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 6b37fd0dd102c2..954a87c473e573 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -1468,7 +1468,7 @@ private static LostInputsCheck lostInputsCheck( private static void flushActionFileSystem( @Nullable FileSystem actionFileSystem, @Nullable OutputService outputService) - throws IOException { + throws IOException, InterruptedException { if (outputService != null && actionFileSystem != null) { outputService.flushActionFileSystem(actionFileSystem); } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index f87ab8b508998d..840a6aa142b4b8 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -212,7 +212,8 @@ default void checkActionFileSystemForLostInputs(FileSystem actionFileSystem, Act * Flush the internal state of filesystem returned by {@link #createActionFileSystem} after action * execution, before skyframe checking the action outputs. */ - default void flushActionFileSystem(FileSystem actionFileSystem) throws IOException {} + default void flushActionFileSystem(FileSystem actionFileSystem) + throws IOException, InterruptedException {} default boolean supportsPathResolverForArtifactValues() { return false; diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java index c869f6088be1e2..9ae6943597daa0 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java @@ -1045,9 +1045,11 @@ public void saveOutputMetadata_treeMetadata_localFileMetadataLoaded() throws Exc /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), mockedRemoteArtifactChecker); verify(mockedRemoteArtifactChecker) - .shouldTrustRemoteArtifact(argThat(arg -> arg.getFilename().equals("file1")), any()); + .shouldTrustRemoteArtifact( + argThat(arg -> arg.getExecPathString().endsWith("file1")), any()); verify(mockedRemoteArtifactChecker, never()) - .shouldTrustRemoteArtifact(argThat(arg -> arg.getFilename().equals("file2")), any()); + .shouldTrustRemoteArtifact( + argThat(arg -> arg.getExecPathString().endsWith("file2")), any()); // Not cached since local file changed runAction( action, 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 cedd01510f0427..a010b14eb4341d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -139,6 +139,7 @@ java_library( "//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/skyframe:action_execution_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/util:os", "//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/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index d07d0e4bf690b6..a1aa27e5573579 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 @@ -91,8 +91,7 @@ protected ImmutableList getSpawnModules() { } @Override - protected void assertOutputEquals(Path path, String expectedContent, boolean isLocal) - throws Exception { + protected void assertOutputEquals(Path path, String expectedContent) throws Exception { assertThat(readContent(path, UTF_8)).isEqualTo(expectedContent); } @@ -683,7 +682,90 @@ public void remoteCacheEvictBlobs_whenUploadingInputTree_incrementalBuildCanCont waitDownloads(); // Assert: target was successfully built - assertValidOutputFile( - "a/bar.out", "file-inside\nupdated bar" + lineSeparator(), /* isLocal= */ true); + assertValidOutputFile("a/bar.out", "file-inside\nupdated bar" + lineSeparator()); + } + + @Test + public void downloadToplevel_symlinkFile() throws Exception { + // TODO(chiwang): Make metadata for downloaded symlink non-remote. + assumeFalse(OS.getCurrent() == OS.WINDOWS); + + setDownloadToplevel(); + writeSymlinkRule(); + write( + "BUILD", + "load(':symlink.bzl', 'symlink')", + "genrule(", + " name = 'foo',", + " srcs = [],", + " outs = ['out/foo.txt'],", + " cmd = 'echo foo > $@',", + ")", + "symlink(", + " name = 'foo-link',", + " target = ':foo'", + ")"); + + buildTarget("//:foo-link"); + + assertValidOutputFile("foo-link", "foo\n"); + + // Delete link, re-plant symlink + getOutputPath("foo-link").delete(); + + buildTarget("//:foo-link"); + + assertValidOutputFile("foo-link", "foo\n"); + + // Delete target, re-download it + getOutputPath("foo").delete(); + + assertValidOutputFile("foo-link", "foo\n"); + } + + @Test + public void downloadToplevel_symlinkTree() throws Exception { + // TODO(chiwang): Make metadata for downloaded symlink non-remote. + assumeFalse(OS.getCurrent() == OS.WINDOWS); + + setDownloadToplevel(); + writeSymlinkRule(); + writeOutputDirRule(); + write( + "BUILD", + "load(':output_dir.bzl', 'output_dir')", + "load(':symlink.bzl', 'symlink')", + "output_dir(", + " name = 'foo',", + " content_map = {'file-1': '1', 'file-2': '2', 'file-3': '3'},", + ")", + "symlink(", + " name = 'foo-link',", + " target = ':foo'", + ")"); + + buildTarget("//:foo-link"); + + assertValidOutputFile("foo-link/file-1", "1"); + assertValidOutputFile("foo-link/file-2", "2"); + assertValidOutputFile("foo-link/file-3", "3"); + + getOutputPath("foo-link").deleteTree(); + + // Delete link, re-plant symlink + buildTarget("//:foo-link"); + + assertValidOutputFile("foo-link/file-1", "1"); + assertValidOutputFile("foo-link/file-2", "2"); + assertValidOutputFile("foo-link/file-3", "3"); + + // Delete target, re-download them + getOutputPath("foo").deleteTree(); + + buildTarget("//:foo-link"); + + assertValidOutputFile("foo-link/file-1", "1"); + assertValidOutputFile("foo-link/file-2", "2"); + assertValidOutputFile("foo-link/file-3", "3"); } } 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 83bb5b43e23cd2..a34b117934b03d 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 @@ -20,7 +20,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertThrows; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.ActionExecutedEvent; @@ -30,6 +29,7 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase; import com.google.devtools.build.lib.skyframe.ActionExecutionValue; +import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.io.RecordingOutErr; import com.google.devtools.build.lib.vfs.Path; @@ -46,8 +46,7 @@ public abstract class BuildWithoutTheBytesIntegrationTestBase extends BuildInteg protected abstract void setDownloadAll(); - protected abstract void assertOutputEquals(Path path, String expectedContent, boolean isLocal) - throws Exception; + protected abstract void assertOutputEquals(Path path, String expectedContent) throws Exception; protected abstract void assertOutputContains(String content, String contains) throws Exception; @@ -641,6 +640,8 @@ public void downloadToplevel_treeArtifacts() throws Exception { assertValidOutputFile("foo/file-1", "1"); assertValidOutputFile("foo/file-2", "2"); assertValidOutputFile("foo/file-3", "3"); + assertThat(getMetadata("//:foo").values().stream().noneMatch(FileArtifactValue::isRemote)) + .isTrue(); } @Test @@ -671,8 +672,14 @@ public void downloadToplevel_multipleToplevelTargets() throws Exception { waitDownloads(); assertValidOutputFile("out/foo1.txt", "foo1\n"); + assertThat(getMetadata("//:foo1").values().stream().noneMatch(FileArtifactValue::isRemote)) + .isTrue(); assertValidOutputFile("out/foo2.txt", "foo2\n"); + assertThat(getMetadata("//:foo2").values().stream().noneMatch(FileArtifactValue::isRemote)) + .isTrue(); assertValidOutputFile("out/foo3.txt", "foo3\n"); + assertThat(getMetadata("//:foo3").values().stream().noneMatch(FileArtifactValue::isRemote)) + .isTrue(); } @Test @@ -705,15 +712,27 @@ public void downloadToplevel_incrementalBuild_multipleToplevelTargets() throws E waitDownloads(); assertOutputsDoNotExist("//:foo1"); + assertThat(getMetadata("//:foo1").values().stream().allMatch(FileArtifactValue::isRemote)) + .isTrue(); assertOutputsDoNotExist("//:foo2"); + assertThat(getMetadata("//:foo2").values().stream().allMatch(FileArtifactValue::isRemote)) + .isTrue(); assertOutputsDoNotExist("//:foo3"); + assertThat(getMetadata("//:foo3").values().stream().allMatch(FileArtifactValue::isRemote)) + .isTrue(); buildTarget("//:foo1", "//:foo2", "//:foo3"); waitDownloads(); assertValidOutputFile("out/foo1.txt", "foo1\n"); + assertThat(getMetadata("//:foo1").values().stream().noneMatch(FileArtifactValue::isRemote)) + .isTrue(); assertValidOutputFile("out/foo2.txt", "foo2\n"); + assertThat(getMetadata("//:foo2").values().stream().noneMatch(FileArtifactValue::isRemote)) + .isTrue(); assertValidOutputFile("out/foo3.txt", "foo3\n"); + assertThat(getMetadata("//:foo3").values().stream().noneMatch(FileArtifactValue::isRemote)) + .isTrue(); } @Test @@ -1003,7 +1022,7 @@ public void incrementalBuild_remoteFileMetadataIsReplacedWithLocalFileMetadata() buildTarget("//:foobar"); assertValidOutputFile("out/foo.txt", "foo\n"); assertValidOutputFile("out/foobar.txt", "foo\nbar\n"); - assertThat(getOnlyElement(getFileMetadata("//:foo").values()).isRemote()).isTrue(); + assertThat(getOnlyElement(getMetadata("//:foo").values()).isRemote()).isTrue(); // Act: Do an incremental build without any modifications ActionEventCollector actionEventCollector = new ActionEventCollector(); @@ -1016,21 +1035,34 @@ public void incrementalBuild_remoteFileMetadataIsReplacedWithLocalFileMetadata() assertThat(actionEventCollector.getActionExecutedEvents()).isEmpty(); // Two actions are invalidated but were able to hit the action cache assertThat(actionEventCollector.getCachedActionEvents()).hasSize(2); - assertThat(getOnlyElement(getFileMetadata("//:foo").values()).isRemote()).isFalse(); + assertThat(getOnlyElement(getMetadata("//:foo").values()).isRemote()).isFalse(); } - private ImmutableMap getFileMetadata(String target) - throws Exception { + protected ImmutableMap getMetadata(String target) throws Exception { var result = ImmutableMap.builder(); var evaluator = getRuntimeWrapper().getSkyframeExecutor().getEvaluator(); for (var artifact : getArtifacts(target)) { var value = evaluator.getExistingValue(Artifact.key(artifact)); - Preconditions.checkState(value instanceof ActionExecutionValue); - result.putAll(((ActionExecutionValue) value).getAllFileValues()); + if (value instanceof ActionExecutionValue) { + result.putAll(((ActionExecutionValue) value).getAllFileValues()); + } else if (value instanceof TreeArtifactValue) { + result.putAll(((TreeArtifactValue) value).getChildValues()); + } } return result.buildOrThrow(); } + protected FileArtifactValue getMetadata(Artifact output) throws Exception { + var evaluator = getRuntimeWrapper().getSkyframeExecutor().getEvaluator(); + var value = evaluator.getExistingValue(Artifact.key(output)); + if (value instanceof ActionExecutionValue) { + return ((ActionExecutionValue) value).getAllFileValues().get(output); + } else if (value instanceof TreeArtifactValue) { + return ((TreeArtifactValue) value).getChildValues().get(output); + } + return null; + } + @Test public void incrementalBuild_intermediateOutputModified_rerunGeneratingActions() throws Exception { @@ -1169,8 +1201,7 @@ public void remoteCacheEvictBlobs_whenPrefetchingInputTree_incrementalBuildCanCo buildTarget("//a:bar"); // Assert: target was successfully built - assertValidOutputFile( - "a/bar.out", "file-inside\nupdated bar" + lineSeparator(), /* isLocal= */ true); + assertValidOutputFile("a/bar.out", "file-inside\nupdated bar" + lineSeparator()); } @Test @@ -1262,8 +1293,7 @@ public void remoteTreeFilesExpiredBetweenBuilds_rerunGeneratingActions() throws waitDownloads(); // Assert: target was successfully built - assertValidOutputFile( - "a/bar.out", "file-inside\nupdated bar" + lineSeparator(), /* isLocal= */ true); + assertValidOutputFile("a/bar.out", "file-inside\nupdated bar" + lineSeparator()); } protected void assertOutputsDoNotExist(String target) throws Exception { @@ -1292,22 +1322,37 @@ 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(output.getPath(), content, /* isLocal= */ false); + assertOutputEquals(output.getPath(), content); } 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(getOutputPath(binRelativePath), content, isLocal); + assertOutputEquals(getOutputPath(binRelativePath), content); assertThat(output.isReadable()).isTrue(); assertThat(output.isWritable()).isFalse(); assertThat(output.isExecutable()).isTrue(); } + protected void writeSymlinkRule() throws IOException { + write( + "symlink.bzl", + "def _symlink_impl(ctx):", + " target = ctx.file.target", + " if target.is_directory:", + " link = ctx.actions.declare_directory(ctx.attr.name)", + " else:", + " link = ctx.actions.declare_file(ctx.attr.name)", + " ctx.actions.symlink(output = link, target_file = target)", + " return DefaultInfo(files = depset([link]))", + "", + "symlink = rule(", + " implementation = _symlink_impl,", + " attrs = {", + " 'target': attr.label(mandatory = True, allow_single_file = True),", + " }", + ")"); + } + protected void writeOutputDirRule() throws IOException { write( "output_dir.bzl", diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java index 32f6a0f6c55cb4..3f585c11e9ccdb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java @@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -45,6 +46,7 @@ import com.google.devtools.build.lib.actions.StaticInputMetadataProvider; import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; @@ -68,6 +70,9 @@ /** Tests for {@link RemoteActionFileSystem} */ @RunWith(JUnit4.class) public final class RemoteActionFileSystemTest extends RemoteActionFileSystemTestBase { + private static final RemoteOutputChecker DUMMY_REMOTE_OUTPUT_CHECKER = + new RemoteOutputChecker( + new JavaClock(), "build", /* downloadToplevel= */ false, ImmutableList.of()); private static final DigestHashFunction HASH_FUNCTION = DigestHashFunction.SHA256; @@ -90,6 +95,7 @@ public void setUp() throws IOException { protected RemoteActionFileSystem createActionFileSystem( ActionInputMap inputs, Iterable outputs, InputMetadataProvider fileCache) throws IOException { + doReturn(DUMMY_REMOTE_OUTPUT_CHECKER).when(inputFetcher).getRemoteOutputChecker(); RemoteActionFileSystem remoteActionFileSystem = new RemoteActionFileSystem( fs, @@ -373,7 +379,7 @@ public void getMetadata_notFound() throws Exception { } @Test - public void createSymbolicLink_localFileArtifact() throws IOException { + public void createSymbolicLink_localFileArtifact() throws Exception { // arrange ActionInputMap inputs = new ActionInputMap(1); Artifact localArtifact = createLocalArtifact("local-file", "local contents", inputs); @@ -403,7 +409,7 @@ public void createSymbolicLink_localFileArtifact() throws IOException { } @Test - public void createSymbolicLink_remoteFileArtifact() throws IOException { + public void createSymbolicLink_remoteFileArtifact() throws Exception { // arrange ActionInputMap inputs = new ActionInputMap(1); Artifact remoteArtifact = createRemoteArtifact("remote-file", "remote contents", inputs); @@ -440,7 +446,7 @@ public void createSymbolicLink_remoteFileArtifact() throws IOException { } @Test - public void createSymbolicLink_localTreeArtifact() throws IOException { + public void createSymbolicLink_localTreeArtifact() throws Exception { // arrange ActionInputMap inputs = new ActionInputMap(1); ImmutableMap contentMap = @@ -473,7 +479,7 @@ public void createSymbolicLink_localTreeArtifact() throws IOException { } @Test - public void createSymbolicLink_remoteTreeArtifact() throws IOException { + public void createSymbolicLink_remoteTreeArtifact() throws Exception { // arrange ActionInputMap inputs = new ActionInputMap(1); ImmutableMap contentMap = @@ -511,7 +517,7 @@ public void createSymbolicLink_remoteTreeArtifact() throws IOException { } @Test - public void createSymbolicLink_unresolvedSymlink() throws IOException { + public void createSymbolicLink_unresolvedSymlink() throws Exception { // arrange ActionInputMap inputs = new ActionInputMap(1); SpecialArtifact outputArtifact = 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 12e8de9fe9e28f..61a16d74b33e13 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 @@ -128,6 +128,9 @@ /** Tests for {@link RemoteExecutionService}. */ @RunWith(JUnit4.class) public class RemoteExecutionServiceTest { + private static final RemoteOutputChecker DUMMY_REMOTE_OUTPUT_CHECKER = + new RemoteOutputChecker( + new JavaClock(), "build", /* downloadToplevel= */ false, ImmutableList.of()); @Rule public final RxNoGlobalErrorsRule rxNoGlobalErrorsRule = new RxNoGlobalErrorsRule(); private final DigestUtil digestUtil = @@ -2157,6 +2160,7 @@ private RemoteExecutionService newRemoteExecutionService(RemoteOptions remoteOpt cache, executor, tempPathGenerator, - null); + null, + DUMMY_REMOTE_OUTPUT_CHECKER); } } 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 318f1bd87e918e..22c5ff7bb04a7b 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 @@ -104,6 +104,9 @@ /** Tests for {@link RemoteSpawnCache}. */ @RunWith(JUnit4.class) public class RemoteSpawnCacheTest { + private static final RemoteOutputChecker DUMMY_REMOTE_OUTPUT_CHECKER = + new RemoteOutputChecker( + new JavaClock(), "build", /* downloadToplevel= */ false, ImmutableList.of()); private static final ArtifactExpander SIMPLE_ARTIFACT_EXPANDER = (artifact, output) -> output.add(artifact); @@ -236,7 +239,7 @@ private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { new RemoteExecutionService( directExecutor(), reporter, - /*verboseFailures=*/ true, + /* verboseFailures= */ true, execRoot, remotePathResolver, BUILD_REQUEST_ID, @@ -246,7 +249,8 @@ private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { remoteCache, null, tempPathGenerator, - /* captureCorruptedOutputsDir= */ null)); + /* captureCorruptedOutputsDir= */ null, + DUMMY_REMOTE_OUTPUT_CHECKER)); return new RemoteSpawnCache(execRoot, options, /* verboseFailures=*/ true, service); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index ca4e6ec2d3a01f..e7e22a90ead9f4 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -128,6 +128,9 @@ /** Tests for {@link com.google.devtools.build.lib.remote.RemoteSpawnRunner} */ @RunWith(JUnit4.class) public class RemoteSpawnRunnerTest { + private static final RemoteOutputChecker DUMMY_REMOTE_OUTPUT_CHECKER = + new RemoteOutputChecker( + new JavaClock(), "build", /* downloadToplevel= */ false, ImmutableList.of()); private final Reporter reporter = new Reporter(new EventBus()); private static final ImmutableMap NO_CACHE = @@ -1037,7 +1040,7 @@ private void testParamFilesAreMaterializedForFlag(String flag) throws Exception new RemoteExecutionService( directExecutor(), reporter, - /*verboseFailures=*/ true, + /* verboseFailures= */ true, execRoot, RemotePathResolver.createDefault(execRoot), "build-req-id", @@ -1047,14 +1050,15 @@ private void testParamFilesAreMaterializedForFlag(String flag) throws Exception cache, executor, tempPathGenerator, - /* captureCorruptedOutputsDir= */ null); + /* captureCorruptedOutputsDir= */ null, + DUMMY_REMOTE_OUTPUT_CHECKER); RemoteSpawnRunner runner = new RemoteSpawnRunner( execRoot, remoteOptions, executionOptions, true, - /*cmdlineReporter=*/ null, + /* cmdlineReporter= */ null, retryService, logDir, remoteExecutionService); @@ -1573,7 +1577,7 @@ private RemoteSpawnRunner newSpawnRunner( new RemoteExecutionService( directExecutor(), reporter, - /*verboseFailures=*/ true, + /* verboseFailures= */ true, execRoot, remotePathResolver, "build-req-id", @@ -1583,7 +1587,8 @@ private RemoteSpawnRunner newSpawnRunner( cache, executor, tempPathGenerator, - /*captureCorruptedOutputsDir=*/ null)); + /* captureCorruptedOutputsDir= */ null, + DUMMY_REMOTE_OUTPUT_CHECKER)); return new RemoteSpawnRunner( execRoot, diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java index 11a14cba5a7ff2..86c7faf2e88c26 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java @@ -126,6 +126,9 @@ /** Tests for {@link RemoteSpawnRunner} in combination with {@link GrpcRemoteExecutor}. */ @RunWith(JUnit4.class) public class RemoteSpawnRunnerWithGrpcRemoteExecutorTest { + private static final RemoteOutputChecker DUMMY_REMOTE_OUTPUT_CHECKER = + new RemoteOutputChecker( + new JavaClock(), "build", /* downloadToplevel= */ false, ImmutableList.of()); private static final DigestUtil DIGEST_UTIL = new DigestUtil(SyscallCache.NO_CACHE, DigestHashFunction.SHA256); @@ -309,7 +312,7 @@ public int maxConcurrency() { new RemoteExecutionService( directExecutor(), reporter, - /*verboseFailures=*/ true, + /* verboseFailures= */ true, execRoot, RemotePathResolver.createDefault(execRoot), "build-req-id", @@ -319,7 +322,8 @@ public int maxConcurrency() { remoteCache, executor, tempPathGenerator, - /* captureCorruptedOutputsDir= */ null); + /* captureCorruptedOutputsDir= */ null, + DUMMY_REMOTE_OUTPUT_CHECKER); client = new RemoteSpawnRunner( execRoot,