Add $(rlocationpath(s) ...) expansion (bazelbuild#16668)

The new location expansion pattern `rlocationpath` and its plural version `rlocationpaths` resolve to the path(s) suitable for the Rlocation function offered by runfiles libraries.

Compared to the `rootpath` pattern, which can returns `../repo_name/pkg/file` for a file in an external repository and `pkg/file` for a file in the main repository, the path returned by `rlocationpath` is always of the form `repo_name/pkg/file`.

RELNOTES: The new path variable `$(rlocationpath ...)` and its plural form `$(rlocationpaths ...)` can be used to expand labels to the paths accepted by the `Rlocation` function of runfiles libraries. This is the preferred way to access data dependencies at runtime and works on all platforms, even when runfiles are not enabled (e.g., on Windows by default).

Work towards bazelbuild#16124
Fixes bazelbuild#10923

Closes bazelbuild#16667.

PiperOrigin-RevId: 485930083
Change-Id: I85c0ef29c8332eeabd8473d2611ede546eb24016

Co-authored-by: kshyanashree <[email protected]>

Author: fmeum

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

@@ -262,15 +262,15 @@ title: Make Variables
     </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 @@ title: Make Variables
 
   <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 @@ title: Make Variables
     <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());
   }
 
   /**
@@ -215,7 +219,10 @@ 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());
@@ -230,23 +237,29 @@ private String expand(String value, ErrorReporter reporter) {
 
   @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,
+        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;
     }
@@ -257,10 +270,13 @@ 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);
@@ -269,14 +285,13 @@ public String apply(String arg, RepositoryMapping repositoryMapping) {
             String.format(
                 "invalid label in %s expression: %s", functionName(), e.getMessage()), e);
       }
-      Collection<String> paths = resolveLabel(label);
+      Set<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 {
+    /** Returns all target location(s) of the given label. */
+    private Set<String> resolveLabel(Label unresolved, String workspaceRunfilesDirectory)
+        throws IllegalStateException {
       Collection<Artifact> artifacts = locationMapSupplier.get().get(unresolved);
 
       if (artifacts == null) {
@@ -286,7 +301,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(
@@ -311,24 +326,41 @@ 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(" "));
     }
@@ -346,27 +378,44 @@ 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))
         .build();
   }