From 4d900ceea12919ad62012830a95e51f9ec1a48bb Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Fri, 27 May 2022 22:46:12 +0200 Subject: [PATCH] [5.2] Remote: Fix a bug that outputs of actions tagged with no-remote are u... (#15453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remote: Fix a bug that outputs of actions tagged with no-remote are u… …ploaded to remote cache when remote execution is enabled. Fixes https://github.com/bazelbuild/bazel/issues/14900. Also fixes an issue that action result from just remotely executed action is not saved to disk cache. The root cause is the action result is inlined in the execution response hence not downloaded through remote cache, hence not saved to disk cache. This results in the second build misses the disk cache, but it can still hit the remote cache and fill the disk cache. The third build can hit disk cache. Closes #15212. PiperOrigin-RevId: 441426469 * Fix test Co-authored-by: Chenchu K --- .../ByteStreamBuildEventArtifactUploader.java | 4 +- .../build/lib/remote/RemoteAction.java | 134 +++++++++++++ .../lib/remote/RemoteExecutionService.java | 176 ++++++------------ .../build/lib/remote/RemoteSpawnCache.java | 1 - .../build/lib/remote/RemoteSpawnRunner.java | 1 - .../common/RemoteActionExecutionContext.java | 85 ++++++--- .../SimpleRemoteActionExecutionContext.java | 56 ------ .../remote/disk/DiskAndRemoteCacheClient.java | 50 +++-- .../remote/RemoteExecutionServiceTest.java | 1 - .../lib/remote/RemoteSpawnCacheTest.java | 1 - .../bazel/remote/remote_execution_test.sh | 81 ++++++-- 11 files changed, 355 insertions(+), 235 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java delete mode 100644 src/main/java/com/google/devtools/build/lib/remote/common/SimpleRemoteActionExecutionContext.java diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index ae57825d11a2bb..0e3b79cedf070f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.Step; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.vfs.Path; @@ -271,7 +272,8 @@ private Single upload(Set files) { RequestMetadata metadata = TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "bes-upload", null); - RemoteActionExecutionContext context = RemoteActionExecutionContext.createForBES(metadata); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata); + context.setStep(Step.UPLOAD_BES_FILES); return Single.using( remoteCache::retain, diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java new file mode 100644 index 00000000000000..e680ea484d818c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteAction.java @@ -0,0 +1,134 @@ +// Copyright 2022 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 build.bazel.remote.execution.v2.Action; +import build.bazel.remote.execution.v2.Command; +import build.bazel.remote.execution.v2.Digest; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.ForbiddenActionInputException; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; +import com.google.devtools.build.lib.remote.common.NetworkTime; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; +import com.google.devtools.build.lib.remote.common.RemotePathResolver; +import com.google.devtools.build.lib.remote.merkletree.MerkleTree; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.io.IOException; +import java.util.SortedMap; + +/** A value class representing an action which can be executed remotely. */ +public class RemoteAction { + + private final Spawn spawn; + private final SpawnExecutionContext spawnExecutionContext; + private final RemoteActionExecutionContext remoteActionExecutionContext; + private final RemotePathResolver remotePathResolver; + private final MerkleTree merkleTree; + private final Digest commandHash; + private final Command command; + private final Action action; + private final ActionKey actionKey; + + RemoteAction( + Spawn spawn, + SpawnExecutionContext spawnExecutionContext, + RemoteActionExecutionContext remoteActionExecutionContext, + RemotePathResolver remotePathResolver, + MerkleTree merkleTree, + Digest commandHash, + Command command, + Action action, + ActionKey actionKey) { + this.spawn = spawn; + this.spawnExecutionContext = spawnExecutionContext; + this.remoteActionExecutionContext = remoteActionExecutionContext; + this.remotePathResolver = remotePathResolver; + this.merkleTree = merkleTree; + this.commandHash = commandHash; + this.command = command; + this.action = action; + this.actionKey = actionKey; + } + + public RemoteActionExecutionContext getRemoteActionExecutionContext() { + return remoteActionExecutionContext; + } + + public SpawnExecutionContext getSpawnExecutionContext() { + return spawnExecutionContext; + } + + /** Returns the {@link Spawn} that owns this action. */ + public Spawn getSpawn() { + return spawn; + } + + /** + * Returns the sum of file sizes plus protobuf sizes used to represent the inputs of this action. + */ + public long getInputBytes() { + return merkleTree.getInputBytes(); + } + + /** Returns the number of input files of this action. */ + public long getInputFiles() { + return merkleTree.getInputFiles(); + } + + /** Returns the id this is action. */ + public String getActionId() { + return actionKey.getDigest().getHash(); + } + + /** Returns the {@link ActionKey} of this action. */ + public ActionKey getActionKey() { + return actionKey; + } + + /** Returns underlying {@link Action} of this remote action. */ + public Action getAction() { + return action; + } + + public Digest getCommandHash() { + return commandHash; + } + + public Command getCommand() { + return command; + } + + public MerkleTree getMerkleTree() { + return merkleTree; + } + + /** + * Returns a {@link SortedMap} which maps from input paths for remote action to {@link + * ActionInput}. + */ + public SortedMap getInputMap() + throws IOException, ForbiddenActionInputException { + return remotePathResolver.getInputMapping(spawnExecutionContext); + } + + /** + * Returns the {@link NetworkTime} instance used to measure the network time during the action + * execution. + */ + public NetworkTime getNetworkTime() { + return remoteActionExecutionContext.getNetworkTime(); + } +} 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 063ff493715393..62fecc7a92cee5 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 @@ -67,7 +67,6 @@ import com.google.common.eventbus.Subscribe; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; @@ -96,10 +95,10 @@ import com.google.devtools.build.lib.remote.RemoteExecutionService.ActionResultMetadata.FileMetadata; import com.google.devtools.build.lib.remote.RemoteExecutionService.ActionResultMetadata.SymlinkMetadata; import com.google.devtools.build.lib.remote.common.BulkTransferException; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.OperationObserver; import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.Step; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult; import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; @@ -254,89 +253,6 @@ static Command buildCommand( return command.build(); } - /** A value class representing an action which can be executed remotely. */ - public static class RemoteAction { - private final Spawn spawn; - private final SpawnExecutionContext spawnExecutionContext; - private final RemoteActionExecutionContext remoteActionExecutionContext; - private final RemotePathResolver remotePathResolver; - private final MerkleTree merkleTree; - private final Digest commandHash; - private final Command command; - private final Action action; - private final ActionKey actionKey; - - RemoteAction( - Spawn spawn, - SpawnExecutionContext spawnExecutionContext, - RemoteActionExecutionContext remoteActionExecutionContext, - RemotePathResolver remotePathResolver, - MerkleTree merkleTree, - Digest commandHash, - Command command, - Action action, - ActionKey actionKey) { - this.spawn = spawn; - this.spawnExecutionContext = spawnExecutionContext; - this.remoteActionExecutionContext = remoteActionExecutionContext; - this.remotePathResolver = remotePathResolver; - this.merkleTree = merkleTree; - this.commandHash = commandHash; - this.command = command; - this.action = action; - this.actionKey = actionKey; - } - - public RemoteActionExecutionContext getRemoteActionExecutionContext() { - return remoteActionExecutionContext; - } - - /** Returns the {@link ActionExecutionMetadata} that owns this action. */ - public ActionExecutionMetadata getOwner() { - return spawn.getResourceOwner(); - } - - /** - * Returns the sum of file sizes plus protobuf sizes used to represent the inputs of this - * action. - */ - public long getInputBytes() { - return merkleTree.getInputBytes(); - } - - /** Returns the number of input files of this action. */ - public long getInputFiles() { - return merkleTree.getInputFiles(); - } - - /** Returns the id this is action. */ - public String getActionId() { - return actionKey.getDigest().getHash(); - } - - /** Returns underlying {@link Action} of this remote action. */ - public Action getAction() { - return action; - } - - /** - * Returns a {@link SortedMap} which maps from input paths for remote action to {@link - * ActionInput}. - */ - public SortedMap getInputMap() - throws IOException, ForbiddenActionInputException { - return remotePathResolver.getInputMapping(spawnExecutionContext); - } - - /** - * Returns the {@link NetworkTime} instance used to measure the network time during the action - * execution. - */ - public NetworkTime getNetworkTime() { - return remoteActionExecutionContext.getNetworkTime(); - } - } - private static boolean useRemoteCache(RemoteOptions options) { return !isNullOrEmpty(options.remoteCache) || !isNullOrEmpty(options.remoteExecutor); } @@ -615,11 +531,15 @@ public int hashCode() { @Nullable public RemoteActionResult lookupCache(RemoteAction action) throws IOException, InterruptedException { - checkState(shouldAcceptCachedResult(action.spawn), "spawn doesn't accept cached result"); + checkState(shouldAcceptCachedResult(action.getSpawn()), "spawn doesn't accept cached result"); + + action.getRemoteActionExecutionContext().setStep(Step.CHECK_ACTION_CACHE); CachedActionResult cachedActionResult = remoteCache.downloadActionResult( - action.remoteActionExecutionContext, action.actionKey, /* inlineOutErr= */ false); + action.getRemoteActionExecutionContext(), + action.getActionKey(), + /* inlineOutErr= */ false); if (cachedActionResult == null) { return null; @@ -639,12 +559,12 @@ private ListenableFuture downloadFile(RemoteAction action, FileMet try { ListenableFuture future = remoteCache.downloadFile( - action.remoteActionExecutionContext, + action.getRemoteActionExecutionContext(), remotePathResolver.localPathToOutputPath(file.path()), toTmpDownloadPath(file.path()), file.digest(), new RemoteCache.DownloadProgressReporter( - action.spawnExecutionContext::report, + action.getSpawnExecutionContext()::report, remotePathResolver.localPathToOutputPath(file.path()), file.digest().getSizeBytes())); return transform(future, (d) -> file, directExecutor()); @@ -763,8 +683,8 @@ private void createSymlinks(Iterable symlinks) throws IOExcepti private void injectRemoteArtifact( RemoteAction action, Artifact output, ActionResultMetadata metadata) throws IOException { - RemoteActionExecutionContext context = action.remoteActionExecutionContext; - MetadataInjector metadataInjector = action.spawnExecutionContext.getMetadataInjector(); + RemoteActionExecutionContext context = action.getRemoteActionExecutionContext(); + MetadataInjector metadataInjector = action.getSpawnExecutionContext().getMetadataInjector(); Path path = remotePathResolver.outputPathToLocalPath(output); if (output.isTreeArtifact()) { DirectoryMetadata directory = metadata.directory(path); @@ -947,7 +867,8 @@ ActionResultMetadata parseActionResultMetadata(RemoteAction action, RemoteAction dirMetadataDownloads.put( remotePathResolver.outputPathToLocalPath(dir.getPath()), Futures.transformAsync( - remoteCache.downloadBlob(action.remoteActionExecutionContext, dir.getTreeDigest()), + remoteCache.downloadBlob( + action.getRemoteActionExecutionContext(), dir.getTreeDigest()), (treeBytes) -> immediateFuture(Tree.parseFrom(treeBytes, ExtensionRegistry.getEmptyRegistry())), directExecutor())); @@ -1005,6 +926,8 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re checkState(!shutdown.get(), "shutdown"); checkNotNull(remoteCache, "remoteCache can't be null"); + action.getRemoteActionExecutionContext().setStep(Step.DOWNLOAD_OUTPUTS); + ActionResultMetadata metadata; try (SilentCloseable c = Profiler.instance().profile("Remote.parseActionResultMetadata")) { metadata = parseActionResultMetadata(action, result); @@ -1012,8 +935,8 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re if (result.success()) { // Check that all mandatory outputs are created. - for (ActionInput output : action.spawn.getOutputFiles()) { - if (action.spawn.isMandatoryOutput(output)) { + for (ActionInput output : action.getSpawn().getOutputFiles()) { + if (action.getSpawn().isMandatoryOutput(output)) { // Don't check output that is tree artifact since spawn could generate nothing under that // directory. Remote server typically doesn't create directory ahead of time resulting in // empty tree artifact missing from action cache entry. @@ -1027,16 +950,28 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re && !metadata.symlinks.containsKey(localPath)) { throw new IOException( "Invalid action cache entry " - + action.actionKey.getDigest().getHash() + + action.getActionKey().getDigest().getHash() + ": expected output " + prettyPrint(output) + " does not exist."); } } } + + // When downloading outputs from just remotely executed action, the action result comes from + // Execution response which means, if disk cache is enabled, action result hasn't been + // uploaded to it. Upload action result to disk cache here so next build could hit it. + if (useDiskCache(remoteOptions) + && action.getRemoteActionExecutionContext().getExecuteResponse() != null) { + getFromFuture( + remoteCache.uploadActionResult( + action.getRemoteActionExecutionContext(), + action.getActionKey(), + result.actionResult)); + } } - FileOutErr outErr = action.spawnExecutionContext.getFileOutErr(); + FileOutErr outErr = action.getSpawnExecutionContext().getFileOutErr(); ImmutableList.Builder> downloadsBuilder = ImmutableList.builder(); @@ -1045,7 +980,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re shouldDownloadAllSpawnOutputs( remoteOutputsMode, /* exitCode = */ result.getExitCode(), - hasFilesToDownload(action.spawn.getOutputFiles(), filesToDownload)); + hasFilesToDownload(action.getSpawn().getOutputFiles(), filesToDownload)); if (downloadOutputs) { HashSet queuedFilePaths = new HashSet<>(); @@ -1080,7 +1015,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re FileOutErr tmpOutErr = outErr.childOutErr(); List> outErrDownloads = remoteCache.downloadOutErr( - action.remoteActionExecutionContext, result.actionResult, tmpOutErr); + action.getRemoteActionExecutionContext(), result.actionResult, tmpOutErr); for (ListenableFuture future : outErrDownloads) { downloadsBuilder.add(transform(future, (v) -> null, directExecutor())); } @@ -1100,7 +1035,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re // Ensure that we are the only ones writing to the output files when using the dynamic spawn // strategy. - action.spawnExecutionContext.lockOutputFiles(); + action.getSpawnExecutionContext().lockOutputFiles(); if (downloadOutputs) { moveOutputsToFinalLocation(downloads); @@ -1121,9 +1056,9 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re } else { ActionInput inMemoryOutput = null; Digest inMemoryOutputDigest = null; - PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.spawn); + PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.getSpawn()); - for (ActionInput output : action.spawn.getOutputFiles()) { + for (ActionInput output : action.getSpawn().getOutputFiles()) { if (inMemoryOutputPath != null && output.getExecPath().equals(inMemoryOutputPath)) { Path localPath = remotePathResolver.outputPathToLocalPath(output); FileMetadata m = metadata.file(localPath); @@ -1143,7 +1078,8 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re try (SilentCloseable c = Profiler.instance().profile("Remote.downloadInMemoryOutput")) { if (inMemoryOutput != null) { ListenableFuture inMemoryOutputDownload = - remoteCache.downloadBlob(action.remoteActionExecutionContext, inMemoryOutputDigest); + remoteCache.downloadBlob( + action.getRemoteActionExecutionContext(), inMemoryOutputDigest); waitForBulkTransfer( ImmutableList.of(inMemoryOutputDownload), /* cancelRemainingOnInterrupt=*/ true); byte[] data = getFromFuture(inMemoryOutputDownload); @@ -1169,9 +1105,9 @@ private Single buildUploadManifestAsync( () -> { ImmutableList.Builder outputFiles = ImmutableList.builder(); // Check that all mandatory outputs are created. - for (ActionInput outputFile : action.spawn.getOutputFiles()) { + for (ActionInput outputFile : action.getSpawn().getOutputFiles()) { Path localPath = execRoot.getRelative(outputFile.getExecPath()); - if (action.spawn.isMandatoryOutput(outputFile) && !localPath.exists()) { + if (action.getSpawn().isMandatoryOutput(outputFile) && !localPath.exists()) { throw new IOException( "Expected output " + prettyPrint(outputFile) + " was not created locally."); } @@ -1182,11 +1118,11 @@ private Single buildUploadManifestAsync( remoteOptions, digestUtil, remotePathResolver, - action.actionKey, - action.action, - action.command, + action.getActionKey(), + action.getAction(), + action.getCommand(), outputFiles.build(), - action.spawnExecutionContext.getFileOutErr(), + action.getSpawnExecutionContext().getFileOutErr(), spawnResult.exitCode()); }); } @@ -1211,7 +1147,7 @@ UploadManifest buildUploadManifest(RemoteAction action, SpawnResult spawnResult) public void uploadOutputs(RemoteAction action, SpawnResult spawnResult) throws InterruptedException, ExecException { checkState(!shutdown.get(), "shutdown"); - checkState(shouldUploadLocalResults(action.spawn), "spawn shouldn't upload local result"); + checkState(shouldUploadLocalResults(action.getSpawn()), "spawn shouldn't upload local result"); checkState( SpawnResult.Status.SUCCESS.equals(spawnResult.status()) && spawnResult.exitCode() == 0, "shouldn't upload outputs of failed local action"); @@ -1275,15 +1211,17 @@ private void reportUploadError(Throwable error) { public void uploadInputsIfNotPresent(RemoteAction action, boolean force) throws IOException, InterruptedException { checkState(!shutdown.get(), "shutdown"); - checkState(mayBeExecutedRemotely(action.spawn), "spawn can't be executed remotely"); + checkState(mayBeExecutedRemotely(action.getSpawn()), "spawn can't be executed remotely"); + + action.getRemoteActionExecutionContext().setStep(Step.UPLOAD_INPUTS); RemoteExecutionCache remoteExecutionCache = (RemoteExecutionCache) remoteCache; // Upload the command and all the inputs into the remote cache. Map additionalInputs = Maps.newHashMapWithExpectedSize(2); - additionalInputs.put(action.actionKey.getDigest(), action.action); - additionalInputs.put(action.commandHash, action.command); + additionalInputs.put(action.getActionKey().getDigest(), action.getAction()); + additionalInputs.put(action.getCommandHash(), action.getCommand()); remoteExecutionCache.ensureInputsPresent( - action.remoteActionExecutionContext, action.merkleTree, additionalInputs, force); + action.getRemoteActionExecutionContext(), action.getMerkleTree(), additionalInputs, force); } /** @@ -1296,12 +1234,14 @@ public RemoteActionResult executeRemotely( RemoteAction action, boolean acceptCachedResult, OperationObserver observer) throws IOException, InterruptedException { checkState(!shutdown.get(), "shutdown"); - checkState(mayBeExecutedRemotely(action.spawn), "spawn can't be executed remotely"); + checkState(mayBeExecutedRemotely(action.getSpawn()), "spawn can't be executed remotely"); + + action.getRemoteActionExecutionContext().setStep(Step.EXECUTE_REMOTELY); ExecuteRequest.Builder requestBuilder = ExecuteRequest.newBuilder() .setInstanceName(remoteOptions.remoteInstanceName) - .setActionDigest(action.actionKey.getDigest()) + .setActionDigest(action.getActionKey().getDigest()) .setSkipCacheLookup(!acceptCachedResult); if (remoteOptions.remoteResultCachePriority != 0) { requestBuilder @@ -1315,7 +1255,9 @@ public RemoteActionResult executeRemotely( ExecuteRequest request = requestBuilder.build(); ExecuteResponse reply = - remoteExecutor.executeRemotely(action.remoteActionExecutionContext, request, observer); + remoteExecutor.executeRemotely(action.getRemoteActionExecutionContext(), request, observer); + + action.getRemoteActionExecutionContext().setExecuteResponse(reply); return RemoteActionResult.createFromResponse(reply); } @@ -1344,7 +1286,7 @@ public ServerLogs maybeDownloadServerLogs(RemoteAction action, ExecuteResponse r serverLogs.logCount++; getFromFuture( remoteCache.downloadFile( - action.remoteActionExecutionContext, + action.getRemoteActionExecutionContext(), serverLogs.lastLogPath, e.getValue().getDigest())); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 3222d6a1ecdc27..3ce198104d2e9d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; -import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction; import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; 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 d3f0c008726ff6..8440220704e8f2 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 @@ -56,7 +56,6 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; -import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction; import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; import com.google.devtools.build.lib.remote.RemoteExecutionService.ServerLogs; import com.google.devtools.build.lib.remote.common.BulkTransferException; diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java index c27eeb90463fbe..78cac3980c2a71 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java @@ -13,37 +13,71 @@ // limitations under the License. package com.google.devtools.build.lib.remote.common; +import build.bazel.remote.execution.v2.ExecuteResponse; import build.bazel.remote.execution.v2.RequestMetadata; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.Spawn; import javax.annotation.Nullable; /** A context that provide remote execution related information for executing an action remotely. */ -public interface RemoteActionExecutionContext { - /** The type of the context. */ - enum Type { - REMOTE_EXECUTION, - BUILD_EVENT_SERVICE, +public class RemoteActionExecutionContext { + /** The current step of the context. */ + public enum Step { + INIT, + CHECK_ACTION_CACHE, + UPLOAD_INPUTS, + EXECUTE_REMOTELY, + UPLOAD_OUTPUTS, + DOWNLOAD_OUTPUTS, + UPLOAD_BES_FILES, } - /** Returns the {@link Type} of the context. */ - Type getType(); + @Nullable private final Spawn spawn; + private final RequestMetadata requestMetadata; + private final NetworkTime networkTime; + + @Nullable private ExecuteResponse executeResponse; + private Step step; + + private RemoteActionExecutionContext( + @Nullable Spawn spawn, RequestMetadata requestMetadata, NetworkTime networkTime) { + this.spawn = spawn; + this.requestMetadata = requestMetadata; + this.networkTime = networkTime; + this.step = Step.INIT; + } + + /** Returns current {@link Step} of the context. */ + public Step getStep() { + return step; + } + + /** Sets current {@link Step} of the context. */ + public void setStep(Step step) { + this.step = step; + } /** Returns the {@link Spawn} of the action being executed or {@code null}. */ @Nullable - Spawn getSpawn(); + public Spawn getSpawn() { + return spawn; + } /** Returns the {@link RequestMetadata} for the action being executed. */ - RequestMetadata getRequestMetadata(); + public RequestMetadata getRequestMetadata() { + return requestMetadata; + } /** * Returns the {@link NetworkTime} instance used to measure the network time during the action * execution. */ - NetworkTime getNetworkTime(); + public NetworkTime getNetworkTime() { + return networkTime; + } @Nullable - default ActionExecutionMetadata getSpawnOwner() { + public ActionExecutionMetadata getSpawnOwner() { Spawn spawn = getSpawn(); if (spawn == null) { return null; @@ -52,23 +86,26 @@ default ActionExecutionMetadata getSpawnOwner() { return spawn.getResourceOwner(); } - /** Creates a {@link SimpleRemoteActionExecutionContext} with given {@link RequestMetadata}. */ - static RemoteActionExecutionContext create(RequestMetadata metadata) { - return new SimpleRemoteActionExecutionContext( - /*type=*/ Type.REMOTE_EXECUTION, /*spawn=*/ null, metadata, new NetworkTime()); + public void setExecuteResponse(@Nullable ExecuteResponse executeResponse) { + this.executeResponse = executeResponse; + } + + @Nullable + public ExecuteResponse getExecuteResponse() { + return executeResponse; + } + + /** Creates a {@link RemoteActionExecutionContext} with given {@link RequestMetadata}. */ + public static RemoteActionExecutionContext create(RequestMetadata metadata) { + return new RemoteActionExecutionContext(/*spawn=*/ null, metadata, new NetworkTime()); } /** - * Creates a {@link SimpleRemoteActionExecutionContext} with given {@link Spawn} and {@link + * Creates a {@link RemoteActionExecutionContext} with given {@link Spawn} and {@link * RequestMetadata}. */ - static RemoteActionExecutionContext create(@Nullable Spawn spawn, RequestMetadata metadata) { - return new SimpleRemoteActionExecutionContext( - /*type=*/ Type.REMOTE_EXECUTION, spawn, metadata, new NetworkTime()); - } - - static RemoteActionExecutionContext createForBES(RequestMetadata metadata) { - return new SimpleRemoteActionExecutionContext( - /*type=*/ Type.BUILD_EVENT_SERVICE, /*spawn=*/ null, metadata, new NetworkTime()); + public static RemoteActionExecutionContext create( + @Nullable Spawn spawn, RequestMetadata metadata) { + return new RemoteActionExecutionContext(spawn, metadata, new NetworkTime()); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/SimpleRemoteActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/remote/common/SimpleRemoteActionExecutionContext.java deleted file mode 100644 index 1b099457962490..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/remote/common/SimpleRemoteActionExecutionContext.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2020 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.common; - -import build.bazel.remote.execution.v2.RequestMetadata; -import com.google.devtools.build.lib.actions.Spawn; -import javax.annotation.Nullable; - -/** A {@link RemoteActionExecutionContext} implementation */ -public class SimpleRemoteActionExecutionContext implements RemoteActionExecutionContext { - - private final Type type; - private final Spawn spawn; - private final RequestMetadata requestMetadata; - private final NetworkTime networkTime; - - public SimpleRemoteActionExecutionContext( - Type type, Spawn spawn, RequestMetadata requestMetadata, NetworkTime networkTime) { - this.type = type; - this.spawn = spawn; - this.requestMetadata = requestMetadata; - this.networkTime = networkTime; - } - - @Override - public Type getType() { - return type; - } - - @Nullable - @Override - public Spawn getSpawn() { - return spawn; - } - - @Override - public RequestMetadata getRequestMetadata() { - return requestMetadata; - } - - @Override - public NetworkTime getNetworkTime() { - return networkTime; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java index e5a936cf66d8bc..c337a5ebe3cdf1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java @@ -25,6 +25,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.remote.common.LazyFileOutputStream; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.Step; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.vfs.Path; @@ -55,7 +56,11 @@ public DiskAndRemoteCacheClient( public ListenableFuture uploadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, ActionResult actionResult) { ListenableFuture future = diskCache.uploadActionResult(context, actionKey, actionResult); - if (shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) { + // Only upload action result to remote cache if we are uploading local outputs. This method + // could be called when we are downloading outputs from remote executor if disk cache is enabled + // because we want to upload the action result to it. + if (context.getStep() == Step.UPLOAD_OUTPUTS + && shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) { future = Futures.transformAsync( future, @@ -74,15 +79,20 @@ public void close() { @Override public ListenableFuture uploadFile( RemoteActionExecutionContext context, Digest digest, Path file) { - // For BES upload, we only upload to the remote cache. - if (context.getType() == RemoteActionExecutionContext.Type.BUILD_EVENT_SERVICE) { + RemoteActionExecutionContext.Step step = context.getStep(); + + // For UPLOAD_INPUTS, only upload to remote cache. + if (step == Step.UPLOAD_INPUTS) { return remoteCache.uploadFile(context, digest, file); } - ListenableFuture future = diskCache.uploadFile(context, digest, file); + // For UPLOAD_BES_FILES, only upload to remote cache. + if (step == Step.UPLOAD_BES_FILES) { + return remoteCache.uploadFile(context, digest, file); + } - if (options.isRemoteExecutionEnabled() - || shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) { + ListenableFuture future = diskCache.uploadFile(context, digest, file); + if (shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) { future = Futures.transformAsync( future, v -> remoteCache.uploadFile(context, digest, file), directExecutor()); @@ -93,9 +103,20 @@ public ListenableFuture uploadFile( @Override public ListenableFuture uploadBlob( RemoteActionExecutionContext context, Digest digest, ByteString data) { + RemoteActionExecutionContext.Step step = context.getStep(); + + // For UPLOAD_INPUTS, only upload to remote cache. + if (step == Step.UPLOAD_INPUTS) { + return remoteCache.uploadBlob(context, digest, data); + } + + // For BES upload, only upload to the remote cache. + if (step == Step.UPLOAD_BES_FILES) { + return remoteCache.uploadBlob(context, digest, data); + } + ListenableFuture future = diskCache.uploadBlob(context, digest, data); - if (options.isRemoteExecutionEnabled() - || shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) { + if (shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) { future = Futures.transformAsync( future, v -> remoteCache.uploadBlob(context, digest, data), directExecutor()); @@ -106,17 +127,19 @@ public ListenableFuture uploadBlob( @Override public ListenableFuture> findMissingDigests( RemoteActionExecutionContext context, Iterable digests) { - // If remote execution, find missing digests should only look at + RemoteActionExecutionContext.Step step = context.getStep(); + + // For UPLOAD_INPUTS, find missing digests should only look at // the remote cache, not the disk cache because the remote executor only // has access to the remote cache, not the disk cache. // Also, the DiskCache always returns all digests as missing // and we don't want to transfer all the files all the time. - if (options.isRemoteExecutionEnabled()) { + if (step == Step.UPLOAD_INPUTS) { return remoteCache.findMissingDigests(context, digests); } - // For BES upload, we only check the remote cache. - if (context.getType() == RemoteActionExecutionContext.Type.BUILD_EVENT_SERVICE) { + // For UPLOAD_BES_FILES, we only check the remote cache. + if (step == Step.UPLOAD_BES_FILES) { return remoteCache.findMissingDigests(context, digests); } @@ -169,7 +192,8 @@ public ListenableFuture downloadBlob( final OutputStream tempOut; tempOut = new LazyFileOutputStream(tempPath); - if (options.isRemoteExecutionEnabled() + // Always download outputs for just remotely executed action. + if (context.getExecuteResponse() != null || shouldAcceptCachedResultFromRemoteCache(options, context.getSpawn())) { ListenableFuture download = closeStreamOnError(remoteCache.downloadBlob(context, digest, tempOut), tempOut); 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 dad364ca44b1be..9d39facdc6a0cc 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 @@ -80,7 +80,6 @@ import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.exec.util.SpawnBuilder; -import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction; import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; 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 3c97b692edb029..7a153d64bf1c30 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 @@ -62,7 +62,6 @@ import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.FakeOwner; -import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction; import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 8731a7b1a77852..037a386469dda9 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -88,7 +88,8 @@ EOF || fail "Failed to build //a:foo with remote cache" } -function test_remote_grpc_via_unix_socket_proxy() { +# TODO(b/211478955): Deflake and re-enable. +function DISABLED_test_remote_grpc_via_unix_socket_proxy() { case "$PLATFORM" in darwin|freebsd|linux|openbsd) ;; @@ -126,7 +127,8 @@ EOF rmdir "${socket_dir}" } -function test_remote_grpc_via_unix_socket_direct() { +# TODO(b/211478955): Deflake and re-enable. +function DISABLED_test_remote_grpc_via_unix_socket_direct() { case "$PLATFORM" in darwin|freebsd|linux|openbsd) ;; @@ -2105,7 +2107,6 @@ function test_combined_disk_remote_exec_with_flag_combinations() { # Should be some disk cache hits, just not remote. "--noremote_accept_cached --incompatible_remote_results_ignore_disk" ) - # for flags in "${testcases[@]}"; do genrule_combined_disk_remote_exec "$flags" @@ -2129,15 +2130,13 @@ function genrule_combined_disk_remote_exec() { # if exist in disk cache or remote cache, don't run remote exec, don't update caches. # [CASE]disk_cache, remote_cache: remote_exec, disk_cache, remote_cache - # 1) notexist notexist run OK - , update + # 1) notexist notexist run OK update, update # 2) notexist exist no run update, no update # 3) exist notexist no run no update, no update # 4) exist exist no run no update, no update # 5) another rule that depends on 4, but run before 5 # Our setup ensures the first 2 columns, our validation checks the last 3. - # NOTE that remote_exec will NOT update the disk cache, we expect the remote - # execution to update the remote_cache and when we pull from the remote cache - # we will then mirror to the disk cache. + # NOTE that remote_exec will UPDATE the disk cache. # # We measure if it was run remotely via the "1 remote." in the output and caches # from the cache hit on the same line. @@ -2167,13 +2166,14 @@ EOF echo "INFO: RUNNING testcase($testcase_flags)" # Case 1) # disk_cache, remote_cache: remote_exec, disk_cache, remote_cache - # notexist notexist run OK - , update + # notexist notexist run OK update, update # # Do a build to populate the disk and remote cache. # Then clean and do another build to validate nothing updates. bazel build $spawn_flags $testcase_flags $remote_exec_flags $grpc_flags $disk_flags //a:test &> $TEST_log \ || fail "CASE 1 Failed to build" + rm -rf ${TEST_TMPDIR}/test_expected echo "Hello world" > ${TEST_TMPDIR}/test_expected expect_log "2 processes: 1 internal, 1 remote." "CASE 1: unexpected action line [[$(grep processes $TEST_log)]]" diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \ @@ -2182,10 +2182,13 @@ EOF disk_action_cache_files="$(count_disk_ac_files "$cache")" remote_action_cache_files="$(count_remote_ac_files)" - [[ "$disk_action_cache_files" == 0 ]] || fail "Expected 0 disk action cache entries, not $disk_action_cache_files" + [[ "$disk_action_cache_files" == 1 ]] || fail "CASE 1: Expected 1 disk action cache entries, not $disk_action_cache_files" # Even though bazel isn't writing the remote action cache, we expect the worker to write one or the # the rest of our tests will fail. - [[ "$remote_action_cache_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_action_cache_files" + [[ "$remote_action_cache_files" == 1 ]] || fail "CASE 1: Expected 1 remote action cache entries, not $remote_action_cache_files" + + rm -rf $cache + mkdir $cache # Case 2) # disk_cache, remote_cache: remote_exec, disk_cache, remote_cache @@ -2202,10 +2205,8 @@ EOF # ensure disk and remote cache populated disk_action_cache_files="$(count_disk_ac_files "$cache")" remote_action_cache_files="$(count_remote_ac_files)" - if [[ "$testcase_flags" != --noremote_accept_cached* ]]; then - [[ "$disk_action_cache_files" == 1 ]] || fail "Expected 1 disk action cache entries, not $disk_action_cache_files" - [[ "$remote_action_cache_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_action_cache_files" - fi + [[ "$disk_action_cache_files" == 1 ]] || fail "CASE 2: Expected 1 disk action cache entries, not $disk_action_cache_files" + [[ "$remote_action_cache_files" == 1 ]] || fail "CASE 2: Expected 1 remote action cache entries, not $remote_action_cache_files" # Case 3) # disk_cache, remote_cache: remote_exec, disk_cache, remote_cache @@ -2222,7 +2223,7 @@ EOF bazel clean bazel build $spawn_flags $testcase_flags $remote_exec_flags $grpc_flags $disk_flags //a:test &> $TEST_log \ || fail "CASE 3 failed to build" - if [[ "$testcase_flags" == --noremote_accept_cached* ]]; then + if [[ "$testcase_flags" == --noremote_accept_cached ]]; then expect_log "2 processes: 1 internal, 1 remote." "CASE 3: unexpected action line [[$(grep processes $TEST_log)]]" else expect_log "2 processes: 1 disk cache hit, 1 internal." "CASE 3: unexpected action line [[$(grep processes $TEST_log)]]" @@ -2236,7 +2237,7 @@ EOF bazel clean bazel build $spawn_flags $testcase_flags $remote_exec_flags $grpc_flags $disk_flags //a:test &> $TEST_log \ || fail "CASE 4 failed to build" - if [[ "$testcase_flags" == --noremote_accept_cached* ]]; then + if [[ "$testcase_flags" == --noremote_accept_cached ]]; then expect_log "2 processes: 1 internal, 1 remote." "CASE 4: unexpected action line [[$(grep processes $TEST_log)]]" else expect_log "2 processes: 1 disk cache hit, 1 internal." "CASE 4: unexpected action line [[$(grep processes $TEST_log)]]" @@ -2255,7 +2256,7 @@ EOF bazel clean bazel build $spawn_flags $testcase_flags --genrule_strategy=remote $remote_exec_flags $grpc_flags $disk_flags //a:test2 &> $TEST_log \ || fail "CASE 5 failed to build //a:test2" - if [[ "$testcase_flags" == --noremote_accept_cached* ]]; then + if [[ "$testcase_flags" == --noremote_accept_cached ]]; then expect_log "3 processes: 1 internal, 2 remote." "CASE 5: unexpected action line [[$(grep processes $TEST_log)]]" else expect_log "3 processes: 1 disk cache hit, 1 internal, 1 remote." "CASE 5: unexpected action line [[$(grep processes $TEST_log)]]" @@ -2263,6 +2264,7 @@ EOF } function test_combined_disk_remote_exec_nocache_tag() { + rm -rf ${TEST_TMPDIR}/test_expected local cache="${TEST_TMPDIR}/disk_cache" local flags=("--disk_cache=$cache" "--remote_cache=grpc://localhost:${worker_port}" @@ -2427,13 +2429,24 @@ EOF rm -rf $cache } -function test_combined_cache_with_no_remote_cache_tag() { +function test_combined_cache_with_no_remote_cache_tag_remote_cache() { # Test that actions with no-remote-cache tag can hit disk cache of a combined cache but # remote cache is disabled. + combined_cache_with_no_remote_cache_tag "--remote_cache=grpc://localhost:${worker_port}" +} + +function test_combined_cache_with_no_remote_cache_tag_remote_execution() { + # Test that actions with no-remote-cache tag can hit disk cache of a combined cache but + # remote cache is disabled. + + combined_cache_with_no_remote_cache_tag "--remote_executor=grpc://localhost:${worker_port}" +} + +function combined_cache_with_no_remote_cache_tag() { local cache="${TEST_TMPDIR}/cache" local disk_flags="--disk_cache=$cache" - local grpc_flags="--remote_cache=grpc://localhost:${worker_port}" + local grpc_flags="$@" mkdir -p a cat > a/BUILD < $TEST_log \ || fail "Failed to build //a:test" expect_not_log "1 remote cache hit" "Should not get cache hit from grpc cache" - expect_log "1 .*-sandbox" "Rebuild target failed" diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \ || fail "Rebuilt target generated different result" } +function test_combined_cache_with_no_remote_tag() { + # Test that outputs of actions tagged with no-remote should not be uploaded + # to remote cache when remote execution is enabled. See + # https://github.com/bazelbuild/bazel/issues/14900. + mkdir -p a + cat > a/BUILD < $TEST_log \ + || fail "Failed to build //a:test" + + remote_ac_files="$(count_remote_ac_files)" + [[ "$remote_ac_files" == 0 ]] || fail "Expected 0 remote action cache entries, not $remote_ac_files" + remote_cas_files="$(count_remote_cas_files)" + [[ "$remote_cas_files" == 0 ]] || fail "Expected 0 remote cas entries, not $remote_cas_files" +} + function test_repo_remote_exec() { # Test that repository_ctx.execute can execute a command remotely.