Add $(rlocationpath(s) ...) expansion

src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm

@@ -262,15 +262,15 @@
     </p>
     <p>
       Output files are staged similarly, but are also prefixed with the subpath
-      <code>bazel-out/cpu-compilation_mode/bin</code> (or for certain outputs:
-      <code>bazel-out/cpu-compilation_mode/genfiles</code>, or for the outputs
-      of host tools: <code>bazel-out/host/bin</code>). In the above example,
-      <code>//testapp:app</code> is a host tool because it appears in
+      <code>bazel-out/cpu-compilation_mode/bin</code> (or for the outputs of
+      tools: <code>bazel-out/cpu-opt-exec-hash/bin</code>). In the above example,
+      <code>//testapp:app</code> is a tool because it appears in
       <code>show_app_output</code>'s <code><a
       href="$expander.expandRef("genrule.tools")">tools</a></code> attribute.
       So its output file <code>app</code> is written to
-      <code>bazel-myproject/bazel-out/host/bin/testapp/app</code>. The exec path
-      is thus <code>bazel-out/host/bin/testapp/app</code>. This extra prefix
+      <code>bazel-myproject/bazel-out/cpu-opt-exec-hash/bin/testapp/app</code>.
+      The exec path is thus <code>
+      bazel-out/cpu-opt-exec-hash/bin/testapp/app</code>. This extra prefix
       makes it possible to build the same target for, say, two different CPUs in
       the same build without the results clobbering each other.
     </p>
@@ -284,15 +284,57 @@
 
   <li>
     <p>
-      <code>rootpath</code>: Denotes the runfiles path that a built binary can
-      use to find its dependencies at runtime.
-    </p>
+      <code>rootpath</code>: Denotes the path that a built binary can use to
+      find a dependency at runtime relative to the subdirectory of its runfiles
+      directory corresponding to the main repository.
+      <strong>Note:</strong> This only works if <a
+            href="/reference/command-line-reference#flag--enable_runfiles">
+      <code>--enable_runfiles</code></a> is enabled, which is not the case on
+      Windows by default. Use <code>rlocationpath</code> instead for
+      cross-platform support.
     <p>
-      This is the same as <code>execpath</code> but strips the output prefixes
-      described above. In the above example this means both
+      This is similar to <code>execpath</code> but strips the configuration
+      prefixes described above. In the example from above this means both
       <code>empty.source</code> and <code>app</code> use pure workspace-relative
       paths: <code>testapp/empty.source</code> and <code>testapp/app</code>.
     </p>
+    <p>
+      The <code>rootpath</code> of a file in an external repository
+      <code>repo</code> will start with <code>../repo/</code>, followed by the
+      repository-relative path.
+    </p>
+    <p>
+      This has the same "one output only" requirements as <code>execpath</code>.
+    </p>
+  </li>
+
+  <li>
+    <p>
+      <code>rlocationpath</code>: The path a built binary can pass to the <code>
+      Rlocation</code> function of a runfiles library to find a dependency at
+      runtime, either in the runfiles directory (if available) or using the
+      runfiles manifest.
+    </p>
+    <p>
+      This is similar to <code>rootpath</code> in that it does not contain
+      configuration prefixes, but differs in that it always starts with the
+      name of the repository. In the example from above this means that <code>
+      empty.source</code> and <code>app</code> result in the following
+      paths: <code>myproject/testapp/empty.source</code> and <code>
+      myproject/testapp/app</code>.
+    </p>
+    <p>
+      The <code>rlocationpath</code> of a file in an external repository
+      <code>repo</code> will start with <code>repo/</code>, followed by the
+      repository-relative path.
+    </p>
+    <p>
+      Passing this path to a binary and resolving it to a file system path using
+      the runfiles libraries is the preferred approach to find dependencies at
+      runtime. Compared to <code>rootpath</code>, it has the advantage that it
+      works on all platforms and even if the runfiles directory is not
+      available.
+    </p>
     <p>
       This has the same "one output only" requirements as <code>execpath</code>.
     </p>
