RELNOTES[NEW]: The "args" attribute of *_binary and *_test rules now support expanding $(location //some/deps)

*_binary and *_test rules were supporting make variables substitution but not location expansion like genrule() does. Now the $(location //some/label) where //some/label is the label of a dependency of the rule will be replaced at runtime by the actual location of that dependency. In the same manner $(locations //some/label) will be replaced by the space separated list of files of the //some/label dependency. A longer usage explanation is provided in the build encyclopedia.

--
MOS_MIGRATED_REVID=87927345

3 files changed, 95 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
index ae70699c13..1735c1a4aa 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.analysis;
 
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
@@ -44,11 +45,22 @@ import java.util.Map;
  * Note that //mypackage:myhelper should have just one output.
  */
 public class LocationExpander {
+  
+  /**
+   * List of options to tweak the LocationExpander.
+   */
+  public static enum Options {
+    /** output the execPath instead of the relative path */
+    EXEC_PATHS,
+    /** Allow to take label from the data attribute */
+    ALLOW_DATA,
+  }
+  
   private static final int MAX_PATHS_SHOWN = 5;
   private static final String LOCATION = "$(location";
   private final RuleContext ruleContext;
+  private final ImmutableSet<Options> options;
   private Map<Label, Collection<Artifact>> locationMap;
-  private boolean allowDataAttributeEntriesInLabel = false;
 
   /**
    * Creates location expander helper bound to specific target and with default
@@ -57,18 +69,51 @@ public class LocationExpander {
    * @param ruleContext BUILD rule
    */
   public LocationExpander(RuleContext ruleContext) {
-    this(ruleContext, false);
+    this(ruleContext, Options.EXEC_PATHS);
   }
 
-  public LocationExpander(RuleContext ruleContext,
-      boolean allowDataAttributeEntriesInLabel) {
+  /**
+   * Creates location expander helper bound to specific target and with default location map.
+   *
+   * @param ruleContext BUILD rule
+   * @param allowDataAttributeEntriesInLabel set to true if the <code>data</code> attribute should
+   *        be used too.
+   */
+  public LocationExpander(RuleContext ruleContext, boolean allowDataAttributeEntriesInLabel) {
+    this.ruleContext = ruleContext;
+    ImmutableSet.Builder<Options> builder = ImmutableSet.builder();
+    builder.add(Options.EXEC_PATHS);
+    if (allowDataAttributeEntriesInLabel) {
+      builder.add(Options.ALLOW_DATA);
+    }
+    this.options = builder.build();
+  }
+
+  /**
+   * Creates location expander helper bound to specific target.
+   * 
+   * @param ruleContext the BUILD rule's context
+   * @param options the list of options, see {@link Options}.
+   */
+  public LocationExpander(RuleContext ruleContext, ImmutableSet<Options> options) {
+    this.ruleContext = ruleContext;
+    this.options = options;
+  }
+
+  /**
+   * Creates location expander helper bound to specific target.
+   * 
+   * @param ruleContext the BUILD rule's context
+   * @param options the list of options, see {@link Options}.
+   */
+  public LocationExpander(RuleContext ruleContext, Options... options) {
     this.ruleContext = ruleContext;
-    this.allowDataAttributeEntriesInLabel = allowDataAttributeEntriesInLabel;
+    this.options = ImmutableSet.copyOf(options);
   }
 
   public Map<Label, Collection<Artifact>> getLocationMap() {
     if (locationMap == null) {
-      locationMap = buildLocationMap(ruleContext, allowDataAttributeEntriesInLabel);
+      locationMap = buildLocationMap(ruleContext, options.contains(Options.ALLOW_DATA));
     }
     return locationMap;
   }
@@ -141,7 +186,7 @@ public class LocationExpander {
                               + "declared prerequisite of this rule");
         return attrValue;
       }
-      List<String> paths = getPaths(artifacts);
+      List<String> paths = getPaths(artifacts, options.contains(Options.EXEC_PATHS));
       if (paths.isEmpty()) {
         ruleContext.attributeError(attrName,
                               "label '" + label + "' in " + message + " expression expands to no "
@@ -227,12 +272,14 @@ public class LocationExpander {
    * artifacts.
    *
    * @param artifacts to get the paths of
+   * @param takeExecPath if false, the root relative path will be taken
    * @return all associated executable paths
    */
-  private static List<String> getPaths(Collection<Artifact> artifacts) {
+  private static List<String> getPaths(Collection<Artifact> artifacts, boolean takeExecPath) {
     List<String> paths = Lists.newArrayListWithCapacity(artifacts.size());
     for (Artifact artifact : artifacts) {
-      PathFragment execPath = artifact.getExecPath();
+      PathFragment execPath =
+          takeExecPath ? artifact.getExecPath() : artifact.getRootRelativePath();
       if (execPath != null) {  // omit middlemen etc
         paths.add(execPath.getPathString());
       }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index eef371b475..0de3117017 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -593,14 +593,27 @@ public final class RuleContext extends TargetContext
   }
 
   /**
-   * Gets an attribute of type STRING_LIST expanding Make variables and
-   * tokenizes the result.
+   * Gets an attribute of type STRING_LIST expanding Make variables and tokenizes
+   * the result.
    *
    * @param attributeName the name of the attribute to process
-   * @return a list of strings containing the expanded and tokenized values for the
-   *         attribute
+   * @return a list of strings containing the expanded and tokenized values for the attribute
    */
   public List<String> getTokenizedStringListAttr(String attributeName) {
+    return getTokenizedStringListAttr(attributeName, null);
+  }
+
+  /**
+   * Gets an attribute of type STRING_LIST expanding Make variables, $(location) tags into the
+   * dependency location (see {@link LocationExpander} for details) and tokenizes the result.
+   *
+   * @param attributeName the name of the attribute to process
+   * @param ruleContext the rule context to look for $(location) tag replacement, or null if
+   *        location should not be expanded
+   * @return a list of strings containing the expanded and tokenized values for the attribute
+   */
+  public List<String> getTokenizedStringListAttr(String attributeName,
+      @Nullable RuleContext ruleContext) {
     if (!getRule().isAttrDefined(attributeName, Type.STRING_LIST)) {
       // TODO(bazel-team): This should be an error.
       return ImmutableList.of();
@@ -610,8 +623,12 @@ public final class RuleContext extends TargetContext
       return ImmutableList.of();
     }
     List<String> tokens = new ArrayList<>();
+    LocationExpander locationExpander =
+        ruleContext != null ? new LocationExpander(ruleContext, LocationExpander.Options.ALLOW_DATA)
+            : null;
+
     for (String token : original) {
-      tokenizeAndExpandMakeVars(tokens, attributeName, token);
+      tokenizeAndExpandMakeVars(tokens, attributeName, token, locationExpander);
     }
     return ImmutableList.copyOf(tokens);
   }
@@ -621,10 +638,23 @@ public final class RuleContext extends TargetContext
    *
    * <p>This methods should be called only during initialization.
    */
+  public void tokenizeAndExpandMakeVars(List<String> tokens, String attributeName, String value) {
+    tokenizeAndExpandMakeVars(tokens, attributeName, value, null);
+  }
+
+  /**
+   * Expands make variables and $(location) tag in value and tokenizes the result into tokens.
+   *
+   * <p>This methods should be called only during initialization.
+   */
   public void tokenizeAndExpandMakeVars(List<String> tokens, String attributeName,
-                                        String value) {
+                                        String value, @Nullable LocationExpander locationExpander) {
     try {
-      ShellUtils.tokenize(tokens, expandMakeVariables(attributeName, value));
+      if (locationExpander != null) {
+        value = locationExpander.expand(attributeName, value);
+      }
+      value = expandMakeVariables(attributeName, value);
+      ShellUtils.tokenize(tokens, value);
     } catch (ShellUtils.TokenizationException e) {
       attributeError(attributeName, e.getMessage());
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
index a790e72b3e..0be417d26c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -116,8 +116,9 @@ public class RunfilesSupport {
     this.runfilesManifest = createRunfilesAction(ruleContext, runfiles);
     this.runfilesMiddleman = createRunfilesMiddleman(ruleContext, runfiles.getAllArtifacts());
     sourcesManifest = createSourceManifest(ruleContext, runfiles);
+
     args = ImmutableList.<String>builder()
-        .addAll(ruleContext.getTokenizedStringListAttr("args"))
+        .addAll(ruleContext.getTokenizedStringListAttr("args", ruleContext))
         .addAll(appendingArgs)
         .build();
   }