Support gi.pygtkcompat in gi reactor

[itamar]

Newer versions of gi provide a compatibility layer for the older pygtk API. Currently, if pygtk has been loaded we don't allow loading gi reactor, since this is incompatible and cause segfaults. Likewise, once gi reactor is loaded we prevent imports of pygtk modules like gobject. However, if gi.pygtkcompat is available, we should instead:

1. If we can figure out that pygtkcompat was enabled, don't prevent import of gi reactor even if we see pygtk loaded, since we know it's not actually the real pygtk.
2. Enable the compatibility layer instead of preventing import of gobject and friends.

[itamarst]

(In [34392]) Branching to 'pygtkcompat-5676'

[itamar]

OK, ready for review. It'd be nice to get this into 12.1. Buildbot is in progress, I'll check status in a bit:

​http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/pygtkcompat-5676

[jesstess]

Thanks for working on this, itamar. Your changes look good; a couple of comments:

• Using both this branch and the 12.1.0 release, this is what happens when I try to import gireactor or gtk3reactor:

$ python -c "import twisted; print twisted.__version__; from twisted.internet import gtk3reactor"
12.0.0+r34756
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "twisted/internet/gtk3reactor.py", line 22, in <module>
    from twisted.internet import gireactor
  File "twisted/internet/gireactor.py", line 53, in <module>
    _oldGiInit()
  File "twisted/internet/gireactor.py", line 42, in _oldGiInit
    GLib.threads_init()
  File "/usr/lib/python2.7/dist-packages/gi/module.py", line 268, in __getattr__
    return getattr(self._introspection_module, name)
  File "/usr/lib/python2.7/dist-packages/gi/module.py", line 101, in __getattr__
    self.__name__, name))
AttributeError: 'gi.repository.GLib' object has no attribute 'threads_init'
$ python -c "import twisted; print twisted.__version__; from twisted.internet import gireactor"
12.0.0+r34756
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "twisted/internet/gireactor.py", line 53, in <module>
    _oldGiInit()
  File "twisted/internet/gireactor.py", line 42, in _oldGiInit
    GLib.threads_init()
  File "/usr/lib/python2.7/dist-packages/gi/module.py", line 268, in __getattr__
    return getattr(self._introspection_module, name)
  File "/usr/lib/python2.7/dist-packages/gi/module.py", line 101, in __getattr__
    self.__name__, name))
AttributeError: 'gi.repository.GLib' object has no attribute 'threads_init'

The tests know to skip when they don't have the dependencies to support these reactors (​http://twistedmatrix.com/trac/ticket/5494), but this is a pretty bad experience for users. Can you open a ticket for addressing this and printing out a nice error message instead of throwing a traceback?

• We now have this list of glib/gtk libraries ("gobject", "glib", "gio", "gtk") that we do various tests on in 3 places, twice in gireactor.py and once in test_gireactor.py. Is it time to put these in a variable, to avoid the risk of accidentally updating some instances and not others?

• The patch has created an unused import:

  $ pyflakes twisted/internet/gireactor.py
  twisted/internet/gireactor.py:21: 'sys' imported but unused
  

• We need a build slave to test these configurations. Can you open a task ticket to track this and hopefully incentivize someone to do it (perhaps compelled by the high scores list :) )

Once these things have been addressed, I think this is ready to merge.

[itamar]

• I've created a new ticket, #5790.
• Done.
• Done.

I'm concerned about requiring build slave; on the one hand the need is obvious, on the other hand it can keep us from deploying fixes for code we've already released, and it may take a while until one materializes. Hopefully not too long, since we've been offered a bunch of VMs recently.

If I got #5790 fixed would you be willing to rely on tests passing on my machine and yours?

[jesstess]

I didn't mean to imply that we needed a build slave to merge this -- I just want us to at least track the desire to have a build slave configured to be able to exercise these tests.

So yes, with the resolution of #5790 I'd be happy to consider this ready to merge.

[itamar]

I've included a fix for #5790; please let me know if this solves the problem for you on your machine (I couldn't think of a reasonable way to test this, unfortunately.)

[exarkun]

You are aware, of course, that no Python program should ever use hasattr, as it is a broken interface which hides arbitrary exceptions by design. getattr(obj, name, sentinel) is not sentinel is the safe way to spell this check (as is catching the AttributeError from the attribute lookup (not from the call)).

[itamarst]

(In [35160]) Use getattr() instead of hasattr(). Refs #5676

[jesstess]

Thanks for seeing this through, itamar; this fixes the issue on my machine, and ​build results look good. I think this is ready to merge!

[itamar]

Now I'm wondering if we should be enabling gi.pygktcompat at all. Possibly just checking for its existence and then relying on gi and the old python wrappers to interact correctly on their own may be sufficient, and is less side-effecty.

[itamar]

Ah yes, I decided to enable it because if you disable it you get this:

$ trial -r gi twisted
/usr/lib/python2.7/dist-packages/gobject/constants.py:24: Warning: g_boxed_type_register_static: assertion `g_type_from_name (name) == 0' failed
  import gobject._gobject
/usr/lib/python2.7/dist-packages/gtk-2.0/gtk/__init__.py:40: Warning: specified class size for type `PyGtkGenericCellRenderer' is smaller than the parent type's `GtkCellRenderer' class size
  from gtk import _gtk
/usr/lib/python2.7/dist-packages/gtk-2.0/gtk/__init__.py:40: Warning: g_type_get_qdata: assertion `node != NULL' failed
  from gtk import _gtk
Segmentation fault (core dumped)

[itamarst]

(In [35285]) Merge pygtkcompat-5676: Better support for newer and older gi versions.

Author: itamar Review: jesstess Fixes: #5676, #5790

Work with old versions of gi that didn't wrap enable_threads(), and with newer versions that have the gi.pygtkcompat compatability layer.