From d345cf92670a2c15bd16102dc5e15c159c3bdcb7 Mon Sep 17 00:00:00 2001 From: Ulf Adams Date: Tue, 7 Mar 2017 10:04:53 +0000 Subject: StandaloneTestStrategy sets the full list of outputs on the test spawn All spawn strategies already treat all normal outputs as optional. Bazel checks at the action level whether all action outputs are created, but does not check at the spawn level. Spawn.getOptionalOutputs is therefore unnecessary, and removed in this change. The only place where this was set was in StandaloneTestStrategy, which now specifies the full set of outputs, which is now computed by TestRunnerAction. The internal test strategy implementations are also updated in this change. While I'm at it, also remove the use of BaseSpawn and use SimpleSpawn instead. This may go some way towards fixing #1413 and #942. -- PiperOrigin-RevId: 149397100 MOS_MIGRATED_REVID=149397100 --- .../devtools/build/lib/actions/BaseSpawn.java | 33 +--------------------- .../devtools/build/lib/actions/DelegateSpawn.java | 6 ---- .../devtools/build/lib/actions/SimpleSpawn.java | 11 -------- .../google/devtools/build/lib/actions/Spawn.java | 7 ----- 4 files changed, 1 insertion(+), 56 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/actions') diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java index e3143fddcc..901b225afe 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; @@ -31,7 +30,6 @@ public class BaseSpawn implements Spawn { private final ImmutableList arguments; private final ImmutableMap environment; private final ImmutableMap executionInfo; - private final ImmutableSet optionalOutputFiles; private final RunfilesSupplier runfilesSupplier; private final ActionExecutionMetadata action; private final ResourceSet localResources; @@ -42,39 +40,15 @@ public class BaseSpawn implements Spawn { Map executionInfo, RunfilesSupplier runfilesSupplier, ActionExecutionMetadata action, - ResourceSet localResources, - Collection optionalOutputFiles) { + ResourceSet localResources) { this.arguments = ImmutableList.copyOf(arguments); this.environment = ImmutableMap.copyOf(environment); this.executionInfo = ImmutableMap.copyOf(executionInfo); this.runfilesSupplier = runfilesSupplier; this.action = action; this.localResources = localResources; - this.optionalOutputFiles = ImmutableSet.copyOf(optionalOutputFiles); - } - - /** - * Returns a new Spawn. The caller must not modify the parameters after the call; neither will - * this method. - */ - public BaseSpawn( - List arguments, - Map environment, - Map executionInfo, - RunfilesSupplier runfilesSupplier, - ActionExecutionMetadata action, - ResourceSet localResources) { - this( - arguments, - environment, - executionInfo, - runfilesSupplier, - action, - localResources, - ImmutableSet.of()); } - /** Returns a new Spawn. */ public BaseSpawn( List arguments, Map environment, @@ -169,11 +143,6 @@ public class BaseSpawn implements Spawn { return action.getOutputs(); } - @Override - public Collection getOptionalOutputFiles() { - return optionalOutputFiles; - } - @Override public ActionExecutionMetadata getResourceOwner() { return action; diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java index 503979c2a2..8341301053 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; /** @@ -81,11 +80,6 @@ public class DelegateSpawn implements Spawn { return spawn.getOutputFiles(); } - @Override - public Collection getOptionalOutputFiles() { - return spawn.getOptionalOutputFiles(); - } - @Override public ActionExecutionMetadata getResourceOwner() { return spawn.getResourceOwner(); diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java index d6fc5a3a40..564460e726 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java @@ -17,8 +17,6 @@ package com.google.devtools.build.lib.actions; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.vfs.PathFragment; import javax.annotation.concurrent.Immutable; /** @@ -36,7 +34,6 @@ public final class SimpleSpawn implements Spawn { private final RunfilesSupplier runfilesSupplier; private final ImmutableList filesetManifests; private final ImmutableList outputs; - private final ImmutableSet optionalOutputFiles; private final ResourceSet localResources; public SimpleSpawn( @@ -49,7 +46,6 @@ public final class SimpleSpawn implements Spawn { ImmutableList tools, ImmutableList filesetManifests, ImmutableList outputs, - ImmutableSet optionalOutputFiles, ResourceSet localResources) { this.owner = Preconditions.checkNotNull(owner); this.arguments = Preconditions.checkNotNull(arguments); @@ -61,7 +57,6 @@ public final class SimpleSpawn implements Spawn { runfilesSupplier == null ? EmptyRunfilesSupplier.INSTANCE : runfilesSupplier; this.filesetManifests = Preconditions.checkNotNull(filesetManifests); this.outputs = Preconditions.checkNotNull(outputs); - this.optionalOutputFiles = Preconditions.checkNotNull(optionalOutputFiles); this.localResources = Preconditions.checkNotNull(localResources); } @@ -83,7 +78,6 @@ public final class SimpleSpawn implements Spawn { ImmutableList.of(), ImmutableList.of(), outputs, - ImmutableSet.of(), localResources); } @@ -137,11 +131,6 @@ public final class SimpleSpawn implements Spawn { return outputs; } - @Override - public ImmutableSet getOptionalOutputFiles() { - return optionalOutputFiles; - } - @Override public ActionExecutionMetadata getResourceOwner() { return owner; diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index 26f636b806..01e901042b 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; /** @@ -105,12 +104,6 @@ public interface Spawn { */ Collection getOutputFiles(); - /** - * Instructs the spawn strategy to try to fetch these optional output files in addition to the - * usual output artifacts. The PathFragments should be relative to the exec root. - */ - Collection getOptionalOutputFiles(); - /** * Returns the resource owner for local fallback. */ -- cgit v1.2.3