@@ -303,24 +345,25 @@
     <code>rootpath</code>, depending on the attribute being expanded. This is
     legacy pre-Starlark behavior and not recommended unless you really know what
     it does for a particular rule. See <a
-    href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
+          href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
     for details.
   </li>
 </ul>
 
 <p>
-  <code>execpaths</code>, <code>rootpaths</code>, and <code>locations</code> are
-  the plural variations of <code>execpath</code>, <code>rootpath</code>, and
-  <code>location</code>, respectively. They support labels producing multiple
-  outputs, in which case each output is listed separated by a space. Zero-output
-  rules and malformed labels produce build errors.
+  <code>execpaths</code>, <code>rootpaths</code>, <code>rlocationpaths</code>,
+  and <code>locations</code> are the plural variations of <code>execpath</code>,
+  <code>rootpath</code>, <code>rlocationpaths</code>, and<code>location</code>,
+  respectively. They support labels producing multiple outputs, in which case
+  each output is listed separated by a space. Zero-output rules and malformed
+  labels produce build errors.
 </p>
 
 <p>
   All referenced labels must appear in the consuming target's <code>srcs</code>,
   output files, or <code>deps</code>. Otherwise the build fails. C++ targets can
   also reference labels in <code><a
-  href="$expander.expandRef("cc_binary.data")">data</a></code>.
+        href="$expander.expandRef("cc_binary.data")">data</a></code>.
 </p>
 
 <p>

src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java

@@ -17,6 +17,7 @@
 import static java.util.stream.Collectors.joining;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableCollection;
@@ -26,7 +27,9 @@
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction.PathType;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.packages.BuildType;
@@ -63,34 +66,35 @@ public final class LocationExpander {
   private static final boolean EXACTLY_ONE = false;
   private static final boolean ALLOW_MULTIPLE = true;
 
-  private static final boolean USE_LOCATION_PATHS = false;
-  private static final boolean USE_EXEC_PATHS = true;
-
   private final RuleErrorConsumer ruleErrorConsumer;
   private final ImmutableMap<String, LocationFunction> functions;
   private final RepositoryMapping repositoryMapping;
+  private final String workspaceRunfilesDirectory;
 
   @VisibleForTesting
   LocationExpander(
       RuleErrorConsumer ruleErrorConsumer,
       Map<String, LocationFunction> functions,
-      RepositoryMapping repositoryMapping) {
+      RepositoryMapping repositoryMapping,
+      String workspaceRunfilesDirectory) {
     this.ruleErrorConsumer = ruleErrorConsumer;
     this.functions = ImmutableMap.copyOf(functions);
     this.repositoryMapping = repositoryMapping;
+    this.workspaceRunfilesDirectory = workspaceRunfilesDirectory;
   }
 
   private LocationExpander(
-      RuleErrorConsumer ruleErrorConsumer,
+      RuleContext ruleContext,
       Label root,
       Supplier<Map<Label, Collection<Artifact>>> locationMap,
       boolean execPaths,
       boolean legacyExternalRunfiles,
       RepositoryMapping repositoryMapping) {
     this(
-        ruleErrorConsumer,
+        ruleContext,
         allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles),
-        repositoryMapping);
+        repositoryMapping,
+        ruleContext.getWorkspaceName());
   }
 
   /**
@@ -204,7 +208,7 @@ private String expand(String value, ErrorReporter reporter) {
       // (2) Call appropriate function to obtain string replacement.
       String functionValue = value.substring(nextWhitespace + 1, end).trim();
       try {
-        String replacement = functions.get(fname).apply(functionValue, repositoryMapping);
+        String replacement = functions.get(fname).apply(functionValue, repositoryMapping, workspaceRunfilesDirectory);
         result.append(replacement);
       } catch (IllegalStateException ise) {
         reporter.report(ise.getMessage());
@@ -232,23 +236,29 @@ public String expandAttribute(String attrName, String attrValue) {
 
   @VisibleForTesting
   static final class LocationFunction {
+    enum PathType {
+      LOCATION,
+      EXEC,
+      RLOCATION,
+    }
+
     private static final int MAX_PATHS_SHOWN = 5;
 
     private final Label root;
     private final Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier;
-    private final boolean execPaths;
+    private final PathType pathType;
     private final boolean legacyExternalRunfiles;
     private final boolean multiple;
 
     LocationFunction(
-        Label root,
-        Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
-        boolean execPaths,
-        boolean legacyExternalRunfiles,
-        boolean multiple) {
+      Label root,
+      Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
+      PathType pathType,
+      boolean legacyExternalRunfiles,
+      boolean multiple) {
       this.root = root;
       this.locationMapSupplier = locationMapSupplier;
-      this.execPaths = execPaths;
+      this.pathType = Preconditions.checkNotNull(pathType);
       this.legacyExternalRunfiles = legacyExternalRunfiles;
       this.multiple = multiple;
     }
@@ -259,10 +269,11 @@ static final class LocationFunction {
      * using the {@code repositoryMapping}.
      *
      * @param arg The label-like string to be expanded, e.g. ":foo" or "//foo:bar"
-     * @param repositoryMapping map of {@code RepositoryName}s defined in the main workspace
+     * @param repositoryMapping map of apparent repository names to {@code RepositoryName}s
+     * @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository
      * @return The expanded value
      */
