Make remote_download_outputs=toplevel work for tests too
This introduces two new behaviours:
1) bazel test //:foo_test downloads
bazel-testlogs/foo_test/test.{log|xml}
2) bazel build //:foo_test downloads bazel-bin/foo_test
Fixes #8934
Closes #8947.
PiperOrigin-RevId: 259324522
diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD
index 020c2cb..cb4ae3f 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD
@@ -19,6 +19,7 @@
"//src/main/java/com/google/devtools/build/lib:events",
"//src/main/java/com/google/devtools/build/lib:io",
"//src/main/java/com/google/devtools/build/lib:out-err",
+ "//src/main/java/com/google/devtools/build/lib:packages-internal",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib/actions",
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 a64c20b..4f0fb92 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
@@ -19,7 +19,7 @@
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.ActionInput;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.exec.AbstractSpawnStrategy;
@@ -49,7 +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 ImmutableSet<ActionInput> topLevelOutputs = ImmutableSet.of();
private RemoteActionContextProvider(
CommandEnvironment env,
@@ -171,7 +171,7 @@
return cache;
}
- void setTopLevelOutputs(ImmutableSet<Artifact> topLevelOutputs) {
+ void setTopLevelOutputs(ImmutableSet<ActionInput> topLevelOutputs) {
this.topLevelOutputs = Preconditions.checkNotNull(topLevelOutputs, "topLevelOutputs");
}
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 5d10cee..7140b40 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
@@ -22,10 +22,13 @@
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.actions.ActionInput;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.analysis.test.TestProvider;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
@@ -34,6 +37,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.remote.logging.LoggingInterceptor;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
@@ -321,29 +325,45 @@
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();
+ ImmutableSet.Builder<ActionInput> topLevelOutputsBuilder = ImmutableSet.builder();
for (ConfiguredTarget configuredTarget : configuredTargets) {
topLevelOutputsBuilder.addAll(
- TopLevelArtifactHelper.getAllArtifactsToBuild(
- configuredTarget, request.getTopLevelArtifactContext())
- .getImportantArtifacts());
+ getTopLevelTargetOutputs(
+ configuredTarget, request.getTopLevelArtifactContext(), env.getCommandName()));
}
-
- for (AspectValue aspect : aspects) {
- topLevelOutputsBuilder.addAll(
- TopLevelArtifactHelper.getAllArtifactsToBuild(
- aspect, request.getTopLevelArtifactContext())
- .getImportantArtifacts());
- }
-
actionContextProvider.setTopLevelOutputs(topLevelOutputsBuilder.build());
}
}
+ /** Returns a list of build or test outputs produced by the configured target. */
+ private ImmutableList<ActionInput> getTopLevelTargetOutputs(
+ ConfiguredTarget configuredTarget,
+ TopLevelArtifactContext topLevelArtifactContext,
+ String commandName) {
+ if (commandName.equals("test") && isTestRule(configuredTarget)) {
+ TestProvider testProvider = configuredTarget.getProvider(TestProvider.class);
+ if (testProvider == null) {
+ return ImmutableList.of();
+ }
+ return testProvider.getTestParams().getOutputs();
+ } else {
+ return ImmutableList.copyOf(
+ TopLevelArtifactHelper.getAllArtifactsToBuild(configuredTarget, topLevelArtifactContext)
+ .getImportantArtifacts());
+ }
+ }
+
+ private static boolean isTestRule(ConfiguredTarget configuredTarget) {
+ if (configuredTarget instanceof RuleConfiguredTarget) {
+ RuleConfiguredTarget ruleConfiguredTarget = (RuleConfiguredTarget) configuredTarget;
+ return TargetUtils.isTestRuleName(ruleConfiguredTarget.getRuleClassString());
+ }
+ return false;
+ }
+
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 e711b27..1f3908a 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
@@ -27,7 +27,6 @@
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;
@@ -90,7 +89,7 @@
* <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;
+ private final ImmutableSet<ActionInput> topLevelOutputs;
RemoteSpawnCache(
Path execRoot,
@@ -100,7 +99,7 @@
String commandId,
@Nullable Reporter cmdlineReporter,
DigestUtil digestUtil,
- ImmutableSet<Artifact> topLevelOutputs) {
+ ImmutableSet<ActionInput> topLevelOutputs) {
this.execRoot = execRoot;
this.options = options;
this.remoteCache = remoteCache;
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 2a09f3c..639df0a 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
@@ -118,7 +118,7 @@
* <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;
+ private final ImmutableSet<ActionInput> topLevelOutputs;
// Used to ensure that a warning is reported only once.
private final AtomicBoolean warningReported = new AtomicBoolean();
@@ -137,7 +137,7 @@
@Nullable RemoteRetrier retrier,
DigestUtil digestUtil,
Path logDir,
- ImmutableSet<Artifact> topLevelOutputs) {
+ ImmutableSet<ActionInput> topLevelOutputs) {
this.execRoot = execRoot;
this.remoteOptions = remoteOptions;
this.executionOptions = executionOptions;
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 818a4da..fb21413 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
@@ -16,7 +16,6 @@
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;
@@ -100,7 +99,7 @@
/** Returns {@code true} if outputs contains one or more top level outputs. */
public static boolean hasTopLevelOutputs(
- Collection<? extends ActionInput> outputs, ImmutableSet<Artifact> topLevelOutputs) {
+ Collection<? extends ActionInput> outputs, ImmutableSet<ActionInput> topLevelOutputs) {
if (topLevelOutputs.isEmpty()) {
return false;
}
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 567629a..c2611d3 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
@@ -967,7 +967,7 @@
/* topLevelOutputs= */ ImmutableSet.of());
}
- private RemoteSpawnRunner newSpawnRunner(ImmutableSet<Artifact> topLevelOutputs) {
+ private RemoteSpawnRunner newSpawnRunner(ImmutableSet<ActionInput> topLevelOutputs) {
return newSpawnRunner(
/* verboseFailures= */ false, executor, /* reporter= */ null, topLevelOutputs);
}
@@ -976,7 +976,7 @@
boolean verboseFailures,
@Nullable GrpcRemoteExecutor executor,
@Nullable Reporter reporter,
- ImmutableSet<Artifact> topLevelOutputs) {
+ ImmutableSet<ActionInput> topLevelOutputs) {
return new RemoteSpawnRunner(
execRoot,
remoteOptions,
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index 5de687e..a568c7b 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -1058,8 +1058,62 @@
[[ -f bazel-bin/a/foobar.txt ]] \
|| fail "Expected toplevel output bazel-bin/a/foobar.txt to be re-downloaded"
+}
+function test_download_toplevel_test_rule() {
+ # Test that when using --experimental_remote_download_outputs=toplevel with bazel test only
+ # the test.log and test.xml file are downloaded but not the test binary. However when building
+ # a test then the test binary should be downloaded.
+ if [[ "$PLATFORM" == "darwin" ]]; then
+ # TODO(b/37355380): This test is disabled due to RemoteWorker not supporting
+ # setting SDKROOT and DEVELOPER_DIR appropriately, as is required of
+ # action executors in order to select the appropriate Xcode toolchain.
+ return 0
+ fi
+
+ mkdir -p a
+ cat > a/BUILD <<EOF
+package(default_visibility = ["//visibility:public"])
+cc_test(
+ name = 'test',
+ srcs = [ 'test.cc' ],
+)
+EOF
+ cat > a/test.cc <<EOF
+#include <iostream>
+int main() { std::cout << "Hello test!" << std::endl; return 0; }
+EOF
+
+ # When invoking bazel test only test.log and test.xml should be downloaded.
+ bazel test \
+ --remote_executor=grpc://localhost:${worker_port} \
+ --experimental_inmemory_jdeps_files \
+ --experimental_inmemory_dotd_files \
+ --experimental_remote_download_outputs=toplevel \
+ //a:test >& $TEST_log || fail "Failed to test //a:test with remote execution"
+
+ (! [[ -f bazel-bin/a/test ]]) \
+ || fail "Expected test binary bazel-bin/a/test to not be downloaded"
+
+ [[ -f bazel-testlogs/a/test/test.log ]] \
+ || fail "Expected toplevel output bazel-testlogs/a/test/test.log to be downloaded"
+
+ [[ -f bazel-testlogs/a/test/test.xml ]] \
+ || fail "Expected toplevel output bazel-testlogs/a/test/test.log to be downloaded"
+
+ bazel clean
+
+ # When invoking bazel build the test binary should be downloaded.
+ bazel build \
+ --remote_executor=grpc://localhost:${worker_port} \
+ --experimental_inmemory_jdeps_files \
+ --experimental_inmemory_dotd_files \
+ --experimental_remote_download_outputs=toplevel \
+ //a:test >& $TEST_log || fail "Failed to build //a:test with remote execution"
+
+ ([[ -f bazel-bin/a/test ]]) \
+ || fail "Expected test binary bazel-bin/a/test to be downloaded"
}
function test_downloads_minimal_bep() {
