Make pkg_install work without "runfiles" (Windows default)

.bazelci/tests.yml

@@ -56,8 +56,6 @@ win_tests: &win_tests
     - "-//tests/mappings:utf8_manifest_test"
     - "-//tests/mappings/filter_directory/..."
     - "-//tests/zip:unicode_test"
-    # See #387
-    - "-//tests/install/..."
     # rpmbuild(8) is not supported on Windows
     - "-//tests/rpm/..."
     - "-//pkg/legacy/tests/rpm/..."

pkg/install.bzl

@@ -185,7 +185,7 @@ def pkg_install(name, srcs, destdir = None, **kwargs):
         name = name,
         srcs = [":" + name + "_install_script"],
         main = name + "_install_script.py",
-        deps = [Label("//pkg/private:manifest")],
+        deps = [Label("//pkg/private:manifest"), Label("@rules_python//python/runfiles")],
         srcs_version = "PY3",
         python_version = "PY3",
         **kwargs

pkg/private/install.py.tpl

@@ -21,27 +21,29 @@ import argparse
 import logging
 import os
 import pathlib
+import posixpath
 import shutil
 import sys
 import tempfile
 
 from pkg.private import manifest
+from python.runfiles import runfiles
 
 # Globals used for runfile path manipulation.
 #
 # These are necessary because runfiles are different when used as a part of
 # `bazel build` and `bazel run`. # See also
 # https://docs.bazel.build/versions/4.1.0/skylark/rules.html#tools-with-runfiles
+# https://rules-python.readthedocs.io/en/latest/api/py/runfiles/runfiles.runfiles.html
+RUNFILES = runfiles.Create()
+REPOSITORY = RUNFILES.CurrentRepository() or "{WORKSPACE_NAME}"  # the empty string denotes the "_main" repository
 
-# Bazel's documentation claims these are set when calling `bazel run`, but not other
-# modes, like in `build` or `test`.  We'll see.
-CALLED_FROM_BAZEL_RUN = bool(os.getenv("BUILD_WORKSPACE_DIRECTORY") and
-                             os.getenv("BUILD_WORKING_DIRECTORY"))
+def locate(short_path):
+    """Resolve a path relative to the current repository and return its "runfile" location.
 
-WORKSPACE_NAME = "{WORKSPACE_NAME}"
-# This seems to be set when running in `bazel build` or `bazel test`
-# TODO(#382): This may not be the case in Windows.
-RUNFILE_PREFIX = os.path.join(os.getenv("RUNFILES_DIR"), WORKSPACE_NAME) if os.getenv("RUNFILES_DIR") else None
+    Uses `posixpath` because runfile lookups always use forward slashes, even on Windows.
+    """
+    return RUNFILES.Rlocation(posixpath.normpath(posixpath.join(REPOSITORY, short_path)))
 
 
 # This is named "NativeInstaller" because it makes use of "native" python
@@ -184,10 +186,10 @@ class NativeInstaller(object):
         manifest_entries = manifest.read_entries_from(manifest_fh)
 
         for entry in manifest_entries:
-            # Swap out the source with the actual "runfile" location if we're
-            # called as a part of the build rather than "bazel run"
-            if not CALLED_FROM_BAZEL_RUN and entry.src is not None:
-                entry.src = os.path.join(RUNFILE_PREFIX, entry.src)
+            # Swap out the source with the actual "runfile" location, except for
+            # symbolic links as their targets denote installation paths
+            if entry.type != manifest.ENTRY_IS_LINK and entry.src is not None:
+                entry.src = locate(entry.src)
             # Prepend the destdir path to all installation paths, if one is
             # specified.
             if self.destdir is not None:
@@ -287,17 +289,7 @@ def main(args):
         wipe_destdir=args.wipe_destdir,
     )
 
-    if not CALLED_FROM_BAZEL_RUN and RUNFILE_PREFIX is None:
-        logging.critical("RUNFILES_DIR must be set in your environment when this is run as a bazel build tool.")
-        logging.critical("This is most likely an issue on Windows.  See https://github.com/bazelbuild/rules_pkg/issues/387.")
-        return 1
-
-    for f in ["{MANIFEST_INCLUSION}"]:
-        if CALLED_FROM_BAZEL_RUN:
-            installer.include_manifest_path(f)
-        else:
-            installer.include_manifest_path(os.path.join(RUNFILE_PREFIX, f))
-
+    installer.include_manifest_path(locate("{MANIFEST_INCLUSION}"))
     installer.do_the_thing()
 
 

tests/install/test.py

@@ -25,6 +25,8 @@
 
 
 class PkgInstallTestBase(unittest.TestCase):
+    _extension = ".exe" if os.name == "nt" else ""
+
     @classmethod
     def setUpClass(cls):
         cls.runfiles = runfiles.Create()
@@ -47,7 +49,7 @@ def setUpClass(cls):
         env = {}
         env.update(cls.runfiles.EnvVars())
         subprocess.check_call([
-            cls.runfiles.Rlocation("rules_pkg/tests/install/test_installer"),
+            cls.runfiles.Rlocation(f"rules_pkg/tests/install/test_installer{cls._extension}"),
             "--destdir", cls.installdir,
             "--verbose",
         ],
@@ -215,7 +217,7 @@ def test_wipe(self):
         (self.installdir / "should_be_deleted.txt").touch()
 
         subprocess.check_call([
-            self.runfiles.Rlocation("rules_pkg/tests/install/test_installer"),
+            self.runfiles.Rlocation(f"rules_pkg/tests/install/test_installer{self._extension}"),
             "--destdir", self.installdir,
             "--wipe_destdir",
         ],