From d13b135e7c5cef266ee8e62f1dd7a0390ea32c1e Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 3 Feb 2024 18:02:58 -0500 Subject: [PATCH 01/11] Initial commit adding a plugin system and a PySide6 plugin Signed-off-by: Jean-Christophe Morin --- docs/source/conf.py | 3 +- docs/source/index.rst | 1 + docs/source/plugins.rst | 49 +++++++++++++ src/rez_pip/cli.py | 75 ++++++++++++++----- src/rez_pip/install.py | 16 ++--- src/rez_pip/pip.py | 27 ++++++- src/rez_pip/plugins/PySide6.py | 120 +++++++++++++++++++++++++++++++ src/rez_pip/plugins/__init__.py | 118 ++++++++++++++++++++++++++++++ src/rez_pip/plugins/shiboken6.py | 20 ++++++ src/rez_pip/rez.py | 88 +++++++++++++++-------- src/rez_pip/utils.py | 4 +- 11 files changed, 463 insertions(+), 58 deletions(-) create mode 100644 docs/source/plugins.rst create mode 100644 src/rez_pip/plugins/PySide6.py create mode 100644 src/rez_pip/plugins/__init__.py create mode 100644 src/rez_pip/plugins/shiboken6.py diff --git a/docs/source/conf.py b/docs/source/conf.py index 5d33d50..4d763ad 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -63,11 +63,12 @@ # https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html intersphinx_mapping = { + "python": ("https://docs.python.org/3", None), "rez": ("https://rez.readthedocs.io/en/stable/", None), } # Force usage of :external: -intersphinx_disabled_reftypes = ["*"] +# intersphinx_disabled_reftypes = ["*"] # -- Custom ------------------------------------------------------------------ diff --git a/docs/source/index.rst b/docs/source/index.rst index 3cd4830..3aff58c 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -69,5 +69,6 @@ automatically created by the `install.py None + + The pre-pip resolve hook allows a plugin to run some checks *before* resolving the + requested packages using pip. The hook **must** not modify the content of the + arguments passed to it. + + Some use cases are allowing or disallowing the installation of some packages. + + :param packages: List of packages requested by the user. + :param requirements: List of `requirements files `_ if any. + +.. py:function:: postPipResolve(packages: list[rez_pip.pip.PackageInfo]) -> None + + The post-pip resolve hook allows a plugin to run some checks *after* resolving the + requested packages using pip. The hook **must** not modify the content of the + arguments passed to it. + + Some use cases are allowing or disallowing the installation of some packages. + + :param packages: List of resolved packages. + +.. py:function:: groupPackages(packages: list[rez_pip.pip.PackageInfo]) -> list[rez_pip.pip.PackageGroup]: + + Merge packages into groups of packages. The name and version of the first package + in the group will be used as the name and version for the rez package. + + The hook **must** pop grouped packages out of the "packages" variable. + + :param packages: List of resolved packages. + :returns: A list of package groups. + +.. py:function:: metadata(package: rez.package_maker.PackageMaker) -> None + + Modify/inject metadata in the rez package. The plugin is expected to modify + "package" in place. + + :param package: An instanciate PackageMaker. diff --git a/src/rez_pip/cli.py b/src/rez_pip/cli.py index 3527a02..f586fea 100644 --- a/src/rez_pip/cli.py +++ b/src/rez_pip/cli.py @@ -8,6 +8,7 @@ import textwrap import pathlib import tempfile +import itertools import subprocess if sys.version_info >= (3, 10): @@ -18,6 +19,7 @@ import rich import rich.text import rich.panel +import rich.table import rez.version import rich.markup import rich.logging @@ -25,6 +27,7 @@ import rez_pip.pip import rez_pip.rez import rez_pip.data +import rez_pip.plugins import rez_pip.install import rez_pip.download import rez_pip.exceptions @@ -120,6 +123,10 @@ def _createParser() -> argparse.ArgumentParser: help="Print debug information that you can use when reporting an issue on GitHub.", ) + debugGroup.add_argument( + "--list-plugins", action="store_true", help="List all registered plugins" + ) + parser.usage = f""" %(prog)s [options] @@ -202,37 +209,55 @@ def _run(args: argparse.Namespace, pipArgs: typing.List[str], pipWorkArea: str) ) _LOG.info(f"Resolved {len(packages)} dependencies for python {pythonVersion}") + packageGroups: typing.List[rez_pip.pip.PackageGroup] = list( + itertools.chain(*rez_pip.plugins.getHook().groupPackages(packages=packages)) # type: ignore[arg-type] + ) + packageGroups += [rez_pip.pip.PackageGroup([package]) for package in packages] # TODO: Should we postpone downloading to the last minute if we can? _LOG.info("[bold]Downloading...") - wheels = rez_pip.download.downloadPackages(packages, wheelsDir) - _LOG.info(f"[bold]Downloaded {len(wheels)} wheels") - dists: typing.Dict[importlib_metadata.Distribution, bool] = {} + wheelsToDownload = [] + localWheels = [] + for group in packageGroups: + for url in group.downloadUrls: + print(url) + if url.startswith("file://"): + localWheels.append(url[7:]) + else: + wheelsToDownload.extend(group.packages) + + downloadedWheels = rez_pip.download.downloadPackages( + wheelsToDownload, wheelsDir + ) + _LOG.info(f"[bold]Downloaded {len(downloadedWheels)} wheels") + + localWheels += downloadedWheels + # Here, we could have a mapping of : and pass that to installWheel with rich.get_console().status( f"[bold]Installing wheels into {installedWheelsDir!r}" ): - for package, wheel in zip(packages, wheels): - _LOG.info(f"[bold]Installing {package.name}-{package.version} wheel") - dist, isPure = rez_pip.install.installWheel( - package, pathlib.Path(wheel), installedWheelsDir - ) - - dists[dist] = isPure - - distNames = [dist.name for dist in dists.keys()] + for group in packageGroups: + for package, wheel in zip(group.packages, group.downloadUrls): + _LOG.info(f"[bold]Installing {wheel}") + dist = rez_pip.install.installWheel( + package, + pathlib.Path( + wheel[7:] if wheel.startswith("file://") else wheel + ), + os.path.join(installedWheelsDir, package.name), + ) + group.dists.append(dist) with rich.get_console().status("[bold]Creating rez packages..."): - for dist, package in zip(dists, packages): - isPure = dists[dist] + for group in packageGroups: + print(list(package.name for package in group.packages)) rez_pip.rez.createPackage( - dist, - isPure, + group.dists, rez.version.Version(pythonVersion), - distNames, installedWheelsDir, - wheelURL=package.download_info.url, + group.downloadUrls, prefix=args.prefix, release=args.release, ) @@ -313,10 +338,24 @@ def _debug( ) +def _printPlugins() -> None: + table = rich.table.Table("Name", "Hooks", box=None) + for plugin, hooks in rez_pip.plugins._getHookImplementations().items(): + table.add_row(plugin, ", ".join(hooks)) + rich.get_console().print(table) + + def run() -> int: pipWorkArea = tempfile.mkdtemp(prefix="rez-pip-target") args, pipArgs = _parseArgs(sys.argv[1:]) + # Initialize the plugin system + rez_pip.plugins.getManager() + + if args.list_plugins: + _printPlugins() + return 0 + try: _validateArgs(args) diff --git a/src/rez_pip/install.py b/src/rez_pip/install.py index a4406fb..1e59bf8 100644 --- a/src/rez_pip/install.py +++ b/src/rez_pip/install.py @@ -38,9 +38,12 @@ ScriptSection = Literal["console", "gui"] -def isWheelPure(source: installer.sources.WheelSource) -> bool: - stream = source.read_dist_info("WHEEL") - metadata = installer.utils.parse_metadata_file(stream) +def isWheelPure(dist: importlib_metadata.Distribution) -> bool: + path = next( + f for f in dist.files if os.fspath(f.locate()).endswith(".dist-info/WHEEL") + ) + with open(path.locate()) as fd: + metadata = installer.utils.parse_metadata_file(fd.read()) return typing.cast(str, metadata["Root-Is-Purelib"]) == "true" @@ -71,7 +74,7 @@ def installWheel( package: rez_pip.pip.PackageInfo, wheelPath: pathlib.Path, targetPath: str, -) -> typing.Tuple[importlib_metadata.Distribution, bool]: +) -> importlib_metadata.Distribution: # TODO: Technically, target should be optional. We will always want to install in "pip install --target" # mode. So right now it's a CLI option for debugging purposes. @@ -82,11 +85,8 @@ def installWheel( script_kind=installer.utils.get_launcher_kind(), ) - isPure = True _LOG.debug(f"Installing {wheelPath} into {targetPath!r}") with installer.sources.WheelFile.open(wheelPath) as source: - isPure = isWheelPure(source) - installer.install( source=source, destination=destination, @@ -119,7 +119,7 @@ def installWheel( if not dist.files: raise RuntimeError(f"{path!r} does not exist!") - return dist, isPure + return dist # TODO: Document where this code comes from. diff --git a/src/rez_pip/pip.py b/src/rez_pip/pip.py index b02cfaf..adcb6ac 100644 --- a/src/rez_pip/pip.py +++ b/src/rez_pip/pip.py @@ -8,9 +8,15 @@ import subprocess import dataclasses +if sys.version_info >= (3, 10): + import importlib.metadata as importlib_metadata +else: + import importlib_metadata + import dataclasses_json import rez_pip.data +import rez_pip.plugins import rez_pip.exceptions _LOG = logging.getLogger(__name__) @@ -58,6 +64,21 @@ def version(self) -> str: return self.metadata.version +class PackageGroup: + """A group of package""" + + packages: typing.List[PackageInfo] + dists: typing.List[importlib_metadata.Distribution] + + def __init__(self, packages: typing.List[PackageInfo]) -> None: + self.packages = packages + self.dists = [] + + @property + def downloadUrls(self) -> typing.List[str]: + return [p.download_info.url for p in self.packages] + + def getBundledPip() -> str: return os.path.join(os.path.dirname(rez_pip.data.__file__), "pip.pyz") @@ -71,7 +92,9 @@ def getPackages( constraints: typing.List[str], extraArgs: typing.List[str], ) -> typing.List[PackageInfo]: - # python pip.pyz install -q requests --dry-run --ignore-installed --python-version 2.7 --only-binary=:all: --target /tmp/asd --report - + rez_pip.plugins.getHook().prePipResolve( + packages=packageNames, requirements=requirements + ) _fd, tmpFile = tempfile.mkstemp(prefix="pip-install-output", text=True) os.close(_fd) @@ -138,6 +161,8 @@ def getPackages( packageInfo = PackageInfo.from_dict(rawPackage) packages.append(packageInfo) + rez_pip.plugins.getHook().postPipResolve(packages=packages) + return packages diff --git a/src/rez_pip/plugins/PySide6.py b/src/rez_pip/plugins/PySide6.py new file mode 100644 index 0000000..cc02953 --- /dev/null +++ b/src/rez_pip/plugins/PySide6.py @@ -0,0 +1,120 @@ +"""PySide6 plugin. + +For PySide6, we need a merge hook. If User says "install PySide6", we need to install PySide6, PySide6-Addons and PySide6-Essentials and shiboken6. + +But PySide6, PySide6-Addons and PySide6-Essentials have to be merged. Additionally, shiboken6 needs to be broken down to remove PySide6 (core). +Because shiboken6 vendors PySide6-core... See https://inspector.pypi.io/project/shiboken6/6.6.1/packages/bb/72/e54f758e49e8da0dcd9490d006c41a814b0e56898ce4ca054d60cdba97bd/shiboken6-6.6.1-cp38-abi3-manylinux_2_28_x86_64.whl/. + +On Windows, the PySide6/openssl folder has to be added to PATH, see https://inspector.pypi.io/project/pyside6/6.6.1/packages/ec/3d/1da1b88d74cb5318466156bac91f17ad4272c6c83a973e107ad9a9085009/PySide6-6.6.1-cp38-abi3-win_amd64.whl/PySide6/__init__.py#line.81. + +So it's at least a 3 steps process: +1. Merge PySide6, PySide6-Essentials and PySide6-Addons into the same install. Unvendor shiboken. +2. Install shiboken + cleanup. The Cleanup could be its own hook here specific to shiboken. +""" +import os +import sys +import shutil +import typing +import logging + +if sys.version_info >= (3, 10): + import importlib.metadata as importlib_metadata +else: + import importlib_metadata + +import packaging.utils +import packaging.version +import packaging.specifiers +import packaging.requirements + +import rez_pip.pip +import rez_pip.plugins +import rez_pip.exceptions + +# PySide6 was initiall a single package that had shiboken as a dependency. +# Starting from 6.3.0, the package was spit in 3, PySide6, PySide6-Essentials and +# PySide6-Addons. + + +_LOG = logging.getLogger(__name__) + + +@rez_pip.plugins.hookimpl +def prePipResolve( + packages: typing.List[str], +) -> None: + _LOG.debug(f"prePipResolve start") + pyside6Seen = False + variantsSeens = [] + + for package in packages: + req = packaging.requirements.Requirement(package) + name = packaging.utils.canonicalize_name(req.name) + + if name == "pyside6": + pyside6Seen = True + elif name in ["pyside6-essentials", "pyside6-addons"]: + variantsSeens.append(req.name) + + if variantsSeens and not pyside6Seen: + variants = " and ".join(variantsSeens) + verb = "was" if len(variantsSeens) == 1 else "were" + raise rez_pip.exceptions.RezPipError( + f"{variants} {verb} requested but PySide6 was not. You must explicitly request PySide6 in addition to {variants}." + ) + + +@rez_pip.plugins.hookimpl +def postPipResolve(packages: typing.List[rez_pip.pip.PackageInfo]) -> None: + """ + This hook is implemented out of extra caution. We really don't want PySide6-Addons + or PySide6-Essentials to be installed without PySide6. + + In this case, we cover cases where a user requests a package X and that package + depends on PySide6-Addons or PySide6-Essentials. + """ + pyside6Seen = False + variantsSeens = [] + + for package in packages: + name = packaging.utils.canonicalize_name(package.name) + if name == "pyside6": + pyside6Seen = True + elif name in ["pyside6-essentials", "pyside6-addons"]: + variantsSeens.append(package.name) + + if variantsSeens and not pyside6Seen: + variants = " and ".join(variantsSeens) + verb = "is" if len(variantsSeens) == 1 else "are" + raise rez_pip.exceptions.RezPipError( + f"{variants} {verb} part of the resolved packages but PySide6 was not. Dependencies and or you must explicitly request PySide6 in addition to {variants}." + ) + + +@rez_pip.plugins.hookimpl +def groupPackages( + packages: typing.List[rez_pip.pip.PackageInfo], +) -> typing.List[rez_pip.pip.PackageGroup]: + data = [] + for index, package in enumerate(packages[:]): + if packaging.utils.canonicalize_name(package.name) in [ + "pyside6", + "pyside6-addons", + "pyside6-essentials", + ]: + data.append(package) + packages.remove(package) + return [rez_pip.pip.PackageGroup(data)] + + +@rez_pip.plugins.hookimpl +def cleanup(dist: importlib_metadata.Distribution, path: str) -> None: + if packaging.utils.canonicalize_name(dist.name) not in [ + "pyside6", + "pyside6-addons", + "pyside6-essentials", + ]: + return + + shutil.rmtree(os.path.join(path, "python", "shiboken6")) + shutil.rmtree(os.path.join(path, "python", "shiboken6_generator")) diff --git a/src/rez_pip/plugins/__init__.py b/src/rez_pip/plugins/__init__.py new file mode 100644 index 0000000..1f98442 --- /dev/null +++ b/src/rez_pip/plugins/__init__.py @@ -0,0 +1,118 @@ +"""Plugin system.""" +import sys +import typing +import logging +import functools + +import pluggy + +if sys.version_info >= (3, 10): + import importlib.metadata as importlib_metadata +else: + import importlib_metadata + +import rez.package_maker + +if typing.TYPE_CHECKING: + import rez_pip.pip + +__all__ = [ + "hookimpl", +] + + +def __dir__() -> typing.List[str]: + return __all__ + + +__LOG = logging.getLogger(__name__) + +F = typing.TypeVar("F", bound=typing.Callable[..., typing.Any]) +hookspec = typing.cast(typing.Callable[[F], F], pluggy.HookspecMarker("rez-pip")) +hookimpl = typing.cast(typing.Callable[[F], F], pluggy.HookimplMarker("rez-pip")) + + +class PluginSpec: + @hookspec + def prePipResolve( + self, packages: typing.List[str], requirements: typing.List[str] + ) -> None: + """ + Take an action before resolving the packages using pip. + The packages argument should not be modified in any way. + """ + + @hookspec + def postPipResolve(self, packages: typing.List["rez_pip.pip.PackageInfo"]) -> None: + """ + Take an action after resolving the packages using pip. + The packages argument should not be modified in any way. + """ + + @hookspec + def groupPackages( # type: ignore[empty-body] + self, packages: typing.List["rez_pip.pip.PackageInfo"] + ) -> typing.List["rez_pip.pip.PackageGroup"]: + """ + Merge packages into groups of packages. The name and version of the first package + in the group will be used as the name and version for the rez package. + + The hook must pop grouped packages out of the "packages" variable. + """ + + @hookspec + def cleanup(self, dist: importlib_metadata.Distribution, path: str) -> None: + """Cleanup installed distribution""" + + @hookspec + def metadata(self, package: rez.package_maker.PackageMaker) -> None: + """ + Modify/inject metadata in the rez package. The plugin is expected to modify + "package" in place. + """ + + +@functools.lru_cache() +def getManager() -> pluggy.PluginManager: + """ + Returns the plugin manager. The return value will be cached on first call + and the cached value will be return in subsequent calls. + """ + import rez_pip.plugins.PySide6 + import rez_pip.plugins.shiboken6 + + manager = pluggy.PluginManager("rez-pip") + # manager.trace.root.setwriter(print) + # manager.enable_tracing() + + manager.add_hookspecs(PluginSpec) + + manager.register(rez_pip.plugins.PySide6) + manager.register(rez_pip.plugins.shiboken6) + + manager.load_setuptools_entrypoints("rez-pip") + + # print(list(itertools.chain(*manager.hook.prePipResolve(packages=["asd"])))) + return manager + + +def getHook() -> PluginSpec: + """ + Returns the hook attribute from the manager. This is allows + to have type hints at the caller sites. + + Inspired by https://stackoverflow.com/a/54695761. + """ + manager = getManager() + return typing.cast(PluginSpec, manager.hook) + + +def _getHookImplementations() -> typing.Dict[str, typing.List[str]]: + manager = getManager() + + implementations = {} + for name, plugin in manager.list_name_plugin(): + implementations[name] = [ + caller.name for caller in manager.get_hookcallers(plugin) + ] + return implementations diff --git a/src/rez_pip/plugins/shiboken6.py b/src/rez_pip/plugins/shiboken6.py new file mode 100644 index 0000000..27424b7 --- /dev/null +++ b/src/rez_pip/plugins/shiboken6.py @@ -0,0 +1,20 @@ +import os +import sys +import shutil + +import packaging.utils + +import rez_pip.plugins + +if sys.version_info >= (3, 10): + import importlib.metadata as importlib_metadata +else: + import importlib_metadata + + +@rez_pip.plugins.hookimpl +def cleanup(dist: importlib_metadata.Distribution, path: str) -> None: + if packaging.utils.canonicalize_name(dist.name) == "shiboken6": + path = os.path.join(path, "python", "PySide6") + print(f"Removing {path!r}") + shutil.rmtree(path) diff --git a/src/rez_pip/rez.py b/src/rez_pip/rez.py index 17d0133..56c7601 100644 --- a/src/rez_pip/rez.py +++ b/src/rez_pip/rez.py @@ -20,29 +20,46 @@ import rez_pip.pip import rez_pip.utils +import rez_pip.plugins _LOG = logging.getLogger(__name__) def createPackage( - dist: importlib_metadata.Distribution, - isPure: bool, + dists: typing.List[importlib_metadata.Distribution], pythonVersion: rez.version.Version, - nameCasings: typing.List[str], installedWheelsDir: str, - wheelURL: str, + urls: typing.List[str], prefix: typing.Optional[str] = None, release: bool = False, ) -> None: - _LOG.info(f"Creating rez package for {dist.name}") - name = rez_pip.utils.pythontDistributionNameToRez(dist.name) - version = rez_pip.utils.pythonDistributionVersionToRez(dist.version) - - requirements = rez_pip.utils.getRezRequirements(dist, pythonVersion, isPure, []) + _LOG.info( + "Creating rez package for {0}".format(" + ".join(dist.name for dist in dists)) + ) + name = rez_pip.utils.pythontDistributionNameToRez(dists[0].name) + version = rez_pip.utils.pythonDistributionVersionToRez(dists[0].version) - requires = requirements.requires - variant_requires = requirements.variant_requires - metadata = requirements.metadata + requires = [] + variant_requires = [] + metadata: typing.Dict[str, typing.Any] = {} + isPure = True + for dist in dists: + requirements = rez_pip.utils.getRezRequirements(dist, pythonVersion, []) + if not metadata: + # For now we only use the metadata from the first package. Far from ideal... + metadata = requirements.metadata + + # TODO: Remove grouped packages (PySide-Addons, etc) + requires += [ + require for require in requirements.requires if require not in requires + ] + variant_requires += [ + require + for require in requirements.variant_requires + if require not in variant_requires + ] + if isPure: + isPure = metadata["is_pure_python"] if prefix: packagesPath = prefix @@ -63,21 +80,32 @@ def make_root(variant: rez.packages.Variant, path: str) -> None: _LOG.info( rf"Installing {variant.qualified_package_name} \[{formattedRequirements}]" ) - if not dist.files: - raise RuntimeError( - f"{dist.name} package has no files registered! Something is wrong maybe?" - ) - - wheelsDirAbsolute = pathlib.Path(installedWheelsDir).resolve() - for src in dist.files: - srcAbsolute = typing.cast(pathlib.Path, src.locate()).resolve() - dest = os.path.join(path, srcAbsolute.relative_to(wheelsDirAbsolute)) - if not os.path.exists(os.path.dirname(dest)): - os.makedirs(os.path.dirname(dest)) - - _LOG.debug(f"Copying {str(srcAbsolute)!r} to {str(dest)!r}") - shutil.copyfile(srcAbsolute, dest) - shutil.copystat(srcAbsolute, dest) + for dist in dists: + if not dist.files: + raise RuntimeError( + f"{dist.name} package has no files registered! Something is wrong maybe?" + ) + + wheelsDirAbsolute = pathlib.Path(installedWheelsDir).resolve() + for src in dist.files: + srcAbsolute: pathlib.Path = typing.cast( + pathlib.Path, src.locate() + ).resolve() + dest = os.path.join( + path, + os.path.sep.join( + srcAbsolute.relative_to(wheelsDirAbsolute).parts[1:] + ), + ) + # print(dest) + if not os.path.exists(os.path.dirname(dest)): + os.makedirs(os.path.dirname(dest)) + + _LOG.debug(f"Copying {str(srcAbsolute)!r} to {str(dest)!r}") + shutil.copyfile(srcAbsolute, dest) + shutil.copystat(srcAbsolute, dest) + + rez_pip.plugins.getHook().cleanup(dist=dist, path=path) with rez.package_maker.make_package( name, packagesPath, make_root=make_root, skip_existing=True, warn_on_skip=False @@ -113,8 +141,8 @@ def make_root(variant: rez.packages.Variant, path: str) -> None: pkg.pip = { "name": dist.name, "version": dist.version, - "is_pure_python": metadata["is_pure_python"], - "wheel_url": wheelURL, + "is_pure_python": isPure, + "wheel_urls": urls, "rez_pip_version": importlib_metadata.version("rez-pip"), } @@ -126,6 +154,8 @@ def make_root(variant: rez.packages.Variant, path: str) -> None: pkg.pip["metadata"] = remainingMetadata + rez_pip.plugins.getHook().metadata(package=pkg) + _LOG.info( f"[bold]Created {len(pkg.installed_variants)} variants and skipped {len(pkg.skipped_variants)}" ) diff --git a/src/rez_pip/utils.py b/src/rez_pip/utils.py index 641dd01..e4acc00 100644 --- a/src/rez_pip/utils.py +++ b/src/rez_pip/utils.py @@ -14,6 +14,8 @@ import packaging.specifiers import packaging.requirements +import rez_pip.install + _LOG = logging.getLogger(__name__) @@ -466,7 +468,6 @@ def convertMarker(marker: str) -> typing.List[str]: def getRezRequirements( installedDist: importlib_metadata.Distribution, pythonVersion: rez.version.Version, - isPure: bool, nameCasings: typing.Optional[typing.List[str]] = None, ) -> RequirementsDict: """Get requirements of the given dist, in rez-compatible format. @@ -517,6 +518,7 @@ def getRezRequirements( # python build frontends during install has_entry_points_scripts = bool(installedDist.entry_points) + isPure = rez_pip.install.isWheelPure(installedDist) # assume package is platform- and arch- specific if it isn't pure python if not isPure or has_entry_points_scripts: sys_requires.update(["platform", "arch"]) From 54d7e1857ae0eab2db4e5b0ecdb7e8b5c05448c7 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 3 Feb 2024 19:52:02 -0500 Subject: [PATCH 02/11] Fix bug with requirements of merged packages Signed-off-by: Jean-Christophe Morin --- src/rez_pip/cli.py | 3 +-- src/rez_pip/rez.py | 35 ++++++++++++++++++++++++++--------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/rez_pip/cli.py b/src/rez_pip/cli.py index f586fea..69c7698 100644 --- a/src/rez_pip/cli.py +++ b/src/rez_pip/cli.py @@ -254,10 +254,9 @@ def _run(args: argparse.Namespace, pipArgs: typing.List[str], pipWorkArea: str) for group in packageGroups: print(list(package.name for package in group.packages)) rez_pip.rez.createPackage( - group.dists, + group, rez.version.Version(pythonVersion), installedWheelsDir, - group.downloadUrls, prefix=args.prefix, release=args.release, ) diff --git a/src/rez_pip/rez.py b/src/rez_pip/rez.py index 56c7601..31e6b9a 100644 --- a/src/rez_pip/rez.py +++ b/src/rez_pip/rez.py @@ -26,24 +26,33 @@ def createPackage( - dists: typing.List[importlib_metadata.Distribution], + packageGroup: rez_pip.pip.PackageGroup, pythonVersion: rez.version.Version, installedWheelsDir: str, - urls: typing.List[str], prefix: typing.Optional[str] = None, release: bool = False, ) -> None: _LOG.info( - "Creating rez package for {0}".format(" + ".join(dist.name for dist in dists)) + "Creating rez package for {0}".format( + " + ".join(dist.name for dist in packageGroup.dists) + ) + ) + + rezNames = [ + rez_pip.utils.pythontDistributionNameToRez(dist.name) + for dist in packageGroup.dists + ] + + name = rezNames[0] + version = rez_pip.utils.pythonDistributionVersionToRez( + packageGroup.dists[0].version ) - name = rez_pip.utils.pythontDistributionNameToRez(dists[0].name) - version = rez_pip.utils.pythonDistributionVersionToRez(dists[0].version) requires = [] variant_requires = [] metadata: typing.Dict[str, typing.Any] = {} isPure = True - for dist in dists: + for dist in packageGroup.dists: requirements = rez_pip.utils.getRezRequirements(dist, pythonVersion, []) if not metadata: # For now we only use the metadata from the first package. Far from ideal... @@ -51,12 +60,20 @@ def createPackage( # TODO: Remove grouped packages (PySide-Addons, etc) requires += [ - require for require in requirements.requires if require not in requires + require + for require in requirements.requires + if require not in requires + # Check that the rez requirement isn't in the group name since it would be + # an invalid requirement (because we merge them). + and rez.version.Requirement(require).name not in rezNames[1:] ] variant_requires += [ require for require in requirements.variant_requires if require not in variant_requires + # Check that the rez requirement isn't in the group name since it would be + # an invalid requirement (because we merge them). + and rez.version.Requirement(require).name not in rezNames[1:] ] if isPure: isPure = metadata["is_pure_python"] @@ -80,7 +97,7 @@ def make_root(variant: rez.packages.Variant, path: str) -> None: _LOG.info( rf"Installing {variant.qualified_package_name} \[{formattedRequirements}]" ) - for dist in dists: + for dist in packageGroup.dists: if not dist.files: raise RuntimeError( f"{dist.name} package has no files registered! Something is wrong maybe?" @@ -142,7 +159,7 @@ def make_root(variant: rez.packages.Variant, path: str) -> None: "name": dist.name, "version": dist.version, "is_pure_python": isPure, - "wheel_urls": urls, + "wheel_urls": packageGroup.downloadUrls, "rez_pip_version": importlib_metadata.version("rez-pip"), } From 552a169570aa3427a5d63d21f88af6c0e6628c2a Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 3 Feb 2024 20:09:27 -0500 Subject: [PATCH 03/11] Simplify registration of builtin plugins Signed-off-by: Jean-Christophe Morin --- src/rez_pip/plugins/__init__.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rez_pip/plugins/__init__.py b/src/rez_pip/plugins/__init__.py index 1f98442..00beecb 100644 --- a/src/rez_pip/plugins/__init__.py +++ b/src/rez_pip/plugins/__init__.py @@ -2,15 +2,16 @@ import sys import typing import logging +import pkgutil import functools - -import pluggy +import importlib if sys.version_info >= (3, 10): import importlib.metadata as importlib_metadata else: import importlib_metadata +import pluggy import rez.package_maker if typing.TYPE_CHECKING: @@ -78,21 +79,20 @@ def getManager() -> pluggy.PluginManager: Returns the plugin manager. The return value will be cached on first call and the cached value will be return in subsequent calls. """ - import rez_pip.plugins.PySide6 - import rez_pip.plugins.shiboken6 - manager = pluggy.PluginManager("rez-pip") # manager.trace.root.setwriter(print) # manager.enable_tracing() manager.add_hookspecs(PluginSpec) - manager.register(rez_pip.plugins.PySide6) - manager.register(rez_pip.plugins.shiboken6) + # Register the builtin plugins + for module in pkgutil.iter_modules(__path__): + manager.register( + importlib.import_module(f"rez_pip.plugins.{module.name}"), name=module.name + ) manager.load_setuptools_entrypoints("rez-pip") - # print(list(itertools.chain(*manager.hook.prePipResolve(packages=["asd"])))) return manager From 399aa581f44ef317aabae93618687d38a9d55c61 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 3 Feb 2024 20:13:55 -0500 Subject: [PATCH 04/11] Add pluggy to project deps Signed-off-by: Jean-Christophe Morin --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index df30d26..8b05fe8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,6 +21,8 @@ dependencies = [ "dataclasses-json", "rich", "importlib_metadata>=4.6 ; python_version < '3.10'", + # 1.3 introduces type hints. + "pluggy>=1.3" ] classifiers = [ From 50ef9fea77c588199a284af1a757d5082c478c37 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 3 Feb 2024 20:46:50 -0500 Subject: [PATCH 05/11] Fix tests Signed-off-by: Jean-Christophe Morin --- docs/source/plugins.rst | 7 +++++++ pytest.ini | 2 +- src/rez_pip/plugins/__init__.py | 3 ++- tests/test_cli.py | 5 +++++ tests/test_rez.py | 23 ++++++++++++++++++----- 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/docs/source/plugins.rst b/docs/source/plugins.rst index 713fbc2..44ff464 100644 --- a/docs/source/plugins.rst +++ b/docs/source/plugins.rst @@ -47,3 +47,10 @@ Hooks "package" in place. :param package: An instanciate PackageMaker. + +.. py:function:: cleanup(dist: importlib.metadata.Distribution, path: str) -> None + + Cleanup a package post-installation. + + :param dist: Python distribution. + :param path: Root path of the rez variant. diff --git a/pytest.ini b/pytest.ini index d9f1853..aa02126 100644 --- a/pytest.ini +++ b/pytest.ini @@ -7,7 +7,7 @@ addopts = --cov-report=term-missing --cov-report=xml --cov-report=html - --durations=0 + #--durations=0 norecursedirs = rez_repo diff --git a/src/rez_pip/plugins/__init__.py b/src/rez_pip/plugins/__init__.py index 00beecb..d2bb53b 100644 --- a/src/rez_pip/plugins/__init__.py +++ b/src/rez_pip/plugins/__init__.py @@ -88,7 +88,8 @@ def getManager() -> pluggy.PluginManager: # Register the builtin plugins for module in pkgutil.iter_modules(__path__): manager.register( - importlib.import_module(f"rez_pip.plugins.{module.name}"), name=module.name + importlib.import_module(f"rez_pip.plugins.{module.name}"), + name=f"rez_pip.{module.name}", ) manager.load_setuptools_entrypoints("rez-pip") diff --git a/tests/test_cli.py b/tests/test_cli.py index ada7b07..31769fd 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -26,6 +26,7 @@ def test_parseArgs_empty(): assert vars(args) == { "constraint": None, "keep_tmp_dirs": False, + "list_plugins": False, "log_level": "info", "packages": [], "pip": rez_pip.pip.getBundledPip(), @@ -45,6 +46,7 @@ def test_parseArgs_packages(packages): assert vars(args) == { "constraint": None, "keep_tmp_dirs": False, + "list_plugins": False, "log_level": "info", "packages": packages, "pip": rez_pip.pip.getBundledPip(), @@ -64,6 +66,7 @@ def test_parseArgs_no_package_with_requirements(files): assert vars(args) == { "constraint": None, "keep_tmp_dirs": False, + "list_plugins": False, "log_level": "info", "packages": [], "pip": rez_pip.pip.getBundledPip(), @@ -82,6 +85,7 @@ def test_parseArgs_constraints(): assert vars(args) == { "constraint": ["asd", "adasdasd"], "keep_tmp_dirs": False, + "list_plugins": False, "log_level": "info", "packages": [], "pip": rez_pip.pip.getBundledPip(), @@ -102,6 +106,7 @@ def test_parseArgs_pipArgs(): assert vars(args) == { "constraint": None, "keep_tmp_dirs": False, + "list_plugins": False, "log_level": "info", "packages": [], "pip": rez_pip.pip.getBundledPip(), diff --git a/tests/test_rez.py b/tests/test_rez.py index d24bc97..db0dcb0 100644 --- a/tests/test_rez.py +++ b/tests/test_rez.py @@ -17,6 +17,7 @@ else: import importlib_metadata +import rez_pip.pip import rez_pip.rez import rez_pip.utils @@ -62,16 +63,28 @@ def make_file(path: str) -> importlib_metadata.PackagePath: metadata={"is_pure_python": False}, ) + packageGroup = rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata(name="package-a", version="1.0.0.post0"), + download_info=rez_pip.pip.DownloadInfo( + url=f"http://localhost/asd", + archive_info=rez_pip.pip.ArchiveInfo("hash", {}), + ), + is_direct=True, + requested=True, + ) + ] + ) + packageGroup.dists = [dist] + with unittest.mock.patch.object( rez_pip.utils, "getRezRequirements", return_value=expectedRequirements ): rez_pip.rez.createPackage( - dist, - False, + packageGroup, rez.version.Version("3.7.0"), - [], source, - "http://localhost/asd", prefix=repo, ) @@ -84,7 +97,7 @@ def make_file(path: str) -> importlib_metadata.PackagePath: "name": dist.name, "version": dist.version, "is_pure_python": False, - "wheel_url": "http://localhost/asd", + "wheel_urls": ["http://localhost/asd"], "rez_pip_version": importlib_metadata.version("rez-pip"), "metadata": {}, } From 2dc8ef58fef86bafd9d6acc2c06c42864750afcd Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 28 Apr 2024 16:45:38 -0400 Subject: [PATCH 06/11] Fix remaining issues (I think) Signed-off-by: Jean-Christophe Morin --- pyproject.toml | 2 +- pytest.ini | 2 +- src/rez_pip/cli.py | 41 ++++++++++++++------------------ src/rez_pip/download.py | 40 +++++++++++++++++++++---------- src/rez_pip/pip.py | 36 ++++++++++++++++++++++++++++ src/rez_pip/plugins/PySide6.py | 10 ++++---- src/rez_pip/plugins/__init__.py | 41 ++++++++++++++++++++++++++------ src/rez_pip/plugins/shiboken6.py | 16 ++++++++++--- 8 files changed, 136 insertions(+), 52 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8b05fe8..9e30e37 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,7 +22,7 @@ dependencies = [ "rich", "importlib_metadata>=4.6 ; python_version < '3.10'", # 1.3 introduces type hints. - "pluggy>=1.3" + "pluggy>=1.2", ] classifiers = [ diff --git a/pytest.ini b/pytest.ini index aa02126..6657b85 100644 --- a/pytest.ini +++ b/pytest.ini @@ -14,5 +14,5 @@ norecursedirs = rez_repo markers = integration: mark the tests as integration tests py37: mark the tests has using a Python 3.7 rez package - py39: mark the tests has using a Python 3.7 rez package + py39: mark the tests has using a Python 3.9 rez package py311: mark the tests has using a Python 3.11 rez package diff --git a/src/rez_pip/cli.py b/src/rez_pip/cli.py index 69c7698..a661bf6 100644 --- a/src/rez_pip/cli.py +++ b/src/rez_pip/cli.py @@ -6,7 +6,6 @@ import logging import argparse import textwrap -import pathlib import tempfile import itertools import subprocess @@ -212,47 +211,43 @@ def _run(args: argparse.Namespace, pipArgs: typing.List[str], pipWorkArea: str) packageGroups: typing.List[rez_pip.pip.PackageGroup] = list( itertools.chain(*rez_pip.plugins.getHook().groupPackages(packages=packages)) # type: ignore[arg-type] ) + + # Remove empty groups + packageGroups = [group for group in packageGroups if group] + + # Add packages that were not grouped. packageGroups += [rez_pip.pip.PackageGroup([package]) for package in packages] # TODO: Should we postpone downloading to the last minute if we can? _LOG.info("[bold]Downloading...") - wheelsToDownload = [] - localWheels = [] - for group in packageGroups: - for url in group.downloadUrls: - print(url) - if url.startswith("file://"): - localWheels.append(url[7:]) - else: - wheelsToDownload.extend(group.packages) - - downloadedWheels = rez_pip.download.downloadPackages( - wheelsToDownload, wheelsDir - ) - _LOG.info(f"[bold]Downloaded {len(downloadedWheels)} wheels") + downloadedWheels = rez_pip.download.downloadPackages(packageGroups, wheelsDir) + foundLocally = [ + p + for group in packageGroups + for p in group.packages + if not p.isDownloadRequired() + ] - localWheels += downloadedWheels + _LOG.info( + f"[bold]Downloaded {len(downloadedWheels)} wheels, skipped {len(foundLocally)} because they resolved to local files" + ) - # Here, we could have a mapping of : and pass that to installWheel with rich.get_console().status( f"[bold]Installing wheels into {installedWheelsDir!r}" ): for group in packageGroups: - for package, wheel in zip(group.packages, group.downloadUrls): - _LOG.info(f"[bold]Installing {wheel}") + for package in group.packages: + _LOG.info(f"[bold]Installing {package.name} {package.localPath}") dist = rez_pip.install.installWheel( package, - pathlib.Path( - wheel[7:] if wheel.startswith("file://") else wheel - ), + package.localPath, os.path.join(installedWheelsDir, package.name), ) group.dists.append(dist) with rich.get_console().status("[bold]Creating rez packages..."): for group in packageGroups: - print(list(package.name for package in group.packages)) rez_pip.rez.createPackage( group, rez.version.Version(pythonVersion), diff --git a/src/rez_pip/download.py b/src/rez_pip/download.py index 88604ee..0b2d943 100644 --- a/src/rez_pip/download.py +++ b/src/rez_pip/download.py @@ -21,13 +21,13 @@ def downloadPackages( - packages: typing.List[rez_pip.pip.PackageInfo], dest: str + packageGroups: typing.List[rez_pip.pip.PackageGroup], dest: str ) -> typing.List[str]: - return asyncio.run(_downloadPackages(packages, dest)) + return asyncio.run(_downloadPackages(packageGroups, dest)) async def _downloadPackages( - packages: typing.List[rez_pip.pip.PackageInfo], dest: str + packageGroups: typing.List[rez_pip.pip.PackageGroup], dest: str ) -> typing.List[str]: items: typing.List[ typing.Coroutine[typing.Any, typing.Any, typing.Optional[str]] @@ -47,18 +47,33 @@ async def _downloadPackages( tasks: typing.Dict[str, rich.progress.TaskID] = {} # Create all the downlod tasks first - for package in packages: - tasks[package.name] = progress.add_task(package.name) + numPackages = 0 + for group in packageGroups: + for package in group.packages: + if not package.isDownloadRequired(): + continue - # Then create the "total" progress bar. This ensures that total is at the bottom. - mainTask = progress.add_task(f"[bold]Total (0/{len(packages)})", total=0) + numPackages += 1 + tasks[package.name] = progress.add_task(package.name) - for package in packages: - items.append( - _download( - package, dest, session, progress, tasks[package.name], mainTask + # Then create the "total" progress bar. This ensures that total is at the bottom. + mainTask = progress.add_task(f"[bold]Total (0/{numPackages})", total=0) + + for group in packageGroups: + for package in group.packages: + if not package.isDownloadRequired(): + continue + + items.append( + _download( + package, + dest, + session, + progress, + tasks[package.name], + mainTask, + ) ) - ) wheels = await asyncio.gather(*items) @@ -152,4 +167,5 @@ async def _download( mainTaskID, description=f"[bold]Total ({len(completedItems)}/{total})" ) + package.localPath = wheelPath return wheelPath diff --git a/src/rez_pip/pip.py b/src/rez_pip/pip.py index adcb6ac..faee8fa 100644 --- a/src/rez_pip/pip.py +++ b/src/rez_pip/pip.py @@ -55,6 +55,10 @@ class PackageInfo(dataclasses_json.DataClassJsonMixin): undefined=dataclasses_json.Undefined.EXCLUDE ) + # Must be set once the package is downloaded. + # Can be retrieved through the localPath property. + __localPath: typing.Optional[str] = None + @property def name(self) -> str: return self.metadata.name @@ -63,6 +67,25 @@ def name(self) -> str: def version(self) -> str: return self.metadata.version + def isDownloadRequired(self): + return not self.download_info.url.startswith("file://") + + @property + def localPath(self) -> str: + """Path to the package on disk.""" + if not self.isDownloadRequired(): + return self.download_info.url[7:] + + if self.__localPath is None: + raise rez_pip.exceptions.RezPipError( + f"{self.download_info.url} is not yet downloaded." + ) + return self.__localPath + + @localPath.setter + def localPath(self, path: str) -> None: + self.__localPath = path + class PackageGroup: """A group of package""" @@ -74,6 +97,19 @@ def __init__(self, packages: typing.List[PackageInfo]) -> None: self.packages = packages self.dists = [] + def __str__(self) -> str: + return "PackageGroup({})".format( + [f"{p.name}=={p.version}" for p in self.packages] + ) + + def __repr__(self) -> str: + return "PackageGroup({})".format( + [f"{p.name}=={p.version}" for p in self.packages] + ) + + def __bool__(self) -> bool: + return bool(self.packages) + @property def downloadUrls(self) -> typing.List[str]: return [p.download_info.url for p in self.packages] diff --git a/src/rez_pip/plugins/PySide6.py b/src/rez_pip/plugins/PySide6.py index cc02953..0ab31b9 100644 --- a/src/rez_pip/plugins/PySide6.py +++ b/src/rez_pip/plugins/PySide6.py @@ -11,11 +11,11 @@ 1. Merge PySide6, PySide6-Essentials and PySide6-Addons into the same install. Unvendor shiboken. 2. Install shiboken + cleanup. The Cleanup could be its own hook here specific to shiboken. """ + import os import sys import shutil import typing -import logging if sys.version_info >= (3, 10): import importlib.metadata as importlib_metadata @@ -36,14 +36,10 @@ # PySide6-Addons. -_LOG = logging.getLogger(__name__) - - @rez_pip.plugins.hookimpl def prePipResolve( packages: typing.List[str], ) -> None: - _LOG.debug(f"prePipResolve start") pyside6Seen = False variantsSeens = [] @@ -116,5 +112,9 @@ def cleanup(dist: importlib_metadata.Distribution, path: str) -> None: ]: return + # Remove shiboken6 from PySide6 packages... + # PySide6 >=6.3, <6.6.2 were shipping some shiboken6 folders by mistake. + # Not removing these extra folders would stop python from being able to import + # the correct shiboken (that lives in a separate rez package). shutil.rmtree(os.path.join(path, "python", "shiboken6")) shutil.rmtree(os.path.join(path, "python", "shiboken6_generator")) diff --git a/src/rez_pip/plugins/__init__.py b/src/rez_pip/plugins/__init__.py index d2bb53b..1d959d8 100644 --- a/src/rez_pip/plugins/__init__.py +++ b/src/rez_pip/plugins/__init__.py @@ -1,10 +1,12 @@ """Plugin system.""" + import sys import typing import logging import pkgutil import functools import importlib +import collections.abc if sys.version_info >= (3, 10): import importlib.metadata as importlib_metadata @@ -26,7 +28,7 @@ def __dir__() -> typing.List[str]: return __all__ -__LOG = logging.getLogger(__name__) +_LOG = logging.getLogger(__name__) F = typing.TypeVar("F", bound=typing.Callable[..., typing.Any]) hookspec = typing.cast(typing.Callable[[F], F], pluggy.HookspecMarker("rez-pip")) @@ -36,7 +38,9 @@ def __dir__() -> typing.List[str]: class PluginSpec: @hookspec def prePipResolve( - self, packages: typing.List[str], requirements: typing.List[str] + self, + packages: collections.abc.Sequence[str], # Immutable + requirements: collections.abc.Sequence[str], # Immutable ) -> None: """ Take an action before resolving the packages using pip. @@ -44,7 +48,9 @@ def prePipResolve( """ @hookspec - def postPipResolve(self, packages: typing.List["rez_pip.pip.PackageInfo"]) -> None: + def postPipResolve( + self, packages: collections.abc.Sequence["rez_pip.pip.PackageInfo"] # Immutable + ) -> None: """ Take an action after resolving the packages using pip. The packages argument should not be modified in any way. @@ -52,8 +58,8 @@ def postPipResolve(self, packages: typing.List["rez_pip.pip.PackageInfo"]) -> No @hookspec def groupPackages( # type: ignore[empty-body] - self, packages: typing.List["rez_pip.pip.PackageInfo"] - ) -> typing.List["rez_pip.pip.PackageGroup"]: + self, packages: collections.abc.MutableSequence["rez_pip.pip.PackageInfo"] + ) -> collections.abc.Sequence["rez_pip.pip.PackageGroup"]: """ Merge packages into groups of packages. The name and version of the first package in the group will be used as the name and version for the rez package. @@ -73,6 +79,25 @@ def metadata(self, package: rez.package_maker.PackageMaker) -> None: """ +def before( + hookName: str, + hookImpls: collections.abc.Sequence[pluggy.HookImpl], + kwargs: collections.abc.Mapping[str, typing.Any], +) -> None: + """Function that will be called before each hook.""" + _LOG.debug("Calling the %r hooks", hookName) + + +def after( + outcome: pluggy.Result[typing.Any], + hookName: str, + hookImpls: collections.abc.Sequence[pluggy.HookImpl], + kwargs: collections.abc.Mapping[str, typing.Any], +) -> None: + """Function that will be called after each hook.""" + _LOG.debug("Called the %r hooks", hookName) + + @functools.lru_cache() def getManager() -> pluggy.PluginManager: """ @@ -80,8 +105,9 @@ def getManager() -> pluggy.PluginManager: and the cached value will be return in subsequent calls. """ manager = pluggy.PluginManager("rez-pip") - # manager.trace.root.setwriter(print) - # manager.enable_tracing() + if _LOG.getEffectiveLevel() <= logging.DEBUG: + manager.trace.root.setwriter(print) + manager.enable_tracing() manager.add_hookspecs(PluginSpec) @@ -94,6 +120,7 @@ def getManager() -> pluggy.PluginManager: manager.load_setuptools_entrypoints("rez-pip") + manager.add_hookcall_monitoring(before, after) return manager diff --git a/src/rez_pip/plugins/shiboken6.py b/src/rez_pip/plugins/shiboken6.py index 27424b7..e0d7941 100644 --- a/src/rez_pip/plugins/shiboken6.py +++ b/src/rez_pip/plugins/shiboken6.py @@ -1,6 +1,7 @@ import os import sys import shutil +import logging import packaging.utils @@ -11,10 +12,19 @@ else: import importlib_metadata +_LOG = logging.getLogger(__name__) + @rez_pip.plugins.hookimpl def cleanup(dist: importlib_metadata.Distribution, path: str) -> None: - if packaging.utils.canonicalize_name(dist.name) == "shiboken6": - path = os.path.join(path, "python", "PySide6") - print(f"Removing {path!r}") + if packaging.utils.canonicalize_name(dist.name) != "shiboken6": + return + + # Remove PySide6 from shiboken6 packages... + # shiboken6 >=6.3, <6.6.2 were shipping some PySide6 folders by mistake. + # Not removing these extra folders would stop python from being able to import + # the correct PySide6 (that lives in a separate rez package). + path = os.path.join(path, "python", "PySide6") + if os.path.exists(path): + _LOG.debug(f"Removing {path!r}") shutil.rmtree(path) From e57ce9242a1597dd12c1260799d46fc9fbd5a249 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 28 Apr 2024 17:41:23 -0400 Subject: [PATCH 07/11] Fix download tests and add a test for skipped downloads Signed-off-by: Jean-Christophe Morin --- tests/test_download.py | 318 ++++++++++++++++++++++++++--------------- 1 file changed, 202 insertions(+), 116 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index cf8530e..91e830a 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -27,32 +27,54 @@ def rezPipVersion(): yield +class Package: + def __init__(self, name: str, content: str, local: bool): + self.name = name + self.content = content + self.local = local + + +class Group: + def __init__(self, packages: typing.List[Package]): + self.packages = packages + + def getPackage(self, name: str) -> Package: + for package in self.packages: + if package.name == name: + return package + raise KeyError(name) + + @pytest.mark.parametrize( - "packages", + "groups", [ - {"package-a": "package-a data"}, - {"package-a": "package-a data", "package-b": "package-b data"}, + [Group([Package("package-a", "package-a data", False)])], + [ + Group([Package("package-a", "package-a data", False)]), + Group([Package("package-b", "package-b data", False)]), + ], ], - ids=["single-package", "multiple-packages"], + ids=["one-group-with-one-package", "multiple-groups-with-one-package"], ) -def test_download(packages: typing.Dict[str, str], tmp_path: pathlib.Path): +def test_download(groups: typing.List[Group], tmp_path: pathlib.Path): sideEffects = tuple() - for content in packages.values(): - mockedContent = mock.MagicMock() - mockedContent.return_value.__aiter__.return_value = [ - [ - content.encode("utf-8"), - None, + for group in groups: + for package in group.packages: + mockedContent = mock.MagicMock() + mockedContent.return_value.__aiter__.return_value = [ + [ + package.content.encode("utf-8"), + None, + ] ] - ] - sideEffects += ( - mock.Mock( - headers={"content-length": 100}, - status=200, - content=mock.Mock(iter_chunks=mockedContent), - ), - ) + sideEffects += ( + mock.Mock( + headers={"content-length": 100}, + status=200, + content=mock.Mock(iter_chunks=mockedContent), + ), + ) mockedGet = mock.AsyncMock() mockedGet.__aenter__.side_effect = sideEffects @@ -60,43 +82,83 @@ def test_download(packages: typing.Dict[str, str], tmp_path: pathlib.Path): with mock.patch.object(aiohttp.ClientSession, "get") as mocked: mocked.return_value = mockedGet - wheels = rez_pip.download.downloadPackages( - [ - rez_pip.pip.PackageInfo( - metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), - download_info=rez_pip.pip.DownloadInfo( - url=f"https://example.com/{package}.whl", - archive_info=rez_pip.pip.ArchiveInfo("hash", {}), - ), - is_direct=True, - requested=True, + _groups = [] + for group in groups: + infos = [] + for package in group.packages: + infos.append( + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata( + name=package.name, version="1.0.0" + ), + download_info=rez_pip.pip.DownloadInfo( + url=f"https://example.com/{package.name}.whl", + archive_info=rez_pip.pip.ArchiveInfo("hash", {}), + ), + is_direct=True, + requested=True, + ) ) - for package in packages - ], - os.fspath(tmp_path), - ) + _groups.append(rez_pip.pip.PackageGroup(infos)) + + wheels = rez_pip.download.downloadPackages(_groups, os.fspath(tmp_path)) assert sorted(wheels) == sorted( - [os.fspath(tmp_path / f"{package}.whl") for package in packages] + [ + os.fspath(tmp_path / f"{package.name}.whl") + for group in groups + for package in group.packages + ] ) - for wheel in wheels: - with open(wheel, "r") as fd: - content = fd.read() - assert packages[os.path.basename(wheel).split(".")[0]] == content + wheelsMapping = {os.path.basename(wheel).split(".")[0]: wheel for wheel in wheels} + + for group in groups: + for package in group.packages: + with open(wheelsMapping[package.name], "r") as fd: + content = fd.read() + assert content == package.content assert mocked.call_args_list == [ mock.call( - f"https://example.com/{package}.whl", + f"https://example.com/{package.name}.whl", headers={ "Content-Type": "application/octet-stream", "User-Agent": "rez-pip/1.2.3.4.5", }, ) - for package in packages + for group in groups + for package in group.packages ] +def test_download_skip_local(tmp_path: pathlib.Path): + groups = [ + rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata(name="package-a", version="1.0.0"), + download_info=rez_pip.pip.DownloadInfo( + url="file:///example.com/package-a", + archive_info=rez_pip.pip.ArchiveInfo("hash-a", {}), + ), + is_direct=True, + requested=True, + ) + ] + ) + ] + + mockedGet = mock.AsyncMock() + + with mock.patch.object(aiohttp.ClientSession, "get") as mocked: + mocked.return_value = mockedGet + wheels = rez_pip.download.downloadPackages(groups, os.fspath(tmp_path)) + + assert not mocked.called + assert wheels == [] + + def test_download_multiple_packages_with_failure(tmp_path: pathlib.Path): mockedContent = mock.MagicMock() mockedContent.return_value.__aiter__.return_value = [ @@ -127,27 +189,35 @@ def test_download_multiple_packages_with_failure(tmp_path: pathlib.Path): with pytest.raises(RuntimeError): rez_pip.download.downloadPackages( [ - rez_pip.pip.PackageInfo( - metadata=rez_pip.pip.Metadata( - name="package-a", version="1.0.0" - ), - download_info=rez_pip.pip.DownloadInfo( - url="https://example.com/package-a", - archive_info=rez_pip.pip.ArchiveInfo("hash-a", {}), - ), - is_direct=True, - requested=True, + rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata( + name="package-a", version="1.0.0" + ), + download_info=rez_pip.pip.DownloadInfo( + url="https://example.com/package-a", + archive_info=rez_pip.pip.ArchiveInfo("hash-a", {}), + ), + is_direct=True, + requested=True, + ) + ] ), - rez_pip.pip.PackageInfo( - metadata=rez_pip.pip.Metadata( - name="package-b", version="1.0.0" - ), - download_info=rez_pip.pip.DownloadInfo( - url="https://example.com/package-b", - archive_info=rez_pip.pip.ArchiveInfo("hash-b", {}), - ), - is_direct=True, - requested=True, + rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata( + name="package-b", version="1.0.0" + ), + download_info=rez_pip.pip.DownloadInfo( + url="https://example.com/package-b", + archive_info=rez_pip.pip.ArchiveInfo("hash-b", {}), + ), + is_direct=True, + requested=True, + ) + ] ), ], os.fspath(tmp_path), @@ -178,7 +248,7 @@ def test_download_multiple_packages_with_failure(tmp_path: pathlib.Path): def test_download_reuse_if_same_hash(tmp_path: pathlib.Path): """Test that wheels are re-used if the sha256 matches""" sideEffects = tuple() - packages = [] + groups = [] for package in ["package-a", "package-b"]: content = f"{package} data".encode("utf-8") @@ -186,17 +256,21 @@ def test_download_reuse_if_same_hash(tmp_path: pathlib.Path): hash = hashlib.new("sha256") hash.update(content) - packages.append( - rez_pip.pip.PackageInfo( - metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), - download_info=rez_pip.pip.DownloadInfo( - url=f"https://example.com/{package}.whl", - archive_info=rez_pip.pip.ArchiveInfo( - "hash-a", {"sha256": hash.hexdigest()} - ), - ), - is_direct=True, - requested=True, + groups.append( + rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), + download_info=rez_pip.pip.DownloadInfo( + url=f"https://example.com/{package}.whl", + archive_info=rez_pip.pip.ArchiveInfo( + "hash-a", {"sha256": hash.hexdigest()} + ), + ), + is_direct=True, + requested=True, + ) + ] ) ) @@ -222,7 +296,7 @@ def test_download_reuse_if_same_hash(tmp_path: pathlib.Path): with mock.patch.object(aiohttp.ClientSession, "get") as mocked: mocked.return_value = mockedGet1 - rez_pip.download.downloadPackages(packages, str(tmp_path)) + rez_pip.download.downloadPackages(groups, str(tmp_path)) assert mocked.call_args_list == [ mock.call( @@ -241,7 +315,7 @@ def test_download_reuse_if_same_hash(tmp_path: pathlib.Path): ), ] - packages = [] + groups = [] # package-b will be re-used for package in ["package-c", "package-b"]: content = f"{package} data".encode("utf-8") @@ -249,17 +323,21 @@ def test_download_reuse_if_same_hash(tmp_path: pathlib.Path): hash = hashlib.new("sha256") hash.update(content) - packages.append( - rez_pip.pip.PackageInfo( - metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), - download_info=rez_pip.pip.DownloadInfo( - url=f"https://example.com/{package}.whl", - archive_info=rez_pip.pip.ArchiveInfo( - "hash-a", {"sha256": hash.hexdigest()} - ), - ), - is_direct=True, - requested=True, + groups.append( + rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), + download_info=rez_pip.pip.DownloadInfo( + url=f"https://example.com/{package}.whl", + archive_info=rez_pip.pip.ArchiveInfo( + "hash-a", {"sha256": hash.hexdigest()} + ), + ), + is_direct=True, + requested=True, + ) + ] ) ) @@ -285,7 +363,7 @@ def test_download_reuse_if_same_hash(tmp_path: pathlib.Path): with mock.patch.object(aiohttp.ClientSession, "get") as mocked: mocked.return_value = mockedGet2 - wheels = rez_pip.download.downloadPackages(packages, str(tmp_path)) + wheels = rez_pip.download.downloadPackages(groups, str(tmp_path)) assert mocked.call_args_list == [ mock.call( @@ -305,7 +383,7 @@ def test_download_reuse_if_same_hash(tmp_path: pathlib.Path): def test_download_redownload_if_hash_changes(tmp_path: pathlib.Path): """Test that wheels are re-used if the sha256 matches""" sideEffects = tuple() - packages = [] + groups = [] for package in ["package-a", "package-b"]: content = f"{package} data".encode("utf-8") @@ -313,17 +391,21 @@ def test_download_redownload_if_hash_changes(tmp_path: pathlib.Path): hash = hashlib.new("sha256") hash.update(content) - packages.append( - rez_pip.pip.PackageInfo( - metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), - download_info=rez_pip.pip.DownloadInfo( - url=f"https://example.com/{package}.whl", - archive_info=rez_pip.pip.ArchiveInfo( - "hash-a", {"sha256": hash.hexdigest()} - ), - ), - is_direct=True, - requested=True, + groups.append( + rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), + download_info=rez_pip.pip.DownloadInfo( + url=f"https://example.com/{package}.whl", + archive_info=rez_pip.pip.ArchiveInfo( + "hash-a", {"sha256": hash.hexdigest()} + ), + ), + is_direct=True, + requested=True, + ) + ] ) ) @@ -349,7 +431,7 @@ def test_download_redownload_if_hash_changes(tmp_path: pathlib.Path): with mock.patch.object(aiohttp.ClientSession, "get") as mocked: mocked.return_value = mockedGet1 - rez_pip.download.downloadPackages(packages, str(tmp_path)) + rez_pip.download.downloadPackages(groups, str(tmp_path)) assert mocked.call_args_list == [ mock.call( @@ -368,26 +450,30 @@ def test_download_redownload_if_hash_changes(tmp_path: pathlib.Path): ), ] - packages = [] + groups = [] # package-b will be re-used for package in ["package-a", "package-b"]: content = f"{package} data".encode("utf-8") - packages.append( - rez_pip.pip.PackageInfo( - metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), - download_info=rez_pip.pip.DownloadInfo( - url=f"https://example.com/{package}.whl", - archive_info=rez_pip.pip.ArchiveInfo( - # - # Bad sha256. This will trigger a new download - # - "hash-a", - {"sha256": "asd"}, - ), - ), - is_direct=True, - requested=True, + groups.append( + rez_pip.pip.PackageGroup( + [ + rez_pip.pip.PackageInfo( + metadata=rez_pip.pip.Metadata(name=package, version="1.0.0"), + download_info=rez_pip.pip.DownloadInfo( + url=f"https://example.com/{package}.whl", + archive_info=rez_pip.pip.ArchiveInfo( + # + # Bad sha256. This will trigger a new download + # + "hash-a", + {"sha256": "asd"}, + ), + ), + is_direct=True, + requested=True, + ) + ] ) ) @@ -413,7 +499,7 @@ def test_download_redownload_if_hash_changes(tmp_path: pathlib.Path): with mock.patch.object(aiohttp.ClientSession, "get") as mocked: mocked.return_value = mockedGet2 - wheels = rez_pip.download.downloadPackages(packages, str(tmp_path)) + wheels = rez_pip.download.downloadPackages(groups, str(tmp_path)) assert mocked.call_args_list == [ mock.call( From 64de12b8024e0badc345a442ee8deb57f3b9ad35 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 28 Apr 2024 21:46:45 -0400 Subject: [PATCH 08/11] Fix typing Signed-off-by: Jean-Christophe Morin --- src/rez_pip/install.py | 7 +++++-- src/rez_pip/pip.py | 2 +- src/rez_pip/plugins/__init__.py | 10 +++++++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/rez_pip/install.py b/src/rez_pip/install.py index 1e59bf8..693cee5 100644 --- a/src/rez_pip/install.py +++ b/src/rez_pip/install.py @@ -39,6 +39,9 @@ def isWheelPure(dist: importlib_metadata.Distribution) -> bool: + # dist.files should never be empty, but assert to silence mypy. + assert dist.files is not None + path = next( f for f in dist.files if os.fspath(f.locate()).endswith(".dist-info/WHEEL") ) @@ -72,7 +75,7 @@ def getSchemeDict(name: str, target: str) -> typing.Dict[str, str]: def installWheel( package: rez_pip.pip.PackageInfo, - wheelPath: pathlib.Path, + wheelPath: str, targetPath: str, ) -> importlib_metadata.Distribution: # TODO: Technically, target should be optional. We will always want to install in "pip install --target" @@ -86,7 +89,7 @@ def installWheel( ) _LOG.debug(f"Installing {wheelPath} into {targetPath!r}") - with installer.sources.WheelFile.open(wheelPath) as source: + with installer.sources.WheelFile.open(pathlib.Path(wheelPath)) as source: installer.install( source=source, destination=destination, diff --git a/src/rez_pip/pip.py b/src/rez_pip/pip.py index faee8fa..2a337c8 100644 --- a/src/rez_pip/pip.py +++ b/src/rez_pip/pip.py @@ -67,7 +67,7 @@ def name(self) -> str: def version(self) -> str: return self.metadata.version - def isDownloadRequired(self): + def isDownloadRequired(self) -> bool: return not self.download_info.url.startswith("file://") @property diff --git a/src/rez_pip/plugins/__init__.py b/src/rez_pip/plugins/__init__.py index 1d959d8..b7094a9 100644 --- a/src/rez_pip/plugins/__init__.py +++ b/src/rez_pip/plugins/__init__.py @@ -140,7 +140,11 @@ def _getHookImplementations() -> typing.Dict[str, typing.List[str]]: implementations = {} for name, plugin in manager.list_name_plugin(): - implementations[name] = [ - caller.name for caller in manager.get_hookcallers(plugin) - ] + hookcallers = manager.get_hookcallers(plugin) + + # hookcallers will never be None because we get the names from list_name_plugin. + # But it silences mypy. + assert hookcallers is not None + + implementations[name] = [caller.name for caller in hookcallers] return implementations From 4b8672fd65642f49391687f21cbe5a1e1e1c2f5a Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 28 Apr 2024 22:13:44 -0400 Subject: [PATCH 09/11] Fix typing Signed-off-by: Jean-Christophe Morin --- src/rez_pip/cli.py | 6 +----- src/rez_pip/compat.py | 13 +++++++++++++ src/rez_pip/download.py | 7 +------ src/rez_pip/install.py | 6 +----- src/rez_pip/pip.py | 8 +++----- src/rez_pip/plugins/PySide6.py | 9 +++------ src/rez_pip/plugins/__init__.py | 30 +++++++++++++----------------- src/rez_pip/plugins/shiboken6.py | 8 +++----- src/rez_pip/rez.py | 7 +------ src/rez_pip/utils.py | 9 +++------ tests/test_cli.py | 6 +----- tests/test_download.py | 6 +----- tests/test_install.py | 3 +-- tests/test_rez.py | 7 +------ 14 files changed, 46 insertions(+), 79 deletions(-) create mode 100644 src/rez_pip/compat.py diff --git a/src/rez_pip/cli.py b/src/rez_pip/cli.py index a661bf6..f90d972 100644 --- a/src/rez_pip/cli.py +++ b/src/rez_pip/cli.py @@ -10,11 +10,6 @@ import itertools import subprocess -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import rich import rich.text import rich.panel @@ -30,6 +25,7 @@ import rez_pip.install import rez_pip.download import rez_pip.exceptions +from rez_pip.compat import importlib_metadata _LOG = logging.getLogger("rez_pip.cli") diff --git a/src/rez_pip/compat.py b/src/rez_pip/compat.py new file mode 100644 index 0000000..b5b1e57 --- /dev/null +++ b/src/rez_pip/compat.py @@ -0,0 +1,13 @@ +import sys + +if sys.version_info <= (3, 8): + from typing import Sequence, MutableSequence, Mapping +else: + from collections.abc import Sequence, MutableSequence, Mapping + +if sys.version_info >= (3, 10): + import importlib.metadata as importlib_metadata +else: + import importlib_metadata + +__all__ = ["Sequence", "MutableSequence", "Mapping", "importlib_metadata"] diff --git a/src/rez_pip/download.py b/src/rez_pip/download.py index 0b2d943..a877e73 100644 --- a/src/rez_pip/download.py +++ b/src/rez_pip/download.py @@ -1,5 +1,4 @@ import os -import sys import typing import asyncio import hashlib @@ -9,12 +8,8 @@ import aiohttp import rich.progress -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import rez_pip.pip +from rez_pip.compat import importlib_metadata _LOG = logging.getLogger(__name__) _lock = asyncio.Lock() diff --git a/src/rez_pip/install.py b/src/rez_pip/install.py index 693cee5..f24051c 100644 --- a/src/rez_pip/install.py +++ b/src/rez_pip/install.py @@ -11,11 +11,6 @@ import pathlib import sysconfig -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - if typing.TYPE_CHECKING: if sys.version_info >= (3, 8): from typing import Literal @@ -30,6 +25,7 @@ import installer.destinations import rez_pip.pip +from rez_pip.compat import importlib_metadata _LOG = logging.getLogger(__name__) diff --git a/src/rez_pip/pip.py b/src/rez_pip/pip.py index 2a337c8..9f21d08 100644 --- a/src/rez_pip/pip.py +++ b/src/rez_pip/pip.py @@ -8,17 +8,15 @@ import subprocess import dataclasses -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import dataclasses_json import rez_pip.data import rez_pip.plugins import rez_pip.exceptions +if typing.TYPE_CHECKING: + import importlib.metadata as importlib_metadata + _LOG = logging.getLogger(__name__) diff --git a/src/rez_pip/plugins/PySide6.py b/src/rez_pip/plugins/PySide6.py index 0ab31b9..81cecba 100644 --- a/src/rez_pip/plugins/PySide6.py +++ b/src/rez_pip/plugins/PySide6.py @@ -13,15 +13,9 @@ """ import os -import sys import shutil import typing -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import packaging.utils import packaging.version import packaging.specifiers @@ -31,6 +25,9 @@ import rez_pip.plugins import rez_pip.exceptions +if typing.TYPE_CHECKING: + from rez_pip.compat import importlib_metadata + # PySide6 was initiall a single package that had shiboken as a dependency. # Starting from 6.3.0, the package was spit in 3, PySide6, PySide6-Essentials and # PySide6-Addons. diff --git a/src/rez_pip/plugins/__init__.py b/src/rez_pip/plugins/__init__.py index b7094a9..413bd39 100644 --- a/src/rez_pip/plugins/__init__.py +++ b/src/rez_pip/plugins/__init__.py @@ -1,23 +1,17 @@ """Plugin system.""" -import sys import typing import logging import pkgutil import functools import importlib -import collections.abc - -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata import pluggy import rez.package_maker if typing.TYPE_CHECKING: import rez_pip.pip + import rez_pip.compat __all__ = [ "hookimpl", @@ -39,8 +33,8 @@ class PluginSpec: @hookspec def prePipResolve( self, - packages: collections.abc.Sequence[str], # Immutable - requirements: collections.abc.Sequence[str], # Immutable + packages: rez_pip.compat.Sequence[str], # Immutable + requirements: rez_pip.compat.Sequence[str], # Immutable ) -> None: """ Take an action before resolving the packages using pip. @@ -49,7 +43,7 @@ def prePipResolve( @hookspec def postPipResolve( - self, packages: collections.abc.Sequence["rez_pip.pip.PackageInfo"] # Immutable + self, packages: rez_pip.compat.Sequence["rez_pip.pip.PackageInfo"] # Immutable ) -> None: """ Take an action after resolving the packages using pip. @@ -58,8 +52,8 @@ def postPipResolve( @hookspec def groupPackages( # type: ignore[empty-body] - self, packages: collections.abc.MutableSequence["rez_pip.pip.PackageInfo"] - ) -> collections.abc.Sequence["rez_pip.pip.PackageGroup"]: + self, packages: rez_pip.compat.MutableSequence["rez_pip.pip.PackageInfo"] + ) -> rez_pip.compat.Sequence["rez_pip.pip.PackageGroup"]: """ Merge packages into groups of packages. The name and version of the first package in the group will be used as the name and version for the rez package. @@ -68,7 +62,9 @@ def groupPackages( # type: ignore[empty-body] """ @hookspec - def cleanup(self, dist: importlib_metadata.Distribution, path: str) -> None: + def cleanup( + self, dist: rez_pip.compat.importlib_metadata.Distribution, path: str + ) -> None: """Cleanup installed distribution""" @hookspec @@ -81,8 +77,8 @@ def metadata(self, package: rez.package_maker.PackageMaker) -> None: def before( hookName: str, - hookImpls: collections.abc.Sequence[pluggy.HookImpl], - kwargs: collections.abc.Mapping[str, typing.Any], + hookImpls: rez_pip.compat.Sequence[pluggy.HookImpl], + kwargs: rez_pip.compat.Mapping[str, typing.Any], ) -> None: """Function that will be called before each hook.""" _LOG.debug("Calling the %r hooks", hookName) @@ -91,8 +87,8 @@ def before( def after( outcome: pluggy.Result[typing.Any], hookName: str, - hookImpls: collections.abc.Sequence[pluggy.HookImpl], - kwargs: collections.abc.Mapping[str, typing.Any], + hookImpls: rez_pip.compat.Sequence[pluggy.HookImpl], + kwargs: rez_pip.compat.Mapping[str, typing.Any], ) -> None: """Function that will be called after each hook.""" _LOG.debug("Called the %r hooks", hookName) diff --git a/src/rez_pip/plugins/shiboken6.py b/src/rez_pip/plugins/shiboken6.py index e0d7941..8569f59 100644 --- a/src/rez_pip/plugins/shiboken6.py +++ b/src/rez_pip/plugins/shiboken6.py @@ -1,5 +1,5 @@ import os -import sys +import typing import shutil import logging @@ -7,10 +7,8 @@ import rez_pip.plugins -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata +if typing.TYPE_CHECKING: + from rez_pip.compat import importlib_metadata _LOG = logging.getLogger(__name__) diff --git a/src/rez_pip/rez.py b/src/rez_pip/rez.py index 31e6b9a..9f4dc0a 100644 --- a/src/rez_pip/rez.py +++ b/src/rez_pip/rez.py @@ -1,5 +1,4 @@ import os -import sys import copy import shutil import typing @@ -7,11 +6,6 @@ import pathlib import itertools -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import rez.config import rez.version import rez.packages @@ -21,6 +15,7 @@ import rez_pip.pip import rez_pip.utils import rez_pip.plugins +from rez_pip.compat import importlib_metadata _LOG = logging.getLogger(__name__) diff --git a/src/rez_pip/utils.py b/src/rez_pip/utils.py index e4acc00..cb0a97f 100644 --- a/src/rez_pip/utils.py +++ b/src/rez_pip/utils.py @@ -1,13 +1,7 @@ -import sys import typing import logging import dataclasses -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import rez.system import rez.version import packaging.version @@ -16,6 +10,9 @@ import rez_pip.install +if typing.TYPE_CHECKING: + from rez_pip.compat import importlib_metadata + _LOG = logging.getLogger(__name__) diff --git a/tests/test_cli.py b/tests/test_cli.py index 31769fd..c01ff69 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,11 +6,6 @@ import subprocess import unittest.mock -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import pytest import rich.console import packaging.version @@ -19,6 +14,7 @@ import rez_pip.pip import rez_pip.rez import rez_pip.exceptions +from rez_pip.compat import importlib_metadata def test_parseArgs_empty(): diff --git a/tests/test_download.py b/tests/test_download.py index 91e830a..765c696 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -9,16 +9,12 @@ else: from unittest import mock -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import pytest import aiohttp import rez_pip.pip import rez_pip.download +from rez_pip.compat import importlib_metadata @pytest.fixture(scope="module", autouse=True) diff --git a/tests/test_install.py b/tests/test_install.py index 45803eb..7196f3c 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1,6 +1,4 @@ import os -import sys -import glob import pathlib import platform import subprocess @@ -10,6 +8,7 @@ import rez_pip.pip import rez_pip.install +from rez_pip.compat import importlib_metadata from . import utils diff --git a/tests/test_rez.py b/tests/test_rez.py index db0dcb0..ba48d14 100644 --- a/tests/test_rez.py +++ b/tests/test_rez.py @@ -1,5 +1,4 @@ import os -import sys import stat import typing import pathlib @@ -12,14 +11,10 @@ import rez.packages import rez.package_repository -if sys.version_info >= (3, 10): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - import rez_pip.pip import rez_pip.rez import rez_pip.utils +from rez_pip.compat import importlib_metadata def test_createPackage(monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path): From a2a1f9ef3319906a3bdcd47e900c402925998b85 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 28 Apr 2024 22:14:54 -0400 Subject: [PATCH 10/11] Add basic test for plugins module Signed-off-by: Jean-Christophe Morin --- pyproject.toml | 1 + tests/plugins/test_plugins.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 tests/plugins/test_plugins.py diff --git a/pyproject.toml b/pyproject.toml index 9e30e37..3dcbbc5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,7 @@ dependencies = [ "importlib_metadata>=4.6 ; python_version < '3.10'", # 1.3 introduces type hints. "pluggy>=1.2", + "typing-extensions; python_version < '3.8'" ] classifiers = [ diff --git a/tests/plugins/test_plugins.py b/tests/plugins/test_plugins.py new file mode 100644 index 0000000..8649431 --- /dev/null +++ b/tests/plugins/test_plugins.py @@ -0,0 +1,26 @@ +import typing + +import pluggy + +import rez_pip.plugins + + +def test_getManager(): + assert isinstance(rez_pip.plugins.getManager(), pluggy.PluginManager) + + +def test_getHook(): + assert isinstance(rez_pip.plugins.getHook(), pluggy.HookRelay) + + +def test_getHookImplementations(): + implementations = rez_pip.plugins._getHookImplementations() + assert implementations == { + "rez_pip.PySide6": [ + "cleanup", + "groupPackages", + "postPipResolve", + "prePipResolve", + ], + "rez_pip.shiboken6": ["cleanup"], + } From 361601a6cd9a05b31a4a0368e36d920c21494928 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 28 Apr 2024 22:24:22 -0400 Subject: [PATCH 11/11] Fix typing again Signed-off-by: Jean-Christophe Morin --- src/rez_pip/pip.py | 2 +- src/rez_pip/plugins/PySide6.py | 2 +- src/rez_pip/plugins/__init__.py | 21 +++++++++++---------- src/rez_pip/plugins/shiboken6.py | 2 +- src/rez_pip/utils.py | 2 +- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/rez_pip/pip.py b/src/rez_pip/pip.py index 9f21d08..bd03a92 100644 --- a/src/rez_pip/pip.py +++ b/src/rez_pip/pip.py @@ -89,7 +89,7 @@ class PackageGroup: """A group of package""" packages: typing.List[PackageInfo] - dists: typing.List[importlib_metadata.Distribution] + dists: typing.List["importlib_metadata.Distribution"] def __init__(self, packages: typing.List[PackageInfo]) -> None: self.packages = packages diff --git a/src/rez_pip/plugins/PySide6.py b/src/rez_pip/plugins/PySide6.py index 81cecba..df9c801 100644 --- a/src/rez_pip/plugins/PySide6.py +++ b/src/rez_pip/plugins/PySide6.py @@ -101,7 +101,7 @@ def groupPackages( @rez_pip.plugins.hookimpl -def cleanup(dist: importlib_metadata.Distribution, path: str) -> None: +def cleanup(dist: "importlib_metadata.Distribution", path: str) -> None: if packaging.utils.canonicalize_name(dist.name) not in [ "pyside6", "pyside6-addons", diff --git a/src/rez_pip/plugins/__init__.py b/src/rez_pip/plugins/__init__.py index 413bd39..6c80065 100644 --- a/src/rez_pip/plugins/__init__.py +++ b/src/rez_pip/plugins/__init__.py @@ -33,8 +33,8 @@ class PluginSpec: @hookspec def prePipResolve( self, - packages: rez_pip.compat.Sequence[str], # Immutable - requirements: rez_pip.compat.Sequence[str], # Immutable + packages: "rez_pip.compat.Sequence[str]", # Immutable + requirements: "rez_pip.compat.Sequence[str]", # Immutable ) -> None: """ Take an action before resolving the packages using pip. @@ -43,7 +43,8 @@ def prePipResolve( @hookspec def postPipResolve( - self, packages: rez_pip.compat.Sequence["rez_pip.pip.PackageInfo"] # Immutable + self, + packages: 'rez_pip.compat.Sequence["rez_pip.pip.PackageInfo"]', # Immutable ) -> None: """ Take an action after resolving the packages using pip. @@ -52,8 +53,8 @@ def postPipResolve( @hookspec def groupPackages( # type: ignore[empty-body] - self, packages: rez_pip.compat.MutableSequence["rez_pip.pip.PackageInfo"] - ) -> rez_pip.compat.Sequence["rez_pip.pip.PackageGroup"]: + self, packages: 'rez_pip.compat.MutableSequence["rez_pip.pip.PackageInfo"]' + ) -> 'rez_pip.compat.Sequence["rez_pip.pip.PackageGroup"]': """ Merge packages into groups of packages. The name and version of the first package in the group will be used as the name and version for the rez package. @@ -63,7 +64,7 @@ def groupPackages( # type: ignore[empty-body] @hookspec def cleanup( - self, dist: rez_pip.compat.importlib_metadata.Distribution, path: str + self, dist: "rez_pip.compat.importlib_metadata.Distribution", path: str ) -> None: """Cleanup installed distribution""" @@ -77,8 +78,8 @@ def metadata(self, package: rez.package_maker.PackageMaker) -> None: def before( hookName: str, - hookImpls: rez_pip.compat.Sequence[pluggy.HookImpl], - kwargs: rez_pip.compat.Mapping[str, typing.Any], + hookImpls: "rez_pip.compat.Sequence[pluggy.HookImpl]", + kwargs: "rez_pip.compat.Mapping[str, typing.Any]", ) -> None: """Function that will be called before each hook.""" _LOG.debug("Calling the %r hooks", hookName) @@ -87,8 +88,8 @@ def before( def after( outcome: pluggy.Result[typing.Any], hookName: str, - hookImpls: rez_pip.compat.Sequence[pluggy.HookImpl], - kwargs: rez_pip.compat.Mapping[str, typing.Any], + hookImpls: "rez_pip.compat.Sequence[pluggy.HookImpl]", + kwargs: "rez_pip.compat.Mapping[str, typing.Any]", ) -> None: """Function that will be called after each hook.""" _LOG.debug("Called the %r hooks", hookName) diff --git a/src/rez_pip/plugins/shiboken6.py b/src/rez_pip/plugins/shiboken6.py index 8569f59..22fe8d7 100644 --- a/src/rez_pip/plugins/shiboken6.py +++ b/src/rez_pip/plugins/shiboken6.py @@ -14,7 +14,7 @@ @rez_pip.plugins.hookimpl -def cleanup(dist: importlib_metadata.Distribution, path: str) -> None: +def cleanup(dist: "importlib_metadata.Distribution", path: str) -> None: if packaging.utils.canonicalize_name(dist.name) != "shiboken6": return diff --git a/src/rez_pip/utils.py b/src/rez_pip/utils.py index cb0a97f..e4aeb74 100644 --- a/src/rez_pip/utils.py +++ b/src/rez_pip/utils.py @@ -463,7 +463,7 @@ def convertMarker(marker: str) -> typing.List[str]: def getRezRequirements( - installedDist: importlib_metadata.Distribution, + installedDist: "importlib_metadata.Distribution", pythonVersion: rez.version.Version, nameCasings: typing.Optional[typing.List[str]] = None, ) -> RequirementsDict: