signal module shouldn't be imported unless it's used

[nriley]

I'm working on getting Twisted on Jython. Twisted already makes this conditional on runtime.platformType == 'posix'. Jython doesn't yet have a signal module, so this patch makes the import conditional as well.

[nriley]

patch

[glyph]

In terms of a long-term solution, wouldn't wrapping sun.misc.SignalHandler for a simple 'signal' module be almost as easy?

For real inclusion, this change would need some kind of tests (or at least a Jython buildbot that we could start testing bootstrap / import issues like this on).

[nriley]

Replying to glyph:

In terms of a long-term solution, wouldn't wrapping sun.misc.SignalHandler for a simple 'signal' module be almost as easy?

sun.misc.* is not portable; however we're working on wrapping "jna-posix", a library used by JRuby, to implement the bits of Python os, signal, subprocess, etc. that we don't currently. (This is a potential SoC project for this year).

For real inclusion, this change would need some kind of tests (or at least a Jython buildbot that we could start testing bootstrap / import issues like this on).

Uhh. I'm moving an import from one place to another in a file where the usage is conditional anyway; I'm not sure what there is to test.

That said, we are working on getting a machine to use for Jython testing, on which we could likely host a Twisted buildbot. We'll let you know when it's up.

[glyph]

Replying to nriley:

Replying to glyph: sun.misc.* is not portable; however we're working on wrapping "jna-posix", a library used by JRuby, to implement the bits of Python os, signal, subprocess, etc. that we don't currently. (This is a potential SoC project for this year).

Given that jython could dynamically import sun.misc, it seems like a reasonable temporary workaround, but I knew it was

For real inclusion, this change would need some kind of tests (or at least a Jython buildbot that we could start testing bootstrap / import issues like this on).

Uhh. I'm moving an import from one place to another in a file where the usage is conditional anyway; I'm not sure what there is to test.

Normally I'd say any change needs to have unit tests; the reason I specifically included "a buildbot" as an alternative in this case is because issues like this that completely prevent the unit tests from bootstrapping on a particular platform can obviously be counted as a failure, assuming that people know about them ;).

Without that kind of facility available to our core developers, the chances of some future change breaking this fix are very high. (Way, way higher than you'd normally think in a library like Twisted with all these fiddly platform-specific bits - and likely to get higher now that both Jython and IronPython hackers are interested in stretching our portability to even more ridiculous heights).

That said, we are working on getting a machine to use for Jython testing, on which we could likely host a Twisted buildbot. We'll let you know when it's up.

This is really great news. For best results, drop a message to the mailing list when this is available so that one of our buildbot admins finds out about it and give you the appropriate info to get it connected to our buildmaster.

[thijs]

There's now a Jython buildbot available at  http://buildbot.twistedmatrix.com/builders/ubuntu64-jython2.5-select

Unfortunately it's not doing much at the moment because it's blocked by  this jython ticket. I'll check the patch into a branch when that issue(s) is resolved.

[thijs]

(In [24689]) Branching to 'signal-module-3099'

[thijs]

Replying to thijs:

Unfortunately it's not doing much at the moment because it's blocked by  this jython ticket. I'll check the patch into a branch when that issue(s) is resolved.

That ticket was fixed in r5290 of Jython so the buildbot is now working. I applied the patch in r24688, and adjusted setup.py in r24688, but the build  fails:

Traceback (most recent call last):
  File "./bin/trial", line 23, in <module>
    from twisted.scripts.trial import run
  File "/home/buildbot/slaves/twisted-jython/jython2.5-thijs-ubuntu/Twisted/twisted/scripts/trial.py", line 9, in <module>
    from twisted.internet import defer
  File "/home/buildbot/slaves/twisted-jython/jython2.5-thijs-ubuntu/Twisted/twisted/internet/defer.py", line 17, in <module>
    from twisted.python import log, failure, lockfile
  File "/home/buildbot/slaves/twisted-jython/jython2.5-thijs-ubuntu/Twisted/twisted/python/lockfile.py", line 28, in <module>
    from win32api import OpenProcess
ImportError: No module named win32api

[exarkun]

There's a ton more signal imports throughout the codebase. Maybe more than one thing should change in this branch?

On the other hand, it's pretty hard to see what effect any particular fix is having until the test suite can run. Maybe there should be a ticket for all the fixes required to make trial twisted actually produce a summary?

[thijs]

Replying to exarkun:

Maybe there should be a ticket for all the fixes required to make trial twisted actually produce a summary?

Looks like #3413 was opened for that.

[thijs]

Closing this ticket now that there's  initial support for signal in Jython. The buildslave doesn't fail anymore with the win32api ImportError we saw before.