Fix $(location //external:some_tool) expansion for genrules.

Previously, targets in "tools" of genrules could only be expanded in
$(location)s as the target of the bind.

This is another part of fully fixing #90.

--
Change-Id: I91444fbfe551b651c46811014639f08f779c70aa
Reviewed-on: https://bazel-review.googlesource.com/#/c/1750/
MOS_MIGRATED_REVID=101760690

5 files changed, 61 insertions, 40 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
index 382c21ae27..f515bbb507 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
@@ -94,8 +94,9 @@ public final class CommandHelper {
    * @param labelMap adds files to set of known files of label. Used for resolving $(location)
    *     variables.
    */
-  public CommandHelper(RuleContext ruleContext,
-      Iterable<FilesToRunProvider> tools,
+  public CommandHelper(
+      RuleContext ruleContext,
+      Iterable<? extends TransitiveInfoCollection> tools,
       ImmutableMap<Label, Iterable<Artifact>> labelMap) {
     this.ruleContext = ruleContext;
 
@@ -108,8 +109,13 @@ public final class CommandHelper {
       Iterables.addAll(mapGet(tempLabelMap, entry.getKey()), entry.getValue());
     }
 
-    for (FilesToRunProvider tool : tools) { // (Note: host configuration)
-      Label label = tool.getLabel();
+    for (TransitiveInfoCollection dep : tools) { // (Note: host configuration)
+      Label label = dep.getLabel();
+      FilesToRunProvider tool = dep.getProvider(FilesToRunProvider.class);
+      if (tool == null) {
+        continue;
+      }
+
       Collection<Artifact> files = tool.getFilesToRun();
       resolvedToolsBuilder.addAll(files);
       Artifact executableArtifact = tool.getExecutable();
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
index eea5944371..6f0554d52b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
@@ -87,8 +87,9 @@ public class GenRule implements RuleConfiguredTargetFactory {
       labelMap.put(dep.getLabel(), files);
     }
 
-    CommandHelper commandHelper = new CommandHelper(ruleContext, ruleContext
-        .getPrerequisites("tools", Mode.HOST, FilesToRunProvider.class), labelMap.build());
+    CommandHelper commandHelper =
+        new CommandHelper(
+            ruleContext, ruleContext.getPrerequisites("tools", Mode.HOST), labelMap.build());
 
     if (ruleContext.hasErrors()) {
       return null;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
index 4cb6ca52db..fb53cfd700 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.extra.SpawnInfo;
 import com.google.devtools.build.lib.analysis.AbstractConfiguredTarget;
-import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.CommandHelper;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
@@ -490,30 +489,43 @@ public class SkylarkRuleImplementationFunctions {
     }
   };
 
-  @SkylarkSignature(name = "command_helper", doc = "Experimental. Creates a command helper class.",
-      objectType = SkylarkRuleContext.class,
-      returnType = CommandHelper.class,
-      mandatoryPositionals = {
-        @Param(name = "self", type = SkylarkRuleContext.class, doc = "this RuleContext"),
-        @Param(name = "tools", type = SkylarkList.class, generic1 = TransitiveInfoCollection.class,
-            doc = "list of tools (list of targets)"),
-        @Param(name = "label_dict", type = Map.class, defaultValue = "{}",
-            doc = "dictionary of resolved labels and the corresponding list of Files "
-            + "(a dict of Label : list of Files)")})
-  private static final BuiltinFunction createCommandHelper = new BuiltinFunction("command_helper") {
-      @SuppressWarnings("unchecked")
-      // TODO(bazel-team): this cast to Map is unchecked and is not safe.
-      // The best way to fix this probably is to convert CommandHelper to Skylark.
-      public CommandHelper invoke(
-          SkylarkRuleContext ctx, SkylarkList tools, Map<Label, Iterable<Artifact>> labelDict)
-          throws ConversionException, EvalException {
-        return new CommandHelper(ctx.getRuleContext(),
-            AnalysisUtils.getProviders(
-                castList(tools, TransitiveInfoCollection.class),
-                FilesToRunProvider.class),
-            ImmutableMap.copyOf(labelDict));
-      }
-    };
+  @SkylarkSignature(
+    name = "command_helper",
+    doc = "Experimental. Creates a command helper class.",
+    objectType = SkylarkRuleContext.class,
+    returnType = CommandHelper.class,
+    mandatoryPositionals = {
+      @Param(name = "self", type = SkylarkRuleContext.class, doc = "this RuleContext"),
+      @Param(
+        name = "tools",
+        type = SkylarkList.class,
+        generic1 = TransitiveInfoCollection.class,
+        doc = "list of tools (list of targets)"
+      ),
+      @Param(
+        name = "label_dict",
+        type = Map.class,
+        defaultValue = "{}",
+        doc =
+            "dictionary of resolved labels and the corresponding list of Files "
+                + "(a dict of Label : list of Files)"
+      )
+    }
+  )
+  private static final BuiltinFunction createCommandHelper =
+      new BuiltinFunction("command_helper") {
+        @SuppressWarnings("unchecked")
+        // TODO(bazel-team): this cast to Map is unchecked and is not safe.
+        // The best way to fix this probably is to convert CommandHelper to Skylark.
+        public CommandHelper invoke(
+            SkylarkRuleContext ctx, SkylarkList tools, Map<Label, Iterable<Artifact>> labelDict)
+            throws ConversionException, EvalException {
+          return new CommandHelper(
+              ctx.getRuleContext(),
+              castList(tools, TransitiveInfoCollection.class),
+              ImmutableMap.copyOf(labelDict));
+        }
+      };
 
   static {
     SkylarkSignatureProcessor.configureSkylarkFunctions(SkylarkRuleImplementationFunctions.class);
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 e3d9adf4bc..d7f856e54d 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
@@ -19,13 +19,13 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.CommandHelper;
 import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.MakeVariableExpander;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 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.TransitiveInfoCollection;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
@@ -44,10 +44,10 @@ public final class ExtraActionFactory implements RuleConfiguredTargetFactory {
     // this rule instructs the build system to add additional outputs.
     List<Artifact> resolvedData = Lists.newArrayList();
 
-    Iterable<FilesToRunProvider> tools =
-        context.getPrerequisites("tools", Mode.HOST, FilesToRunProvider.class);
-    CommandHelper commandHelper = new CommandHelper(
-        context, tools, ImmutableMap.<Label, Iterable<Artifact>>of());
+    Iterable<? extends TransitiveInfoCollection> tools =
+        context.getPrerequisites("tools", Mode.HOST);
+    CommandHelper commandHelper =
+        new CommandHelper(context, tools, ImmutableMap.<Label, Iterable<Artifact>>of());
 
     resolvedData.addAll(context.getPrerequisiteArtifacts("data", Mode.DATA).list());
     List<String>outputTemplates =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
index 0856345807..d3df3b4f2b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
@@ -20,8 +20,8 @@ import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.CommandHelper;
-import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.analysis.actions.CommandLine;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -118,9 +118,11 @@ public final class ExtraActionSpec implements TransitiveInfoProvider {
 
     Map<String, String> env = owningRule.getConfiguration().getDefaultShellEnvironment();
 
-    CommandHelper commandHelper = new CommandHelper(owningRule,
-        ImmutableList.<FilesToRunProvider>of(),
-        ImmutableMap.<Label, Iterable<Artifact>>of());
+    CommandHelper commandHelper =
+        new CommandHelper(
+            owningRule,
+            ImmutableList.<TransitiveInfoCollection>of(),
+            ImmutableMap.<Label, Iterable<Artifact>>of());
 
     // Multiple actions in the same configured target need to have different names for the artifact
     // that might be created here, so we append something that should be unique for each action.