respond to review comments

Author: cosmicexplorer

src/pip/_internal/commands/download.py

@@ -7,7 +7,6 @@
 from pip._internal.cli.cmdoptions import make_target_python
 from pip._internal.cli.req_command import RequirementCommand, with_cleanup
 from pip._internal.cli.status_codes import SUCCESS
-from pip._internal.network.download import PartialRequirementDownloadCompleter
 from pip._internal.req.req_tracker import get_requirement_tracker
 from pip._internal.utils.misc import ensure_dir, normalize_path, write_output
 from pip._internal.utils.temp_dir import TempDirectory
@@ -135,13 +134,6 @@ def run(self, options, args):
             reqs, check_supported_wheels=True
         )
 
-        # Download any requirements which were only fetched by metadata.
-        download_completer = PartialRequirementDownloadCompleter(
-            session,
-            progress_bar=options.progress_bar,
-            download_dir=options.download_dir)
-        download_completer.complete_requirement_downloads(requirement_set)
-
         downloaded = []  # type: List[str]
         for req in requirement_set.requirements.values():
             if not req.editable and req.satisfied_by is None:

src/pip/_internal/commands/install.py

@@ -18,7 +18,6 @@
 from pip._internal.cli.status_codes import ERROR, SUCCESS
 from pip._internal.exceptions import CommandError, InstallationError
 from pip._internal.locations import distutils_scheme
-from pip._internal.network.download import PartialRequirementDownloadCompleter
 from pip._internal.operations.check import check_install_conflicts
 from pip._internal.req import install_given_reqs
 from pip._internal.req.req_tracker import get_requirement_tracker
@@ -324,15 +323,6 @@ def run(self, options, args):
                 reqs, check_supported_wheels=not options.target_dir
             )
 
-            # Download any requirements which were only fetched by metadata.
-            # Let's download to a temporary directory.
-            tmpdir = TempDirectory(kind="unpack", globally_managed=True).path
-            download_completer = PartialRequirementDownloadCompleter(
-                session,
-                progress_bar=options.progress_bar,
-                download_dir=tmpdir)
-            download_completer.complete_requirement_downloads(requirement_set)
-
             try:
                 pip_req = requirement_set.get_requirement("pip")
             except KeyError:

src/pip/_internal/commands/wheel.py

@@ -11,7 +11,6 @@
 from pip._internal.cli.req_command import RequirementCommand, with_cleanup
 from pip._internal.cli.status_codes import SUCCESS
 from pip._internal.exceptions import CommandError
-from pip._internal.network.download import PartialRequirementDownloadCompleter
 from pip._internal.req.req_tracker import get_requirement_tracker
 from pip._internal.utils.misc import ensure_dir, normalize_path
 from pip._internal.utils.temp_dir import TempDirectory
@@ -157,13 +156,6 @@ def run(self, options, args):
             reqs, check_supported_wheels=True
         )
 
