Merge pull request #8685 from chrahunt/use-preparer-for-lazy-wheels

Author: pradyunsg

src/pip/_internal/cli/req_command.py

@@ -218,6 +218,19 @@ def make_requirement_preparer(
         temp_build_dir_path = temp_build_dir.path
         assert temp_build_dir_path is not None
 
+        if '2020-resolver' in options.features_enabled:
+            lazy_wheel = 'fast-deps' in options.features_enabled
+            if lazy_wheel:
+                logger.warning(
+                    'pip is using lazily downloaded wheels using HTTP '
+                    'range requests to obtain dependency information. '
+                    'This experimental feature is enabled through '
+                    '--use-feature=fast-deps and it is not ready for '
+                    'production.'
+                )
+        else:
+            lazy_wheel = False
+
         return RequirementPreparer(
             build_dir=temp_build_dir_path,
             src_dir=options.src_dir,
@@ -229,6 +242,7 @@ def make_requirement_preparer(
             finder=finder,
             require_hashes=options.require_hashes,
             use_user_site=use_user_site,
+            lazy_wheel=lazy_wheel,
         )
 
     @staticmethod
@@ -259,6 +273,7 @@ def make_resolver(
         # "Resolver" class being redefined.
         if '2020-resolver' in options.features_enabled:
             import pip._internal.resolution.resolvelib.resolver
+
             return pip._internal.resolution.resolvelib.resolver.Resolver(
                 preparer=preparer,
                 finder=finder,
@@ -271,7 +286,6 @@ def make_resolver(
                 force_reinstall=force_reinstall,
                 upgrade_strategy=upgrade_strategy,
                 py_version_info=py_version_info,
-                lazy_wheel='fast-deps' in options.features_enabled,
             )
         import pip._internal.resolution.legacy.resolver
         return pip._internal.resolution.legacy.resolver.Resolver(

src/pip/_internal/operations/prepare.py

@@ -9,6 +9,8 @@
 import os
 import shutil
 
+from pip._vendor.contextlib2 import suppress
+from pip._vendor.packaging.utils import canonicalize_name
 from pip._vendor.six import PY2
 
 from pip._internal.distributions import (
@@ -24,6 +26,11 @@
     PreviousBuildDirError,
     VcsHashUnsupported,
 )
+from pip._internal.models.wheel import Wheel
+from pip._internal.network.lazy_wheel import (
+    HTTPRangeRequestUnsupported,
+    dist_from_wheel_url,
+)
 from pip._internal.utils.filesystem import copy2_fixed
 from pip._internal.utils.hashes import MissingHashes
 from pip._internal.utils.logging import indent_log
@@ -329,6 +336,7 @@ def __init__(
         finder,  # type: PackageFinder
         require_hashes,  # type: bool
         use_user_site,  # type: bool
+        lazy_wheel,  # type: bool
     ):
         # type: (...) -> None
         super(RequirementPreparer, self).__init__()
@@ -362,6 +370,9 @@ def __init__(
         # Should install in user site-packages?
         self.use_user_site = use_user_site
 
+        # Should wheels be downloaded lazily?
+        self.use_lazy_wheel = lazy_wheel
+
     @property
     def _download_should_save(self):
         # type: () -> bool
@@ -448,12 +459,48 @@ def _get_linked_req_hashes(self, req):
         # showing the user what the hash should be.
         return req.hashes(trust_internet=False) or MissingHashes()
 
+    def _fetch_metadata(preparer, link):
+        # type: (Link) -> Optional[Distribution]
+        """Fetch metadata, using lazy wheel if possible."""
+        use_lazy_wheel = preparer.use_lazy_wheel
+        remote_wheel = link.is_wheel and not link.is_file
+        if use_lazy_wheel and remote_wheel and not preparer.require_hashes:
+            wheel = Wheel(link.filename)
+            name = canonicalize_name(wheel.name)
+            # If HTTPRangeRequestUnsupported is raised, fallback silently.
+            with indent_log(), suppress(HTTPRangeRequestUnsupported):
+                logger.info(
+                    'Obtaining dependency information from %s %s',
+                    name, wheel.version,
+                )
+                url = link.url.split('#', 1)[0]
+                session = preparer.downloader._session
+                return dist_from_wheel_url(name, url, session)
+        return None
+
     def prepare_linked_requirement(self, req, parallel_builds=False):
         # type: (InstallRequirement, bool) -> Distribution
         """Prepare a requirement to be obtained from req.link."""
         assert req.link
         link = req.link
         self._log_preparing_link(req)
+        wheel_dist = self._fetch_metadata(link)
+        if wheel_dist is not None:
+            req.needs_more_preparation = True
+            return wheel_dist
+        return self._prepare_linked_requirement(req, parallel_builds)
+
+    def prepare_linked_requirement_more(self, req, parallel_builds=False):
+        # type: (InstallRequirement, bool) -> None
+        """Prepare a linked requirement more, if needed."""
+        if not req.needs_more_preparation:
+            return
+        self._prepare_linked_requirement(req, parallel_builds)
+
+    def _prepare_linked_requirement(self, req, parallel_builds):
+        # type: (InstallRequirement, bool) -> Distribution
+        assert req.link
+        link = req.link
         if link.is_wheel and self.wheel_download_dir:
             # Download wheels to a dedicated dir when doing `pip wheel`.
             download_dir = self.wheel_download_dir

src/pip/_internal/req/req_install.py

@@ -213,6 +213,9 @@ def __init__(
         # but after loading this flag should be treated as read only.
         self.use_pep517 = use_pep517
 
+        # This requirement needs more preparation before it can be built
+        self.needs_more_preparation = False
+
     def __str__(self):
         # type: () -> str
         if self.req:

src/pip/_internal/resolution/resolvelib/candidates.py

@@ -1,22 +1,17 @@
 import logging
 import sys
 
-from pip._vendor.contextlib2 import suppress
 from pip._vendor.packaging.specifiers import InvalidSpecifier, SpecifierSet
 from pip._vendor.packaging.utils import canonicalize_name
 from pip._vendor.packaging.version import Version
 
 from pip._internal.exceptions import HashError, MetadataInconsistent
-from pip._internal.network.lazy_wheel import (
-    HTTPRangeRequestUnsupported,
-    dist_from_wheel_url,
-)
+from pip._internal.models.wheel import Wheel
 from pip._internal.req.constructors import (
     install_req_from_editable,
     install_req_from_line,
 )
 from pip._internal.req.req_install import InstallRequirement
-from pip._internal.utils.logging import indent_log
 from pip._internal.utils.misc import dist_is_editable, normalize_version_info
 from pip._internal.utils.packaging import get_requires_python
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
@@ -147,7 +142,6 @@ def __init__(
         self._name = name
         self._version = version
         self._dist = None  # type: Optional[Distribution]
-        self._prepared = False
 
     def __repr__(self):
         # type: () -> str
@@ -203,12 +197,11 @@ def _prepare_distribution(self):
         # type: () -> Distribution
         raise NotImplementedError("Override in subclass")
 
-    def _check_metadata_consistency(self):
-        # type: () -> None
+    def _check_metadata_consistency(self, dist):
+        # type: (Distribution) -> None
         """Check for consistency of project name and version of dist."""
         # TODO: (Longer term) Rather than abort, reject this candidate
         #       and backtrack. This would need resolvelib support.
-        dist = self._dist  # type: Distribution
         name = canonicalize_name(dist.project_name)
         if self._name is not None and self._name != name:
             raise MetadataInconsistent(self._ireq, "name", dist.project_name)
@@ -218,45 +211,23 @@ def _check_metadata_consistency(self):
 
     def _prepare(self):
         # type: () -> None
-        if self._prepared:
+        if self._dist is not None:
             return
         try:
-            self._dist = self._prepare_distribution()
+            dist = self._prepare_distribution()
         except HashError as e:
             e.req = self._ireq
             raise
 
-        assert self._dist is not None, "Distribution already installed"
-        self._check_metadata_consistency()
-        self._prepared = True
-
-    def _fetch_metadata(self):
-        # type: () -> None
-        """Fetch metadata, using lazy wheel if possible."""
-        preparer = self._factory.preparer
-        use_lazy_wheel = self._factory.use_lazy_wheel
-        remote_wheel = self._link.is_wheel and not self._link.is_file
-        if use_lazy_wheel and remote_wheel and not preparer.require_hashes:
-            assert self._name is not None
-            logger.info('Collecting %s', self._ireq.req or self._ireq)
-            # If HTTPRangeRequestUnsupported is raised, fallback silently.
-            with indent_log(), suppress(HTTPRangeRequestUnsupported):
-                logger.info(
-                    'Obtaining dependency information from %s %s',
-                    self._name, self._version,
-                )
-                url = self._link.url.split('#', 1)[0]
-                session = preparer.downloader._session
-                self._dist = dist_from_wheel_url(self._name, url, session)
-                self._check_metadata_consistency()
-        if self._dist is None:
-            self._prepare()
+        assert dist is not None, "Distribution already installed"
+        self._check_metadata_consistency(dist)
+        self._dist = dist
 
     @property
     def dist(self):
         # type: () -> Distribution
         if self._dist is None:
-            self._fetch_metadata()
+            self._prepare()
         return self._dist
 
     def _get_requires_python_specifier(self):
@@ -309,6 +280,20 @@ def __init__(
             logger.debug("Using cached wheel link: %s", cache_entry.link)
             link = cache_entry.link
         ireq = make_install_req_from_link(link, template)
+        assert ireq.link == link
+        if ireq.link.is_wheel and not ireq.link.is_file:
+            wheel = Wheel(ireq.link.filename)
+            wheel_name = canonicalize_name(wheel.name)
+            assert name == wheel_name, (
+               "{!r} != {!r} for wheel".format(name, wheel_name)
+            )
+            # Version may not be present for PEP 508 direct URLs
+            if version is not None:
+                assert str(version) == wheel.version, (
+                    "{!r} != {!r} for wheel {}".format(
+                        version, wheel.version, name
+                    )
+                )
 
         if (cache_entry is not None and
                 cache_entry.persistent and

src/pip/_internal/resolution/resolvelib/factory.py

@@ -82,7 +82,6 @@ def __init__(
         ignore_installed,  # type: bool
         ignore_requires_python,  # type: bool
         py_version_info=None,  # type: Optional[Tuple[int, ...]]
-        lazy_wheel=False,  # type: bool
     ):
         # type: (...) -> None
         self._finder = finder
@@ -93,7 +92,6 @@ def __init__(
         self._use_user_site = use_user_site
         self._force_reinstall = force_reinstall
         self._ignore_requires_python = ignore_requires_python
-        self.use_lazy_wheel = lazy_wheel
 
         self._link_candidate_cache = {}  # type: Cache[LinkCandidate]
         self._editable_candidate_cache = {}  # type: Cache[EditableCandidate]

src/pip/_internal/resolution/resolvelib/resolver.py

@@ -49,17 +49,8 @@ def __init__(
         force_reinstall,  # type: bool
         upgrade_strategy,  # type: str
         py_version_info=None,  # type: Optional[Tuple[int, ...]]
-        lazy_wheel=False,  # type: bool
     ):
         super(Resolver, self).__init__()
-        if lazy_wheel:
-            logger.warning(
-                'pip is using lazily downloaded wheels using HTTP '
-                'range requests to obtain dependency information. '
-                'This experimental feature is enabled through '
-                '--use-feature=fast-deps and it is not ready for production.'
-            )
-
         assert upgrade_strategy in self._allowed_strategies
 
         self.factory = Factory(
@@ -72,7 +63,6 @@ def __init__(
             ignore_installed=ignore_installed,
             ignore_requires_python=ignore_requires_python,
             py_version_info=py_version_info,
-            lazy_wheel=lazy_wheel,
         )
         self.ignore_dependencies = ignore_dependencies
         self.upgrade_strategy = upgrade_strategy
@@ -169,6 +159,9 @@ def resolve(self, root_reqs, check_supported_wheels):
 
             req_set.add_named_requirement(ireq)
 
+        for actual_req in req_set.all_requirements:
+            self.factory.preparer.prepare_linked_requirement_more(actual_req)
+
         return req_set
 
     def get_installation_order(self, req_set):

tests/unit/test_req.py

@@ -89,6 +89,7 @@ def _basic_resolver(self, finder, require_hashes=False):
                 finder=finder,
                 require_hashes=require_hashes,
                 use_user_site=False,
+                lazy_wheel=False,
             )
             yield Resolver(
                 preparer=preparer,