Fix lock updates for locks with sdist bystanders. (#2325)

Previously, a `pex3 lock update` would fail whenever bystander projects
(those projects in the lock but not targeted for update via `-p`) had an
sdist primary artifact.

Fixes #2324

CHANGES.md

@@ -1,5 +1,12 @@
 # Release Notes
 
+## 2.1.157
+
+This release fixes a bug in `pex3 lock update` for updates that leave
+projects unchanged whose primary artifact is an sdist.
+
+* Fix lock updates for locks with sdist bystanders. (#2325)
+
 ## 2.1.156
 
 This release optimizes wheel install overhead for warm caches. Notably,

pex/resolve/lockfile/updater.py

@@ -34,7 +34,7 @@
 from pex.util import named_temporary_file
 
 if TYPE_CHECKING:
-    from typing import Dict, Iterable, Iterator, List, Mapping, Optional, Tuple, Union
+    from typing import Any, Dict, Iterable, Iterator, List, Mapping, Optional, Tuple, Union
 
     import attr  # vendor:skip
 else:
@@ -316,11 +316,56 @@ def update_resolve(
                 self.update_constraints_by_project_name
                 and project_name not in self.update_constraints_by_project_name
             ):
-                assert updated_pin == original_pin
-                assert updated_requirement.artifact == locked_requirement.artifact
-                assert (
-                    updated_requirement.additional_artifacts
-                    == locked_requirement.additional_artifacts
+                assert updated_pin == original_pin, (
+                    "The locked requirement {original} should have been undisturbed by the lock "
+                    "update, but it changed to {updated}.".format(
+                        original=original_pin.as_requirement(), updated=updated_pin.as_requirement()
+                    )
+                )
+
+                # N.B.: We use a custom key for artifact equality comparison since `Artifact`
+                # contains a `verified` attribute that can both vary based on Pex's current
+                # knowledge about the trustworthiness of an artifact hash and is not relevant to
+                # whether the artifact refers to the same artifact.
+                def artifact_key(artifact):
+                    # type: (Artifact) -> Any
+                    return artifact.url, artifact.fingerprint
+
+                assert artifact_key(updated_requirement.artifact) == artifact_key(
+                    locked_requirement.artifact
+                ), (
+                    "The locked requirement {original} should have been undisturbed by the lock "
+                    "update, but its primary artifact changed from:\n"
+                    "{original_artifact}\n"
+                    "to:\n"
+                    "{updated_artifact}".format(
+                        original=original_pin.as_requirement(),
+                        original_artifact=artifact_key(locked_requirement.artifact),
+                        updated_artifact=artifact_key(updated_requirement.artifact),
+                    )
+                )
+                assert set(map(artifact_key, updated_requirement.additional_artifacts)) == set(
+                    map(artifact_key, locked_requirement.additional_artifacts)
+                ), (
+                    "The locked requirement {original} should have been undisturbed by the lock "
+                    "update, but its additional artifact set changed from:\n"
+                    "{original_artifacts}\n"
+                    "to:\n"
+                    "{updated_artifacts}".format(
+                        original=original_pin.as_requirement(),
+                        original_artifacts="\n".join(
+                            map(
+                                lambda a: str(artifact_key(a)),
+                                locked_requirement.additional_artifacts,
+                            )
+                        ),
+                        updated_artifacts="\n".join(
+                            map(
+                                lambda a: str(artifact_key(a)),
+                                updated_requirement.additional_artifacts,
+                            )
+                        ),
+                    )
                 )
             elif original_pin != updated_pin:
                 updates[project_name] = VersionUpdate(

pex/version.py

@@ -1,4 +1,4 @@
 # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-__version__ = "2.1.156"
+__version__ = "2.1.157"

tests/integration/test_issue_2324.py

@@ -0,0 +1,80 @@
+# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import print_function
+
+import itertools
+import os.path
+
+from pex.pep_440 import Version
+from pex.pep_503 import ProjectName
+from pex.resolve.locked_resolve import FileArtifact
+from pex.resolve.lockfile import json_codec
+from pex.resolve.resolved_requirement import Pin
+from pex.typing import TYPE_CHECKING
+from testing.cli import run_pex3
+
+if TYPE_CHECKING:
+    from typing import Any
+
+
+def test_update_sdists_not_updated(tmpdir):
+    # type: (Any) -> None
+
+    constraints = os.path.join(str(tmpdir), "constraints.txt")
+    with open(constraints, "w") as fp:
+        print("ansicolors<1.1.8", file=fp)
+        print("cowsay<6", file=fp)
+
+    lock = os.path.join(str(tmpdir), "lock.json")
+
+    def assert_lock(*pins):
+        # type: (*Pin) -> None
+
+        lockfile = json_codec.load(lock)
+        assert 1 == len(lockfile.locked_resolves)
+        locked_resolve = lockfile.locked_resolves[0]
+        locked_requirements = {
+            locked_req.pin: tuple(locked_req.iter_artifacts())
+            for locked_req in locked_resolve.locked_requirements
+        }
+        assert set(pins) == set(locked_requirements)
+        assert all(
+            isinstance(artifact, FileArtifact) and artifact.is_source
+            for artifact in itertools.chain.from_iterable(locked_requirements.values())
+        )
+
+    run_pex3(
+        "lock",
+        "create",
+        "--no-wheel",
+        "--constraints",
+        constraints,
+        "ansicolors",
+        "cowsay",
+        "--indent",
+        "2",
+        "-o",
+        lock,
+    ).assert_success()
+    assert_lock(
+        Pin(ProjectName("ansicolors"), Version("1.1.7")), Pin(ProjectName("cowsay"), Version("5.0"))
+    )
+
+    # N.B.: Pre-fix this test would lead to an artifact comparison assertion for cowsay, which is
+    # expected to be unmodified by the lock update.
+    #
+    # E       Traceback (most recent call last):
+    # E         File "/home/jsirois/dev/pantsbuild/jsirois-pex/pex/result.py", line 105, in catch
+    # E           return func(*args, **kwargs)
+    # E                  ^^^^^^^^^^^^^^^^^^^^^
+    # E         File "/home/jsirois/dev/pantsbuild/jsirois-pex/pex/resolve/lockfile/updater.py", line 320, in update_resolve
+    # E           assert updated_requirement.artifact == locked_requirement.artifact
+    # E                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    # E       AssertionError
+    # E       Encountered 1 error updating /tmp/pytest-of-jsirois/pytest-8/test_update_sdists_not_updated0/lock.json:
+    # E       1.) cp311-cp311-manylinux_2_35_x86_64:
+    run_pex3("lock", "update", "-v", "-p", "ansicolors<1.1.9", lock).assert_success()
+    assert_lock(
+        Pin(ProjectName("ansicolors"), Version("1.1.8")), Pin(ProjectName("cowsay"), Version("5.0"))
+    )