Ensure that SpawnResults make their way back to the SpawnActions that caused them to be created.

RELNOTES: None.
PiperOrigin-RevId: 170058295

6 files changed, 41 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/SandboxedSpawnActionContext.java b/src/main/java/com/google/devtools/build/lib/actions/SandboxedSpawnActionContext.java
index 9d9cdb439e..24292c8c5b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SandboxedSpawnActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SandboxedSpawnActionContext.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.actions;
 
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -42,8 +43,12 @@ public interface SandboxedSpawnActionContext extends SpawnActionContext {
    * <p>If the {@link AtomicReference} is not null (thus {@code #compareAndSet} fails) and not set
    * to the unique reference of the strategy, the {@link SandboxedSpawnActionContext} should abandon
    * all results and raise {@link InterruptedException}.
+   *
+   * @return a Set of {@link SpawnResult}s containing metadata about the Spawn's execution. This
+   *   will typically contain one element, but could contain no elements if spawn execution did not
+   *   complete, or contain multiple elements if multiple sub-spawns were executed
    */
-  void exec(
+  Set<SpawnResult> exec(
       Spawn spawn,
       ActionExecutionContext actionExecutionContext,
       AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles)
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java
index e37887ffd8..b4066ec870 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnActionContext.java
@@ -13,13 +13,15 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import java.util.Set;
+
 /**
  * A context that allows execution of {@link Spawn} instances.
  */
 @ActionContextMarker(name = "spawn")
 public interface SpawnActionContext extends ActionContext {
 
-  /** Executes the given spawn. */
-  void exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
+  /** Executes the given spawn and returns metadata about the execution. */
+  Set<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index a6af9fc364..688d2aaa94 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -42,6 +42,7 @@ import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
 import com.google.devtools.build.lib.actions.extra.SpawnInfo;
@@ -66,6 +67,7 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import javax.annotation.CheckReturnValue;
 import javax.annotation.Nullable;
 
@@ -253,9 +255,9 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
    *
    * <p>Called by {@link #execute}.
    */
-  protected void internalExecute(ActionExecutionContext actionExecutionContext)
+  protected Set<SpawnResult> internalExecute(ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException, CommandLineExpansionException {
-    getContext(actionExecutionContext)
+    return getContext(actionExecutionContext)
         .exec(getSpawn(actionExecutionContext.getClientEnv()), actionExecutionContext);
   }
 
@@ -263,6 +265,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
   public void execute(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     try {
+      // TODO(b/62588075): internalExecute() now returns a set of SpawnResults, we could capture it.
       internalExecute(actionExecutionContext);
     } catch (ExecException e) {
       String failMessage;
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
index 1042bf9401..64de06b29d 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.exec;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -43,6 +44,7 @@ import java.io.IOException;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 import java.util.SortedMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -59,13 +61,13 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte
   }
 
   @Override
-  public void exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
+  public Set<SpawnResult> exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException {
-    exec(spawn, actionExecutionContext, null);
+    return exec(spawn, actionExecutionContext, null);
   }
 
   @Override
-  public void exec(
+  public Set<SpawnResult> exec(
       Spawn spawn,
       ActionExecutionContext actionExecutionContext,
       AtomicReference<Class<? extends SpawnActionContext>> writeOutputFiles)
@@ -86,17 +88,17 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte
     if (cache == null || !Spawns.mayBeCached(spawn)) {
       cache = SpawnCache.NO_CACHE;
     }
-    SpawnResult result;
+    SpawnResult spawnResult;
     try {
       try (CacheHandle cacheHandle = cache.lookup(spawn, policy)) {
         if (cacheHandle.hasResult()) {
-          result = Preconditions.checkNotNull(cacheHandle.getResult());
+          spawnResult = Preconditions.checkNotNull(cacheHandle.getResult());
         } else {
           // Actual execution.
-          result = spawnRunner.exec(spawn, policy);
+          spawnResult = spawnRunner.exec(spawn, policy);
           if (cacheHandle.willStore()) {
             cacheHandle.store(
-                result, listExistingOutputFiles(spawn, actionExecutionContext.getExecRoot()));
+                spawnResult, listExistingOutputFiles(spawn, actionExecutionContext.getExecRoot()));
           }
         }
       }
@@ -104,7 +106,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte
       throw new EnvironmentalExecException("Unexpected IO error.", e);
     }
 
-    if ((result.status() != Status.SUCCESS) || (result.exitCode() != 0)) {
+    if ((spawnResult.status() != Status.SUCCESS) || (spawnResult.exitCode() != 0)) {
       String cwd = actionExecutionContext.getExecRoot().getPathString();
       String message =
           CommandFailureUtils.describeCommandFailure(
@@ -113,8 +115,9 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte
               spawn.getEnvironment(),
               cwd);
       throw new SpawnExecException(
-          message, result, /*forciblyRunRemotely=*/false, /*catastrophe=*/false);
+          message, spawnResult, /*forciblyRunRemotely=*/false, /*catastrophe=*/false);
     }
+    return ImmutableSet.of(spawnResult);
   }
 
   private List<Path> listExistingOutputFiles(Spawn spawn, Path execRoot) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
index 5e8828d6bb..4955c2b183 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.genrule;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
@@ -23,10 +24,12 @@ import com.google.devtools.build.lib.actions.CommandLineExpansionException;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
+import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.analysis.actions.CommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.events.EventHandler;
 import java.util.List;
+import java.util.Set;
 
 /**
  * A spawn action for genrules. Genrules are handled specially in that inputs and outputs are
@@ -66,15 +69,17 @@ public class GenRuleAction extends SpawnAction {
   }
 
   @Override
-  protected void internalExecute(ActionExecutionContext actionExecutionContext)
+  protected Set<SpawnResult> internalExecute(ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException {
     EventHandler reporter = actionExecutionContext.getEventHandler();
     checkInputsForDirectories(reporter, actionExecutionContext.getActionInputFileCache());
+    Set<SpawnResult> spawnResults = ImmutableSet.of();
     try {
-      super.internalExecute(actionExecutionContext);
+      spawnResults = super.internalExecute(actionExecutionContext);
     } catch (CommandLineExpansionException e) {
       throw new AssertionError("GenRuleAction command line expansion cannot fail");
     }
     checkOutputsForDirectories(reporter);
+    return spawnResults;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
index 1ede1d062d..5baec3274f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.CommandLine;
@@ -54,6 +55,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -135,16 +137,18 @@ public class JavaHeaderCompileAction extends SpawnAction {
   }
 
   @Override
-  protected void internalExecute(ActionExecutionContext actionExecutionContext)
+  protected Set<SpawnResult> internalExecute(ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException {
     SpawnActionContext context = getContext(actionExecutionContext);
     try {
-      context.exec(getDirectSpawn(), actionExecutionContext);
+      return context.exec(getDirectSpawn(), actionExecutionContext);
     } catch (ExecException e) {
       // if the direct input spawn failed, try again with transitive inputs to produce better
       // better messages
       try {
-        context.exec(getSpawn(actionExecutionContext.getClientEnv()), actionExecutionContext);
+        return context.exec(
+            getSpawn(actionExecutionContext.getClientEnv()),
+            actionExecutionContext);
       } catch (CommandLineExpansionException commandLineExpansionException) {
         throw new UserExecException(commandLineExpansionException);
       }