Fix remaining issues (I think)

Signed-off-by: Jean-Christophe Morin <[email protected]>

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 = [

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

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 <merged package>: <dists> 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),

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

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]

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"))

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,24 +38,28 @@ 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.
         The packages argument should not be modified in any way.
         """
 
     @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.
         """
 
     @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,15 +79,35 @@ 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:
     """
     Returns the plugin manager. The return value will be cached on first call
     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
 
 

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)