GNOME Bugzilla – Bug 622924
Migrate from dbus-glib to glib's GDBus
Last modified: 2015-01-05 18:43:05 UTC
For GLib 2.25.5 GDBus D-Bus support was merged, providing an API to replace dbus-glib. See http://library.gnome.org/devel/gio/unstable/gdbus.html and http://library.gnome.org/devel/gio/unstable/ch28.html . According to a quick grep this module seems to use dbus-glib: ./gnome-session/configure.in: dbus-glib-1 >= $DBUS_GLIB_REQUIRED) ./gnome-session/configure.in:PKG_CHECK_MODULES(DBUS_GLIB, dbus-glib-1 >= $DBUS_GLIB_REQUIRED)
Created attachment 289358 [details] [review] gsm-consolekit: fix GCC warnings
Created attachment 289359 [details] [review] build: remove old EggSMClient leftover
Created attachment 289360 [details] [review] gsm-manager: port to GDBus
Created attachment 289361 [details] [review] gsm-client: port to GDBus
Created attachment 289362 [details] [review] gsm-dbus-client: port to GDBus
Created attachment 289363 [details] [review] gsm-app: port to GDBus
Created attachment 289364 [details] [review] gsm-inhibitor: port to GDBus
Created attachment 289365 [details] [review] gsm-presence: port to GDBus
Created attachment 289366 [details] [review] gsm-shell: port to GDBus
Created attachment 289367 [details] [review] gsm-autostart-app: port to GDBus
Created attachment 289368 [details] [review] gsm-util: port to GDBus
Created attachment 289369 [details] [review] tests: port to GDBus
Created attachment 289370 [details] [review] gnome-session-quit: port to GDBus
Created attachment 289371 [details] [review] gsm-system: allow disabling ConsoleKit backend This allows a dbus-glib-free compilation of gnome-session when the ConsoleKit backend is not in use.
Created attachment 289372 [details] [review] build: don't depend anymore on dbus-glib All the code with the exception of the ConsoleKit helper is now ported to GDBus.
I worked on this patchset during the weekend, which ports gnome-session to GDBus. I have tested it quite a bit without noticeable regressions so far.
Created attachment 289373 [details] [review] tests: port to GDBus
Review of attachment 289358 [details] [review]: sure
Review of attachment 289359 [details] [review]: yes
Review of attachment 289371 [details] [review]: makes sense
Review of attachment 289370 [details] [review]: looks good
Review of attachment 289372 [details] [review]: yay
Review of attachment 289373 [details] [review]: This patch need to go before the one that drops dbus-glib. ::: gnome-session/test-client-dbus.c @@ +89,3 @@ + reason), + G_DBUS_CALL_FLAGS_NONE, + -1, NULL, &error); Does this leak the GVariant returned by g_dbus_proxy_call_sync ? @@ +166,3 @@ } + g_variant_get (object_path_variant, "(o)", &client_id); leaking object_path_variant here ? @@ +205,3 @@ + g_variant_new ("(o)", client_id), + G_DBUS_CALL_FLAGS_NONE, + -1, NULL, &error); Here too: leaked return value ::: gnome-session/test-inhibit.c @@ +133,3 @@ + g_variant_new ("(u)", cookie), + G_DBUS_CALL_FLAGS_NONE, + -1, NULL, &error); Here too: leaked return value.
Attachment 289358 [details] pushed as 42111b2 - gsm-consolekit: fix GCC warnings Attachment 289359 [details] pushed as 4418042 - build: remove old EggSMClient leftover
Review of attachment 289371 [details] [review]: ::: gnome-session/Makefile.am @@ +64,2 @@ gnome_session_SOURCES += gsm-consolekit.c gsm-consolekit.h +endif i think either these files need to get added to EXTRA_DIST if not HAVE_CONSOLEKIT or DISTCHECK_CONFIGURE_FLAGS needs to force consolekit or these files will drop out of the tarball at make dist time right?
Review of attachment 289360 [details] [review]: cool ::: gnome-session/Makefile.am @@ +131,3 @@ + --generate-c-code gsm-manager-generated \ + --c-namespace Gsm \ + $(srcdir)/org.gnome.SessionManager.xml Note a huge deal, but I like adding e.g., --annotate "org.gnome.SessionManager" "org.gtk.GDBus.C.Name" Manager to the codegen lines to make the generated code e.g. gsm_manager_emit_client_added instead of gsm_org_gnome_session_manager_emit_client_added Also, it's totally a subjective style thing, but i like naming the generated C files after the xml file... so --generate-c-code org.gnome.SessionManager Of course, we used -glue suffix before, so do whatever you want. ::: gnome-session/gsm-manager.c @@ -174,3 @@ - PROP_INHIBITED_ACTIONS, - PROP_SESSION_NAME, - PROP_SESSION_IS_ACTIVE, I think if you kept these as properties you could just us g_bind_property to tie them to the proxy and not call the setters manually @@ -2338,3 @@ - - dbus_connection_send (connection, message, NULL); - dbus_message_unref (message); nice chunk of removed code here
Pushed with minor changes. Attachment 289360 [details] pushed as a7c3b02 - gsm-manager: port to GDBus Attachment 289361 [details] pushed as 8ce2bbb - gsm-client: port to GDBus Attachment 289362 [details] pushed as 96ce81a - gsm-dbus-client: port to GDBus Attachment 289363 [details] pushed as dfd9078 - gsm-app: port to GDBus Attachment 289364 [details] pushed as f91274d - gsm-inhibitor: port to GDBus Attachment 289365 [details] pushed as 45299eb - gsm-presence: port to GDBus Attachment 289366 [details] pushed as 687e693 - gsm-shell: port to GDBus Attachment 289367 [details] pushed as ff0eae6 - gsm-autostart-app: port to GDBus Attachment 289368 [details] pushed as 1a10239 - gsm-util: port to GDBus Attachment 289370 [details] pushed as 083a4a4 - gnome-session-quit: port to GDBus Attachment 289371 [details] pushed as d638df1 - gsm-system: allow disabling ConsoleKit backend Attachment 289372 [details] pushed as 24ba982 - build: don't depend anymore on dbus-glib Attachment 289373 [details] pushed as b47a3a3 - tests: port to GDBus I'm leaving the bug open for a bit because I want to go a little further and use GDBusObjectManager etc.
i changed my mind. i'll close this now, and we can do a new bug if we ever add the objectmanager bits...doesn't seem super important right now.
Created attachment 293867 [details] [review] manager: add back session-name property It got stripped out in the gdbus port, but it's used internally. Fixes memory corruption crasher
Comment on attachment 293867 [details] [review] manager: add back session-name property Attachment 293867 [details] pushed as 837a0bf - manager: add back session-name property