From a33b6795578ee76ad44088324eda102d4b212d4c Mon Sep 17 00:00:00 2001 From: John Sirois Date: Thu, 7 Apr 2022 16:19:58 -0700 Subject: [PATCH 1/2] Don't clear lock link database during analysis. When we clear the database and a resolve back-tracks, we lose the additional artifact urls needed to fill in the backtracked requirement lock encountered later in the resolve. Previously the clearing was done to relieve memory pressure during the analysis but measurements show even moderately large resolves collect only a handful of MBs worth of link data. We could collect 100x more links in a very large resolve and still be OK here on all but the most constrained machines. As such, we simply stop clearing the crawled link database during analysis. Fixes #1711 --- pex/pip/tool.py | 3 - .../cli/commands/test_issue_1711.py | 106 ++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 tests/integration/cli/commands/test_issue_1711.py diff --git a/pex/pip/tool.py b/pex/pip/tool.py index 53c761c33..7550fd029 100644 --- a/pex/pip/tool.py +++ b/pex/pip/tool.py @@ -415,8 +415,6 @@ def analyze(self, line): # VCS requirements are satisfied by a singular source; so we need not consult # links collected in this round. - self._links.clear() - self._resolved_requirements.append( ResolvedRequirement( requirement=requirement, @@ -467,7 +465,6 @@ def analyze(self, line): additional_artifacts = self._links[project_name_and_version] additional_artifacts.discard(partial_artifact) - self._links.clear() self._resolved_requirements.append( ResolvedRequirement( diff --git a/tests/integration/cli/commands/test_issue_1711.py b/tests/integration/cli/commands/test_issue_1711.py new file mode 100644 index 000000000..67639bcf9 --- /dev/null +++ b/tests/integration/cli/commands/test_issue_1711.py @@ -0,0 +1,106 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +import os + +from pex.cli.testing import run_pex3 +from pex.pep_440 import Version +from pex.pep_503 import ProjectName +from pex.resolve import lockfile +from pex.resolve.locked_resolve import Artifact, LockedRequirement +from pex.resolve.resolved_requirement import Fingerprint +from pex.testing import run_pex_command +from pex.typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any + + +def pypi_artifact( + hash, # type: str + path, # type: str +): + # type: (...) -> Artifact + return Artifact.from_url( + url="https://files.pythonhosted.org/packages/{}".format(path), + fingerprint=Fingerprint(algorithm="sha256", hash=hash), + ) + + +def test_backtrack_links_preserved(tmpdir): + # type: (Any) -> None + + lock = os.path.join(str(tmpdir), "lock") + create_lock_args = [ + "lock", + "create", + "--resolver-version", + "pip-2020-resolver", + "--style", + "universal", + "--interpreter-constraint", + ">=3.7,<3.10", + "psutil", + "psutil<5.5", # force a back-track + "-o", + lock, + "--indent", + "2", + ] + + def lock_as_json(): + with open(lock) as fp: + return fp.read() + + def assert_psutil_basics(): + # type: () -> LockedRequirement + lock_file = lockfile.load(lock) + assert 1 == len( + lock_file.locked_resolves + ), "Expected 1 resolve for universal style:\n{json}".format(json=lock_as_json()) + locked_resolve = lock_file.locked_resolves[0] + + locked_requirements_by_project_name = { + locked_requirement.pin.project_name: locked_requirement + for locked_requirement in locked_resolve.locked_requirements + } + psutil = locked_requirements_by_project_name.get(ProjectName("psutil")) + assert psutil is not None, "Expected lock to resolve psutil:\n{json}".format( + json=lock_as_json() + ) + assert Version("5.4.8") == psutil.pin.version, ( + "Expected lock to resolve psutil to <5.5 due to the second requirement but otherwise " + "as high as possible, which should be 5.4.8 but was: {version}\n{json}".format( + version=psutil.pin.version, json=lock_as_json() + ) + ) + return psutil + + # 1st prove this does the wrong thing on prior broken versions of Pex. + run_pex_command(args=["pex==2.1.77", "-c", "pex3", "--"] + create_lock_args).assert_success() + psutil_old = assert_psutil_basics() + assert 0 == len(psutil_old.additional_artifacts), ( + "Expected old versions of Pex to incorrectly wipe out the additional artifacts database " + "when backtracking needs to retrieve saved links later." + ) + + # Now show it currently works. + run_pex3(*create_lock_args).assert_success() + psutil_current = assert_psutil_basics() + assert { + pypi_artifact( + hash="1c71b9716790e202a00ab0931a6d1e25db1aa1198bcacaea2f5329f75d257fff", + path="50/00/ae52663b879333aa5c65fc9a87ddc24169f8fdd1831762a1ba9c9be7740d/psutil-5.4.8-cp37-cp37m-win_amd64.whl", + ), + pypi_artifact( + hash="bfcea4f189177b2d2ce4a34b03c4ac32c5b4c22e21f5b093d9d315e6e253cd81", + path="21/1e/fe6731e5f03ddf2e57d5b307f25bba294262bc88e27a0fbefdb3515d1727/psutil-5.4.8-cp37-cp37m-win32.whl", + ), + pypi_artifact( + hash="6e265c8f3da00b015d24b842bfeb111f856b13d24f2c57036582568dc650d6c3", + path="e3/58/0eae6e4466e5abf779d7e2b71fac7fba5f59e00ea36ddb3ed690419ccb0f/psutil-5.4.8.tar.gz", + ), + } == set(psutil_current.iter_artifacts()), ( + "Expected a full set of artifacts even after the lock resolve backtracked from " + "psutil latest to psutil<5.5 before settling:\n{json}".format(json=lock_as_json()) + ) From bbf48032c8e1357dedc28eb7b17c0a2dd3f48a55 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Thu, 7 Apr 2022 18:57:41 -0700 Subject: [PATCH 2/2] Skip legacy failure test for Python 2. No clue why this works under Python 2. --- .../cli/commands/test_issue_1711.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/integration/cli/commands/test_issue_1711.py b/tests/integration/cli/commands/test_issue_1711.py index 67639bcf9..f1787255f 100644 --- a/tests/integration/cli/commands/test_issue_1711.py +++ b/tests/integration/cli/commands/test_issue_1711.py @@ -4,6 +4,7 @@ import os from pex.cli.testing import run_pex3 +from pex.compatibility import PY3 from pex.pep_440 import Version from pex.pep_503 import ProjectName from pex.resolve import lockfile @@ -77,12 +78,19 @@ def assert_psutil_basics(): return psutil # 1st prove this does the wrong thing on prior broken versions of Pex. - run_pex_command(args=["pex==2.1.77", "-c", "pex3", "--"] + create_lock_args).assert_success() - psutil_old = assert_psutil_basics() - assert 0 == len(psutil_old.additional_artifacts), ( - "Expected old versions of Pex to incorrectly wipe out the additional artifacts database " - "when backtracking needs to retrieve saved links later." - ) + # N.B.: For some reason, this works with old Pex under Python 2.7; i.e.: It appears Pip behaves + # differently - likely because of some collection implementation difference. + if PY3: + run_pex_command( + args=["pex==2.1.77", "-c", "pex3", "--"] + create_lock_args + ).assert_success() + psutil_old = assert_psutil_basics() + assert 0 == len(psutil_old.additional_artifacts), ( + "Expected old versions of Pex to incorrectly wipe out the additional artifacts " + "database when backtracking needs to retrieve saved links later:\n{json}".format( + json=lock_as_json() + ) + ) # Now show it currently works. run_pex3(*create_lock_args).assert_success()