Warn when --platform resolves fail tag checks.
#2533
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The addition of wheel tag compatibility to the resolve chech adding in pex-tool#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 pex-tool#2512 by emitting a detailed warning at build time with remediation steps instead of failing the build outright. Fixes pex-tool#2532
jsirois
commented
Sep 16, 2024
Comment on lines
+100
to
+109
| 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 |
|
@cburroughs added you FYI. Avoid Pex 2.19.0 as a |
|
Thanks for the alert. FYI, although I imagine it doesn't particularly change anything on the Pex side: Pants almost doesn't use it any more, as the last use on |
|
Yeah, I did that research here: pantsbuild/pants#21425 (comment) |
jsirois
added a commit
that referenced
this pull request
Dec 2, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The addition of a wheel tag compatibility check to the overall
post-resolve check in #2512 regressed users of abbreviated
--platformin 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