GNOME Bugzilla – Bug 764065
[Camel] Port more classes to GObject
Last modified: 2016-11-08 15:10:43 UTC
Let's simplify some points of the API to allow à better GObject Introspection. I started a branch here https://github.com/tintou/evolution-data-server so that it's possible to follow my work
Few notes from the IRC (and some added): - drop CamelMessageInfoBase and use CamelMessageInfo only - if the CamelMessageInfo will be done as a GObject, then: a) hide all structure members into the private structure and add for them the usually get/dup/set API functions ('dup' is for strings, like [1]) b) each member will have also its GObject property c) CamelFolderSummaryClass::message_info_size and CamelFolderSummaryClass::content_info_size will be GType-s, not gsize This will require many changes in the code all over the place. To accept any of these changes you might also cover at least the main projects, hosted in the GNOME git. Namely: evolution, evolution-mapi, evolution-ews, evolution-rss and eventually also evolution-rspam and evolution-activesync, but I do not know in what state these two are. As my knowledge of the github interface is very limited (and I find their interface very confusing), then please paste here direct links to your commits, if there will be more. Attaching patches here the usual way works for me too (maybe even better than referencing github clone), but I'm not forcing you to do it. [1] https://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-source-webdav.c#n717
Ping, what is the expected ETA for these changes, please? I'd not be willing to commit any such change just before the feature/API freeze, I'd rather have it done earlier in the development cycle, thus other projects have enough time to adapt to the changes. If it'll be too close to the end of the development cycle I'd prefer to postpone this to 3.23 or even later. The review process will take its time as well.
This will take a bunch of time for me to port it to GObject so this might indeed go to 3.23
Okay, thanks. Targeting 3.23.1 would be wonderful. The date might be around the second half of the October. It's not set yet at https://wiki.gnome.org/ThreePointTwentyone
When you'll be playing with the CamelMessageInfo changes, please hide the members in a private structure and make the interface thread safe, which means that for text values there will be (with white-space fixed for the proper coding style): void camel_message_info_set_text (CamelMessageInfo *info, const gchar *text) { g_return_if_fail (CAMEL_IS_MESSAGE_INFO (info)); g_mutex_lock (&info->priv->property_lock); if (g_strcmp0 (info->priv->text, text) == 0) { g_mutex_unlock (&info->priv->property_lock); return; } camel_pstring_free (info->priv->text); info->priv->text = camel_pstring_strdup (text); g_mutex_unlock (&info->priv->property_lock); g_object_notify (G_OBJECT (info), "text"); } /* Not a thread-safe function, but still provided */ const gchar * camel_message_info_get_text (CamelMessageInfo *info) { g_return_val_if_fail (CAMEL_IS_MESSAGE_INFO (info), NULL); return info->priv->text; } /* Thread safe, use g_free() to free */ gchar * camel_message_info_dup_text (CamelMessageInfo *info) { gchar *res; g_return_val_if_fail (CAMEL_IS_MESSAGE_INFO (info), NULL); g_mutex_lock (&info->priv->property_lock); res = g_strdup (info->priv->text); g_mutex_unlock (&info->priv->property_lock); return res; } /* Thread safe, use camel_pstring_free() to free */ // it's because the strings are stored in the string pool, thus to avoid // unnecessary allocation for callers whom care const gchar * camel_message_info_dup_text_pstring (CamelMessageInfo *info) { const gchar *res; g_return_val_if_fail (CAMEL_IS_MESSAGE_INFO (info), NULL); g_mutex_lock (&info->priv->property_lock); res = camel_pstring_strdup (info->priv->string); g_mutex_unlock (&info->priv->property_lock); return res; } Similarly for the other non-integer properties. These changes will help to address bug #680471. You may even want to replace the camel_flag/camel_tag with something more convenient, like GSList of string (for CamelFlag) and GSList of CamelTag, which will be simple pair of { gchar *name, *value; } where none will be allocated as part of the structure. All these strings can be part of the camel_pstring_strdup/_free() too, because those strings are repeating. If you'd like to help with this, then I can do it. By the way, projects usually affected by the camel soname version bump shown in Fedora are: almanah bijiben california ekiga evolution evolution-ews evolution-mapi evolution-rss evolution-rspam ffgtk folks giggle glabels gnome-calendar gnome-contacts gnome-phone-manager gnome-shell libopensync-plugin-evolution2 Some of them may just require a rebuild, but it's to be figure out. There can be more project affected, thus it would be nice to have also a guide how to port from pre-change to the-changed version of the camel, once it's all done.
Created attachment 330153 [details] [review] EDS-Getter patch
Created attachment 330154 [details] [review] Evolution-Getter patch
Created attachment 330155 [details] [review] EvolutionWS-Getter patch
Here is a set of three patches for the three main components. The port process is simple, camel_message_info_foo macros are replaced by camel_message_info_get_foo (It's a preparation of the GObject move of the CamelMessageInfo object) In top of that, using methods instead of macros allows the Python binding to use these functions (and every interpreted language)
Thanks for the update. Apart of some minor whitespace issues (mostly parameter indentation), I received compiler warnings about passing incompatible pointers into the new functions in the eds itself. That led me to change the API of the getters to not use 'const CamelMessageInfo *', but 'gconstpointer' instead. Then I added some safety checkers into these new functions. I also added "Since: 3.22" to the developer documentation. As I do not want to scare the Camel library users yet (the full change for this bug will mean quite some work for the users of the Camel, thus I want to relax them for now), I added backward compatibility #define-s to the header, thus no other changes are required for the library users, except of the rebuild, for which I bumped a soname version. Created commit c9da071 in eds master (3.21.4+) Created commit_0a907dd in evo master (3.21.4+) [1] Created commit_4e09c6d in ews master (3.21.4+) [2] Created commit_fc9dc4b in ema master (3.21.4+) [3] [1] https://git.gnome.org/browse/evolution/commit/?id=0a907dd [2] https://git.gnome.org/browse/evolution-ews/commit/?id=4e09c6d [3] https://git.gnome.org/browse/evolution-mapi/commit/?id=fc9dc4b
(In reply to Milan Crha from comment #5) > almanah > bijiben > california > ekiga > evolution > evolution-ews > evolution-mapi > evolution-rss > evolution-rspam > ffgtk > folks > giggle > glabels > gnome-calendar > gnome-contacts > gnome-phone-manager > gnome-shell > libopensync-plugin-evolution2 I've a good news, most of the above projects/packages do not use the libcamel at all, they link it only due to other reasons, like the libcamel being advertised by libedataserver-1.2.pc file. I tried to address it by commit 0dafc9b.
I made a work-in-progress branches "wip/camel-more-gobject" for the core evolution products in the GNOME git, to have a place to work on this: https://git.gnome.org/browse/evolution-data-server/log/?h=wip/camel-more-gobject https://git.gnome.org/browse/evolution/log/?h=wip/camel-more-gobject https://git.gnome.org/browse/evolution-ews/log/?h=wip/camel-more-gobject https://git.gnome.org/browse/evolution-mapi/log/?h=wip/camel-more-gobject The evolution-rss, and eventually also evolution-rspam, will need some changes too, but we can cover them separately, depending on the overall changes being done in the camel. As we spoke on IRC, I'll take care of the CamelMessageInfo and CamelMessageContentInfo (basically structures defined in camel-folder-summary.h) and you'll have the rest.
I created additional work-in-progress branches for this work here: https://git.gnome.org/browse/evolution-rss/log/?h=wip/camel-more-gobject I set its expectation to eds 3.23.1 (it uses conditional compile for various versions of the evolution-data-server/evolution), which requires some tweaking in the configure files before we'll branch for 3.23.1. I'd like to cover also evolution-rspam, but that doesn't reside in GNOME's infrastructure. Luckily, the changes being done in the camel-more-gobject branches do not influence it. It depends on your further changes whether any work will be required there or not. In any case, I basically finished the CamelMessageInfo changes, I'll do some testing and cleanup by the mid/end of this week, but it should not clash with your work much (probably not at all), thus consider the branches ready for your work. I'll "rebase" [1] the branches to 3.23.1 soon after the master will be branched. I'll let you know beforehand. [1] most likely I'll just drop the branch and will recreate it with one squashed commit for done changes in the code.
Possibly the final project to cover, at least from the GNOME infrastructure: https://git.gnome.org/browse/evolution-activesync?h=wip%2Fcamel-more-gobject
I rebased the changes to current git master of the evolution core products, which is also after the merge for the CMake work. The 3.23.1 is going to be released on the next Monday, October 24th. How much work do you plan to add to the current state, please? I'd like to make life easier to any Camel library user as much as possible, which basically means to not change API/ABI too often, neither do soname version bumps. The current changes already contain one soname version bump. Do you have planned other changes which would change API/ABI, or the current state is basically what you've been missing and it can be merged? I do not mind to commit this for 3.23.1 or 3.23.2, I only do not want to bump soname version multiple times, when we can avoid it. It can be that there will be Camel soname version bump(s) in the 3.23.x cycle for other reasons, though I do not plan any in particular at the moment.
For the record, the class CamelFolderChangeInfo just need some GBoxed enablement and it would be better to use CamelNameValueArray for the 'headers' member of the CamelMesageInfo (see IRC for more details)
Thanks for the summary. I'll look on it some time after the 3.23.1 release, and I'm targeting these changes for 3.23.2. I'll keep you update through this bug report. Feel free to add more requests meanwhile (to cover as many API changes as possible, where API additions aren't any problem).
Debian has one other package that links against libcamel: https://tracker.debian.org/pkg/mail-notification
(In reply to Jeremy Bicha from comment #18) > Debian has one other package that links against libcamel: > https://tracker.debian.org/pkg/mail-notification Thanks, I'll look on it. Is there any upstream, where the change (if needed) would be sent? I see that Fedora has the package too [1], but it doesn't build the evolution part, it seems [2] (the build log says "evolution: no". [1] http://pkgs.fedoraproject.org/cgit/rpms/mail-notification.git/ [2] https://kojipkgs.fedoraproject.org//packages/mail-notification/5.4/88.git.9ae8768.fc25/data/logs/x86_64/build.log
(In reply to Milan Crha from comment #19) > Thanks, I'll look on it. Is there any upstream, where the change (if needed) > would be sent? No, the debian/changelog lists the last new version as being from 2008, so Debian has a lot of patches to keep the software compiling: https://sources.debian.net/src/mail-notification/5.4.dfsg.1-14/debian/patches/
Created attachment 338889 [details] [review] patch for mail-notification This fixes the mail-notification. The build itself, without this patch, didn't fail, but the build/build.log contained warnings about implicit function declarations. This patch fixes it. If you have any place where to share this more widely, then I think people will appreciate it. The Fedora itself references a clone of the mail-notification from https://github.com/epienbroek/mail-notification/ but I do not have a github account to let the maintainer know. I opened a Fedora bug here: https://bugzilla.redhat.com/show_bug.cgi?id=1390590
Created attachment 339113 [details] [review] patch for evolution-rspam This is a patch for evolution-rspam, for changes introduced in bug #773787
I marked committed patches as obsolete here, to make it easier to see what patches here are related to other projects. Those are currently for evolution-rspam and mail-notification projects.
This landed for the 3.23.2 release of the evolution-data-server and other core evolution products. Thank you for your help with this. https://git.gnome.org/browse/evolution-data-server/commit/?id=9af0c834e https://git.gnome.org/browse/evolution/commit/?id=8b74f9146 https://git.gnome.org/browse/evolution-ews/commit/?id=e4dcf633b https://git.gnome.org/browse/evolution-mapi/commit/?id=341a2f4ea https://git.gnome.org/browse/evolution-rss/commit/?id=c78ce3988 https://git.gnome.org/browse/evolution-activesync/commit/?id=f093b09e0