Simplify the Spawn interface

Remove getSpawnInfo and asShellCommand, which are never really
overridden in a useful way. asShellCommand moves to the Spawns class,
and getSpawnInfo is only ever called by SpawnAction, and the
implementation moves there.

I'm considering using Spawn as the general lower-level abstraction for both
local and remote execution. It sort of is that already, except it's not
used consistently - we often pass a tuple of (args, env) plus possibly
input and output files through parameter-heavy method call hierarchies
instead of using this existing abstraction.

However, I'm concerned about the amount of baggage it's carrying as well
as the number of implementations for what is supposed to be a simple
interface (or possibly even a simple value class), and this is an attempt
to slim it down a bit.

This should have no visible effects on builds.

--
PiperOrigin-RevId: 146109838
MOS_MIGRATED_REVID=146109838

9 files changed, 52 insertions, 74 deletions

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 6208db67ce..d98ec9ff83 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
@@ -18,11 +18,6 @@ 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.actions.extra.EnvironmentVariable;
-import com.google.devtools.build.lib.actions.extra.SpawnInfo;
-import com.google.devtools.build.lib.util.CommandDescriptionForm;
-import com.google.devtools.build.lib.util.CommandFailureUtils;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 import java.util.List;
@@ -115,11 +110,6 @@ public class BaseSpawn implements Spawn {
   }
 
   @Override
-  public String asShellCommand(Path workingDir) {
-    return asShellCommand(getArguments(), workingDir, getEnvironment());
-  }
-
-  @Override
   public RunfilesSupplier getRunfilesSupplier() {
     return runfilesSupplier;
   }
@@ -130,28 +120,6 @@ public class BaseSpawn implements Spawn {
   }
 
   @Override
