Skip to content

Commit

Permalink
Warn when --platform resolves fail tag checks. (#2533)
Browse files Browse the repository at this point in the history
The addition of a wheel tag compatibility check to the overall
post-resolve check in #2512 regressed users of abbreviated `--platform`
in some cases by failing PEX builds that would otherwise succeed and,
later, actually work at runtime. Keep the spirit of #2512 by emitting a
detailed warning at build time with remediation steps instead of failing
the build outright.

Fixes #2532
  • Loading branch information
jsirois authored Sep 17, 2024
1 parent 5e689f9 commit 9098057
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 9 deletions.
10 changes: 10 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Release Notes

## 2.19.1

This release fixes a regression introduced by #2512 in the 2.19.0
release when building PEXes using abbreviated `--platform` targets.
Instead of failing certain builds that used to succeed, Pex now warns
that the resulting PEX may fail at runtime and that
`--complete-platform` should be used instead.

# Only warn when `--platform` resolves fail tag checks. (#2533)

## 2.19.0

This release adds support for a new `--pre-resolved-dists` resolver as
Expand Down
55 changes: 47 additions & 8 deletions pex/resolve/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from abc import abstractmethod
from collections import OrderedDict, defaultdict

from pex import pex_warnings
from pex.common import pluralize
from pex.dependency_configuration import DependencyConfiguration
from pex.dist_metadata import Distribution, Requirement
Expand All @@ -17,7 +18,7 @@
from pex.pip.version import PipVersionValue
from pex.resolve.lockfile.model import Lockfile
from pex.sorted_tuple import SortedTuple
from pex.targets import Target, Targets
from pex.targets import AbbreviatedPlatform, Target, Targets
from pex.typing import TYPE_CHECKING

if TYPE_CHECKING:
Expand Down Expand Up @@ -99,6 +100,7 @@ def check_resolve(
resolved_distribution.distribution.metadata.project_name, []
).append(resolved_distribution)

maybe_unsatisfied = defaultdict(list) # type: DefaultDict[Target, List[str]]
unsatisfied = defaultdict(list) # type: DefaultDict[Target, List[str]]
for resolved_distribution in itertools.chain.from_iterable(
resolved_distributions_by_project_name.values()
Expand Down Expand Up @@ -129,14 +131,15 @@ def check_resolve(
installed_requirement_dist.distribution
for installed_requirement_dist in installed_requirement_dists
]
if not any(
(
requirement.specifier.contains(resolved_dist.version, prereleases=True)
and target.wheel_applies(resolved_dist)
)
version_matches = any(
requirement.specifier.contains(resolved_dist.version, prereleases=True)
for resolved_dist in resolved_dists
):
unsatisfied[target].append(
)
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,
Expand All @@ -149,6 +152,17 @@ def check_resolve(
),
)
)
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)

if unsatisfied:
unsatisfieds = []
Expand All @@ -169,6 +183,31 @@ def check_resolve(
)
)

if maybe_unsatisfied:
maybe_unsatisfieds = []
for target, missing in maybe_unsatisfied.items():
maybe_unsatisfieds.append(
"{target} may not be compatible with:\n {missing}".format(
target=target.render_description(), missing="\n ".join(missing)
)
)
pex_warnings.warn(
"The resolved distributions for {count} {targets} may not be compatible:\n"
"{failures}\n"
"\n"
"Its generally advisable to use `--complete-platform` instead of `--platform` to\n"
"ensure resolved distributions will be compatible with the target platform at\n"
"runtime. For instructions on how to generate a `--complete-platform` see:\n"
" https://docs.pex-tool.org/buildingpex.html#complete-platform ".format(
count=len(maybe_unsatisfieds),
targets=pluralize(maybe_unsatisfieds, "target"),
failures="\n".join(
"{index}: {failure}".format(index=index, failure=failure)
for index, failure in enumerate(maybe_unsatisfieds, start=1)
),
)
)


@attr.s(frozen=True)
class ResolveResult(object):
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.19.0"
__version__ = "2.19.1"
118 changes: 118 additions & 0 deletions tests/integration/resolve/test_issue_2532.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# Copyright 2024 Pex project contributors.
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import

import os
import subprocess
from textwrap import dedent

from pex.typing import TYPE_CHECKING
from testing import WheelBuilder
from testing.docker import skip_unless_docker

if TYPE_CHECKING:
from typing import Any


@skip_unless_docker
def test_resolved_wheel_tag_platform_mismatch_warns(
tmpdir, # type: Any
pex_project_dir, # type: str
):
# type: (...) -> None

context = os.path.join(str(tmpdir), "context")
pex_wheel = WheelBuilder(pex_project_dir, wheel_dir=context).bdist()
with open(os.path.join(context, "Dockerfile"), "w") as fp:
fp.write(
dedent(
r"""
FROM almalinux:8.10
RUN dnf install -y \
python3.11-devel \
gcc \
make \
libffi-devel
RUN mkdir /wheels
COPY {pex_wheel} {pex_wheel}
RUN python3.11 -mvenv /pex/venv && \
/pex/venv/bin/pip install {pex_wheel} && \
rm {pex_wheel}
ENV PATH=/pex/venv/bin:$PATH
RUN mkdir /work
WORKDIR = /work
""".format(
pex_wheel=os.path.basename(pex_wheel)
)
)
)
subprocess.check_call(args=["docker", "build", "-t", "pex_test_issue_2532", context])

process = subprocess.Popen(
args=[
"docker",
"run",
"--rm",
"pex_test_issue_2532",
"bash",
"-c",
dedent(
r"""
pex \
--python python3.11 \
--platform manylinux_2_28_x86_64-cp-3.11.9-cp311 \
cryptography==42.0.8 \
cffi==1.16.0 \
-o component_deps.pex
./component_deps.pex -c 'import cffi; print(cffi.__file__)'
"""
),
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
stdout, stderr = process.communicate()
assert 0 == process.returncode

# N.B.: The tags calculated for manylinux_2_28_x86_64-cp-3.11.9-cp311 via `pip -v debug ...`
# are:
# ----
# cp311-cp311-manylinux_2_28_x86_64
# cp311-abi3-manylinux_2_28_x86_64
# cp311-none-manylinux_2_28_x86_64
# ...
#
# This does not match either of the wheel tags of:
# + cp311-cp311-manylinux_2_17_x86_64
# + cp311-cp311-manylinux_2014_x86_64
#
# Instead of failing the resolve check though, we should just see a warning since both of these
# tags may be compatible at runtime, and, in fact, they are.
error = stderr.decode("utf-8")
assert (
dedent(
"""\
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:
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
Its generally advisable to use `--complete-platform` instead of `--platform` to
ensure resolved distributions will be compatible with the target platform at
runtime. For instructions on how to generate a `--complete-platform` see:
https://docs.pex-tool.org/buildingpex.html#complete-platform
"""
).strip()
in error
), error

output = stdout.decode("utf-8")
assert (
"cffi-1.16.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl" in output
), output

0 comments on commit 9098057

Please sign in to comment.