Merge pull request #12197 from cosmicexplorer/fix-metadata-sdist-download

Author: uranusjr

news/12191.bugfix.rst

@@ -0,0 +1 @@
+Prevent downloading sdists twice when PEP 658 metadata is present.

src/pip/_internal/operations/prepare.py

@@ -7,6 +7,7 @@
 import mimetypes
 import os
 import shutil
+from pathlib import Path
 from typing import Dict, Iterable, List, Optional
 
 from pip._vendor.packaging.utils import canonicalize_name
@@ -20,7 +21,6 @@
     InstallationError,
     MetadataInconsistent,
     NetworkConnectionError,
-    PreviousBuildDirError,
     VcsHashUnsupported,
 )
 from pip._internal.index.package_finder import PackageFinder
@@ -47,7 +47,6 @@
     display_path,
     hash_file,
     hide_url,
-    is_installable_dir,
 )
 from pip._internal.utils.temp_dir import TempDirectory
 from pip._internal.utils.unpacking import unpack_file
@@ -319,21 +318,7 @@ def _ensure_link_req_src_dir(
             autodelete=True,
             parallel_builds=parallel_builds,
         )
-
-        # If a checkout exists, it's unwise to keep going.  version
-        # inconsistencies are logged later, but do not fail the
-        # installation.
-        # FIXME: this won't upgrade when there's an existing
-        # package unpacked in `req.source_dir`
-        # TODO: this check is now probably dead code
-        if is_installable_dir(req.source_dir):
-            raise PreviousBuildDirError(
-                "pip can't proceed with requirements '{}' due to a"
-                "pre-existing build directory ({}). This is likely "
-                "due to a previous installation that failed . pip is "
-                "being responsible and not assuming it can delete this. "
-                "Please delete it and try again.".format(req, req.source_dir)
-            )
+        req.ensure_pristine_source_checkout()
 
     def _get_linked_req_hashes(self, req: InstallRequirement) -> Hashes:
         # By the time this is called, the requirement's link should have
@@ -481,20 +466,19 @@ def _complete_partial_requirements(
         for link, (filepath, _) in batch_download:
             logger.debug("Downloading link %s to %s", link, filepath)
             req = links_to_fully_download[link]
+            # Record the downloaded file path so wheel reqs can extract a Distribution
+            # in .get_dist().
             req.local_file_path = filepath
-            # TODO: This needs fixing for sdists
-            # This is an emergency fix for #11847, which reports that
-            # distributions get downloaded twice when metadata is loaded
-            # from a PEP 658 standalone metadata file. Setting _downloaded
-            # fixes this for wheels, but breaks the sdist case (tests
-            # test_download_metadata). As PyPI is currently only serving
-            # metadata for wheels, this is not an immediate issue.
-            # Fixing the problem properly looks like it will require a
-            # complete refactoring of the `prepare_linked_requirements_more`
-            # logic, and I haven't a clue where to start on that, so for now
-            # I have fixed the issue *just* for wheels.
-            if req.is_wheel:
-                self._downloaded[req.link.url] = filepath
+            # Record that the file is downloaded so we don't do it again in
+            # _prepare_linked_requirement().
+            self._downloaded[req.link.url] = filepath
+
+            # If this is an sdist, we need to unpack it after downloading, but the
+            # .source_dir won't be set up until we are in _prepare_linked_requirement().
+            # Add the downloaded archive to the install requirement to unpack after
+            # preparing the source dir.
+            if not req.is_wheel:
+                req.needs_unpacked_archive(Path(filepath))
 
         # This step is necessary to ensure all lazy wheels are processed
         # successfully by the 'download', 'wheel', and 'install' commands.

src/pip/_internal/req/req_install.py

@@ -6,6 +6,7 @@
 import uuid
 import zipfile
 from optparse import Values
+from pathlib import Path
 from typing import Any, Collection, Dict, Iterable, List, Optional, Sequence, Union
 
 from pip._vendor.packaging.markers import Marker
@@ -17,7 +18,7 @@
 from pip._vendor.pyproject_hooks import BuildBackendHookCaller
 
 from pip._internal.build_env import BuildEnvironment, NoOpBuildEnvironment
-from pip._internal.exceptions import InstallationError
+from pip._internal.exceptions import InstallationError, PreviousBuildDirError
 from pip._internal.locations import get_scheme
 from pip._internal.metadata import (
     BaseDistribution,
@@ -47,11 +48,13 @@
     backup_dir,
     display_path,
     hide_url,
+    is_installable_dir,
     redact_auth_from_url,
 )
 from pip._internal.utils.packaging import safe_extra
 from pip._internal.utils.subprocess import runner_with_spinner_message
 from pip._internal.utils.temp_dir import TempDirectory, tempdir_kinds
+from pip._internal.utils.unpacking import unpack_file
 from pip._internal.utils.virtualenv import running_under_virtualenv
 from pip._internal.vcs import vcs
 
@@ -180,6 +183,9 @@ def __init__(
         # This requirement needs more preparation before it can be built
         self.needs_more_preparation = False
 
+        # This requirement needs to be unpacked before it can be installed.
+        self._archive_source: Optional[Path] = None
+
     def __str__(self) -> str:
         if self.req:
             s = str(self.req)
@@ -645,6 +651,27 @@ def ensure_has_source_dir(
                 parallel_builds=parallel_builds,
             )
 
+    def needs_unpacked_archive(self, archive_source: Path) -> None:
+        assert self._archive_source is None
+        self._archive_source = archive_source
+
+    def ensure_pristine_source_checkout(self) -> None:
+        """Ensure the source directory has not yet been built in."""
+        assert self.source_dir is not None
+        if self._archive_source is not None:
+            unpack_file(str(self._archive_source), self.source_dir)
+        elif is_installable_dir(self.source_dir):
+            # If a checkout exists, it's unwise to keep going.
+            # version inconsistencies are logged later, but do not fail
+            # the installation.
+            raise PreviousBuildDirError(
+                f"pip can't proceed with requirements '{self}' due to a "
+                f"pre-existing build directory ({self.source_dir}). This is likely "
+                "due to a previous installation that failed . pip is "
+                "being responsible and not assuming it can delete this. "
+                "Please delete it and try again."
+            )
+
     # For editable installations
     def update_editable(self) -> None:
         if not self.link: