Installing on Pypy fails because of CPython-specific extension modules

[lvh]

Some of the extension modules (specifically the Cython extensions) are CPython specific and break on Pypy. This makes pip install -e some_twisted_checkout not work. Eventually it blows up with an error like:

/home/lvh/tmp/pypy/include/pypy_decl.h:129:29: note: expected ‘struct PyThreadState *’ but argument is of type ‘int’

There is a complete pip log attached to this ticket.

Armin Rigo has assured me this is a consequence of Cython. Cython support in Pypy is a work in progress. There are a number of potential eventual resolutions for this, but the correct thing to do right now is to disable compilation of the module on not-CPython. The right thing to do *eventually* might be re-enable it, or just use `select.epoll` as available on recent Pythons.

The affected modules are twisted.test.raiser, twisted.internet.iocpreactor.iocpsupport.iocpsupport and twisted.python.epoll`.

[lvh]

Here's a patch. It's missing tests and docstrings because I'm not sure how to test this without shoving fake modules into sys.modules and other less-than-savory things like del sys.subversion.

If that's what has to happen, that's what I'll do, but...

[exarkun]

Thanks for working on this important yet often neglected part of Twisted, lvh.

1. Yea, tests. Fortunately, I don't think this will be very hard or gross to test. Whenever the only way to proceed appears to be to jam some fakes into a global like sys.modules, the solution is usually the same - add some more parameters to something. _isCPython should accept some arguments; perhaps the platform module and the sys module would suffice? However, consider the next point first.
2. I think you should drop the IronPython and Jython fixes from this patch. They make the change much more complicated and we presently have no chance of testing them, as we lack Jython and IronPython builders. _isCPython is a lot simpler if it doesn't know about them.
3. _isCPython and hasEpoll should have docstrings, of course.
4. To return to testing, I suggest moving _isCPython and hasEpoll into twisted.python._dist where they can be imported properly (and so unit tested properly). While you're doing that, you should discover that hasEpoll has a bug.
5. It might be cool to have a CPythonExtension subclass of Extension which takes care of adding in the _isCPython check, on top of whatever other conditional it is given. On the other hand, trying to create cool distutils-based code is sort of like trying to air condition hell. Regardless, the approach you've already taken is sufficient for this ticket, so don't worry about it for now, but file another ticket for refactoring this code if you like.
6. I haven't tried this on PyPy yet, so I'm assuming that you did and it worked. :) I notice that twisted/runner/ still builds its rpc extension, even if not on CPython. I guess that extension works on PyPy?

Please re-submit for review once you've addressed these points. Also, perhaps talk to me then about adding a build_ext step to our PyPy builder so we can see what happens there.

Thanks again!

[exarkun]

Hm. One other thought. twisted.python.runtime might actually be the best place for this logic. We can always move a private implementation of this detection from twisted.python.dist to a public interface in twisted.python.runtime later, though.

[lvh]

1. Done
2. Done
3. Done
4. Done & fixed
5. Yeah, did the easy thing now, different ticket
6. Yes

All new stuff is private because of your second comment and point 5.

[exarkun]

Probably looks good. I'd like to see the results on buildbot before suggesting to merge, though.

[lvh]

(In [32360]) Branching to 'pypy-5158'

[lvh]

 http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/pypy-5158

Looks like only some unrelated Windows troubles to me. So, do we do the build_ext thing now?

[exarkun]

Here's the result of a build_ext step on PyPy:  http://buildbot.twistedmatrix.com/builders/lucid32-pypy/builds/371/steps/shell_2/logs/stdio

[lvh]

Okay, recap of IRC discussion:

Problem: this fails because I have a PyPy with cpyext. Only some of the C extensions fail to build on PyPy. This one (portmap) doesn't, iff you can compile C extensions at all (it's mostly just the Cython cexts that don't work).

Solution: either we only support cpyext enabled PyPys (this is similar to how CPython works now, you need Python.h and a C compiler available as well), or we check if we're on cpyext-enabled PyPy and only compile then. I think the latter is preferable but more work. Input welcome.

[lvh]

Just to clarify: This fails on the buildslave but not on my machine because I have a cpyext-enabled Pypy, and the buildslave doesn't.

[exarkun]

The buildslave now has PyPy with cpyext. The branch succeeded a "build_ext" step on that builder:  http://buildbot.twistedmatrix.com/builders/lucid32-pypy/builds/401

[exarkun]

(In [32636]) docstring adjustments

banish "should" and "correctly" from your vocabulary

refs #5158

[exarkun]

(In [32639]) Merge pypy-5158

Author: lvh Reviewer: exarkun Fixes: #5158

Disable CPython extension modules which fail to build on PyPy, when building on PyPy.

[jell]

Maybe that would be better solution for this bug.
With attached patch twisted cython extension mimics cpython >= 2.5 api, and twisted/internet/epollreactor is adapted to this api.
But it's needed only under cpython < 2.5

Is python 2.4 still supported by twisted? If not - this cython extension should now be deprecated or even removed.

[exarkun]

Thanks for your interest in this. However, please don't re-open tickets (see ReviewProcess for details of how we use the issue tracker).

The attached patch looks like it duplicates a lot of the work in #3114.

[jell]

fixed patch with some missing parts