SpawnAction.setShellCommand: expect shell path

SpawnAction.setShellCommand(String) now expects
the shell interpreter's path as an argument. This
change enables two things:
- rules can report an error if the shell is
  missing
- SpawnAction no longer has to know about a
  default shell

The new ShToolchain class will later also be
responsible for retrieving the active shell
toolchain (added in https://github.com/bazelbuild/bazel/commit/81ed3add408adb20bddbc3ba1818c65806738dc5).

This change brings Bazel closer to not depend on
the shell unless it has to (e.g. to run shell
scripts).

See https://github.com/bazelbuild/bazel/issues/4319

RELNOTES: none
PiperOrigin-RevId: 193885943

2 files changed, 30 insertions, 16 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
index bbce349638..5299d9a8b6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
+import com.google.devtools.build.lib.analysis.ShToolchain;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
@@ -306,14 +307,25 @@ public final class TestActionBuilder {
           coverageArtifacts.add(coverageArtifact);
         }
 
-        env.registerAction(new TestRunnerAction(
-            ruleContext.getActionOwner(), inputs,
-            testSetupScript, collectCoverageScript,
-            testLog, cacheStatus,
-            coverageArtifact,
-            testProperties, extraTestEnv, executionSettings,
-            shard, run, config, ruleContext.getWorkspaceName(),
-            useTestRunner));
+        PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext);
+        env.registerAction(
+            new TestRunnerAction(
+                ruleContext.getActionOwner(),
+                inputs,
+                testSetupScript,
+                collectCoverageScript,
+                testLog,
+                cacheStatus,
+                coverageArtifact,
+                testProperties,
+                extraTestEnv,
+                executionSettings,
+                shard,
+                run,
+                config,
+                ruleContext.getWorkspaceName(),
+                shExecutable,
+                useTestRunner));
         results.add(cacheStatus);
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
index b8baf1838a..eb754bdbf4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -33,7 +33,6 @@ import com.google.devtools.build.lib.actions.CommandLineExpansionException;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit;
 import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
-import com.google.devtools.build.lib.analysis.ShellConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
 import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants;
@@ -80,6 +79,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa
   private final Artifact cacheStatus;
   private final PathFragment testWarningsPath;
   private final PathFragment unusedRunfilesLogPath;
+  private final PathFragment shExecutable;
   private final PathFragment splitLogsPath;
   private final PathFragment splitLogsDir;
   private final PathFragment undeclaredOutputsDir;
@@ -125,18 +125,18 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa
   }
 
   /**
-   * Create new TestRunnerAction instance. Should not be called directly.
-   * Use {@link TestActionBuilder} instead.
+   * Create new TestRunnerAction instance. Should not be called directly. Use {@link
+   * TestActionBuilder} instead.
    *
-   * @param shardNum The shard number. Must be 0 if totalShards == 0
-   *     (no sharding). Otherwise, must be >= 0 and < totalShards.
+   * @param shardNum The shard number. Must be 0 if totalShards == 0 (no sharding). Otherwise, must
+   *     be >= 0 and < totalShards.
    * @param runNumber test run number
    */
   TestRunnerAction(
       ActionOwner owner,
       Iterable<Artifact> inputs,
-      Artifact testSetupScript,  // Must be in inputs
-      @Nullable Artifact collectCoverageScript,  // Must be in inputs, if not null
+      Artifact testSetupScript, // Must be in inputs
+      @Nullable Artifact collectCoverageScript, // Must be in inputs, if not null
       Artifact testLog,
       Artifact cacheStatus,
       Artifact coverageArtifact,
@@ -147,6 +147,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa
       int runNumber,
       BuildConfiguration configuration,
       String workspaceName,
+      PathFragment shExecutable,
       boolean useTestRunner) {
     super(
         owner,
@@ -182,6 +183,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa
     this.testWarningsPath = baseDir.getChild("test.warnings");
     this.unusedRunfilesLogPath = baseDir.getChild("test.unused_runfiles_log");
     this.testStderr = baseDir.getChild("test.err");
+    this.shExecutable = shExecutable;
     this.splitLogsDir = baseDir.getChild("test.raw_splitlogs");
     // See note in {@link #getSplitLogsPath} on the choice of file name.
     this.splitLogsPath = splitLogsDir.getChild("test.splitlogs");
@@ -756,7 +758,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa
   }
 
   public PathFragment getShExecutable() {
-    return configuration.getFragment(ShellConfiguration.class).getShellExecutable();
+    return shExecutable;
   }
 
   public ImmutableMap<String, String> getLocalShellEnvironment() {