elementary / granite

Library that extends GTK with common widgets and utilities
https://elementary.io
GNU Lesser General Public License v3.0
280 stars 61 forks source link

Python bindings issues #241

Open ivankra opened 5 years ago

ivankra commented 5 years ago

I'm trying to use libgranite from python via gir bindings and running into a couple of issues with them.

First, .gir file that's produced during the build is missing a shared-library= attribute, as a result you can't instantiate any Granite.* classes:

$ python3 -c 'from gi.repository import Granite; Granite.WidgetsSourceList()'
...
TypeError: could not get a reference to type class

I reported this at first to Debian (bug, patch) as I thought it was distro problem, but the culprit seems to build scripts in this repository and seems to also affect Elementary OS. Please consider cherrypicking their patch.

Second issue I ran into is that overriding get_context_menu virtual method for Granite.Widgets.SourceList.Item doesn't work, and as a result I can't create a custom menu for SourceList:

#!/usr/bin/env python3
import gi
gi.require_version('Gtk', '3.0')
gi.require_version('Granite', '1.0')
from gi.repository import Granite, Gtk

class Item(Granite.WidgetsSourceListItem):
    def __init__(self):
        super().__init__(name='item')

    def do_get_context_menu(self):
        print('do_get_context_menu called')
        return Gtk.Menu()

item = Item()
print(item.get_context_menu())

Output: None, and my virtual method override (with do_ prefix) never gets called.

Not sure if the bug is in granite or vala or (py-)gobject. Do you have any ideas how make it work from python?

jmc-88 commented 5 years ago

The Debian patch added this flag to the CMake build, which has been since removed in favour of Meson, and that does have the --shared-library flag as expected: https://github.com/elementary/granite/blob/f1b29f52e3aaf0f5d6bba44c42617da265f679c8/lib/meson.build#L103-L104

So, the first reported issue should be fixed by now and just needs a new release. Building it locally from head and running the following completes without issue:

$ python3 -c 'import gi; gi.require_version("Granite", "1.0"); from gi.repository import Granite; Granite.WidgetsSourceList()'

As for the second part of the report, I don't understand the need for the do_ prefix. Overriding the get_context_menu() method works as expected.

ivankra commented 5 years ago

Still broken, both on debian buster and elementary:

$ sudo apt install gir1.2-granite-1.0
$ python3 -c 'from gi.repository import Granite; Granite.WidgetsSourceList()'
-c:1: PyGIWarning: Granite was imported without specifying a version first. Use gi.require_version('Granite', '1.0') before import to ensure that the right version gets loaded.

** (-c:4565): WARNING **: 06:40:30.433: Failed to load shared library '/build/granite-w6PYSo/granite-5.2.4+r1403+pkg108~ubuntu5.0.1/obj-x86_64-linux-gnu/lib/libgranite.so.5.2.4' referenced by the typelib: /build/granite-w6PYSo/granite-5.2.4+r1403+pkg108~ubuntu5.0.1/obj-x86_64-linux-gnu/lib/libgranite.so.5.2.4: cannot open shared object file: No such file or directory
/usr/lib/python3/dist-packages/gi/overrides/__init__.py:326: Warning: cannot retrieve class for invalid (unclassed) type 'void'
  return super_init_func(self, **new_kwargs)
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/gi/overrides/__init__.py", line 326, in new_init
    return super_init_func(self, **new_kwargs)
TypeError: could not get a reference to type class

libgranite version in repositories (5.2.4+r1403+pkg108~ubuntu5.0.1) is more recent than the changes to meson.build that you mention, so I don't think a new release will fix it.

Looks like the bindings are now getting built with shared-library=/build/... path or something like that, which doesn't of course exist outside build machines.

For the second part: if you mean to fix it by changing def do_get_context_menu to def get_context_menu, well, this would only work for python callers, but the call to get_context_menu method that I care about in order to specify a custom menu happens in libgranite library and it's vala, not python.

do_ prefix is py-gobject's convention that tells that I'm supplying an implementation of a virtual method with the name specified after the prefix. gobject is supposed to hook up some stub to the object's vtable, that will call out into python whenever something (python code in above example, but vala in prod) calls that virtual method. For some reason, this doesn't work as it's supposed to with libgranite's python bindings.

jmc-88 commented 5 years ago

Ah, I see, the first part of the issue comes from the fact that the --shared-library refers to the local full path at build time, which doesn't correspond to the one that is found in the installed system. I was trying this on a local build without going through the Debian package, so that's why they were the same in my test.

jmc-88 commented 5 years ago

It's worse: the referenced library path was the one in my build directory, i.e. in my home. So it's wrong in both local and upstream builds.

I think that can be fixed by only specifying the shared library name without its full path, letting it be dynamically resolved to its correct path on install. I'll send a PR out.

Before changing it:

$ g-ir-inspect --print-shlibs Granite
shlib: /home/jmc/Projects/elementary/granite/build/lib/libgranite.so.5.2.4

after changing it:

$ g-ir-inspect --print-shlibs Granite
shlib: libgranite.so.5.2.4

$ python3 -c 'import gi; gi.require_version("Granite", "1.0"); from gi.repository import Granite; Granite.WidgetsSourceList()'
# still no errors logged
jmc-88 commented 4 years ago

@ivankra I tried your command from https://github.com/elementary/granite/issues/241#issuecomment-508971447 and it now works for me:

$ python3 -c "import gi; gi.require_version('Granite', '1.0'); from gi.repository import Granite; Granite.WidgetsSourceList()"
# no output

Can you confirm whether that's the same on your end?

ivankra commented 4 years ago

Yes, works for me now on debian with libgranite 5.2.5-1. Thanks for fixing!

The second issue I mentioned - not being able to override gobject virtual method get_context_menu from python - still seems to broken.

jmc-88 commented 4 years ago

Another data point: I've extracted a minimal test case with a Vala class and a virtual method, and it works as expected. Same when I move the get_context_menu() method from Granite.WidgetsSourceList.Item to the parent Granite.WidgetsSourceList and subclass that instead.

In short, I think this may an upstream issue in gobject-introspection's handling of virtual methods for nested classes, for some reason.

jmc-88 commented 4 years ago

Actually, scratch that. There was an issue with the test case: now it works, and I can only reproduce the failure when using Granite.

jmc-88 commented 4 years ago

Note: I didn't test get_context_menu() again, but I did separately experience issues with the behavior of vfunc overrides in PyGObject which may be related (see https://gitlab.gnome.org/GNOME/pygobject/-/issues/367).