Skip to content

Commit

Permalink
Fix resolve check to cover dists satisfying root reqs.
Browse files Browse the repository at this point in the history
  • Loading branch information
jsirois committed Dec 2, 2024
1 parent b87bf3c commit 86efcd2
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 57 deletions.
12 changes: 12 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -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 an 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
Expand Down
117 changes: 69 additions & 48 deletions pex/resolve/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand All @@ -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):
Expand All @@ -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 = []
Expand Down
3 changes: 2 additions & 1 deletion pex/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion pex/version.py
Original file line number Diff line number Diff line change
@@ -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"
3 changes: 3 additions & 0 deletions tests/integration/resolve/test_issue_2532.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions tests/integration/test_issue_940.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 24 additions & 4 deletions tests/integration/test_issue_996.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
)
Expand All @@ -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")
Expand Down

0 comments on commit 86efcd2

Please sign in to comment.