Using --complete-platform with --resolve-local-platforms should build sdists when local platform provides a subset of complete-platforms
#2026
Comments
shalabhc
changed the title
--complete-platform with --resolve-local-platforms should build sdists when local is a subset of complete--complete-platform with --resolve-local-platforms should build sdists when local plaform is a subset of complete-platforms
|
Here's a test that I think should pass, but doesn't: |
shalabhc
changed the title
--complete-platform with --resolve-local-platforms should build sdists when local plaform is a subset of complete-platforms--complete-platform with --resolve-local-platforms should build sdists when local platform is a subset of complete-platforms
shalabhc
changed the title
--complete-platform with --resolve-local-platforms should build sdists when local platform is a subset of complete-platforms--complete-platform with --resolve-local-platforms should build sdists when local platform provides a subset of complete-platforms
|
Why are you using --platform or --complete-platform at all here? |
|
Here's our use case:
I'm trying to optimize the build and deploy time. Using a custom builder docker image that can build sdists is expensive time-wise - eg takes about 1 additional minute in github to download. So I'm trying to do this:
Happy to answer more questions about this. |
|
So, the issue here is there is no interpreter in the manylinux image that has a subset of the complete platform info you built in the slim image. It goes exactly like this:
|
|
The definition of 'matches' is what I'm confused about. If a builder can build |
|
i.e instead of a subset match it does two checks:
I think 1 should not be necessary? |
|
Matches means subset right now. Quite literally, this is the problem: |
|
Note I see in your diff above that the builder (right) is a proper subset of the target complete-platforms (left). Did I miss something? |
|
@shalabhc why do you see the line you pulled as relevant? Its the block I point to that should be the relevant one. |
|
The This is then used in the |
|
Ok, you've gathered some good info here. I'll need some time to process. |
|
See my test (slightly modified version of existing test) above - it should succeed if 'subset' is the only criteria. |
|
@jsirois I have a fix for my new test but it breaks some existing tests :) I can tweak it until all tests pass and make a PR - may take a couple of iterations to get it right. |
|
Sounds great. Thanks @shalabhc |
jsirois
pushed a commit
that referenced
this issue
Jan 10, 2023
…lve-local-platforms (#2031) Fixes #2026. Previously the interpreter resolution was more strict than necessary. The `supported_tags` were only checked if the most specific tag of the complete platform matched the platform of the local interpreter. This prevented resolution in cases when it was possible and safe (ie all `supported_tags` of the interpreter as present in the `supported_tags` of the complete-platform). Added an additional test for the above case. Fixed existing code and also improved various existing tests by adding `expected_complete_platforms`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
If a builder platform is capable of building sdists into wheels that are compatible with a target platform, then
pex --complete-platform=cp-generated-from-target.json --resolve-local-platformsshould build any pex on the builder, including any sdists.Example where this doesn't work
quay.io/pypa/manylinux2014_x86_64python:3.10-slimsourceSteps to reproduce
Generate the complete-platform.json (py310-complete-platform.json.zip)
Prepare the builder
Build the PEX (note there is no wheel for
pendulumfor Python 3.10)Alternatives tried
--interpreter-constraintinstead of--python-> same error as above--platform=manylinux2014_x86_64-cp-310-cp310instead of--complete-platformmanylinux_2_17:The text was updated successfully, but these errors were encountered: