Fix artifact URL recording for pip>=23.3. (#2421)

When support for Pip 23.3.1 was added in #2276 a latent bug in artifact 
URL recording was exposed in cases where the index being used issued
re-directs. Fix up artifact URL recording to grab the primary index URL
and not subsequent re-directs.

Implementing the fix above led to a test failure that revealed another
bug whereby lock file artifact downloads were not respecting the locked
resolve target when it was a foreign platform, which is now fixed.

Finally, fixing the un-patched foreign platform target issue in lock 
file artifact downloads revealed that artifact URLs with hashes were not
being taken advantage of in all cases. Now, when there is a version of
an artifact URL seen that contains hashes - the best of those is always
used to prevent needless post-processing to download and hash the
artifact at lock creation time.

Fixes #2414

CHANGES.md

@@ -1,5 +1,17 @@
 # Release Notes
 
+## 2.3.3
+
+This release fixes `pex3 lock create` support for `--pip-version`s
+23.3.1 and newer. Previously, when locking using indexes that serve
+artifacts via re-directs, the resulting lock file would contain the
+final re-directed URL instead of the originating index artifact URL.
+This could lead to issues when the indexes re-direction scheme changed
+or else if authentication parameters in the original index URL were
+stripped in the Pip logs.
+
+* Fix artifact URL recording for `pip>=23.3`. (#2421)
+
 ## 2.3.2
 
 This release fixes a regression for users of gevent monkey patching. The

pex/resolve/downloads.py

@@ -11,6 +11,7 @@
 from pex.common import safe_mkdir, safe_mkdtemp
 from pex.hashing import Sha256
 from pex.jobs import Job, Raise, SpawnedJob, execute_parallel
+from pex.pip import foreign_platform
 from pex.pip.download_observer import DownloadObserver
 from pex.pip.installation import get_pip
 from pex.pip.tool import PackageIndexConfiguration, Pip
@@ -109,10 +110,10 @@ def _download(
                 break
 
         # Although we don't actually need to observe the download, we do need to patch Pip to not
-        # care about wheel tags, environment markers or Requires-Python. The locker's download
-        # observer does just this for universal locks with no target system or requires python
-        # restrictions.
-        download_observer = DownloadObserver(
+        # care about wheel tags, environment markers or Requires-Python if the lock target is
+        # either foreign or universal. The locker.patch below handles the universal case or else
+        # generates no patches if the lock is not universal.
+        download_observer = foreign_platform.patch(self.target) or DownloadObserver(
             analyzer=None,
             patch_set=locker.patch(lock_configuration=self.lock_configuration),
         )

pex/resolve/locker.py

@@ -258,6 +258,7 @@ def __init__(
         self._links = defaultdict(
             OrderedDict
         )  # type: DefaultDict[Pin, OrderedDict[ArtifactURL, PartialArtifact]]
+        self._known_fingerprints = {}  # type: Dict[ArtifactURL, Fingerprint]
         self._artifact_build_observer = None  # type: Optional[ArtifactBuildObserver]
         self._local_projects = OrderedSet()  # type: OrderedSet[str]
         self._lock_result = None  # type: Optional[LockResult]
@@ -276,6 +277,13 @@ def should_collect(self, returncode):
         # type: (int) -> bool
         return returncode == 0
 
+    def parse_url_and_maybe_record_fingerprint(self, url):
+        # type: (str) -> ArtifactURL
+        artifact_url = ArtifactURL.parse(url)
+        if artifact_url.fingerprint:
+            self._known_fingerprints[artifact_url] = artifact_url.fingerprint
+        return artifact_url
+
     @staticmethod
     def _extract_resolve_data(artifact_url):
         # type: (ArtifactURL) -> Tuple[Pin, PartialArtifact]
@@ -286,18 +294,26 @@ def _extract_resolve_data(artifact_url):
 
     def _maybe_record_wheel(self, url):
         # type: (str) -> ArtifactURL
-        artifact_url = ArtifactURL.parse(url)
+        artifact_url = self.parse_url_and_maybe_record_fingerprint(url)
         if artifact_url.is_wheel:
             pin, partial_artifact = self._extract_resolve_data(artifact_url)
 
-            additional_artifacts = self._links[pin]
-            additional_artifacts.pop(artifact_url, None)
-            self._resolved_requirements[pin] = ResolvedRequirement(
-                pin=pin,
-                artifact=partial_artifact,
-                additional_artifacts=tuple(additional_artifacts.values()),
-            )
-            self._selected_path_to_pin[os.path.basename(artifact_url.path)] = pin
+            # A wheel selected in a Pip download resolve can be noted more than one time. Notably,
+            # this occurs across all supported versions of Pip when an index serves re-directs.
+            # We want the original URL in the lock since that points to the index the user selected
+            # and not the re-directed final (implementation detail) URL that may change but should
+            # not affect the lock.
+            #
+            # See: https://github.com/pex-tool/pex/issues/2414
+            if pin not in self._resolved_requirements:
+                additional_artifacts = self._links[pin]
+                additional_artifacts.pop(artifact_url, None)
+                self._resolved_requirements[pin] = ResolvedRequirement(
+                    pin=pin,
+                    artifact=partial_artifact,
+                    additional_artifacts=tuple(additional_artifacts.values()),
+                )
+                self._selected_path_to_pin[os.path.basename(artifact_url.path)] = pin
         return artifact_url
 
     def analyze(self, line):
@@ -354,7 +370,7 @@ def analyze(self, line):
                     )
                     verified = True
                     selected_path = os.path.basename(archive_path)
-                    artifact_url = ArtifactURL.parse(
+                    artifact_url = self.parse_url_and_maybe_record_fingerprint(
                         self._vcs_url_manager.normalize_url(artifact_url.raw_url)
                     )
                     self._selected_path_to_pin[selected_path] = build_result.pin
@@ -466,7 +482,7 @@ def analyze(self, line):
                     ),
                     re.compile(r"WARNING: Discarding {url}".format(url=re.escape(file_url))),
                 ),
-                artifact_url=ArtifactURL.parse(file_url),
+                artifact_url=self.parse_url_and_maybe_record_fingerprint(file_url),
             )
             return self.Continue()
 
@@ -481,7 +497,7 @@ def analyze(self, line):
         if self.style in (LockStyle.SOURCES, LockStyle.UNIVERSAL):
             match = re.search(r"Found link (?P<url>[^\s]+)(?: \(from .*\))?, version: ", line)
             if match:
-                url = ArtifactURL.parse(match.group("url"))
+                url = self.parse_url_and_maybe_record_fingerprint(match.group("url"))
                 pin, partial_artifact = self._extract_resolve_data(url)
                 self._links[pin][url] = partial_artifact
                 return self.Continue()
@@ -496,15 +512,20 @@ def analysis_completed(self):
             if resolved_requirement.pin in self._saved
         ]
 
+        artifacts = []
+        for resolved_requirement in resolved_requirements:
+            for artifact in resolved_requirement.iter_artifacts():
+                if not artifact.fingerprint:
+                    fingerprint = self._known_fingerprints.get(artifact.url)
+                    if fingerprint:
+                        artifact = attr.evolve(artifact, fingerprint=fingerprint)
+                artifacts.append(artifact)
+
         fingerprinted_artifacts = {
             artifact.url: artifact
             for artifact in self._fingerprint_service.fingerprint(
                 endpoints=self._pep_691_endpoints,
-                artifacts=tuple(
-                    artifact
-                    for resolved_requirement in resolved_requirements
-                    for artifact in resolved_requirement.iter_artifacts()
-                ),
+                artifacts=tuple(artifacts),
             )
         }
 

pex/version.py

@@ -1,4 +1,4 @@
 # Copyright 2015 Pex project contributors.
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-__version__ = "2.3.2"
+__version__ = "2.3.3"