feat: add to_rlocation_path and to_repository_relative_path functions…

… to paths.bzl (#362)

lib/paths.bzl

@@ -3,9 +3,13 @@
 load("//lib/private:paths.bzl", "paths")
 
 relative_file = paths.relative_file
-to_manifest_path = paths.to_manifest_path
-to_workspace_path = paths.to_workspace_path
 to_output_relative_path = paths.to_output_relative_path
+to_repository_relative_path = paths.to_repository_relative_path
+to_rlocation_path = paths.to_rlocation_path
+
+# deprecated namings
+to_manifest_path = paths.to_manifest_path  # equivalent to to_rlocation_path
+to_workspace_path = paths.to_workspace_path  # equivalent to to_repository_relative_path
 
 # Bash helper function for looking up runfiles.
 # See windows_utils.bzl for the cmd.exe equivalent.

lib/private/paths.bzl

@@ -4,11 +4,11 @@ load("@bazel_skylib//lib:paths.bzl", _spaths = "paths")
 
 def _relative_file(to_file, frm_file):
     """Resolves a relative path between two files, "to_file" and "frm_file".
-    
+
     If neither of the paths begin with ../ it is assumed that they share the same root. When finding the relative path,
     the incoming files are treated as actual files (not folders) so the resulting relative path may differ when compared
     to passing the same arguments to python's "os.path.relpath()" or NodeJs's "path.relative()".
-    
+
     For example, 'relative_file("../foo/foo.txt", "bar/bar.txt")' will return '../../foo/foo.txt'
 
     Args:
@@ -18,7 +18,6 @@ def _relative_file(to_file, frm_file):
     Returns:
         The relative path from frm_file to to_file, including the file name
     """
-
     to_parent_count = to_file.count("../")
     frm_parent_count = frm_file.count("../")
 
@@ -36,10 +35,10 @@ def _relative_file(to_file, frm_file):
     if len(to_segments) == 0 and len(frm_segments) == 0:
         return to_file
 
-    # since we prefix a "/" and normalize, the first segment is always "". So split point will be at least 1.
+    # since we prefix a "/" and normalize, the first segment is always "". So split point will be at least 1
     split_point = 1
 
-    # If either of the paths starts with ../ then assume that any shared paths are a coincidence. 
+    # if either of the paths starts with ../ then assume that any shared paths are a coincidence
     if to_segments[0] != ".." and frm_segments != "..":
         longest_common = []
         for to_seg, frm_seg in zip(to_segments, frm_segments):
@@ -58,53 +57,72 @@ def _relative_file(to_file, frm_file):
         )
     )
 
-def _to_output_relative_path(f):
-    "The relative path from bazel-out/[arch]/bin to the given File object"
-    if f.is_source:
+def _to_output_relative_path(file):
+    """
+    The relative path from bazel-out/[arch]/bin to the given File object
+
+    Args:
+        file: a `File` object
+
+    Returns:
+        The output relative path for the `File`
+    """
+    if file.is_source:
         execroot = "../../../"
     else:
         execroot = ""
-    if f.short_path.startswith("../"):
-        path = "external/" + f.short_path[3:]
+    if file.short_path.startswith("../"):
+        path = "external/" + file.short_path[3:]
     else:
-        path = f.short_path
+        path = file.short_path
     return execroot + path
 
