Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adding PyPy support #185

Merged
merged 26 commits into from
Feb 19, 2020
Merged

Adding PyPy support #185

merged 26 commits into from
Feb 19, 2020

Conversation

YannickJadoul
Copy link
Member

@YannickJadoul YannickJadoul commented Oct 30, 2019 •
edited
Loading

There's probably still a lot to discuss on choosing defaults, option names, build tag names, ... but we can already give this a go and see if we can get the technical part working :)

Cfr. #143

@YannickJadoul YannickJadoul changed the title Adding PyPy builds Adding PyPy support Oct 30, 2019
@YannickJadoul
Copy link
Member Author

Tests are currently failing because the version of auditwheel inside of the pypywheels/manylinux2010-pypy_x86_64 Docker image is too old (==2.0) and does not include the release that uses the AUDITWHEEL_PLAT environment variable (>=2.1), yet.

@YannickJadoul YannickJadoul force-pushed the pypy branch 4 times, most recently from 91cf8e0 to 2964825 Compare October 30, 2019 16:33
@joerick
Copy link
Contributor

joerick commented Nov 4, 2019

Wow, I can't believe this works so easily!

So the idea is that CPython extensions should be able to build without modification for pypy as well? If that's the case, I'd love to just add this to CIBW_BUILD by default and users could get pypy wheels as a bonus - it'd be easy enough to turn them off with CIBW_SKIP if they weren't desired. If the compatibility is good, we could be producing pypy wheels be accident!

@YannickJadoul
Copy link
Member Author

So the idea is that CPython extensions should be able to build without modification for pypy as well?

Yes, PyPy support for the C API is now complete (though it's still not as efficient), and cffi extensions both work on CPython as well as PyPy, so PyPy seems to be making a push towards building wheels and getting them uploaded to the PyPI :)

If that's the case, I'd love to just add this to CIBW_BUILD by default and users could get pypy wheels as a bonus - it'd be easy enough to turn them off with CIBW_SKIP if they weren't desired.

Yes! Sounds good to me :)

This is just manylinux2010, right now, though (manylinux1 isn't supported by the PyPy interpreter itself, as far as I know). I still need to have a look at Windows and macOS; which might be more annoying since they don't have a nice drop-in Docker image, so we'll have to manually download and install/extract PyPy.

@mayeut
Copy link
Member

mayeut commented Nov 5, 2019

Does it make sense to try and build macOS and/or windows wheels as well ?

@YannickJadoul
Copy link
Member Author

Does it make sense to try and build macOS and/or windows wheels as well ?

Yes, it's on my TODO list. But for Windows, I was waiting for a resolution of the Windows-installation choice. And for macOS, I need to dive into the versioning of macOS (cfr. #156), since for PyPy3 the download page reads "(High Sierra >= 10.13, not for Sierra and below)".

In either case, it won't be as simple as replacing a Docker image, since I can't see .pkg packages for macOS, or find PyPy on NuGet :-/ So we'll have to download and extract, I think.

@YannickJadoul YannickJadoul force-pushed the pypy branch 18 times, most recently from b85bd00 to 1bc35d2 Compare November 7, 2019 13:41
@YannickJadoul
Copy link
Member Author

PyPy on Windows now also works! :)

Two remarks:

  • We need a workaround for an bug in PyPy related to encodings and code pages: simple_shell(['chcp', '437']) # Workaround for https://bitbucket.org/pypy/pypy/issues/3081/
  • The '{package_name}-{package_version}-pp372-pp372-win32.whl' wheel filename that's produced by PyPy 3 is inconsistent, but will get fixed in a future release.

@YannickJadoul
Copy link
Member Author

@Czaki: 1bba356

@YannickJadoul YannickJadoul force-pushed the pypy branch 3 times, most recently from 30f3b10 to 3c925ad Compare February 17, 2020 16:35
@YannickJadoul
Copy link
Member Author

Getting close to finishing this! :-)

The pypy/manylinux PR with wheel==0.34 was just merged.
Just waiting for pypa/virtualenv#1561 / pypa/virtualenv#1641, now!

@Czaki
Copy link
Contributor

Czaki commented Feb 17, 2020

The pypy/manylinux PR with wheel==0.34 was just merged.
Just waiting for pypa/virtualenv#1561 / pypa/virtualenv#1641, now!

But if we pin virtualenv bellow 20 then it works. And if they finish then minor bugfix can be released.
@mattip is waiting for release.

@mattip
Copy link
Contributor

mattip commented Feb 17, 2020

I'm not waiting, take your time and do it right. I already released the blog post with a little caveat about cibuildwheel. It didn;t get alot of traction anyway, I guess I shouldn't post on a Sunday.

Czaki
Czaki reviewed Feb 18, 2020
View reviewed changes
README.md Outdated
Comment on lines 25 to 26
| PyPy 2.7 v7.3.0 | ✅² | | ✅ | ✅ | |
| PyPy 3.6 v7.3.0 | ✅² | | ✅ | ✅ | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing description for ²

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

@YannickJadoul
Copy link
Member Author

Actually, since we merged #270 separately, this PR is ready to be merged, I think! We will revert #270 separately, once there's a solution, and I've vote to hold off releasing until then, but ... I think this one is ready :-)

@joerick, anyone else, any objections left? :-)

joerick
joerick reviewed Feb 18, 2020
View reviewed changes
Copy link
Contributor

@joerick joerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congratulations on this monumental effort @YannickJadoul! I have a few comments, they are inline.

@YannickJadoul
Copy link
Member Author

@joerick, I addressed the few new remarks, I think :-)

@YannickJadoul YannickJadoul mentioned this pull request Feb 19, 2020
Merged
@YannickJadoul YannickJadoul merged commit 4f47ab8 into pypa:master Feb 19, 2020
@YannickJadoul
Copy link
Member Author

Woohoooooo, merged! Thanks, everyone! :-)

@YannickJadoul YannickJadoul deleted the pypy branch February 19, 2020 23:06
polm added a commit to SamuraiT/mecab-python3 that referenced this pull request Mar 15, 2020
@polm
Try skipping pypy wheel builds
ccfaff3 
cibuildwheel [added support for
pypy](pypa/cibuildwheel#185), but those
builds don't work. It's not clear if the problem is the old Python
version or swig, but for the moment it should be fine to just disable
them.
@YannickJadoul YannickJadoul mentioned this pull request May 4, 2020
Merged
@YannickJadoul YannickJadoul mentioned this pull request Jun 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattip mattip mattip left review comments

@Czaki Czaki Czaki left review comments

@mayeut mayeut mayeut left review comments

@joerick joerick joerick approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@YannickJadoul @joerick @mayeut @mattip @Czaki