Created attachment 227216 [details]
Test which raises an exception (do not commit)

This patch shows the overridden function that is assigned into the introspection module is never reached. Using this with make check, the exception is never raised.

I'm wrong about this. It turns out overridden functions only occur three times in all the current overrides and none of them are tested. Furthermore, they are not added to the __all__ table in overrides which is probably why the clobbering exists in the first place.

Gtk.main_quit
Gtk.stock_lookup
Gdk.color_parse

The system should refrain from clobbering the original method in the introspection module if possible. I will look at adding overridden functions to the registry used by types.

The only remaining one is now Gtk.main_quit, the other two already disappeared with 
http://git.gnome.org/browse/pygobject/commit/?id=4f6ebcfe060.

http://git.gnome.org/browse/pygobject/commit/?id=1edc4ba adds tests for these three functions, and http://git.gnome.org/browse/pygobject/commit/?id=1dc2bc drops overridefunc.

So what I was complaining about is we clobber the original introspection modules version of the function, not add it to the override module. So instead of:

Gtk.stock_lookup = strip_boolean_result(Gtk.stock_lookup)

We would have:

stock_lookup = strip_boolean_result(Gtk.stock_lookup)
__all__.append('stock_lookup')

This leaves the original module unaltered and the overrides module as layer on top without side-affects.

We might consider making sure we don't break compatibility in the case of overrides outside of PyGObject. So if someone is using "override" to decorate a function it shouldn't break, this could be achieved by just returning the original arg if it is a function:

def override(type_):
     '''Decorator for registering an override'''
     if isinstance(type_, types.FunctionType):
         return type_
     ...

(In reply to comment #4)
> So what I was complaining about is we clobber the original introspection
> modules version of the function, not add it to the override module.

Oh sorry, I see what you mean. Fixed in

http://git.gnome.org/browse/pygobject/commit/?id=25a6f9

(In reply to comment #5)
> We might consider making sure we don't break compatibility in the case of
> overrides outside of PyGObject. So if someone is using "override" to decorate a
> function it shouldn't break, this could be achieved by just returning the
> original arg if it is a function:
> 
> def override(type_):
>      '''Decorator for registering an override'''
>      if isinstance(type_, types.FunctionType):
>          return type_
>      ...

I tried that, but with that the overridden function is not recognized at all (see e. g. test_overrides_gtk.TestGtk.test_gtk_main). Presumably they need to be added to __all__, but that's not what e. g. the current GEdit plugins do. This caused bug 695199.

So I reverted http://git.gnome.org/browse/pygobject/commit/?id=1dc2bc for now, and reopen this. If we want to simplify this, we need to find a way to do the above but automatically add the newly defined function to __all__ (or do the equivalent).

Created attachment 348435 [details] [review]
Remove gi.overrides.overridefunc

Move the code into override()

----

With the new override import code it no longer clobbers the introspection module since sys.modules contains a proxy module now. But moving it into override() can't hurt I guess.