From 02008af93fe2c5586636e5c1013667f07dd5a484 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Mon, 28 Apr 2025 14:31:26 -0700 Subject: [PATCH 1/3] Fix Pex locking for source requirements. Previously, locking VCS requirements would fail for projects with non-normalized project names, e.g.: PySocks vs its normalized form of pysocks. Additionally, locking would fail when the requirements were specified at least in part via requirements files (`-r` / `--requirements`) and there was either a local project or a VCS requirement contained in the requirements files. --- CHANGES.md | 13 ++ pex/pip/vcs.py | 15 +-- pex/resolve/locker.py | 20 ++-- pex/version.py | 2 +- .../cli/commands/test_issue_1665.py | 2 +- .../commands/test_lock_requirements_file.py | 112 ++++++++++++++++++ 6 files changed, 147 insertions(+), 17 deletions(-) create mode 100644 tests/integration/cli/commands/test_lock_requirements_file.py diff --git a/CHANGES.md b/CHANGES.md index 72f82e400..b4d6d31fa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,18 @@ # Release Notes +## 2.36.1 + +This release fixes a few issues with creating Pex locks when source requirements were involved. + +Previously, locking VCS requirements would fail for projects with non-normalized project names, +e.g.: PySocks vs its normalized form of pysocks. + +Additionally, locking would fail when the requirements were specified at least in part via +requirements files (`-r` / `--requirements`) and there was either a local project or a VCS +requirement contained in the requirements files. + +* Fix Pex locking for source requirements. (#2750) + ## 2.36.0 This release brings support for creating PEXes that target Android. The Pip 25.1 upgrade in Pex diff --git a/pex/pip/vcs.py b/pex/pip/vcs.py index e954590f3..744157493 100644 --- a/pex/pip/vcs.py +++ b/pex/pip/vcs.py @@ -34,15 +34,16 @@ def _find_built_source_dist( # encoded in: `pip._internal.req.req_install.InstallRequirement.archive`. listing = os.listdir(build_dir) - pattern = re.compile( - r"{project_name}-(?P.+)\.zip".format( - project_name=project_name.normalized.replace("-", "[-_.]+") - ) - ) + pattern = re.compile(r"(?P[^-]+)-(?P.+)\.zip") for name in listing: match = pattern.match(name) - if match and Version(match.group("version")) == version: - return os.path.join(build_dir, name) + if not match: + continue + if ProjectName(match.group("project_name")) != project_name: + continue + if Version(match.group("version")) != version: + continue + return os.path.join(build_dir, name) return Error( "Expected to find built sdist for {project_name} {version} in {build_dir} but only found:\n" diff --git a/pex/resolve/locker.py b/pex/resolve/locker.py index c1f3adb6d..7ccc650b3 100644 --- a/pex/resolve/locker.py +++ b/pex/resolve/locker.py @@ -216,7 +216,7 @@ def build_result(self, line): # type: (str) -> Optional[ArtifactBuildResult] match = re.search( - r"Source in .+ has version (?P[^\s]+), which satisfies requirement " + r"Source in .+ has version (?P\S+), which satisfies requirement " r"(?P.+) .*from {url}".format(url=re.escape(self._artifact_url.raw_url)), line, ) @@ -434,7 +434,7 @@ def analyze(self, line): return self.Continue() match = re.search( - r"Fetched page (?P[^\s]+) as (?P{content_types})".format( + r"Fetched page (?P.+\S) as (?P{content_types})".format( content_types="|".join( re.escape(content_type) for content_type in self._fingerprint_service.accept ) @@ -447,18 +447,20 @@ def analyze(self, line): ) return self.Continue() - match = re.search(r"Looking up \"(?P[^\s]+)\" in the cache", line) + match = re.search(r"Looking up \"(?P.+\S)\" in the cache", line) if match: self._maybe_record_wheel(match.group("url")) - match = re.search(r"Processing (?P.*\.(whl|tar\.(gz|bz2|xz)|tgz|tbz2|txz|zip))", line) + match = re.search(r"Processing (?P.+\.(whl|tar\.(gz|bz2|xz)|tgz|tbz2|txz|zip))", line) if match: self._maybe_record_wheel( "file://{path}".format(path=os.path.abspath(match.group("path"))) ) match = re.search( - r"Added (?P.+) from (?P[^\s]+) .*to build tracker", + r"Added (?P.+) from (?P.+\S) \(from", line + ) or re.search( + r"Added (?P.+) from (?P.+\S) to build tracker", line, ) if match: @@ -478,13 +480,15 @@ def analyze(self, line): ) return self.Continue() - match = re.search(r"Added (?Pfile:.+) to build tracker", line) + match = re.search(r"Added (?Pfile:.+\S) \(from", line) or re.search( + r"Added (?Pfile:.+\S) to build tracker", line + ) if match: file_url = match.group("file_url") self._artifact_build_observer = ArtifactBuildObserver( done_building_patterns=( re.compile( - r"Removed .+ from {file_url} from build tracker".format( + r"Removed .+ from {file_url} (?:.* )?from build tracker".format( file_url=re.escape(file_url) ) ), @@ -503,7 +507,7 @@ def analyze(self, line): return self.Continue() if self.style in (LockStyle.SOURCES, LockStyle.UNIVERSAL): - match = re.search(r"Found link (?P[^\s]+)(?: \(from .*\))?, version: ", line) + match = re.search(r"Found link (?P\S+)(?: \(from .*\))?, version: ", line) if match: url = self.parse_url_and_maybe_record_fingerprint(match.group("url")) pin, partial_artifact = self._extract_resolve_data(url) diff --git a/pex/version.py b/pex/version.py index 9923fa392..bec980359 100644 --- a/pex/version.py +++ b/pex/version.py @@ -1,4 +1,4 @@ # Copyright 2015 Pex project contributors. # Licensed under the Apache License, Version 2.0 (see LICENSE). -__version__ = "2.36.0" +__version__ = "2.36.1" diff --git a/tests/integration/cli/commands/test_issue_1665.py b/tests/integration/cli/commands/test_issue_1665.py index 360cb0c84..edf32cfed 100644 --- a/tests/integration/cli/commands/test_issue_1665.py +++ b/tests/integration/cli/commands/test_issue_1665.py @@ -45,4 +45,4 @@ def assert_lock(*extra_lock_args, **extra_popen_args): cwd = os.path.join(str(tmpdir), "cwd") tmpdir = os.path.join(cwd, ".tmp") os.makedirs(tmpdir) - assert_lock("--tmpdir", ".tmp", cwd=cwd) + assert_lock(cwd=cwd) diff --git a/tests/integration/cli/commands/test_lock_requirements_file.py b/tests/integration/cli/commands/test_lock_requirements_file.py new file mode 100644 index 000000000..560e12b57 --- /dev/null +++ b/tests/integration/cli/commands/test_lock_requirements_file.py @@ -0,0 +1,112 @@ +# Copyright 2025 Pex project contributors. +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import absolute_import, print_function + +import difflib +import filecmp +import os + +import pytest + +from pex.pip.version import PipVersion +from testing.cli import run_pex3 +from testing.pytest_utils.tmp import Tempdir + + +def diff( + file1, # type: str + file2, # type: str +): + # type: (...) -> str + + with open(file1) as fp1, open(file2) as fp2: + return os.linesep.join( + difflib.context_diff(fp1.readlines(), fp2.readlines(), fp1.name, fp2.name) + ) + + +def assert_locks_match( + tmpdir, # type: Tempdir + *requirements # type: str +): + # type: (...) -> None + + lock1 = tmpdir.join("lock1.json") + run_pex3( + "lock", + "create", + "--pip-version", + "latest-compatible", + "-o", + lock1, + "--indent", + "2", + *requirements + ).assert_success() + + requirements_file = tmpdir.join("requirements.txt") + with open(requirements_file, "w") as fp: + for requirement in requirements: + print(requirement, file=fp) + + lock2 = tmpdir.join("lock2.json") + run_pex3( + "lock", + "create", + "--pip-version", + "latest-compatible", + "-o", + lock2, + "--indent", + "2", + "-r", + requirements_file, + ).assert_success() + + assert filecmp.cmp(lock1, lock2, shallow=False), diff(lock1, lock2) + + +def test_lock_by_name(tmpdir): + # type: (Tempdir) -> None + + assert_locks_match(tmpdir, "cowsay<6") + + +def test_lock_vcs(tmpdir): + # type: (Tempdir) -> None + + assert_locks_match( + tmpdir, "ansicolors @ git+https://github.com/jonathaneunice/colors.git@c965f5b9" + ) + + +@pytest.mark.skipif( + PipVersion.LATEST_COMPATIBLE is PipVersion.VENDORED, + reason="Vendored Pip cannot handle modern pyproject.toml with heterogeneous arrays.", +) +def test_lock_local_project( + tmpdir, # type: Tempdir + pex_project_dir, # type: str +): + # type: (...) -> None + + assert_locks_match(tmpdir, pex_project_dir) + + +def test_lock_mixed( + tmpdir, # type: Tempdir + pex_project_dir, # type: str +): + # type: (...) -> None + + requirements = [ + "cowsay<6", + "ansicolors @ git+https://github.com/jonathaneunice/colors.git@c965f5b9", + ] + # N.B.: Vendored Pip cannot handle modern pyproject.toml with heterogeneous arrays, which ours + # uses. + if PipVersion.LATEST_COMPATIBLE is not PipVersion.VENDORED: + requirements.append(pex_project_dir) + + assert_locks_match(tmpdir, *requirements) From d1ade4b03f43e9a92a8de410a095a39b0bc82722 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Mon, 28 Apr 2025 14:21:23 -0700 Subject: [PATCH 2/3] Fix concurrent test issues with root dir pollution. Get rid of `os.chdir` fixture which was inherently unsafe and also fix the Pex build backend to not ask setuptools to help calculate extra build requirements - we know the answers for these ourselves. --- build-backend/pex_build/setuptools/build.py | 30 ++++++++++++--------- tests/integration/conftest.py | 15 +---------- tests/integration/test_integration.py | 29 +++++++++++--------- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/build-backend/pex_build/setuptools/build.py b/build-backend/pex_build/setuptools/build.py index d2f049271..bbc112a4d 100644 --- a/build-backend/pex_build/setuptools/build.py +++ b/build-backend/pex_build/setuptools/build.py @@ -19,7 +19,6 @@ from pex import hashing, toml, windows from pex.common import open_zip, safe_copy, safe_mkdir, temporary_dir -from pex.orderedset import OrderedSet from pex.pep_376 import Hash, InstalledFile, Record from pex.typing import cast from pex.version import __version__ @@ -28,6 +27,15 @@ from typing import Any, Dict, List, Optional +def get_requires_for_build_sdist(config_settings=None): + # type: (Optional[Dict[str, Any]]) -> List[str] + + # N.B.: The default setuptools implementation would eventually return nothing, but only after + # running code that can temporarily pollute our project directory, foiling concurrent test runs; + # so we short-circuit the answer here. Faster and safer. + return [] + + def build_sdist( sdist_directory, # type: str config_settings=None, # type: Optional[Dict[str, Any]] @@ -77,17 +85,15 @@ def prepare_metadata_for_build_editable( def get_requires_for_build_wheel(config_settings=None): # type: (Optional[Dict[str, Any]]) -> List[str] - reqs = OrderedSet( - setuptools.build_meta.get_requires_for_build_wheel(config_settings=config_settings) - ) # type: OrderedSet[str] - if pex_build.INCLUDE_DOCS: - pyproject_data = toml.load("pyproject.toml") - return cast( - "List[str]", - # Here we skip any included dependency groups and just grab the direct doc requirements. - [req for req in pyproject_data["dependency-groups"]["docs"] if isinstance(req, str)], - ) - return list(reqs) + if not pex_build.INCLUDE_DOCS: + return [] + + pyproject_data = toml.load("pyproject.toml") + return cast( + "List[str]", + # Here we skip any included dependency groups and just grab the direct doc requirements. + [req for req in pyproject_data["dependency-groups"]["docs"] if isinstance(req, str)], + ) def prepare_metadata_for_build_wheel( diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index ad19a0edd..376fc669b 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -9,7 +9,6 @@ import pytest from pex.atomic_directory import atomic_directory -from pex.common import temporary_dir from pex.interpreter import PythonInterpreter from pex.os import WINDOWS from pex.pip.version import PipVersion @@ -19,7 +18,7 @@ from testing.mitmproxy import Proxy if TYPE_CHECKING: - from typing import Any, Callable, Iterator + from typing import Any, Callable @pytest.fixture(scope="session") @@ -92,18 +91,6 @@ def pex_bdist( return wheels[0] -@pytest.fixture -def tmp_workdir(): - # type: () -> Iterator[str] - cwd = os.getcwd() - with temporary_dir() as tmpdir: - os.chdir(tmpdir) - try: - yield os.path.realpath(tmpdir) - finally: - os.chdir(cwd) - - @pytest.fixture(scope="session") def mitmdump_venv(shared_integration_test_tmpdir): # type: (str) -> Virtualenv diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index c87791f1a..d4322a7fd 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -59,6 +59,7 @@ from testing.pep_427 import get_installable_type_flag from testing.pip import skip_if_only_vendored_pip_supported from testing.pytest_utils import IS_CI +from testing.pytest_utils.tmp import Tempdir if TYPE_CHECKING: from typing import Any, Callable, Iterator, List, Optional, Tuple @@ -1552,8 +1553,9 @@ def test_unzip_mode(tmpdir): assert "PEXWarning: The `PEX_UNZIP` env var is deprecated." in error2.decode("utf-8") -def test_tmpdir_absolute(tmp_workdir): - # type: (str) -> None +def test_tmpdir_absolute(tmpdir): + # type: (Tempdir) -> None + tmp_workdir = str(tmpdir) result = run_pex_command( args=[ "--tmpdir", @@ -1569,26 +1571,27 @@ def test_tmpdir_absolute(tmp_workdir): print(tempfile.gettempdir()) """ ), - ] + ], + cwd=tmp_workdir, ) result.assert_success() assert [tmp_workdir, tmp_workdir] == result.output.strip().splitlines() -def test_tmpdir_dne(tmp_workdir): - # type: (str) -> None - tmpdir_dne = os.path.join(tmp_workdir, ".tmp") - result = run_pex_command(args=["--tmpdir", ".tmp", "--", "-c", ""]) +def test_tmpdir_dne(tmpdir): + # type: (Tempdir) -> None + tmpdir_dne = tmpdir.join(".tmp") + result = run_pex_command(args=["--tmpdir", ".tmp", "--", "-c", ""], cwd=str(tmpdir)) result.assert_failure() assert tmpdir_dne in result.error assert "does not exist" in result.error -def test_tmpdir_file(tmp_workdir): - # type: (str) -> None - tmpdir_file = os.path.join(tmp_workdir, ".tmp") +def test_tmpdir_file(tmpdir): + # type: (Tempdir) -> None + tmpdir_file = tmpdir.join(".tmp") touch(tmpdir_file) - result = run_pex_command(args=["--tmpdir", ".tmp", "--", "-c", ""]) + result = run_pex_command(args=["--tmpdir", ".tmp", "--", "-c", ""], cwd=str(tmpdir)) result.assert_failure() assert tmpdir_file in result.error assert "is not a directory" in result.error @@ -1601,8 +1604,8 @@ def test_tmpdir_file(tmp_workdir): ) -def test_requirements_network_configuration(proxy, tmp_workdir): - # type: (Proxy, str) -> None +def test_requirements_network_configuration(proxy): + # type: (Proxy) -> None def req( contents, # type: str line_no, # type: int From 8c12e0a13e877cd226427f88ba8553e765382265 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Mon, 28 Apr 2025 16:05:34 -0700 Subject: [PATCH 3/3] Fix bad vcs re. Allow for project names containing dash. --- pex/pip/vcs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pex/pip/vcs.py b/pex/pip/vcs.py index 744157493..e246818e5 100644 --- a/pex/pip/vcs.py +++ b/pex/pip/vcs.py @@ -34,7 +34,7 @@ def _find_built_source_dist( # encoded in: `pip._internal.req.req_install.InstallRequirement.archive`. listing = os.listdir(build_dir) - pattern = re.compile(r"(?P[^-]+)-(?P.+)\.zip") + pattern = re.compile(r"(?P.+)-(?P.+)\.zip") for name in listing: match = pattern.match(name) if not match: