remote: implement --experimental_remote_download_outputs=toplevel
This implements the second milestone of "Remote builds without the
Bytes" and introduces the "toplevel" option for the
--experimental_remote_download_outputs flag. This mode will only
download outputs of top level targets but not outputs of intermediate
actions.
On a build of Bazel against RBE I am still seeing a 50% speed up
compared to the "all" mode.
Progress towards #6862.
Closes #7984.
PiperOrigin-RevId: 243029119
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index 6af3eb8..a897ffc 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -110,7 +110,12 @@
}
for (BlazeModule module : env.getRuntime().getBlazeModules()) {
- module.afterAnalysis(env, request, buildOptions, analysisResult.getTargetsToBuild());
+ module.afterAnalysis(
+ env,
+ request,
+ buildOptions,
+ analysisResult.getTargetsToBuild(),
+ analysisResult.getAspects());
}
reportTargets(analysisResult);
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 e73a44c..a64c20b 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
@@ -17,7 +17,9 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionContext;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.exec.AbstractSpawnStrategy;
@@ -47,6 +49,7 @@
private final DigestUtil digestUtil;
@Nullable private final Path logDir;
private final AtomicReference<SpawnRunner> fallbackRunner = new AtomicReference<>();
+ private ImmutableSet<Artifact> topLevelOutputs = ImmutableSet.of();
private RemoteActionContextProvider(
CommandEnvironment env,
@@ -99,7 +102,8 @@
buildRequestId,
commandId,
env.getReporter(),
- digestUtil);
+ digestUtil,
+ topLevelOutputs);
return ImmutableList.of(spawnCache);
} else {
RemoteSpawnRunner spawnRunner =
@@ -116,7 +120,8 @@
executor,
retrier,
digestUtil,
- logDir);
+ logDir,
+ topLevelOutputs);
return ImmutableList.of(new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner));
}
}
@@ -166,6 +171,10 @@
return cache;
}
+ void setTopLevelOutputs(ImmutableSet<Artifact> topLevelOutputs) {
+ this.topLevelOutputs = Preconditions.checkNotNull(topLevelOutputs, "topLevelOutputs");
+ }
+
@Override
public void executionPhaseEnding() {
if (cache != null) {
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 90c39fd..9420d8c 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
@@ -19,8 +19,13 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListeningScheduledExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
@@ -39,6 +44,7 @@
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.ServerBuilder;
+import com.google.devtools.build.lib.skyframe.AspectValue;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.AsynchronousFileOutputStream;
@@ -313,6 +319,38 @@
}
}
+ @Override
+ public void afterAnalysis(
+ CommandEnvironment env,
+ BuildRequest request,
+ BuildOptions buildOptions,
+ Iterable<ConfiguredTarget> configuredTargets,
+ ImmutableSet<AspectValue> aspects) {
+ if (remoteOutputsMode != null && remoteOutputsMode.downloadToplevelOutputsOnly()) {
+ Preconditions.checkState(actionContextProvider != null, "actionContextProvider was null");
+ // TODO(buchgr): Consider only storing the action owners instead of the artifacts
+ // Collect all top level output artifacts of regular targets as well as aspects. This
+ // information is used by remote spawn runners to decide whether to download an artifact
+ // if --experimental_remote_download_outputs=toplevel is set
+ ImmutableSet.Builder<Artifact> topLevelOutputsBuilder = ImmutableSet.builder();
+ for (ConfiguredTarget configuredTarget : configuredTargets) {
+ topLevelOutputsBuilder.addAll(
+ TopLevelArtifactHelper.getAllArtifactsToBuild(
+ configuredTarget, request.getTopLevelArtifactContext())
+ .getImportantArtifacts());
+ }
+
+ for (AspectValue aspect : aspects) {
+ topLevelOutputsBuilder.addAll(
+ TopLevelArtifactHelper.getAllArtifactsToBuild(
+ aspect, request.getTopLevelArtifactContext())
+ .getImportantArtifacts());
+ }
+
+ actionContextProvider.setTopLevelOutputs(topLevelOutputsBuilder.build());
+ }
+ }
+
private static void cleanAndCreateRemoteLogsDir(Path logDir) throws AbruptExitException {
try {
// Clean out old logs files.
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 3b1732d..4bd91ae 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
@@ -16,13 +16,18 @@
import static com.google.common.base.Strings.isNullOrEmpty;
import static com.google.devtools.build.lib.remote.util.Utils.createSpawnResult;
import static com.google.devtools.build.lib.remote.util.Utils.getInMemoryOutputPath;
+import static com.google.devtools.build.lib.remote.util.Utils.hasTopLevelOutputs;
+import static com.google.devtools.build.lib.remote.util.Utils.shouldDownloadAllSpawnOutputs;
import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionResult;
import build.bazel.remote.execution.v2.Command;
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.Platform;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -64,6 +69,7 @@
name = {"remote-cache"},
contextType = SpawnCache.class)
final class RemoteSpawnCache implements SpawnCache {
+
private final Path execRoot;
private final RemoteOptions options;
@@ -77,6 +83,14 @@
private final DigestUtil digestUtil;
+ /**
+ * Set of artifacts that are top level outputs
+ *
+ * <p>This set is empty unless {@link RemoteOutputsMode#TOPLEVEL} is specified. If so, this set is
+ * used to decide whether to download an output.
+ */
+ private final ImmutableSet<Artifact> topLevelOutputs;
+
RemoteSpawnCache(
Path execRoot,
RemoteOptions options,
@@ -84,7 +98,8 @@
String buildRequestId,
String commandId,
@Nullable Reporter cmdlineReporter,
- DigestUtil digestUtil) {
+ DigestUtil digestUtil,
+ ImmutableSet<Artifact> topLevelOutputs) {
this.execRoot = execRoot;
this.options = options;
this.remoteCache = remoteCache;
@@ -92,8 +107,10 @@
this.buildRequestId = buildRequestId;
this.commandId = commandId;
this.digestUtil = digestUtil;
+ this.topLevelOutputs = Preconditions.checkNotNull(topLevelOutputs, "topLevelOutputs");
}
+
@Override
public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
throws InterruptedException, IOException, ExecException {
@@ -138,31 +155,34 @@
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
result = remoteCache.getCachedActionResult(actionKey);
}
+ // In case the remote cache returned a failed action (exit code != 0) we treat it as a
+ // cache miss
if (result != null && result.getExitCode() == 0) {
- // In case if failed action returned (exit code != 0) we treat it as a cache miss
- // Otherwise, we know that result exists.
- PathFragment inMemoryOutputPath = getInMemoryOutputPath(spawn);
InMemoryOutput inMemoryOutput = null;
- switch (remoteOutputsMode) {
- case MINIMAL:
- try (SilentCloseable c =
- prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs minimal")) {
- inMemoryOutput =
- remoteCache.downloadMinimal(
- result,
- spawn.getOutputFiles(),
- inMemoryOutputPath,
- context.getFileOutErr(),
- execRoot,
- context.getMetadataInjector());
- }
- break;
- case ALL:
- try (SilentCloseable c =
- prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs")) {
- remoteCache.download(result, execRoot, context.getFileOutErr());
- }
- break;
+ boolean downloadOutputs =
+ shouldDownloadAllSpawnOutputs(
+ remoteOutputsMode,
+ /* exitCode = */ 0,
+ hasTopLevelOutputs(spawn.getOutputFiles(), topLevelOutputs));
+ if (downloadOutputs) {
+ try (SilentCloseable c =
+ prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs")) {
+ remoteCache.download(result, execRoot, context.getFileOutErr());
+ }
+ } else {
+ PathFragment inMemoryOutputPath = getInMemoryOutputPath(spawn);
+ // inject output metadata
+ try (SilentCloseable c =
+ prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs minimal")) {
+ inMemoryOutput =
+ remoteCache.downloadMinimal(
+ result,
+ spawn.getOutputFiles(),
+ inMemoryOutputPath,
+ context.getFileOutErr(),
+ execRoot,
+ context.getMetadataInjector());
+ }
}
SpawnResult spawnResult =
createSpawnResult(
@@ -174,10 +194,10 @@
} catch (IOException e) {
String errorMsg = e.getMessage();
if (isNullOrEmpty(errorMsg)) {
- errorMsg = e.getClass().getSimpleName();
+ errorMsg = e.getClass().getSimpleName();
}
- errorMsg = "Reading from Remote Cache:\n" + errorMsg;
- report(Event.warn(errorMsg));
+ errorMsg = "Reading from Remote Cache:\n" + errorMsg;
+ report(Event.warn(errorMsg));
} finally {
withMetadata.detach(previous);
}
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 464f373..3e57b5c 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
@@ -20,6 +20,8 @@
import static com.google.devtools.build.lib.remote.util.Utils.createSpawnResult;
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
import static com.google.devtools.build.lib.remote.util.Utils.getInMemoryOutputPath;
+import static com.google.devtools.build.lib.remote.util.Utils.hasTopLevelOutputs;
+import static com.google.devtools.build.lib.remote.util.Utils.shouldDownloadAllSpawnOutputs;
import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionResult;
@@ -35,6 +37,7 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
import com.google.devtools.build.lib.actions.ActionInput;
@@ -109,6 +112,14 @@
private final DigestUtil digestUtil;
private final Path logDir;
+ /**
+ * Set of artifacts that are top level outputs
+ *
+ * <p>This set is empty unless {@link RemoteOutputsMode#TOPLEVEL} is specified. If so, this set is
+ * used to decide whether to download an output.
+ */
+ private final ImmutableSet<Artifact> topLevelOutputs;
+
// Used to ensure that a warning is reported only once.
private final AtomicBoolean warningReported = new AtomicBoolean();
@@ -125,7 +136,8 @@
@Nullable GrpcRemoteExecutor remoteExecutor,
@Nullable RemoteRetrier retrier,
DigestUtil digestUtil,
- Path logDir) {
+ Path logDir,
+ ImmutableSet<Artifact> topLevelOutputs) {
this.execRoot = execRoot;
this.remoteOptions = remoteOptions;
this.executionOptions = executionOptions;
@@ -139,6 +151,7 @@
this.retrier = retrier;
this.digestUtil = digestUtil;
this.logDir = logDir;
+ this.topLevelOutputs = Preconditions.checkNotNull(topLevelOutputs, "topLevelOutputs");
}
@Override
@@ -291,34 +304,29 @@
SpawnExecutionContext context,
RemoteOutputsMode remoteOutputsMode)
throws ExecException, IOException, InterruptedException {
- SpawnResult.Status actionStatus =
- actionResult.getExitCode() == 0 ? Status.SUCCESS : Status.NON_ZERO_EXIT;
- // In case the action failed, download all outputs. It might be helpful for debugging
- // and there is no point in injecting output metadata of a failed action.
- RemoteOutputsMode effectiveOutputsStrategy =
- actionStatus == Status.SUCCESS ? remoteOutputsMode : RemoteOutputsMode.ALL;
- PathFragment inMemoryOutputPath = getInMemoryOutputPath(spawn);
+ boolean downloadOutputs =
+ shouldDownloadAllSpawnOutputs(
+ remoteOutputsMode,
+ /* exitCode = */ actionResult.getExitCode(),
+ hasTopLevelOutputs(spawn.getOutputFiles(), topLevelOutputs));
InMemoryOutput inMemoryOutput = null;
- switch (effectiveOutputsStrategy) {
- case MINIMAL:
- try (SilentCloseable c =
- Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs minimal")) {
- inMemoryOutput =
- remoteCache.downloadMinimal(
- actionResult,
- spawn.getOutputFiles(),
- inMemoryOutputPath,
- context.getFileOutErr(),
- execRoot,
- context.getMetadataInjector());
- }
- break;
-
- case ALL:
- try (SilentCloseable c = Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs")) {
- remoteCache.download(actionResult, execRoot, context.getFileOutErr());
- }
- break;
+ if (downloadOutputs) {
+ try (SilentCloseable c = Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs")) {
+ remoteCache.download(actionResult, execRoot, context.getFileOutErr());
+ }
+ } else {
+ PathFragment inMemoryOutputPath = getInMemoryOutputPath(spawn);
+ try (SilentCloseable c =
+ Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs minimal")) {
+ inMemoryOutput =
+ remoteCache.downloadMinimal(
+ actionResult,
+ spawn.getOutputFiles(),
+ inMemoryOutputPath,
+ context.getFileOutErr(),
+ execRoot,
+ context.getMetadataInjector());
+ }
}
return createSpawnResult(actionResult.getExitCode(), cacheHit, getName(), inMemoryOutput);
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOutputsMode.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOutputsMode.java
index 632e67d..d1bb349 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOutputsMode.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOutputsMode.java
@@ -24,10 +24,21 @@
* Generally don't download remote action outputs. The only outputs that are being downloaded are:
* stdout, stderr and .d and .jdeps files for C++ and Java compilation actions.
*/
- MINIMAL;
+ MINIMAL,
+
+ /**
+ * Downloads outputs of top level targets, but generally not intermediate outputs. The only
+ * intermediate outputs to be downloaded are .d and .jdeps files for C++ and Java compilation
+ * actions.
+ */
+ TOPLEVEL;
/** Returns {@code true} iff action outputs should always be downloaded. */
public boolean downloadAllOutputs() {
return this == ALL;
}
+
+ public boolean downloadToplevelOutputsOnly() {
+ return this == TOPLEVEL;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD
index 307858c..6974d85 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD
@@ -13,6 +13,7 @@
deps = [
"//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib/actions",
+ "//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//third_party:guava",
"//third_party/grpc:grpc-jar",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
index 7fdbf61..1ab04da 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
@@ -13,15 +13,20 @@
// limitations under the License.
package com.google.devtools.build.lib.remote.util;
+import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.SpawnResult.Status;
+import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.protobuf.ByteString;
import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
import java.util.concurrent.ExecutionException;
import javax.annotation.Nullable;
@@ -78,6 +83,30 @@
return builder.build();
}
+ /** Returns {@code true} if all spawn outputs should be downloaded to disk. */
+ public static boolean shouldDownloadAllSpawnOutputs(
+ RemoteOutputsMode remoteOutputsMode, int exitCode, boolean hasTopLevelOutputs) {
+ return remoteOutputsMode.downloadAllOutputs()
+ ||
+ // In case the action failed, download all outputs. It might be helpful for debugging
+ // and there is no point in injecting output metadata of a failed action.
+ exitCode != 0
+ ||
+ // If one output of a spawn is a top level output then download all outputs. Spawns
+ // are typically structured in a way that either all or no outputs are top level and
+ // it's much simpler to implement under this assumption.
+ (remoteOutputsMode.downloadToplevelOutputsOnly() && hasTopLevelOutputs);
+ }
+
+ /** Returns {@code true} if outputs contains one or more top level outputs. */
+ public static boolean hasTopLevelOutputs(
+ Collection<? extends ActionInput> outputs, ImmutableSet<Artifact> topLevelOutputs) {
+ if (topLevelOutputs.isEmpty()) {
+ return false;
+ }
+ return !Collections.disjoint(outputs, topLevelOutputs);
+ }
+
/** An in-memory output file. */
public static final class InMemoryOutput {
private final ActionInput output;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index dda1465..13ed9f9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -15,6 +15,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
@@ -30,6 +31,7 @@
import com.google.devtools.build.lib.exec.ExecutorBuilder;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
+import com.google.devtools.build.lib.skyframe.AspectValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.io.OutErr;
@@ -260,7 +262,8 @@
CommandEnvironment env,
BuildRequest request,
BuildOptions buildOptions,
- Iterable<ConfiguredTarget> configuredTargets)
+ Iterable<ConfiguredTarget> configuredTargets,
+ ImmutableSet<AspectValue> aspects)
throws InterruptedException, ViewCreationFailedException {}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
index 52ef53e..2f8f7da 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
@@ -306,7 +306,9 @@
executor,
RemoteModule.createExecuteRetrier(remoteOptions, retryService),
DIGEST_UTIL,
- logDir);
+ logDir,
+ /* topLevelOutputs= */ ImmutableSet.of());
+
inputDigest = fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz");
command =
Command.newBuilder()
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 8fd9016..3b59f98 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
@@ -31,6 +31,7 @@
import build.bazel.remote.execution.v2.RequestMetadata;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
@@ -238,7 +239,9 @@
"build-req-id",
"command-id",
reporter,
- digestUtil);
+ digestUtil,
+ /* topLevelOutputs= */ ImmutableSet.of());
+
fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz");
}
@@ -570,7 +573,8 @@
"build-req-id",
"command-id",
reporter,
- digestUtil);
+ digestUtil,
+ /* topLevelOutputs= */ ImmutableSet.of());
ActionResult success = ActionResult.newBuilder().setExitCode(0).build();
when(remoteCache.getCachedActionResult(any())).thenReturn(success);
@@ -597,7 +601,9 @@
"build-req-id",
"command-id",
reporter,
- digestUtil);
+ digestUtil,
+ /* topLevelOutputs= */ ImmutableSet.of());
+
IOException downloadFailure = new IOException("downloadMinimal failed");
ActionResult success = ActionResult.newBuilder().setExitCode(0).build();
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 7870c86..fed06d7 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
@@ -37,6 +37,7 @@
import build.bazel.remote.execution.v2.Platform;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.eventbus.EventBus;
import com.google.common.io.ByteStreams;
import com.google.common.util.concurrent.ListeningScheduledExecutorService;
@@ -47,6 +48,7 @@
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
+import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
@@ -794,7 +796,8 @@
executor,
retrier,
digestUtil,
- logDir);
+ logDir,
+ /* topLevelOutputs= */ ImmutableSet.of());
ExecuteResponse succeeded =
ExecuteResponse.newBuilder()
@@ -926,23 +929,63 @@
verify(cache, never()).download(any(ActionResult.class), any(Path.class), eq(outErr));
}
- private static Spawn newSimpleSpawn() {
+ @Test
+ public void testDownloadTopLevel() throws Exception {
+ // arrange
+ RemoteOptions options = Options.getDefaults(RemoteOptions.class);
+ options.remoteOutputsMode = RemoteOutputsMode.TOPLEVEL;
+
+ ArtifactRoot outputRoot = ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("outs"));
+ Artifact topLevelOutput = new Artifact(outputRoot.getRoot().getRelative("foo.bin"), outputRoot);
+
+ ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build();
+ when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(succeededAction);
+
+ RemoteSpawnRunner runner = newSpawnRunner(ImmutableSet.of(topLevelOutput));
+
+ Spawn spawn = newSimpleSpawn(topLevelOutput);
+ SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn);
+
+ // act
+ SpawnResult result = runner.exec(spawn, policy);
+ assertThat(result.exitCode()).isEqualTo(0);
+ assertThat(result.status()).isEqualTo(Status.SUCCESS);
+
+ // assert
+ verify(cache).download(eq(succeededAction), any(Path.class), eq(outErr));
+ verify(cache, never())
+ .downloadMinimal(eq(succeededAction), anyCollection(), any(), any(), any(), any());
+ }
+
+ private static Spawn newSimpleSpawn(Artifact... outputs) {
return new SimpleSpawn(
new FakeOwner("foo", "bar"),
/*arguments=*/ ImmutableList.of(),
/*environment=*/ ImmutableMap.of(),
/*executionInfo=*/ ImmutableMap.of(),
/*inputs=*/ ImmutableList.of(),
- /*outputs=*/ ImmutableList.<ActionInput>of(),
+ /*outputs=*/ ImmutableList.copyOf(outputs),
ResourceSet.ZERO);
}
private RemoteSpawnRunner newSpawnRunner() {
- return newSpawnRunner(/* verboseFailures= */ false, executor, /* reporter= */ null);
+ return newSpawnRunner(
+ /* verboseFailures= */ false,
+ executor,
+ /* reporter= */ null,
+ /* topLevelOutputs= */ ImmutableSet.of());
+ }
+
+ private RemoteSpawnRunner newSpawnRunner(ImmutableSet<Artifact> topLevelOutputs) {
+ return newSpawnRunner(
+ /* verboseFailures= */ false, executor, /* reporter= */ null, topLevelOutputs);
}
private RemoteSpawnRunner newSpawnRunner(
- boolean verboseFailures, @Nullable GrpcRemoteExecutor executor, @Nullable Reporter reporter) {
+ boolean verboseFailures,
+ @Nullable GrpcRemoteExecutor executor,
+ @Nullable Reporter reporter,
+ ImmutableSet<Artifact> topLevelOutputs) {
return new RemoteSpawnRunner(
execRoot,
remoteOptions,
@@ -956,15 +999,24 @@
executor,
retrier,
digestUtil,
- logDir);
+ logDir,
+ topLevelOutputs);
}
private RemoteSpawnRunner newSpawnRunnerWithoutExecutor() {
- return newSpawnRunner(/* verboseFailures= */ false, /* executor= */ null, /* reporter= */ null);
+ return newSpawnRunner(
+ /* verboseFailures= */ false,
+ /* executor= */ null,
+ /* reporter= */ null,
+ /* topLevelOutputs= */ ImmutableSet.of());
}
private RemoteSpawnRunner newSpawnRunnerWithoutExecutor(Reporter reporter) {
- return newSpawnRunner(/* verboseFailures= */ false, /* executor= */ null, reporter);
+ return newSpawnRunner(
+ /* verboseFailures= */ false,
+ /* executor= */ null,
+ reporter,
+ /* topLevelOutputs= */ ImmutableSet.of());
}
// TODO(buchgr): Extract a common class to be used for testing.
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index 17c5ea2..f035647 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -938,8 +938,9 @@
}
function test_downloads_minimal_native_prefetch() {
- # Test that when using --experimental_remote_outputs=minimal a remotely stored output that's an
- # input to a native action (ctx.actions.expand_template) is staged lazily for action execution.
+ # Test that when using --experimental_remote_download_outputs=minimal a remotely stored output
+ # that's an input to a native action (ctx.actions.expand_template) is staged lazily for action
+ # execution.
mkdir -p a
cat > a/substitute_username.bzl <<'EOF'
def _substitute_username_impl(ctx):
@@ -1000,6 +1001,59 @@
|| fail "Expected bazel-bin/a/template.txt to have been deleted again"
}
+function test_downloads_toplevel() {
+ # Test that when using --experimental_remote_download_outputs=toplevel only the output of the
+ # toplevel target is being downloaded.
+ mkdir -p a
+ cat > a/BUILD <<'EOF'
+genrule(
+ name = "foo",
+ srcs = [],
+ outs = ["foo.txt"],
+ cmd = "echo \"foo\" > \"$@\"",
+)
+
+genrule(
+ name = "foobar",
+ srcs = [":foo"],
+ outs = ["foobar.txt"],
+ cmd = "cat $(location :foo) > \"$@\" && echo \"bar\" >> \"$@\"",
+)
+EOF
+
+ bazel build \
+ --genrule_strategy=remote \
+ --remote_executor=localhost:${worker_port} \
+ --experimental_inmemory_jdeps_files \
+ --experimental_inmemory_dotd_files \
+ --experimental_remote_download_outputs=toplevel \
+ //a:foobar || fail "Failed to build //a:foobar"
+
+ (! [[ -f bazel-bin/a/foo.txt ]]) \
+ || fail "Expected intermediate output bazel-bin/a/foo.txt to not be downloaded"
+
+ [[ -f bazel-bin/a/foobar.txt ]] \
+ || fail "Expected toplevel output bazel-bin/a/foobar.txt to be downloaded"
+
+
+ # Delete the file to test that the action is re-run
+ rm -f bazel-bin/a/foobar.txt
+
+ bazel build \
+ --genrule_strategy=remote \
+ --remote_executor=localhost:${worker_port} \
+ --experimental_inmemory_jdeps_files \
+ --experimental_inmemory_dotd_files \
+ --experimental_remote_download_outputs=toplevel \
+ //a:foobar >& $TEST_log || fail "Failed to build //a:foobar"
+
+ expect_log "1 process: 1 remote cache hit"
+
+ [[ -f bazel-bin/a/foobar.txt ]] \
+ || fail "Expected toplevel output bazel-bin/a/foobar.txt to be re-downloaded"
+
+
+}
# TODO(alpha): Add a test that fails remote execution when remote worker
# supports sandbox.
