Bug 48821

Summary: add support for GVariant-based method calls
Product: dbus Reporter: Simon McVittie <smcv>
Component: GLibAssignee: Simon McVittie <smcv>
Status: RESOLVED WONTFIX QA Contact: D-Bus Maintainers <dbus>
Severity: normal    
Priority: medium CC: jonny.lamb, rob.taylor
Version: unspecifiedKeywords: patch
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: [1/3] dbus_g_message_read_variants, dbus_g_message_write_variants: add and test
[2/3] dbus_g_connection_call_async: add and test
[3/3] lcov.am: fix to work in out-of-tree builds
[01/11] dbus_g_message_read_variants, dbus_g_message_write_variants: add and test
[02/11] dbus_g_connection_call_async: add and test
[03/11] lcov.am: fix to work in out-of-tree builds
[04/11] define MY_OBJECT_IFACE for tests to use
[05/11] Factor out a common test_assert_no_error macro
[06/11] test_run_until_round_trip: add
[07/11] Fix building other projects against an uninstalled dbus-glib
[08/11] Add GVariant-based signal subscriptions
[09/11] Add a test for DBusGProxy
[10/11] Add GVariant-based async method replies
[11/11] Add non-GError-based async method errors

Description Simon McVittie 2012-04-17 07:39:24 UTC
Porting telepathy-glib to GDBus is clearly going to be a bit of a nightmare, but we can attack it from multiple angles.

One relatively easy thing we can do is to make some of our method calls using GVariant, rather than DBusGProxy.
Comment 1 Simon McVittie 2012-04-17 07:40:01 UTC
Created attachment 60172 [details] [review]
[1/3] dbus_g_message_read_variants,  dbus_g_message_write_variants: add and test

These will let us send GVariant-based messages over libdbus connections,
to facilitate porting from dbus-glib to GDBus.
Comment 2 Simon McVittie 2012-04-17 07:40:27 UTC
Created attachment 60173 [details] [review]
[2/3] dbus_g_connection_call_async: add and test

This is basically the same API as g_dbus_connection_call_async, but
implemented in terms of libdbus. This means we can use a DBusGConnection
in telepathy-glib to make method calls that look suspiciously like a
GDBus call, reducing the size of the "flag day" commit we'll have to
make when we switch from dbus-glib to GDBus.

To support this, make GIO a public dependency.
Comment 3 Simon McVittie 2012-04-17 07:40:43 UTC
Created attachment 60174 [details] [review]
[3/3] lcov.am: fix to work in out-of-tree builds
Comment 4 Dan Williams 2012-10-22 23:42:19 UTC
Comment on attachment 60172 [details] [review]
[1/3] dbus_g_message_read_variants,  dbus_g_message_write_variants: add and test

Review of attachment 60172 [details] [review]:
-----------------------------------------------------------------

If you don't have DBusBasicValue you're also not going to have DBus8ByteStruct, so we'll need a private copy of that in dbus-gvariant.c for !HAVE_DBUSBASICVALUE.  Other than that, nothing egregious pops out at me, and it passes the testcases.
Comment 5 Dan Williams 2012-10-22 23:55:22 UTC
Comment on attachment 60173 [details] [review]
[2/3] dbus_g_connection_call_async: add and test

Review of attachment 60173 [details] [review]:
-----------------------------------------------------------------

Perhaps I'm missing something huge, but where in dbus_g_connection_call_async() are the GVariant parameters set on the outgoing message?  Return parameters are processed correctly, but it doesn't look like the outgoing message will have any arguments at all?
Comment 6 Dan Williams 2012-10-22 23:56:18 UTC
Comment on attachment 60174 [details] [review]
[3/3] lcov.am: fix to work in out-of-tree builds

Review of attachment 60174 [details] [review]:
-----------------------------------------------------------------

not really qualified to review this, but looks sane
Comment 7 Simon McVittie 2012-12-04 18:30:08 UTC
I'll return to this one day... but this is not that day.

(In reply to comment #4)
> If you don't have DBusBasicValue you're also not going to have
> DBus8ByteStruct, so we'll need a private copy of that in dbus-gvariant.c for
> !HAVE_DBUSBASICVALUE.

True. At this point I think we could also just require dbus 1.6 - if you don't have that then you're just not keeping up.

(In reply to comment #5)
> Perhaps I'm missing something huge, but where in
> dbus_g_connection_call_async() are the GVariant parameters set on the
> outgoing message?

Er... nowhere, but the test doesn't exercise parameters either, so I didn't catch this error. This needs fixing when I have time to resume dragging telepathy-glib into the future (or at least the present).
Comment 8 Simon McVittie 2013-10-21 17:25:11 UTC
Created attachment 87939 [details] [review]
[01/11] dbus_g_message_read_variants,  dbus_g_message_write_variants: add and test

These will let us send GVariant-based messages over libdbus connections,
to facilitate porting from dbus-glib to GDBus.

---

Revised to actually handle (and test) parameters, and add documentation.
Comment 9 Simon McVittie 2013-10-21 17:26:52 UTC
Created attachment 87940 [details] [review]
[02/11] dbus_g_connection_call_async: add and test

This is basically the same API as g_dbus_connection_call_async, but
implemented in terms of libdbus. This means we can use a DBusGConnection
in telepathy-glib to make method calls that look suspiciously like a
GDBus call, reducing the size of the "flag day" commit we'll have to
make when we switch from dbus-glib to GDBus.

---

Er, no, it's this one that was revised to handle parameters and add docs; the previous one was amended to not use DBus8ByteStruct (which it didn't actually need, because "char bytes[8]" was enough to force the size). You get the general idea, though.
Comment 10 Simon McVittie 2013-10-21 17:27:25 UTC
Created attachment 87941 [details] [review]
[03/11] lcov.am: fix to work in out-of-tree builds

---

The same as before.
Comment 11 Simon McVittie 2013-10-21 17:27:35 UTC
Created attachment 87942 [details] [review]
[04/11] define MY_OBJECT_IFACE for tests to use
Comment 12 Simon McVittie 2013-10-21 17:27:47 UTC
Created attachment 87943 [details] [review]
[05/11] Factor out a common test_assert_no_error macro
Comment 13 Simon McVittie 2013-10-21 17:27:58 UTC
Created attachment 87944 [details] [review]
[06/11] test_run_until_round_trip: add
Comment 14 Simon McVittie 2013-10-21 17:28:12 UTC
Created attachment 87945 [details] [review]
[07/11] Fix building other projects against an uninstalled  dbus-glib

The magic variables used in the -uninstalled.pc.in don't seem to be
having the desired effect, and in any case, the standard Autoconf
substitutions do what we want.
Comment 15 Simon McVittie 2013-10-21 17:28:25 UTC
Created attachment 87946 [details] [review]
[08/11] Add GVariant-based signal subscriptions
Comment 16 Simon McVittie 2013-10-21 17:28:58 UTC
Created attachment 87947 [details] [review]
[09/11] Add a test for DBusGProxy

---

... specifically, the new GVariant-based signal listening (yeah, this patch could have been named better).
Comment 17 Simon McVittie 2013-10-21 17:29:27 UTC
Created attachment 87948 [details] [review]
[10/11] Add GVariant-based async method replies
Comment 18 Simon McVittie 2013-10-21 17:29:39 UTC
Created attachment 87949 [details] [review]
[11/11] Add non-GError-based async method errors
Comment 19 Simon McVittie 2015-02-09 14:11:10 UTC
I'm abandoning this. Most dbus-glib projects have moved to GDBus anyway, the major exception being telepathy-glib which has a different migration strategy (although that migration has stalled).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.