Fix locking of sdists rejected by Pip.

Previously, the Pex locking logic would miss when an sdist was initially
picked by Pip, but then rejected and a wheel was chosen instead.

Fixes pex-tool#2519

pex/cli/commands/lock.py

@@ -655,6 +655,7 @@ def _add_update_arguments(cls, update_parser):
         resolver_options.register_network_options(resolver_options_parser)
         resolver_options.register_max_jobs_option(resolver_options_parser)
         resolver_options.register_use_pip_config(resolver_options_parser)
+        resolver_options.register_preserve_pip_download_log(resolver_options_parser)
 
     @classmethod
     def add_update_lock_options(
@@ -1085,6 +1086,7 @@ def _create_lock_update_request(
             max_jobs=resolver_options.get_max_jobs_value(self.options),
             use_pip_config=resolver_options.get_use_pip_config_value(self.options),
             dependency_configuration=dependency_config,
+            preserve_log=resolver_options.get_preserve_pip_download_log(self.options),
         )
 
         target_configuration = target_options.configure(self.options)

pex/resolve/locker.py

@@ -300,12 +300,20 @@ def _maybe_record_wheel(self, url):
 
             # 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.
+            # We want the original wheel 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:
+            #
+            # Sometimes, there will be a prior URL, but it will be for a failed sdist build, in
+            # which case we always want to replace.
+            #
+            # See: https://github.com/pex-tool/pex/issues/2519
+            resolved_requirement = self._resolved_requirements.get(pin)
+            if not resolved_requirement or not any(
+                artifact.url.is_wheel for artifact in resolved_requirement.iter_artifacts()
+            ):
                 additional_artifacts = self._links[pin]
                 additional_artifacts.pop(artifact_url, None)
                 self._resolved_requirements[pin] = ResolvedRequirement(

pex/resolve/lockfile/updater.py

@@ -619,6 +619,7 @@ def create(
         max_jobs,  # type: int
         use_pip_config,  # type: bool
         dependency_configuration,  # type: DependencyConfiguration
+        preserve_log,  # type: bool
     ):
         # type: (...) -> LockUpdater
 
@@ -637,6 +638,7 @@ def create(
             network_configuration=network_configuration,
             max_jobs=max_jobs,
             use_pip_config=use_pip_config,
+            preserve_log=preserve_log,
         )
         return cls(
             lock_file=lock_file,

pex/resolve/resolver_options.py

@@ -288,15 +288,25 @@ def valid_project_name(arg):
         help="Whether to transitively resolve requirements.",
     )
     register_max_jobs_option(parser)
+    register_preserve_pip_download_log(parser)
+
+
+def register_preserve_pip_download_log(parser):
+    # type: (_ActionsContainer) -> None
     parser.add_argument(
         "--preserve-pip-download-log",
         "--no-preserve-pip-download-log",
-        default=default_resolver_configuration.preserve_log,
+        default=PipConfiguration().preserve_log,
         action=HandleBoolAction,
         help="Preserve the `pip download` log and print its location to stderr.",
     )
 
 
+def get_preserve_pip_download_log(options):
+    # type: (Namespace) -> bool
+    return cast(bool, options.preserve_pip_download_log)
+
+
 def register_use_pip_config(parser):
     # type: (_ActionsContainer) -> None
     """Register an option to control Pip config hermeticity.
@@ -554,7 +564,7 @@ def create_pip_configuration(options):
         build_configuration=build_configuration,
         transitive=options.transitive,
         max_jobs=get_max_jobs_value(options),
-        preserve_log=options.preserve_pip_download_log,
+        preserve_log=get_preserve_pip_download_log(options),
         version=pip_version,
         resolver_version=resolver_version,
         allow_version_fallback=options.allow_pip_version_fallback,

tests/integration/cli/commands/test_issue_2519.py

@@ -0,0 +1,157 @@
+# Copyright 2024 Pex project contributors.
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+import os.path
+import re
+
+import pytest
+
+from pex.compatibility import PY2
+from pex.pip.version import PipVersion, PipVersionValue
+from pex.targets import LocalInterpreter
+from pex.typing import TYPE_CHECKING
+from testing import IntegResults
+from testing.cli import run_pex3
+
+if TYPE_CHECKING:
+    from typing import Any
+
+
+def assert_lock_sync(
+    lock,  # type: str
+    pip_version,  # type: PipVersionValue
+    *extra_args  # type: str
+):
+    # type: (...) -> IntegResults
+    result = run_pex3(
+        "lock",
+        "sync",
+        "--style",
+        "strict",
+        "--pip-version",
+        str(pip_version),
+        "bloom-filter2>=2.0.0",
+        "--indent",
+        "2",
+        "--lock",
+        lock,
+        *extra_args
+    )
+    result.assert_success()
+    return result
+
+
+TARGET = LocalInterpreter.create()
+PLATFORM_TAG = TARGET.platform.tag
+
+skip_if_py2 = pytest.mark.skipif(
+    PY2, reason="The bloom-filter2 project only claims support for Python 3.x"
+)
+
+
+@skip_if_py2
+@pytest.mark.parametrize(
+    "pip_version",
+    [
+        pytest.param(version, id=str(version))
+        for version in PipVersion.values()
+        if version < PipVersion.v23_2 and version.requires_python_applies(TARGET)
+    ],
+)
+def test_sync_version_mismatch_old_pip(
+    pip_version,  # type: PipVersionValue
+    tmpdir,  # type: Any
+):
+    # type: (...) -> None
+
+    lock = os.path.join(str(tmpdir), "lockfile.json")
+    assert_lock_sync(lock, pip_version)
+
+    result = assert_lock_sync(lock, pip_version)
+    assert re.match(
+        r"^{lead_in}\n"
+        r"    \+ http.*{wheel}\n"
+        r"    - http.*{sdist}$\n".format(
+            lead_in=re.escape(
+                "Updates for lock generated by {platform_tag}:\n"
+                "  Updated bloom-filter2 2 artifacts:".format(platform_tag=PLATFORM_TAG)
+            ),
+            wheel=re.escape("bloom_filter2-2.0.0-py3-none-any.whl"),
+            sdist=re.escape("bloom-filter2-2.0.0-1.tar.gz"),
+        ),
+        result.error,
+    ), result.error
+
+    result = assert_lock_sync(lock, pip_version)
+    assert (
+        "No updates for lock generated by {platform_tag}.\n".format(platform_tag=PLATFORM_TAG)
+        == result.error
+    )
+
+    result = assert_lock_sync(lock, pip_version)
+    assert (
+        "No updates for lock generated by {platform_tag}.\n".format(platform_tag=PLATFORM_TAG)
+        == result.error
+    )
+
+
+@skip_if_py2
+@pytest.mark.parametrize(
+    "pip_version",
+    [
+        pytest.param(version, id=str(version))
+        for version in PipVersion.values()
+        if version >= PipVersion.v23_2 and version.requires_python_applies(TARGET)
+    ],
+)
+def test_sync_version_mismatch_new_pip(
+    pip_version,  # type: PipVersionValue
+    tmpdir,  # type: Any
+):
+    # type: (...) -> None
+
+    lock = os.path.join(str(tmpdir), "lockfile.json")
+    assert_lock_sync(lock, pip_version)
+
+    result = assert_lock_sync(lock, pip_version)
+    assert (
+        "No updates for lock generated by {platform_tag}.\n".format(platform_tag=PLATFORM_TAG)
+        == result.error
+    )
+
+    result = assert_lock_sync(lock, pip_version)
+    assert (
+        "No updates for lock generated by {platform_tag}.\n".format(platform_tag=PLATFORM_TAG)
+        == result.error
+    )
+
+
+@skip_if_py2
+@pytest.mark.parametrize(
+    "pip_version",
+    [
+        pytest.param(version, id=str(version))
+        for version in PipVersion.values()
+        if version.requires_python_applies(TARGET)
+    ],
+)
+def test_sync_version_mismatch_prefer_binary_all_pips(
+    pip_version,  # type: PipVersionValue
+    tmpdir,  # type: Any
+):
+    # type: (...) -> None
+
+    lock = os.path.join(str(tmpdir), "lockfile.json")
+    assert_lock_sync(lock, pip_version, "--prefer-binary")
+
+    result = assert_lock_sync(lock, pip_version)
+    assert (
+        "No updates for lock generated by {platform_tag}.\n".format(platform_tag=PLATFORM_TAG)
+        == result.error
+    )
+
+    result = assert_lock_sync(lock, pip_version)
+    assert (
+        "No updates for lock generated by {platform_tag}.\n".format(platform_tag=PLATFORM_TAG)
+        == result.error
+    )