-  public SpawnInfo getExtraActionInfo() {
-    SpawnInfo.Builder info = SpawnInfo.newBuilder();
-
-    info.addAllArgument(getArguments());
-    for (Map.Entry<String, String> variable : getEnvironment().entrySet()) {
-      info.addVariable(
-          EnvironmentVariable.newBuilder()
-              .setName(variable.getKey())
-              .setValue(variable.getValue())
-              .build());
-    }
-    for (ActionInput input : getInputFiles()) {
-      // Explicitly ignore middleman artifacts here.
-      if (!(input instanceof Artifact) || !((Artifact) input).isMiddlemanArtifact()) {
-        info.addInputFile(input.getExecPathString());
-      }
-    }
-    info.addAllOutputFile(ActionInputHelper.toExecPaths(getOutputFiles()));
-    return info.build();
-  }
-
-  @Override
   public ImmutableList<String> getArguments() {
     // TODO(bazel-team): this method should be final, as the correct value of the args can be
     // injected in the ctor.
@@ -226,16 +194,6 @@ public class BaseSpawn implements Spawn {
     return action.getMnemonic();
   }
 
-  /** Convert a working dir + environment map + arg list into a Bourne shell command. */
-  public static String asShellCommand(
-      Collection<String> arguments, Path workingDirectory, Map<String, String> environment) {
-    // We print this command out in such a way that it can safely be
-    // copied+pasted as a Bourne shell command.  This is extremely valuable for
-    // debugging.
-    return CommandFailureUtils.describeCommand(
-        CommandDescriptionForm.COMPLETE, arguments, environment, workingDirectory.getPathString());
-  }
-
   /** A local spawn requiring zero resources. */
   public static class Local extends BaseSpawn {
     public Local(
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 821407bd86..41c278087d 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,8 +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.actions.extra.SpawnInfo;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 
@@ -54,21 +52,11 @@ public class DelegateSpawn implements Spawn {
   }
 
   @Override
-  public String asShellCommand(Path workingDir) {
-    return spawn.asShellCommand(workingDir);
-  }
-
-  @Override
   public RunfilesSupplier getRunfilesSupplier() {
     return spawn.getRunfilesSupplier();
   }
 
   @Override
-  public SpawnInfo getExtraActionInfo() {
-    return spawn.getExtraActionInfo();
-  }
-
-  @Override
   public ImmutableList<String> getArguments() {
     return spawn.getArguments();
   }
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 ae2358f991..cf01172173 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,8 +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.actions.extra.SpawnInfo;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 
@@ -50,13 +48,6 @@ public interface Spawn {
   ImmutableMap<String, String> getExecutionInfo();
 
   /**
-   * Returns this Spawn as a Bourne shell command.
-   *
-   * @param workingDir the initial working directory of the command
-   */
-  String asShellCommand(Path workingDir);
-
-  /**
    * Returns the {@link RunfilesSupplier} helper encapsulating the runfiles for this spawn.
    */
   RunfilesSupplier getRunfilesSupplier();
@@ -67,11 +58,6 @@ public interface Spawn {
   ImmutableList<Artifact> getFilesetManifests();
 
   /**
-   * Returns a protocol buffer describing this spawn for use by the extra_action functionality.
-   */
-  SpawnInfo getExtraActionInfo();
-
-  /**
    * Returns the command (the first element) and its arguments.
    */
   ImmutableList<String> getArguments();
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
index 75046a9fd2..af8ca07881 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
@@ -14,6 +14,12 @@
 
 package com.google.devtools.build.lib.actions;
 
+import com.google.devtools.build.lib.util.CommandDescriptionForm;
+import com.google.devtools.build.lib.util.CommandFailureUtils;
+import com.google.devtools.build.lib.vfs.Path;
+import java.util.Collection;
+import java.util.Map;
+
 /** Helper methods relating to implementations of {@link Spawn}. */
 public final class Spawns {
   private Spawns() {}
@@ -41,4 +47,19 @@ public final class Spawns {
       throw new UserExecException("could not parse timeout: ", e);
     }
   }
+
+  /** Convert a spawn into a Bourne shell command. */
+  public static String asShellCommand(Spawn spawn, Path workingDirectory) {
+    return asShellCommand(spawn.getArguments(), workingDirectory, spawn.getEnvironment());
+  }
+
+  /** Convert a working dir + environment map + arg list into a Bourne shell command. */
+  public static String asShellCommand(
+      Collection<String> arguments, Path workingDirectory, Map<String, String> environment) {
+    // We print this command out in such a way that it can safely be
+    // copied+pasted as a Bourne shell command.  This is extremely valuable for
+    // debugging.
+    return CommandFailureUtils.describeCommand(
+        CommandDescriptionForm.COMPLETE, arguments, environment, workingDirectory.getPathString());
+  }
 }
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 cf0f4ebaaf..05920e1bf5 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
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.BaseSpawn;
@@ -42,6 +43,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.extra.EnvironmentVariable;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
 import com.google.devtools.build.lib.actions.extra.SpawnInfo;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
@@ -381,7 +383,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     ExtraActionInfo.Builder builder = super.getExtraActionInfo();
     if (extraActionInfoSupplier == null) {
       Spawn spawn = getSpawn();
-      SpawnInfo spawnInfo = spawn.getExtraActionInfo();
+      SpawnInfo spawnInfo = getExtraActionInfo(spawn);
 
       return builder
           .setExtension(SpawnInfo.spawnInfo, spawnInfo);
@@ -391,6 +393,27 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     }
   }
 
+  private static SpawnInfo getExtraActionInfo(Spawn spawn) {
+    SpawnInfo.Builder info = SpawnInfo.newBuilder();
+
+    info.addAllArgument(spawn.getArguments());
+    for (Map.Entry<String, String> variable : spawn.getEnvironment().entrySet()) {
+      info.addVariable(
+          EnvironmentVariable.newBuilder()
+              .setName(variable.getKey())
+              .setValue(variable.getValue())
+              .build());
+    }
+    for (ActionInput input : spawn.getInputFiles()) {
+      // Explicitly ignore middleman artifacts here.
+      if (!(input instanceof Artifact) || !((Artifact) input).isMiddlemanArtifact()) {
+        info.addInputFile(input.getExecPathString());
+      }
+    }
+    info.addAllOutputFile(ActionInputHelper.toExecPaths(spawn.getOutputFiles()));
+    return info.build();
+  }
+
   @Override
   public ImmutableMap<String, String> getEnvironment() {
     return environment;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
index 5d5f542b1d..88c6557e99 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.ActionStatusMessage;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Executor;
 import com.google.devtools.build.lib.actions.Spawn;
+import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
@@ -51,7 +52,7 @@ public final class SandboxHelpers {
               + " ["
               + spawn.getResourceOwner().prettyPrint()
               + "]",
-          spawn.asShellCommand(executor.getExecRoot()));
+          Spawns.asShellCommand(spawn, executor.getExecRoot()));
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
index e076f4d617..040a6548b0 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
@@ -70,7 +70,7 @@ public class StandaloneSpawnStrategy implements SpawnActionContext {
     if (executor.reportsSubcommands()) {
       executor.reportSubcommand(
           Label.print(spawn.getOwner().getLabel()) + " [" + spawn.getResourceOwner().prettyPrint()
-              + "]", spawn.asShellCommand(executor.getExecRoot()));
+              + "]", Spawns.asShellCommand(spawn, executor.getExecRoot()));
     }
 
     executor
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
index d01b83e868..69927d30e1 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
@@ -16,7 +16,7 @@ package com.google.devtools.build.lib.worker;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.hash.HashCode;
-import com.google.devtools.build.lib.actions.BaseSpawn;
+import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -129,6 +129,6 @@ final class WorkerKey {
 
   @Override
   public String toString() {
-    return BaseSpawn.asShellCommand(args, execRoot, env);
+    return Spawns.asShellCommand(args, execRoot, env);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
index 08d24b2652..45845464a4 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.actions.Executor;
 import com.google.devtools.build.lib.actions.SandboxedSpawnActionContext;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -215,7 +216,7 @@ public final class WorkerSpawnStrategy implements SandboxedSpawnActionContext {
               + " ["
               + spawn.getResourceOwner().prettyPrint()
               + "]",
-          spawn.asShellCommand(executor.getExecRoot()));
+              Spawns.asShellCommand(spawn, executor.getExecRoot()));
     }
 
     if (!spawn.getExecutionInfo().containsKey("supports-workers")