-
Notifications
You must be signed in to change notification settings - Fork 760
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
Add support for PyPy wheels #1028
Conversation
"ironpython" => Err(TagsError::UnsupportedImplementation(s.to_string())), | ||
"jython" => Err(TagsError::UnsupportedImplementation(s.to_string())), | ||
// Unknown implementations. | ||
_ => Err(TagsError::UnknownImplementation(s.to_string())), |
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.
@konstin - Not sure if you have an opinion on this. Should I just skip all the tags that rely on calling abi_tag
? So just include 3, 4, and 5 in from_env
, and skip the ones that rely on the ABI and language version?
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.
I guess we need to read EXT_SUFFIX
to exactly mimic packaging
. We could add that to our environment markers...? But not required for this PR.
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.
wdym by "calling abi_tag"?
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.
Above, we have cases like:
// 1. This exact c api version
for platform_tag in &platform_tags {
tags.push((
implementation.language_tag(python_version),
implementation.abi_tag(python_version, implementation_version),
platform_tag.clone(),
));
tags.push((
implementation.language_tag(python_version),
"none".to_string(),
platform_tag.clone(),
));
}
// 2. abi3 and no abi (e.g. executable binary)
if matches!(implementation, Implementation::CPython) {
// For some reason 3.2 is the minimum python for the cp abi
for minor in 2..=python_version.1 {
for platform_tag in &platform_tags {
tags.push((
implementation.language_tag((python_version.0, minor)),
"abi3".to_string(),
platform_tag.clone(),
));
}
}
}
// 3. no abi (e.g. executable binary)
for minor in 0..=python_version.1 {
for platform_tag in &platform_tags {
tags.push((
format!("py{}{}", python_version.0, minor),
"none".to_string(),
platform_tag.clone(),
));
}
}
// 4. major only
...
If we don't know how to create ABI tags (e.g., you're using ironpython), we could skip (1), (2), and (3), but allow (4) and (5) (the no-ABI tags). Right now, we error out saying we don't support them.
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.
I'm for continuing to error out, otherwise it gets confusing when it seems to work but then doesn't pick up any binary wheels.
c745e73
to
5a83d8a
Compare
"ironpython" => Err(TagsError::UnsupportedImplementation(s.to_string())), | ||
"jython" => Err(TagsError::UnsupportedImplementation(s.to_string())), | ||
// Unknown implementations. | ||
_ => Err(TagsError::UnknownImplementation(s.to_string())), |
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.
wdym by "calling abi_tag"?
/// Return the major version of the implementation (e.g., `CPython` or `PyPy`). | ||
pub fn implementation_major(&self) -> u8 { | ||
let major = self.markers.python_full_version.version.release()[0]; | ||
u8::try_from(major).expect("invalid major version") | ||
} | ||
|
||
/// Return the minor version of the implementation (e.g., `CPython` or `PyPy`). | ||
pub fn implementation_minor(&self) -> u8 { | ||
let minor = self.markers.python_full_version.version.release()[1]; | ||
u8::try_from(minor).expect("invalid minor version") |
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.
For pypy the implementation version is different from the python version. E.g. i have pypy 7.3.1 installed which implements python 3.10. The current implementation says the tag is pypy310_pp310
, while it's actually pypy310_pp73
. It's the version we track in the markers' implementation_version
.
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.
Sorry, I know this and thought I did it correctly (?), it’s just a typo.
5a83d8a
to
90af694
Compare
# Conflicts: # crates/puffin/tests/pip_compile.rs
# Conflicts: # crates/puffin/tests/pip_compile.rs
Summary
This PR adds support for PyPy wheels by changing the compatible tags based on the implementation name and version of the current interpreter.
For now, we only support CPython and PyPy, and explicitly error out when given other interpreters. (Is this right? Should we just fallback to CPython tags...? Or skip the ABI-specific tags for unknown interpreters?)
The logic is based on https://github.com/pypa/packaging/blob/4d8534061364e3cbfee582192ab81a095ec2db51/src/packaging/tags.py#L247. Note, however, that
packaging
uses theEXT_SUFFIX
variable fromsysconfig
... Instead, I looked at the way that PyPy formats the tags, and recreated them based on the Python and implementation version. For example, PyPy wheels look likecchardet-2.1.7-pp37-pypy37_pp73-win_amd64.whl
-- so that'spp37
for PyPy with Python version 3.7, and thenpypy37_pp73
for PyPy with Python version 3.7 and PyPy version 7.3.Closes #1013.
Test Plan
I tested this manually, but I couldn't find macOS universal PyPy wheels... So instead I added
cchardet
to arequirements.in
, rancargo run pip sync requirements.in --index-url https://pypy.kmtea.eu/simple --verbose
, and added logging to verify that the platform tags matched (even if the architecture didn't).