diff --git a/.bazelci/tests.yml b/.bazelci/tests.yml index a045d56b..2da54d95 100644 --- a/.bazelci/tests.yml +++ b/.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/..." diff --git a/pkg/install.bzl b/pkg/install.bzl index 7b415aba..fec35c8e 100644 --- a/pkg/install.bzl +++ b/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 diff --git a/pkg/private/install.py.tpl b/pkg/private/install.py.tpl index 1ced5e6f..d4d0cb18 100644 --- a/pkg/private/install.py.tpl +++ b/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() diff --git a/tests/install/test.py b/tests/install/test.py index 01d97ddb..270e835b 100644 --- a/tests/install/test.py +++ b/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", ],