Bug 18055 - [abi-break] telepathy-glib should use glib-mkenums for enums not exposed on D-Bus
Summary: [abi-break] telepathy-glib should use glib-mkenums for enums not exposed on D...
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard: ABI-break
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-14 01:48 UTC by Murray Cumming
Modified: 2010-05-10 08:53 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
0001-2008-10-14-Murray-Cumming-murrayc-murrayc.com.patch (1018 bytes, patch)
2008-10-14 01:48 UTC, Murray Cumming
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Murray Cumming 2008-10-14 01:48:19 UTC
telepathy-glib should use glib-mkenums to generate tp-enum-types.[h|c], so that it can register correct types for signals and properties.

For instance, that would allow the attached patch to work.

It's difficult to integrate glib-mkenums into a Makefile.am, particularly when you are generating other code already. Pango is probably a good example:
http://svn.gnome.org/viewvc/pango/trunk/pango/Makefile.am?view=markup
Comment 1 Murray Cumming 2008-10-14 01:48:59 UTC
Created attachment 19649 [details] [review]
0001-2008-10-14-Murray-Cumming-murrayc-murrayc.com.patch
Comment 2 Will Thompson 2008-10-24 09:15:19 UTC
In the case you attach a patch for, we can't fix this without breaking ABI; we'll aim to do this right in future when adding new enums not exposed in the D-Bus API, and I'm marking this bug as one we can deal with when we decide to break telepathy-glib's ABI.

In the cases where enums come from the Telepathy D-Bus specification, we can't fix this because dbus-glib expects G_TYPE_UINT for them, rather than some GType corresponding to a GEnum.
Comment 3 Murray Cumming 2009-01-12 08:02:25 UTC
This would also allow us to use g_value_set_enum() instead of g_value_set_int(), as people expect when writing code, for instance when setting a GValue to TP_HANDLE_TYPE_CONTACT.
Comment 4 Murray Cumming 2009-01-12 08:03:41 UTC
(In reply to comment #2)
> In the cases where enums come from the Telepathy D-Bus specification, we can't
> fix this because dbus-glib expects G_TYPE_UINT for them, rather than some GType
> corresponding to a GEnum.

This seems like an error in your code that calls dbus-glib. Surely you can just transform the type where necessary.

Comment 5 Danielle Madeley 2009-09-18 04:16:23 UTC
So I recently asked why the new TpAccount wrapper-API wasn't using GEnums and was pointed to this bug.

It seems to me like we could possibly use GEnum in new API, except for the following problems:
 * API would be inconsistent between old and new APIs
 * GEnum is strict about guarding against values outside the range, this is a problem if the other end is using a newer version of the Telepathy spec.

It's possible that the second problem could be fixed with the addition of a g_value_force_enum(), which would be like set_enum() but without the bounds checking. The idea being that you enforce enum bounds checking on the API side, but you accept any incoming value on the D-Bus side.

I wonder if, even if it's not used in the API, it's worth adding glib-mkenums to the build, so that we can dump the names of specified enum values for debugging with g_enum_get_value().
Comment 6 Simon McVittie 2010-05-10 08:53:27 UTC
I'm going to call this FIXED, since Danielle added enum/flags types for all the hand-written enums.

TpConnectionManager:info-source is still of type UINT since otherwise it'd be an ABI break, but this is now flagged in the documentation.


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.