Skip to content

Commit

Permalink
Accept more foreign --platform "YOLO-mode" wheels. (#2607)
Browse files Browse the repository at this point in the history
Previously, when speculatively building a wheel from an sdist for a
foreign platform target, the wheel tags needed to match the foreign
platform target's tags exactly in all cases. For `--complete-platform`
this makes sense, we have complete information about the foreign
platform target's tags, but for an abbreviated `--platform` it does not
since we have abbreviated information that is not enough to know a tag
mismatch definitively signals the wheel will never work on the foreign
platform target. Now only those definitive cases (e.g.: the speculative
wheel is Linux but the foreign abbreviated platform is macOS) are
rejected. This will let some eventually failing wheels though, but the
warning added in #2533 already covers this risk.
  • Loading branch information
jsirois authored Dec 2, 2024
1 parent e1c9c8d commit ce30d98
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 58 deletions.
14 changes: 14 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# Release Notes

## 2.24.2

This release fixes a long-standing bug in "YOLO-mode" foreign platform
speculative wheel builds. Previously if the speculatively built wheel
had tags that did not match the foreign platform, the process errored
pre-emptively. This was correct for complete foreign platforms, where
all tag information is known, but not for all cases of abbreviated
platforms, where the failure was overly aggressive in some cases. Now
foreign abbreviated platform speculative builds are only rejected when
there is enough information to be sure the speculatively built wheel
definitely cannot work on the foreign abbreviated platform.

* Accept more foreign `--platform` "YOLO-mode" wheels. (#2607)

## 2.24.1

This release fixes `pex3 cache prune` handling of cached Pips.
Expand Down
6 changes: 5 additions & 1 deletion docker/user/create_docker_image_user.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@
set -xeuo pipefail

if (( $# != 4 )); then
echo >2 "Usage $0 <user> <uid> <group> <gid>"
echo >&2 "Usage $0 <user> <uid> <group> <gid>"
exit 1
fi

uid=$2
gid=$4

if id -un ubuntu; then
userdel -r ubuntu
fi

if ! id -g ${gid} >/dev/null; then
group=$3
addgroup --gid=${gid} ${group} >&2
Expand Down
26 changes: 12 additions & 14 deletions pex/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,20 +527,18 @@ def _root_requirements_iter(
)
unavailable_dists = self._unavailable_dists_by_project_name.get(project_name)
if unavailable_dists:
message += (
"\nFound {count} {distributions} for {project_name} that do not apply:\n"
"{unavailable_dists}".format(
count=len(unavailable_dists),
distributions=pluralize(unavailable_dists, "distribution"),
project_name=project_name,
unavailable_dists="\n".join(
"{index}.) {message}".format(
index=index,
message=unavailable_dist.render_message(self._target),
)
for index, unavailable_dist in enumerate(unavailable_dists, start=1)
),
)
message += "\nFound {count} {distributions} for {project_name} that {does} not apply:\n" "{unavailable_dists}".format(
count=len(unavailable_dists),
distributions=pluralize(unavailable_dists, "distribution"),
project_name=project_name,
does="does" if len(unavailable_dists) == 1 else "do",
unavailable_dists="\n".join(
"{index}.) {message}".format(
index=index,
message=unavailable_dist.render_message(self._target),
)
for index, unavailable_dist in enumerate(unavailable_dists, start=1)
),
)
raise ResolveError(message)
candidates = [
Expand Down
48 changes: 47 additions & 1 deletion pex/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from pex.network_configuration import NetworkConfiguration
from pex.orderedset import OrderedSet
from pex.pep_376 import InstalledWheel
from pex.pep_425 import CompatibilityTags
from pex.pep_427 import InstallableType, WheelError, install_wheel_chroot
from pex.pep_503 import ProjectName
from pex.pip.download_observer import DownloadObserver
Expand All @@ -44,6 +45,7 @@
check_resolve,
)
from pex.targets import AbbreviatedPlatform, CompletePlatform, LocalInterpreter, Target, Targets
from pex.third_party.packaging.tags import Tag
from pex.tracer import TRACER
from pex.typing import TYPE_CHECKING
from pex.util import CacheHelper
Expand All @@ -53,6 +55,7 @@
from typing import (
DefaultDict,
Dict,
FrozenSet,
Iterable,
Iterator,
List,
Expand Down Expand Up @@ -368,7 +371,50 @@ def finalize_build(self, check_compatible=True):
wheel_path = wheels[0]
if check_compatible and self.request.target.is_foreign:
wheel = Distribution.load(wheel_path)
if not self.request.target.wheel_applies(wheel):
wheel_tag_match = self.request.target.wheel_applies(wheel)
incompatible = isinstance(self.request.target, CompletePlatform) and not wheel_tag_match
if (
not incompatible
and not wheel_tag_match
and isinstance(self.request.target, AbbreviatedPlatform)
):

def collect_platforms(tags):
# type: (Iterable[Tag]) -> Tuple[FrozenSet[str], bool]
platforms = [] # type: List[str]
is_linux = False
for tag in tags:
platforms.append(tag.platform)
if "linux" in tag.platform:
is_linux = True
return frozenset(platforms), is_linux

wheel_platform_tags, is_linux_wheel = collect_platforms(
CompatibilityTags.from_wheel(wheel.location)
)
abbreviated_target_platform_tags, is_linux_abbreviated_target = collect_platforms(
self.request.target.supported_tags
)
# N.B.: We can't say much about whether an abbreviated platform will match in the
# end unless the platform is a mismatch (i.e. linux vs mac). We check only for that
# sort of mismatch here. Further, we don't wade into manylinux compatibility and
# just consider a locally built linux wheel may match a linux target.
if not (is_linux_wheel and is_linux_abbreviated_target):
if is_linux_wheel ^ is_linux_abbreviated_target:
incompatible = True
else:
common_platforms = abbreviated_target_platform_tags.intersection(
wheel_platform_tags
)
if not common_platforms:
incompatible = True
elif common_platforms == frozenset(["any"]):
# N.B.: In the "any" platform case, we know we have complete information
# about the foreign abbreviated target platform (the `pip debug` command
# we run to learn compatible tags has enough information to give us all
# the "any" tags accurately); so we can expect an exact wheel tag match.
incompatible = not wheel_tag_match
if incompatible:
raise ValueError(
"No pre-built wheel was available for {project_name} {version}.{eol}"
"Successfully built the wheel {wheel} from the sdist {sdist} but it is not "
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.1"
__version__ = "2.24.2"
6 changes: 3 additions & 3 deletions tests/integration/resolve/test_issue_2532.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def test_resolved_wheel_tag_platform_mismatch_warns(
ENV PATH=/pex/venv/bin:$PATH
RUN mkdir /work
WORKDIR = /work
WORKDIR /work
""".format(
pex_wheel=os.path.basename(pex_wheel)
)
Expand Down Expand Up @@ -77,7 +77,8 @@ def test_resolved_wheel_tag_platform_mismatch_warns(
stderr=subprocess.PIPE,
)
stdout, stderr = process.communicate()
assert 0 == process.returncode
error = stderr.decode("utf-8")
assert 0 == process.returncode, error

# N.B.: The tags calculated for manylinux_2_28_x86_64-cp-3.11.9-cp311 via `pip -v debug ...`
# are:
Expand All @@ -93,7 +94,6 @@ def test_resolved_wheel_tag_platform_mismatch_warns(
#
# 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(
"""\
Expand Down
66 changes: 30 additions & 36 deletions tests/integration/test_issue_996.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,19 @@

import multiprocessing
import os
import subprocess

from pex.common import temporary_dir
from pex.interpreter import PythonInterpreter
from pex.typing import TYPE_CHECKING
from testing import (
PY39,
PY310,
IntegResults,
ensure_python_interpreter,
make_env,
run_pex_command,
run_simple_pex,
)
from testing import PY39, PY310, IntegResults, ensure_python_interpreter, make_env, run_pex_command
from testing.pytest.tmp import Tempdir

if TYPE_CHECKING:
from typing import List


def test_resolve_local_platform():
# type: () -> None
def test_resolve_local_platform(tmpdir):
# type: (Tempdir) -> None
python39 = ensure_python_interpreter(PY39)
python310 = ensure_python_interpreter(PY310)
pex_python_path = os.pathsep.join((python39, python310))
Expand All @@ -35,27 +28,28 @@ def create_platform_pex(args):
env=make_env(PEX_PYTHON_PATH=pex_python_path),
)

with temporary_dir() as td:
pex_file = os.path.join(td, "pex_file")

# N.B.: We use psutil since only an sdist is available for linux and osx and the
# distribution has no dependencies.
args = ["psutil==5.7.0", "-o", pex_file]

# By default, no --platforms are resolved and so distributions must be available in binary
# form.
results = create_platform_pex(args)
results.assert_failure()

# If --platform resolution is enabled however, we should be able to find a corresponding
# local interpreter to perform a full-featured resolve with.
results = create_platform_pex(["--resolve-local-platforms"] + args)
results.assert_success()

output, returncode = run_simple_pex(
pex=pex_file,
args=("-c", "import psutil; print(psutil.cpu_count())"),
interpreter=PythonInterpreter.from_binary(python310),
)
assert 0 == returncode
assert int(output.strip()) >= multiprocessing.cpu_count()
pex_file = tmpdir.join("pex_file")

# N.B.: We use psutil since only an sdist is available for linux and osx and the
# 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)

# 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()
process = subprocess.Popen(check_args, env=check_env, stderr=subprocess.PIPE)
_, stderr = process.communicate()
error = stderr.decode("utf-8")
assert process.returncode != 0, error
assert (
"Found 1 distribution for psutil that does not apply:\n"
" 1.) The wheel tags for psutil 5.7.0 are "
) in error

# If --platform resolution is enabled however, we should be able to find a corresponding
# local interpreter to perform a full-featured resolve with.
create_platform_pex(["--resolve-local-platforms"] + build_args).assert_success()
output = subprocess.check_output(check_args, env=check_env).decode("utf-8")
assert int(output.strip()) >= multiprocessing.cpu_count()
4 changes: 2 additions & 2 deletions tests/integration/test_pex_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,14 +393,14 @@ def test_boot_resolve_fail(
r"No interpreter compatible with the requested constraints was found:\n"
r"\n"
r" A distribution for psutil could not be resolved for {py39_exe}.\n"
r" Found 1 distribution for psutil that do not apply:\n"
r" Found 1 distribution for psutil that does not apply:\n"
r" 1\.\) The wheel tags for psutil 5\.9\.0 are .+ which do not match the supported tags "
r"of {py39_exe}:\n"
r" cp39-cp39-.+\n"
r" ... \d+ more ...\n"
r"\n"
r" A distribution for psutil could not be resolved for {py310_exe}.\n"
r" Found 1 distribution for psutil that do not apply:\n"
r" Found 1 distribution for psutil that does not apply:\n"
r" 1\.\) The wheel tags for psutil 5\.9\.0 are .+ which do not match the supported tags "
r"of {py310_exe}:\n"
r" cp310-cp310-.+\n"
Expand Down

0 comments on commit ce30d98

Please sign in to comment.