CommandHelper: do not look up shell's path

The CommandHelper no longer looks up the shell
interpreter's path itself. Instead its ctor takes
the path as an argument.

This change will allow incrementally migrating
rules that use CommandHelper to start depending on
the shell toolchain.

Shell-using rules today only use the
ShellConfiguration config fragment to look up the
shell's path. In the future, more and more rules
will also retrieve the active shell toolchain, and
be able to:

1. use the auto-detected local shell interpreter's
   location, especially when it's not the default
   /bin/bash

2. use define different shell toolchains
  (different interpreter paths) for remote builds

3. gracefully fail the build if the machine has no
   shell installed but the action graph included a
   shell action

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

Change-Id: I4da4e77e7d1fe57e8e4f5eb8820d03a840915e20

Closes #5283.

Change-Id: I4da4e77e7d1fe57e8e4f5eb8820d03a840915e20
PiperOrigin-RevId: 198298315

2 files changed, 29 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
index 92d468c5d1..8364d04b45 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
@@ -27,12 +27,14 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
+import com.google.devtools.build.lib.analysis.ShToolchain;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.extra.ExtraActionSpec;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.List;
 
 /**
@@ -76,15 +78,21 @@ public final class ExtraActionFactory implements RuleConfiguredTargetFactory {
     boolean requiresActionOutput =
         context.attributes().get("requires_action_output", Type.BOOLEAN);
 
-    ExtraActionSpec spec = new ExtraActionSpec(
-        commandHelper.getResolvedTools(),
-        new CompositeRunfilesSupplier(commandHelper.getToolsRunfilesSuppliers()),
-        resolvedData,
-        outputTemplates,
-        command,
-        context.getLabel(),
-        TargetUtils.getExecutionInfo(context.getRule()),
-        requiresActionOutput);
+    PathFragment shExecutable = ShToolchain.getPathOrError(context);
+    if (context.hasErrors()) {
+      return null;
+    }
+    ExtraActionSpec spec =
+        new ExtraActionSpec(
+            shExecutable,
+            commandHelper.getResolvedTools(),
+            new CompositeRunfilesSupplier(commandHelper.getToolsRunfilesSuppliers()),
+            resolvedData,
+            outputTemplates,
+            command,
+            context.getLabel(),
+            TargetUtils.getExecutionInfo(context.getRule()),
+            requiresActionOutput);
     return new RuleConfiguredTargetBuilder(context)
         .addProvider(ExtraActionSpec.class, spec)
         .add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
index 7f133d75a9..52efdf15de 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
+import com.google.devtools.build.lib.analysis.ShToolchain;
 import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
@@ -195,8 +196,17 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
     FilesToRunProvider genruleSetup =
         ruleContext.getPrerequisite("$genrule_setup", Mode.HOST, FilesToRunProvider.class);
     inputs.addTransitive(genruleSetup.getFilesToRun());
-    List<String> argv = commandHelper.buildCommandLine(command, inputs, ".genrule_script.sh",
-          ImmutableMap.copyOf(executionInfo));
+    PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext);
+    if (ruleContext.hasErrors()) {
+      return null;
+    }
+    List<String> argv =
+        commandHelper.buildCommandLine(
+            shExecutable,
+            command,
+            inputs,
+            ".genrule_script.sh",
+            ImmutableMap.copyOf(executionInfo));
 
     // TODO(bazel-team): Make the make variable expander pass back a list of these.
     if (requiresCrosstool(baseCommand)) {