-
Notifications
You must be signed in to change notification settings - Fork 150
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
fix PyPy SOABI parsing #484
Conversation
for more information, see https://pre-commit.ci
If the logic was indeed wrong and this was not caught by the test suite, how do I know future refactorings won't break it? |
Codecov ReportBase: 66.85% // Head: 67.70% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #484 +/- ##
==========================================
+ Coverage 66.85% 67.70% +0.85%
==========================================
Files 12 12
Lines 902 901 -1
==========================================
+ Hits 603 610 +7
+ Misses 299 291 -8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Good question. I will add tests to test_bdist_wheel. |
for more information, see https://pre-commit.ci
Thanks! |
@@ -69,8 +68,8 @@ def get_flag(var, fallback, expected=True, warn=True): | |||
|
|||
|
|||
def get_abi_tag(): | |||
"""Return the ABI tag based on SOABI (if available) or emulate SOABI (PyPy).""" | |||
soabi = get_config_var("SOABI") | |||
"""Return the ABI tag based on SOABI (if available) or emulate SOABI (PyPy2).""" |
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.
(not really related, just noticed this) Is PyPy2 really still supported by wheel? I thought it requires Python 3.7+?
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 think all platforms these days have a valid SOABI, even windows. So "PyPy2" is more of a generic placeholder for "weird platform".
In #373 I started fixing the SOABI parsing to trim off the platform tag from the ABI tag. I used the wrong logic: the
soabi.startswith("pypy-")
would not be True. This means the code on line 96 was used for PyPy. No harm was done, but if PyPy updates its SOABI tag to fix this issue, the fix here will be needed.Edit: to be clear: the SOABI is currently
pypy36-pp73
, which does not start withpypy-