-        # Download any requirements which were only fetched by metadata.
-        download_completer = PartialRequirementDownloadCompleter(
-            session,
-            progress_bar=options.progress_bar,
-            download_dir=options.wheel_dir)
-        download_completer.complete_requirement_downloads(requirement_set)
-
         reqs_to_build = [
             r for r in requirement_set.requirements.values()
             if should_build_for_wheel_command(r)

src/pip/_internal/network/download.py

@@ -176,7 +176,7 @@ def __call__(self, link, location):
         return filepath, content_type
 
 
-class BatchDownloader(object):
+class _BatchDownloader(object):
 
     def __init__(
         self,
@@ -212,39 +212,35 @@ def __call__(self, links, location):
             yield link, (filepath, content_type)
 
 
-class PartialRequirementDownloadCompleter(object):
-
-    def __init__(
-        self,
-        session,           # type: PipSession
-        progress_bar,      # type: str
-        download_dir,      # type: str
-    ):
-        # type: (...) -> None
-        self._batch_downloader = BatchDownloader(session, progress_bar)
-        self._download_dir = download_dir
-
-    def complete_requirement_downloads(self, req_set):
-        # type: (RequirementSet) -> None
-        """Download any requirements which were only partially downloaded with
-        --use-feature=fast-deps."""
-        reqs_to_fully_download = [
-            r for r in req_set.requirements.values()
-            if r.needs_more_preparation
-        ]
-
-        # Map each link to the requirement that owns it. This allows us to set
-        # `req.local_file_path` on the appropriate requirement after passing
-        # all the links at once into BatchDownloader.
-        links_to_fully_download = {}  # type: Dict[Link, InstallRequirement]
-        for req in reqs_to_fully_download:
-            assert req.link
-            links_to_fully_download[req.link] = req
-
-        batch_download = self._batch_downloader(
-            links_to_fully_download.keys(),
-            self._download_dir)
-        for link, (filepath, _) in batch_download:
-            logger.debug("Downloading link %s to %s", link, filepath)
-            req = links_to_fully_download[link]
-            req.local_file_path = filepath
+def complete_partial_requirement_downloads(
+    session,       # type: PipSession
+    progress_bar,  # type: str
+    req_set,       # type: RequirementSet
+    download_dir,  # type: str
+):
+    # type: (...) -> None
+    """Download any requirements which were only partially downloaded with
+    --use-feature=fast-deps."""
+    batch_downloader = _BatchDownloader(session, progress_bar)
+
+    reqs_to_fully_download = [
+        r for r in req_set.requirements.values()
+        if r.needs_more_preparation
+    ]
+
+    # Map each link to the requirement that owns it. This allows us to set
+    # `req.local_file_path` on the appropriate requirement after passing
+    # all the links at once into BatchDownloader.
+    links_to_fully_download = {}  # type: Dict[Link, InstallRequirement]
+    for req in reqs_to_fully_download:
+        assert req.link
+        links_to_fully_download[req.link] = req
+
+    batch_download = batch_downloader(
+        links_to_fully_download.keys(),
+        download_dir,
+    )
+    for link, (filepath, _) in batch_download:
+        logger.debug("Downloading link %s to %s", link, filepath)
+        req = links_to_fully_download[link]
+        req.local_file_path = filepath

src/pip/_internal/operations/prepare.py

@@ -26,7 +26,10 @@
     VcsHashUnsupported,
 )
 from pip._internal.models.wheel import Wheel
-from pip._internal.network.download import Downloader
+from pip._internal.network.download import (
+    Downloader,
+    complete_partial_requirement_downloads,
+)
 from pip._internal.network.lazy_wheel import (
     HTTPRangeRequestUnsupported,
     dist_from_wheel_url,
@@ -53,6 +56,7 @@
 
     from pip._internal.index.package_finder import PackageFinder
     from pip._internal.models.link import Link
+    from pip._internal.models.req_set import RequirementSet
     from pip._internal.network.session import PipSession
     from pip._internal.req.req_install import InstallRequirement
     from pip._internal.req.req_tracker import RequirementTracker
@@ -352,6 +356,10 @@ def __init__(
 
         # Should wheels be downloaded lazily?
         self.use_lazy_wheel = lazy_wheel
+        # TODO: this field is only needed in
+        # .complete_partial_requirements(). When the v1 resolver can be
+        # removed, partial downloads can be completed outside of the resolver.
+        self._progress_bar = progress_bar
 
         # Memoized downloaded files, as mapping of url: (path, mime type)
         self._downloaded = {}  # type: Dict[str, Tuple[str, str]]
@@ -490,6 +498,17 @@ def _fetch_metadata_using_lazy_wheel(self, link):
             logger.debug('%s does not support range requests', url)
             return None
 
+    def complete_partial_requirements(self, req_set):
+        # type: (RequirementSet) -> None
+        """Download any requirements which were only fetched by metadata."""
+        download_location = self.wheel_download_dir or self.download_dir
+        complete_partial_requirement_downloads(
+            self._session,
+            self._progress_bar,
+            req_set,
+            download_location,
+        )
+
     def prepare_linked_requirement(self, req, parallel_builds=False):
         # type: (InstallRequirement, bool) -> Distribution
         """Prepare a requirement to be obtained from req.link."""
@@ -518,7 +537,7 @@ def prepare_linked_requirements_more(self, reqs, parallel_builds=False):
                     req.needs_more_preparation = False
 
         # Prepare requirements we found were already downloaded for some
-        # reason. The rest will be downloaded outside of the resolver.
+        # reason. The other downloads will be completed elsewhere.
         for req in reqs:
             if not req.needs_more_preparation:
                 self._prepare_linked_requirement(req, parallel_builds)

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

@@ -162,6 +162,11 @@ def resolve(self, root_reqs, check_supported_wheels):
 
         reqs = req_set.all_requirements
         self.factory.preparer.prepare_linked_requirements_more(reqs)
+
+        # TODO: extricate this call from the resolver.resolve() code path once
+        # we can drop the v1 resolver.
+        self.factory.preparer.complete_partial_requirements(req_set)
+
         return req_set
 
     def get_installation_order(self, req_set):