Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Dbus wrapper, and use glib/gio GDbus via introspection, remove the dbus-python dependency #277

Open
diegogangl opened this issue Apr 8, 2020 · 10 comments · May be fixed by #484
Open
Labels
enhancement low-hanging-fruit "Easy picks" suitable for new contributors to tackle packaging Flatpak packages (anything else = NOPE.png) priority:low

Comments

@diegogangl
Copy link
Contributor

GTG used to expose some of its functionality to dbus using some custom code in the dbus wrapper class (dbus.py). Nowadays, GtkApplication registers itself with the same name as the app id which conflicts with the old way, since both end up with the same bus name.

My " solution " was to change the well known name in the dbus wrapper. So now GTG has two dbus interfaces. It would be a shame to throw away the old one since it was nicer and more specific to gtg. A real solution would be to add those functions to the well know name exposed by GtkApplication.

It would be great if someone with dbus knowledge could help out here!

@diegogangl diegogangl added enhancement priority:low low-hanging-fruit "Easy picks" suitable for new contributors to tackle labels Apr 8, 2020
diegogangl added a commit that referenced this issue May 9, 2020
It's a shame to drop this interface, but it's becoming problematic to use with
Flatpak. We should revisit these in the future,  and plug them in
org.gnome.GTG.

ref #277
@nekohayo nekohayo changed the title Fix Dbus wrapper Fix Dbus wrapper, and use glib/gio GDbus via introspection, remove the dbus-python dependency Jul 5, 2020
@nekohayo
Copy link
Member

nekohayo commented Jul 5, 2020

Mart suggested that we should eliminate the dbus-python dependency by using glib/gio GDBus stuff via introspection to achieve the same thing. I guess this should be done as part of this ticket, so renaming the title (in case anyone out there is looking at this ticket and wants to tackle it).

@nekohayo nekohayo added the packaging Flatpak packages (anything else = NOPE.png) label Jul 5, 2020
johnnybubonic pushed a commit to johnnybubonic/gtg that referenced this issue Jul 20, 2020
It's a shame to drop this interface, but it's becoming problematic to use with
Flatpak. We should revisit these in the future,  and plug them in
org.gnome.GTG.

ref getting-things-gnome#277
@diegogangl
Copy link
Contributor Author

I managed to port the timer to gio dbus (which is the only place we use dbus ATM). Unfortunately I can't get it to attach a callback to the proper signal. It seems to be a problem with the system bus, I can attach callbacks to signals in the session bus just fine but when it comes to the system bus it just... doesn't do it. There are no errors or anything.

Haven't had any luck asking in #gtk, #python and Gnome Discourse. Leaving this code here in case someone knows the proper incantation:

        system_bus = Gio.bus_get_sync(Gio.BusType.SYSTEM, None)
        system_bus.signal_subscribe('org.freedesktop.login1',
                                    'org.freedesktop.login1.Manager',
                                    'PrepareForSleep',
                                    '/org/freedesktop/login1',
                                    None,
                                    Gio.DBusSignalFlags.NONE,
                                    on_prepare_for_sleep,
                                    None)

The callback:

def on_prepare_for_sleep(connection, sender_name, object_path,
                         interface_name, signal_name, parameters,
                         *user_data):
    """Handle dbus prepare for sleep signal."""

    print('Called')

@Neui
Copy link
Contributor

Neui commented Sep 25, 2020

@diegogangl
Adding the following code after calling signal_subscribe I receive the signals:

        from gi.repository import GLib
        GLib.MainLoop().run()

However, as you can see, this will stop application flow, in other words GTG won't continue executing.
I've found an Arch forum post where someone also does that. Many other resources uses dbus-python or something. But I've found that D-Feet uses DBus via Gio and connects some signals, maybe I'll look how they do it.

