After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 633347 - Cannot add GtkHBox to a GtkButton
Cannot add GtkHBox to a GtkButton
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 631631 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-10-28 13:21 UTC by Holger Berndt
Modified: 2010-11-07 18:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add a overrides registry so we can refrence overrides inside the module (5.80 KB, patch)
2010-11-04 16:06 UTC, johnp
none Details | Review
Add tests (1.69 KB, patch)
2010-11-04 18:46 UTC, Sebastian Pölsterl
none Details | Review

Description Holger Berndt 2010-10-28 13:21:02 UTC
GtkButton seems to do too harsh type checks when acting as a container. The code

Gtk.Button().add(Gtk.HBox())

throws:
TypeError: argument 1: Must be Gtk.Widget, not HBox
Comment 1 Sebastian Pölsterl 2010-10-31 11:13:43 UTC
Thanks for the bug report. This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find.

*** This bug has been marked as a duplicate of bug 631631 ***
Comment 2 johnp 2010-11-02 15:48:10 UTC
This works fine for me, even without the patch in bug 631631.  Can you check if the latest pygobject in git works for you?
Comment 3 Holger Berndt 2010-11-02 20:46:53 UTC
I just tried in an interactive python console (in a jhbuild shell) with latest pygobject from git via jhbuild.

[jhbuild] hb@host:~$ python
Python 2.6.6 (r266:84292, Sep 15 2010, 15:52:39) 
[GCC 4.4.5] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pygtk
>>> pygtk.require("2.0")
>>> from gi.repository import Gtk
>>> Gtk.Button().add(Gtk.Label())
Traceback (most recent call last):
  • File "<stdin>", line 1 in <module>
  • File "/opt/gnome3/lib/python2.6/site-packages/gtk-2.0/gi/types.py", line 40 in function
    return info.invoke(*args)
TypeError: instance: Must be Gtk.Container, not Button
>>>
Comment 4 johnp 2010-11-03 13:52:56 UTC
Ok, so I see the issue.  I'm not sure why my instance isn't erroring out.  In any case we override both Widget and Container so all of out other overrides must inherit from them. I just committed a fix.  See if that works for you.
Comment 5 Sebastian Pölsterl 2010-11-03 17:01:33 UTC
While trying to come up with a unittest to check inheritance I encountered a problem:

>>> from gi.repository import Gtk
>>> ali = Gtk.Alignment()
>>> ali.get_name()
Traceback (most recent call last):
  • File "<stdin>", line 1 in <module>
  • File "gi/types.py", line 40 in function
    return info.invoke(*args)
TypeError: instance: Must be Gtk.Widget, not Alignment

We do not override Alignment, but Widget, of course. In the patch you mentioned you manually inherited from the overwritten classes, does this mean we have to overwrite every class that inherits from Widget to get this right? This of course not only applies to Widget but all overrides.
Comment 6 johnp 2010-11-03 18:47:11 UTC
I was under the impression that if there is an override when constructing the wrapper we automatically inherit from the override
Comment 7 johnp 2010-11-03 20:35:46 UTC
So the interesting thing is we are inheriting the methods from the overridden object but we aren't an instance of that object.  I'm not sure how we remedy it.  My patch doesn't help in this code path
Comment 8 johnp 2010-11-03 21:56:48 UTC
Narrowed it down to a race condition.  Basically the overrides module doesn't exist until it is fully imported.  Inside the overrides module we are instantiating and caching each base type in the class hierarchy for any class that is overridden.  In the case of GtkAlignment, it inherits from GtkBin which in turn inherits from GtkContainer, etc.  When GtkButton is overridden in the overrides it too inherits from GtkBin, so it sets up the GtkBin wrapper class which in turn inherits from GtkContainer.  If this was done outside the overrides, GtkBin would find Gtk.overrides.Container and everything would be right in the world.

Since it is done inside the overrides, GtkBin is now cached with a base of GtkContainer.  When GtkAlignment is instantiated it looks up its base which happens to be GtkBin.  This is where we lose.  Since GtkBin is cached with the wrong base hierarchy we end up not inheriting from Gtk.overrides.Container (or Widget for that matter).
Comment 9 johnp 2010-11-03 22:17:51 UTC
to simplify the explanation, it is a reentrancy error.  We are constructing a class in the overrides which jumps out to modules.py which in turn tries to reenter the overrides module to check if there are any overrides.  Since the override module isn't fully loaded, module.py assumes there are no overrides and proceeds to construct the actual Gtk class instead.  I'm not quite sure how to fix this issue.
Comment 10 johnp 2010-11-04 16:06:30 UTC
Created attachment 173830 [details] [review]
Add a overrides registry so we can refrence overrides inside the module

* Overrides have a reentrancy issue when doing inheritance.  If an override
  inherits from another override down the stack it won't see the override
  because the module is not finished loading and will inherit from the
  non-overriden object instead.  This causes type errors later.
* By adding the overrides to a registry outside of the module we can order
  registration and make the override available as soon as the class is parsed,
  not when the whole module is parsed.
Comment 11 johnp 2010-11-04 16:08:47 UTC
This patch is just a first pass.  It still needs tests and it breaks the dialogs tests for some reason
Comment 12 johnp 2010-11-04 16:17:03 UTC
It breaks dialogs because I was relying on the broken inheritance behavior to work around a name clash in the various constructors.  Namely that Gtk.Dialog overloads the buttons keyword to take a list of buttons while some of the Gtk.Dialog sub classes send in an Enum (which is the original behavior).  So in otherwords the dialogs got broken in a good way.  It means this patch is working correctly.
Comment 13 Sebastian Pölsterl 2010-11-04 18:46:19 UTC
Created attachment 173841 [details] [review]
Add tests

This patch adds the test I was talking about in comment 5. I admit it's kind of dirty, but I couldn't find a better way to get all super classes. The test currently stays in the Gtk namespace to avoid name clashes such as GActionGroup and GtkActionGroup.
Comment 14 johnp 2010-11-04 20:22:31 UTC

*** This bug has been marked as a duplicate of bug 631631 ***
Comment 15 johnp 2010-11-04 20:23:08 UTC
Opps, Wrong way around
Comment 16 johnp 2010-11-04 20:25:10 UTC
*** Bug 631631 has been marked as a duplicate of this bug. ***
Comment 17 Holger Berndt 2010-11-05 20:57:56 UTC
Since IRC discussion and debugging turned out extremely weird results on my setup, I now did a fresh jhbuild checkout and build, and don't have this problem anymore with current git master (no additional patches applied, neither from this bug nor from the duplicate). I suspect I had some funky version mismatch of gtk2/gtk3 mixtures.

Nice to know that you still found an issue (so I'm leaving the bug open), and that I didn't just send you on useless missions of ghost hunting. Thanks for your patient support.
Comment 18 johnp 2010-11-05 21:17:38 UTC
Ya it was great.  I need to remind people that if they bug me enough I spend more time on the issue :)  Really if someone show interest enough to engage me I end up really wanting to solve their issue so feel free to hit us up again if you run into other problems.  

As it is this patch is still not complete.  The test case brought up a ref counting issue with the base_info's so I need to fix that and hopefully I can get a release out this weekend during the GNOME Summit.
Comment 19 johnp 2010-11-05 21:20:08 UTC
Also I will eventually move to getting all overrides registered, not just GObject overrides.  This will allow us to remove most of the funky calls to __import__.
Comment 20 johnp 2010-11-07 18:14:16 UTC
Ref counting error went away in a gobject-introspection HEAD