-def _to_manifest_path(ctx, file):
-    """The runfiles manifest entry path for a file
+def _to_rlocation_path(ctx, file):
+    """The rlocation path for a `File`
+
+    The path a built binary can pass to the `Rlocation` function of a runfiles library to find a
+    dependency at runtime, either in the runfiles directory (if available) or using the runfiles
+    manifest.
+
+    This is similar to root path (a.k.a. [short_path](https://bazel.build/rules/lib/File#short_path))
+    in that it does not contain configuration prefixes, but differs in that it always starts with the
+    name of the repository.
 
-    This is the full runfiles path of a file including its workspace name as
-    the first segment. We refert to it as the manifest path as it is the path
-    flavor that is used for in the runfiles MANIFEST file.
+    The rlocation path of a `File` in an external repository repo will start with `repo/`, followed by the
+    repository-relative path.
 
-    We must avoid using non-normalized paths (workspace/../other_workspace/path)
-    in order to locate entries by their key.
+    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 root path, it has the
+    advantage that it works on all platforms and even if the runfiles directory is not available.
+
+    Based on the $(rlocation) predefined source/output path variable:
+    https://bazel.build/reference/be/make-variables#predefined_genrule_variables.
 
     Args:
         ctx: starlark rule execution context
-        file: a File object
+        file: a `File` object
 
     Returns:
-        The runfiles manifest entry path for a file
+        The rlocation path for the `File`
     """
-
     if file.short_path.startswith("../"):
         return file.short_path[3:]
     else:
         return ctx.workspace_name + "/" + file.short_path
 
-def _to_workspace_path(file):
-    """The workspace relative path for a file
+def _to_repository_relative_path(file):
+    """The repository relative path for a `File`
+
+    This is the full runfiles path of a `File` excluding its workspace name.
 
-    This is the full runfiles path of a file excluding its workspace name.
-    This differs from root path and manifest path as it does not include the
-    repository name if the file is from an external repository.
+    This differs from  root path (a.k.a. [short_path](https://bazel.build/rules/lib/File#short_path)) and
+    rlocation path as it does not include the repository name if the `File` is from an external repository.
 
     Args:
-        file: a File object
+        file: a `File` object
 
     Returns:
-        The workspace relative path for a file
+        The repository relative path for the `File`
     """
 
     if file.short_path.startswith("../"):
@@ -114,7 +132,11 @@ def _to_workspace_path(file):
 
 paths = struct(
     relative_file = _relative_file,
-    to_manifest_path = _to_manifest_path,
     to_output_relative_path = _to_output_relative_path,
-    to_workspace_path = _to_workspace_path,
+    to_repository_relative_path = _to_repository_relative_path,
+    to_rlocation_path = _to_rlocation_path,
+    # TODO(2.0): remove to_workspace_path? it is replaced by to_repository_relative_path
+    to_workspace_path = _to_repository_relative_path,
+    # TODO(2.0): remove to_manifest_path? it is replaced by to_rlocation_path
+    to_manifest_path = _to_rlocation_path,
 )

lib/tests/paths_test.bzl

@@ -189,14 +189,22 @@ def _relative_file_test_impl(ctx):
 
     return unittest.end(env)
 
-def _manifest_path_test_impl(ctx):
+def _rlocation_path_test_impl(ctx):
     env = unittest.begin(ctx)
+    asserts.equals(env, "bazel_skylib/LICENSE", paths.to_rlocation_path(ctx, ctx.file.f1))
+    asserts.equals(env, "aspect_bazel_lib/lib/paths.bzl", paths.to_rlocation_path(ctx, ctx.file.f2))
+
+    # deprecated naming
     asserts.equals(env, "bazel_skylib/LICENSE", paths.to_manifest_path(ctx, ctx.file.f1))
     asserts.equals(env, "aspect_bazel_lib/lib/paths.bzl", paths.to_manifest_path(ctx, ctx.file.f2))
     return unittest.end(env)
 
-def _workspace_path_test_impl(ctx):
+def _repository_relative_path_test_impl(ctx):
     env = unittest.begin(ctx)
+    asserts.equals(env, "LICENSE", paths.to_repository_relative_path(ctx.file.f1))
+    asserts.equals(env, "lib/paths.bzl", paths.to_repository_relative_path(ctx.file.f2))
+
+    # deprecated naming
     asserts.equals(env, "LICENSE", paths.to_workspace_path(ctx.file.f1))
     asserts.equals(env, "lib/paths.bzl", paths.to_workspace_path(ctx.file.f2))
     return unittest.end(env)
@@ -221,15 +229,15 @@ _ATTRS = {
 }
 
 relative_file_test = unittest.make(_relative_file_test_impl)
-manifest_path_test = unittest.make(_manifest_path_test_impl, attrs = _ATTRS)
+rlocation_path_test = unittest.make(_rlocation_path_test_impl, attrs = _ATTRS)
 output_relative_path_test = unittest.make(_output_relative_path_test_impl, attrs = _ATTRS)
-workspace_path_test = unittest.make(_workspace_path_test_impl, attrs = _ATTRS)
+repository_relative_path_test = unittest.make(_repository_relative_path_test_impl, attrs = _ATTRS)
 
 def paths_test_suite():
     unittest.suite(
         "paths_tests",
         partial.make(relative_file_test, timeout = "short"),
-        partial.make(manifest_path_test, timeout = "short"),
+        partial.make(rlocation_path_test, timeout = "short"),
         partial.make(output_relative_path_test, timeout = "short"),
-        partial.make(workspace_path_test, timeout = "short"),
+        partial.make(repository_relative_path_test, timeout = "short"),
     )