I tried subscribing in the startup and activate signal of the Application object, but no change. I tried to run an main loop in a new python thread (seems you can't create a new thread via GI/GLib), trying to use either the "default" or an new MainContext.

@diegogangl
Copy link
Contributor Author

You are right, this might be the thing. Maybe you need to connect this way to subscribe

            self.connection = Gio.DBusConnection.new_for_address_sync(
                self.__bus_address,
                Gio.DBusConnectionFlags.AUTHENTICATION_CLIENT |
                Gio.DBusConnectionFlags.MESSAGE_BUS_CONNECTION,
                None, None)

@Neui
Copy link
Contributor

Neui commented Sep 28, 2020

I looked into D-Feet a bit, and it seems it doesn't do anything special.
Here's the "stacktrace" when connecting the signal (went through the code by hand):

I tried subscribing inside GTGs MainWindow.__init__, but no success.
Note that I haven't tested if the signal D-Feet uses actually works; From the name, it seems it is something that is rather unlikely to happen.

I also tried your code (which is actually from D-Feet), with the bus address being the system bus address Gio returns, but no success either:

        system_address = Gio.dbus_address_get_for_bus_sync(Gio.BusType.SYSTEM, None)
        system_bus = Gio.DBusConnection.new_for_address_sync(system_address,
             Gio.DBusConnectionFlags.AUTHENTICATION_CLIENT | Gio.DBusConnectionFlags.MESSAGE_BUS_CONNECTION,
             None,
             None)

And yes, I made sure that the method gets called.

@diegogangl
Copy link
Contributor Author

Wait, I was wrong. That's not how the connection gets done.

I took it from:
https://gitlab.gnome.org/GNOME/d-feet/-/blob/master/src/dfeet/bus_watch.py#L132

Looking at the if block they actually connect in the same way as the first snippet I posted
I wonder what we are doing differently 🤔

@Neui
Copy link
Contributor

Neui commented Sep 29, 2020

I tried to check if D-Feets signal works and trace it down "until it works". I used the following git diff:

diff --git a/src/dfeet/bus_watch.py b/src/dfeet/bus_watch.py
index a6472ed..fd552cd 100644
--- a/src/dfeet/bus_watch.py
+++ b/src/dfeet/bus_watch.py
@@ -115,6 +115,24 @@ class BusNameBox(Gtk.VBox):
 class BusWatch(object):
     """watch for a given bus"""
     def __init__(self, data_dir, bus_address):
+        # print("Registering for", bus_address)
+        if bus_address == Gio.BusType.SYSTEM:
+            # print("Registering")
+            # Gio.bus_get_sync(Gio.BusType.SYSTEM, None).start_message_processing()
+            # Gio.bus_get_sync(Gio.BusType.SYSTEM, None).flush_sync(None)
+            def test(*args):
+                print("I AM HERE", args)
+            Gio.bus_get_sync(Gio.BusType.SYSTEM, None).signal_subscribe(None, None, None, None, None, 0, test, None)
+            # Gio.bus_get_sync(Gio.BusType.SYSTEM, None).flush_sync(None)
+            def test(*args):
+                print("SECOND", args)
+            Gio.bus_get_sync(Gio.BusType.SYSTEM, None).signal_subscribe(None, None, None, None, None, 0, test, None)
+            def test(*args):
+                print("THIRD", args)
+            Gio.bus_get_sync(Gio.BusType.SYSTEM, None).signal_subscribe(None, None, None, None, None, 0, test, None)
+            def test(*args):
+                print("FOURTH", args)
+            Gio.bus_get_sync(Gio.BusType.SYSTEM, None).signal_subscribe(None, None, None, None, None, 0, test, None)
         self.__data_dir = data_dir
         self.__bus_address = bus_address
         # setup UI
@@ -146,6 +164,12 @@ class BusWatch(object):
         self.connection.signal_subscribe(None, "org.freedesktop.DBus", "NameOwnerChanged",
                                          None, None, 0, self.__name_owner_changed_cb, None)

+        # def test(*args):
+        #     print(self.__bus_address, args)
+
+        # self.connection.signal_subscribe(None, None, None,
+        #                                  None, None, 0, test, None)
+
         # refilter if someone wants to filter the busbox list
         self.__bus_name_filter.connect("changed",
                                        self.__bus_name_filter_changed_cb)
diff --git a/src/dfeet/window.py b/src/dfeet/window.py
index 01b6032..dfa0306 100644
--- a/src/dfeet/window.py
+++ b/src/dfeet/window.py
@@ -126,8 +126,12 @@ class DFeetWindow(Gtk.ApplicationWindow):
     def __action_connect_system_bus_cb(self, action, parameter):
         """connect to system bus"""
         try:
+            # print("connecting to systembus cb")
             if self.system_bus is not None:
                 return
+            # def test(*args):
+            #     print("BEFORE BUS WATCH", args)
+            # Gio.bus_get_sync(Gio.BusType.SYSTEM, None).signal_subscribe(None, None, None, None, None, 0, test, None)
             bw = BusWatch(self.data_dir, Gio.BusType.SYSTEM)
             self.system_bus = bw.box_bus
             self.stack.add_titled(self.system_bus, 'System Bus', 'System Bus')

For some reason, it "randomly" works. I also noticed if I don't start_message_processing + flush (and even then it would just "sometimes" work), the first "working" signal would get an NameAcquired signal, but only once:

SECOND (<Gio.DBusConnection object at 0x7fe3d496e700 (GDBusConnection at 0x12ce4a0)>, 'org.freedesktop.DBus', '/org/freedesktop/DBus', 'org.freedesktop.DBus', 'NameAcquired', GLib.Variant('(s)', (':1.2640',)), None)

Then that and further ones would print other incoming signals.
I guess there is some kind of "race" of an name being acquired or something.
I tried to look if there is some promising sounding method, but I couldn't really find anything.

@diegogangl
Copy link
Contributor Author

I guess there is some kind of "race" of an name being acquired or something.

They are calling it at some point late in their main window, while we are creating the timer in the application constructor. I remember moving it to the end of do_active() but without much luck. Maybe we need to do this later?

@Neui
Copy link
Contributor

Neui commented Oct 2, 2020

I guess there is some kind of "race" of an name being acquired or something.

They are calling it at some point late in their main window, while we are creating the timer in the application constructor. I remember moving it to the end of do_active() but without much luck. Maybe we need to do this later?

Actually, I found it out. See the PR #479, or: D-Feet keeps a reference to the system dbus, which keeps the connection alive. In our case, we don't and thus when it goes out of scope, the connection gets destroyed and the signals get disconnected.
I also noticed that a bit later in D-Feet, it listens for "NameAcquired", and that is likely what I received for (or something).

@Neui Neui linked a pull request Oct 9, 2020 that will close this issue
5 tasks
@khurshid-alam
Copy link

Is there any plan to implement this in next release ? @Neui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement low-hanging-fruit "Easy picks" suitable for new contributors to tackle packaging Flatpak packages (anything else = NOPE.png) priority:low
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants