From 374ede818cde11de4acc12b051411fb550fa7f2f Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sat, 13 Jan 2024 16:57:22 -0800 Subject: [PATCH] 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 | 7 +++ pex/resolve/lockfile/updater.py | 57 +++++++++++++++++--- pex/version.py | 2 +- tests/integration/test_issue_2324.py | 80 ++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 7 deletions(-) create mode 100644 tests/integration/test_issue_2324.py diff --git a/CHANGES.md b/CHANGES.md index 94c21ed1c..20f84c221 100644 --- a/CHANGES.md +++ b/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, diff --git a/pex/resolve/lockfile/updater.py b/pex/resolve/lockfile/updater.py index 79b5db0b8..a33cf76ef 100644 --- a/pex/resolve/lockfile/updater.py +++ b/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( diff --git a/pex/version.py b/pex/version.py index 41524877d..e1707b712 100644 --- a/pex/version.py +++ b/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" diff --git a/tests/integration/test_issue_2324.py b/tests/integration/test_issue_2324.py new file mode 100644 index 000000000..5f0403b4f --- /dev/null +++ b/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")) + )