-    public String apply(String arg, RepositoryMapping repositoryMapping) {
+    public String apply(String arg, RepositoryMapping repositoryMapping, String workspaceRunfilesDirectory) {
       Label label;
       try {
         label = root.getRelativeWithRemapping(arg, repositoryMapping);
@@ -271,14 +282,14 @@ public String apply(String arg, RepositoryMapping repositoryMapping) {
             String.format(
                 "invalid label in %s expression: %s", functionName(), e.getMessage()), e);
       }
-      Collection<String> paths = resolveLabel(label);
+      Collection<String> paths = resolveLabel(label, workspaceRunfilesDirectory);
       return joinPaths(paths);
     }
 
     /**
      * Returns all target location(s) of the given label.
      */
-    private Collection<String> resolveLabel(Label unresolved) throws IllegalStateException {
+    private Collection<String> resolveLabel(Label unresolved, String workspaceRunfilesDirectory) throws IllegalStateException {
       Collection<Artifact> artifacts = locationMapSupplier.get().get(unresolved);
 
       if (artifacts == null) {
@@ -288,7 +299,7 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
                 unresolved, functionName()));
       }
 
-      Set<String> paths = getPaths(artifacts);
+      Set<String> paths = getPaths(artifacts, workspaceRunfilesDirectory);
       if (paths.isEmpty()) {
         throw new IllegalStateException(
             String.format(
@@ -313,24 +324,37 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
      * Extracts list of all executables associated with given collection of label artifacts.
      *
      * @param artifacts to get the paths of
+     * @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository
      * @return all associated executable paths
      */
-    private Set<String> getPaths(Collection<Artifact> artifacts) {
+    private Set<String> getPaths(Collection<Artifact> artifacts, String workspaceRunfilesDirectory) {
       TreeSet<String> paths = Sets.newTreeSet();
       for (Artifact artifact : artifacts) {
-        PathFragment execPath =
-            execPaths
-                ? artifact.getExecPath()
-                : legacyExternalRunfiles
-                    ? artifact.getPathForLocationExpansion()
-                    : artifact.getRunfilesPath();
-        if (execPath != null) {  // omit middlemen etc
-          paths.add(execPath.getCallablePathString());
+        PathFragment path = getPath(artifact, workspaceRunfilesDirectory);
+        if (path != null) {  // omit middlemen etc
+          paths.add(path.getCallablePathString());
         }
       }
       return paths;
     }
 
+    private PathFragment getPath(Artifact artifact, String workspaceRunfilesDirectory) {
+      switch (pathType) {
+        case LOCATION:
+          return legacyExternalRunfiles ? artifact.getPathForLocationExpansion() : artifact.getRunfilesPath();
+        case EXEC:
+          return artifact.getExecPath();
+        case RLOCATION:
+          PathFragment runfilesPath = artifact.getRunfilesPath();
+          if (runfilesPath.startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX)) {
+            return runfilesPath.relativeTo(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX);
+          } else {
+            return PathFragment.create(workspaceRunfilesDirectory).getRelative(runfilesPath);
+          }
+      }
+      throw new IllegalStateException("Unexpected PathType: " + pathType);
+    }
+
     private String joinPaths(Collection<String> paths) {
       return paths.stream().map(ShellEscaper::escapeString).collect(joining(" "));
     }
@@ -348,27 +372,35 @@ static ImmutableMap<String, LocationFunction> allLocationFunctions(
     return new ImmutableMap.Builder<String, LocationFunction>()
         .put(
             "location",
-            new LocationFunction(root, locationMap, execPaths, legacyExternalRunfiles, EXACTLY_ONE))
+            new LocationFunction(root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
         .put(
             "locations",
             new LocationFunction(
-                root, locationMap, execPaths, legacyExternalRunfiles, ALLOW_MULTIPLE))
+                root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
         .put(
             "rootpath",
             new LocationFunction(
-                root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
+                root, locationMap, PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
         .put(
             "rootpaths",
             new LocationFunction(
-                root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
+                root, locationMap, PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
         .put(
             "execpath",
             new LocationFunction(
-                root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
+                root, locationMap, PathType.EXEC, legacyExternalRunfiles, EXACTLY_ONE))
         .put(
             "execpaths",
             new LocationFunction(
-                root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
+                root, locationMap, PathType.EXEC, legacyExternalRunfiles, ALLOW_MULTIPLE))
+        .put(
+            "rlocationpath",
+            new LocationFunction(
+                root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, EXACTLY_ONE))
+        .put(
+            "rlocationpaths",
+            new LocationFunction(
+                root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
         .buildOrThrow();
   }
 

src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java

@@ -50,6 +50,7 @@ final class LocationTemplateContext implements TemplateContext {
   private final ImmutableMap<String, LocationFunction> functions;
   private final RepositoryMapping repositoryMapping;
   private final boolean windowsPath;
+  private final String workspaceRunfilesDirectory;
 
   private LocationTemplateContext(
       TemplateContext delegate,
@@ -58,12 +59,14 @@ private LocationTemplateContext(
       boolean execPaths,
       boolean legacyExternalRunfiles,
       RepositoryMapping repositoryMapping,
-      boolean windowsPath) {
+      boolean windowsPath,
+      String workspaceRunfilesDirectory) {
     this.delegate = delegate;
     this.functions =
         LocationExpander.allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles);
     this.repositoryMapping = repositoryMapping;
     this.windowsPath = windowsPath;
+    this.workspaceRunfilesDirectory = workspaceRunfilesDirectory;
   }
 
   public LocationTemplateContext(
@@ -83,7 +86,8 @@ public LocationTemplateContext(
         execPaths,
         ruleContext.getConfiguration().legacyExternalRunfiles(),
         ruleContext.getRule().getPackage().getRepositoryMapping(),
-        windowsPath);
+        windowsPath,
+        ruleContext.getWorkspaceName());
   }
 
   @Override
@@ -108,7 +112,7 @@ private String lookupFunctionImpl(String name, String param) throws ExpansionExc
     try {
       LocationFunction f = functions.get(name);
       if (f != null) {
-        return f.apply(param, repositoryMapping);
+        return f.apply(param, repositoryMapping, workspaceRunfilesDirectory);
       }
     } catch (IllegalStateException e) {
       throw new ExpansionException(e.getMessage(), e);