-
-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Attempt "cross-builds" of sdists for foreign platforms. #2075
Conversation
The "cross-building" is in scare-quotes because this is not actually cross-building, it is just attempting a build of the sdist, and, if successful, seeing if the resulting wheel matches the foreign platform. This enables sdist-only projects to be used in foregin platform Pex operations when it turns out the sdist is multi-platform. Closes pex-tool#2073
) | ||
), | ||
reason="Test requires a manylinux2014_x86_64 compatible interpreter.", | ||
) | ||
@applicable_pip_versions | ||
def test_download_platform_issues_1355( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test changed substantially since ansicolors is now an example of a distribution which is "cross-buildable" for foreign platforms, but it did so to reflect the actual original issue: #1355
That issue is now verified directly and exactly.
This is a bit of a graduation for Pex. As of this change, which many prior changes over the last 2-3 years have laid the groundwork for, I see no more buggy-by-design behaviors left over from the Twitter years that can be fixed without breaking people. Time permitting, Pex 3.0.0 is ready to be prepared for (writing a real doc site) and cut. |
I'm on Linux. Old:
New:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! A couple of minor nits that are not worth fixing and triggering another CI run, but maybe worth tacking on to a future unrelated change.
@@ -119,6 +120,13 @@ def pex_repository(py27, py310, foreign_platform, manylinux): | |||
) | |||
|
|||
|
|||
@pytest.mark.skipif( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, it looks like these 2 Python 3.5 failures are not bitrot. Main is definitely green for them. I'll circle back in the am and figure these two out before proceeding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, yeah - this caught a bug. When I eliminated iter_platform_args()
, that killed sending Pip the --platform ... --implementation ... --python-version ... --abi ...
quad. The foreign_platform patches cover --platform ... --implementation ... --abi ...
, aka: wheel tag matching, but they do not cover --python-version ...
, aka Requires-Python
matching. Pip requires you send the quad as a whole; so the --python-version ...
/ Requires-Python
matching must be patched too. That patching is already implemented in the locker patches; so I'll need to share that code here for foreign platform resolves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I've reverted the test skips and updated the PR to include patching Requires-Python
for foreign platform resolves. The PR update is large, but its mainly shuffling. The code to patch Requires-Python
already existed to support universal lock resolves and so 95% of the delta here is extracting that bit of patching and re-arranging for re-use in the foreign platform patches.
…ires-Python outlawing 3.5." This reverts commit dd3868a.
The "cross-building" is in scare-quotes because this is not actually
cross-building, it is just attempting a build of the sdist, and, if
successful, seeing if the resulting wheel matches the foreign platform.
This enables sdist-only projects to be used in foreign platform Pex
operations when it turns out the sdist is multi-platform.
Closes #2073