From 0e9d1a2c8063e0f48fc224dbb4e62260f18a928c Mon Sep 17 00:00:00 2001 From: John Sirois Date: Mon, 2 Dec 2024 14:02:57 -0800 Subject: [PATCH] Fix resolve check to cover dists satisfying root reqs. (#2610) --- CHANGES.md | 12 ++ pex/resolve/resolvers.py | 117 +++++++++++-------- pex/resolver.py | 3 +- pex/version.py | 2 +- tests/integration/resolve/test_issue_2532.py | 3 + tests/integration/test_issue_940.py | 5 +- tests/integration/test_issue_996.py | 28 ++++- 7 files changed, 113 insertions(+), 57 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 91fae5d04..3b375e9e6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,17 @@ # Release Notes +## 2.24.3 + +This release fixes a long-standing bug in resolve checking. Previously, +only resolve dependency chains where checked, but not the resolved +distributions that satisfied the input root requirements. + +In addition, the 2.24.2 release included a wheel with no compression +(~11MB instead of ~3.5MB). The Pex wheel is now fixed to be compressed. + +* Fix resolve check to cover dists satisfying root reqs. (#2610) +* Fix build process to produce a compressed `.whl`. (#2609) + ## 2.24.2 This release fixes a long-standing bug in "YOLO-mode" foreign platform diff --git a/pex/resolve/resolvers.py b/pex/resolve/resolvers.py index ef98e9651..d838b0b3f 100644 --- a/pex/resolve/resolvers.py +++ b/pex/resolve/resolvers.py @@ -92,6 +92,7 @@ def check_resolve( resolved_distributions, # type: Iterable[ResolvedDistribution] ): # type: (...) -> None + resolved_distributions_by_project_name = ( OrderedDict() ) # type: OrderedDict[ProjectName, List[ResolvedDistribution]] @@ -102,11 +103,77 @@ def check_resolve( maybe_unsatisfied = defaultdict(list) # type: DefaultDict[Target, List[str]] unsatisfied = defaultdict(list) # type: DefaultDict[Target, List[str]] + + def check( + target, # type: Target + dist, # type: Distribution + requirement, # type: Requirement + root=False, # type: bool + ): + # type: (...) -> None + + required_by = ( + "{requirement} was requested".format(requirement=requirement) + if root + else "{dist} requires {requirement}".format(dist=dist, requirement=requirement) + ) + installed_requirement_dists = resolved_distributions_by_project_name.get( + requirement.project_name + ) + if not installed_requirement_dists: + unsatisfied[target].append( + "{required_by} but no version was resolved".format(required_by=required_by) + ) + else: + resolved_dists = [ + installed_requirement_dist.distribution + for installed_requirement_dist in installed_requirement_dists + ] + version_matches = any( + ( + # We don't attempt a match against a legacy version in order to avoid false + # negatives. + resolved_dist.metadata.version.is_legacy + or requirement.specifier.contains(resolved_dist.version, prereleases=True) + ) + for resolved_dist in resolved_dists + ) + tags_match = any( + target.wheel_applies(resolved_dist) for resolved_dist in resolved_dists + ) + if not version_matches or not tags_match: + message = ( + "{required_by} but {count} incompatible {dists_were} resolved:\n" + " {dists}".format( + required_by=required_by, + count=len(resolved_dists), + dists_were="dists were" if len(resolved_dists) > 1 else "dist was", + dists="\n ".join( + os.path.basename(resolved_dist.location) + for resolved_dist in resolved_dists + ), + ) + ) + if version_matches and not tags_match and isinstance(target, AbbreviatedPlatform): + # We don't know for sure an abbreviated platform doesn't match a wheels tags + # until we are running on that platform; so just warn for these instead of + # hard erroring. + maybe_unsatisfied[target].append(message) + else: + unsatisfied[target].append(message) + for resolved_distribution in itertools.chain.from_iterable( resolved_distributions_by_project_name.values() ): - dist = resolved_distribution.distribution target = resolved_distribution.target + for root_req in resolved_distribution.direct_requirements: + check( + target=target, + dist=resolved_distribution.distribution, + requirement=root_req, + root=True, + ) + dist = resolved_distribution.distribution for requirement in dist.requires(): if dependency_configuration.excluded_by(requirement): @@ -116,53 +183,7 @@ def check_resolve( ) if not target.requirement_applies(requirement): continue - - installed_requirement_dists = resolved_distributions_by_project_name.get( - requirement.project_name - ) - if not installed_requirement_dists: - unsatisfied[target].append( - "{dist} requires {requirement} but no version was resolved".format( - dist=dist.as_requirement(), requirement=requirement - ) - ) - else: - resolved_dists = [ - installed_requirement_dist.distribution - for installed_requirement_dist in installed_requirement_dists - ] - version_matches = any( - requirement.specifier.contains(resolved_dist.version, prereleases=True) - for resolved_dist in resolved_dists - ) - tags_match = any( - target.wheel_applies(resolved_dist) for resolved_dist in resolved_dists - ) - if not version_matches or not tags_match: - message = ( - "{dist} requires {requirement} but {count} incompatible {dists_were} " - "resolved:\n {dists}".format( - dist=dist, - requirement=requirement, - count=len(resolved_dists), - dists_were="dists were" if len(resolved_dists) > 1 else "dist was", - dists="\n ".join( - os.path.basename(resolved_dist.location) - for resolved_dist in resolved_dists - ), - ) - ) - if ( - version_matches - and not tags_match - and isinstance(target, AbbreviatedPlatform) - ): - # We don't know for sure an abbreviated platform doesn't match a wheels tags - # until we are running on that platform; so just warn for these instead of - # hard erroring. - maybe_unsatisfied[target].append(message) - else: - unsatisfied[target].append(message) + check(target, dist, requirement) if unsatisfied: unsatisfieds = [] diff --git a/pex/resolver.py b/pex/resolver.py index 368f12e29..fdfac75aa 100644 --- a/pex/resolver.py +++ b/pex/resolver.py @@ -1029,10 +1029,11 @@ def add_installation(install_result): except WheelError as e: raise Untranslatable("Failed to install a wheel: {err}".format(err=e)) + installations = list(direct_requirements.adjust(installations)) if not ignore_errors: with TRACER.timed("Checking install"): check_resolve(self._dependency_configuration, installations) - return direct_requirements.adjust(installations) + return installations def _parse_reqs( diff --git a/pex/version.py b/pex/version.py index dc4606e32..d5e8f4a47 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.24.2" +__version__ = "2.24.3" diff --git a/tests/integration/resolve/test_issue_2532.py b/tests/integration/resolve/test_issue_2532.py index 4966fe68f..01b4a6ce6 100644 --- a/tests/integration/resolve/test_issue_2532.py +++ b/tests/integration/resolve/test_issue_2532.py @@ -99,6 +99,9 @@ def test_resolved_wheel_tag_platform_mismatch_warns( """\ PEXWarning: The resolved distributions for 1 target may not be compatible: 1: abbreviated platform cp311-cp311-manylinux_2_28_x86_64 may not be compatible with: + cffi==1.16.0 was requested but 2 incompatible dists were resolved: + cffi-1.16.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl + cffi-1.16.0-cp311-cp311-linux_x86_64.whl cryptography 42.0.8 requires cffi>=1.12; platform_python_implementation != "PyPy" but 2 incompatible dists were resolved: cffi-1.16.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl cffi-1.16.0-cp311-cp311-linux_x86_64.whl diff --git a/tests/integration/test_issue_940.py b/tests/integration/test_issue_940.py index e6bbad6dc..f04b67357 100644 --- a/tests/integration/test_issue_940.py +++ b/tests/integration/test_issue_940.py @@ -51,10 +51,9 @@ def prepare_project(project_dir): ) as whl: pex_root = os.path.join(str(tmpdir), "pex_root") pex_file = os.path.join(str(tmpdir), "pex") - results = run_pex_command( + run_pex_command( args=["-o", pex_file, "--pex-root", pex_root, "--runtime-pex-root", pex_root, whl] - ) - results.assert_success() + ).assert_success() output, returncode = run_simple_pex(pex_file, args=["-c", "import foo"]) assert returncode == 0, output diff --git a/tests/integration/test_issue_996.py b/tests/integration/test_issue_996.py index 011c81104..19472d18e 100644 --- a/tests/integration/test_issue_996.py +++ b/tests/integration/test_issue_996.py @@ -3,9 +3,12 @@ import multiprocessing import os +import re import subprocess +from textwrap import dedent from pex.interpreter import PythonInterpreter +from pex.targets import AbbreviatedPlatform from pex.typing import TYPE_CHECKING from testing import PY39, PY310, IntegResults, ensure_python_interpreter, make_env, run_pex_command from testing.pytest.tmp import Tempdir @@ -20,10 +23,13 @@ def test_resolve_local_platform(tmpdir): python310 = ensure_python_interpreter(PY310) pex_python_path = os.pathsep.join((python39, python310)) + platform = PythonInterpreter.from_binary(python310).platform + target = AbbreviatedPlatform.create(platform) + def create_platform_pex(args): # type: (List[str]) -> IntegResults return run_pex_command( - args=["--platform", str(PythonInterpreter.from_binary(python310).platform)] + args, + args=["--platform", str(platform)] + args, python=python39, env=make_env(PEX_PYTHON_PATH=pex_python_path), ) @@ -34,11 +40,25 @@ def create_platform_pex(args): # distribution has no dependencies. build_args = ["psutil==5.7.0", "-o", pex_file] check_args = [python310, pex_file, "-c", "import psutil; print(psutil.cpu_count())"] - check_env = make_env(PEX_PYTHON_PATH=python310, PEX_VERBOSE=1) + check_env = make_env(PEX_PYTHON_PATH=python310) # By default, no --platforms are resolved and so the yolo build process will produce a wheel - # using Python 3.9, which is not compatible with Python 3.10. - create_platform_pex(build_args).assert_success() + # using Python 3.9, which is not compatible with Python 3.10. Since we can't be sure of this, + # we allow the build but warn it may be incompatible. + create_platform_pex(build_args).assert_success( + expected_error_re=dedent( + r""" + .* PEXWarning: The resolved distributions for 1 target may not be compatible: + 1: {target} may not be compatible with: + psutil==5\.7\.0 was requested but 1 incompatible dist was resolved: + psutil-5\.7\.0-cp39-cp39-.+\.whl + .* + """ + ) + .format(target=re.escape(target.render_description())) + .strip(), + re_flags=re.DOTALL | re.MULTILINE, + ) process = subprocess.Popen(check_args, env=check_env, stderr=subprocess.PIPE) _, stderr = process.communicate() error = stderr.decode("utf-8")