GNOME Bugzilla – Bug 580021
Port to external libgdata
Last modified: 2010-05-19 21:23:25 UTC
As agreed on the mailing list, I'm porting e-d-s to libgdata[1]. The initial find-and-replace port is complete, and I've pushed it to the libgdata-port branch on git.gnome.org. Basic calendar functionality is working, but there's still work to be done to tidy it up and better use libgdata's features. [1]: http://live.gnome.org/libgdata
I wonder whether your changes will fix properly also bug #548702, rather than the hack I just uploaded there. Oh, also, what's the actual state for this?
Copying my comment from bug #583742 about schedule: Need an pronouncement from the release team on libgdata first, which is scheduled for late July. And, given that libgdata would be a _required_ dependency (unlike with libgweather, for example), I'd rather wait until after we branch for GNOME 2.28. (Our policy is that our unstable releases must be buildable on the latest stable GNOME release. libgdata isn't in the latest stable GNOME release.) We _may_ branch early for 2.28 though, so we can get a head start on the kill-bonobo and eds-dbus merges. Still in discussion with release team.
(In reply to comment #2) > Copying my comment from bug #583742 about schedule: > > Need an pronouncement from the release team on libgdata first, which is > scheduled for late July. And, given that libgdata would be a _required_ > dependency (unlike with libgweather, for example), I'd rather wait until after > we branch for GNOME 2.28. ...which is done now, so patches are welcome.
Philip, do you have any future plans to work on this? Just wondering...
I'm also curious if there's any advantage over CalDAV for Google calendars. If not, maybe we should just drop the Google calendar backend altogether (providing automated Google -> CalDAV migration). We've already been telling people to stop using it, yet we still ship it and it's displayed prominently as an option for new calendars. Most users won't know to choose CalDAV when they see Google explicitly listed. What about address books? Is the Google backend still the only way to reach Google Contacts? (Google has no plans for LDAP support as of last month, from what I'm seeing.) If so, libgdata integration would definitely be welcome there.
(In reply to comment #4) > Philip, do you have any future plans to work on this? Just wondering... I'm not getting much free time from university work at the moment, and when I do, I get confounded by libnss, libsmime and D-Bus ganging up to stop my build environment from working with the re-architected e-d-s. I have the up-to-date modifications in a local branch, which applied cleanly to head a week or so ago, but I can't test them easily. Sorry. :-( (In reply to comment #5) > I'm also curious if there's any advantage over CalDAV for Google calendars. If > not, maybe we should just drop the Google calendar backend altogether > (providing automated Google -> CalDAV migration). I think that was Milan's plan. The Google Calendar backend isn't particularly brilliant, and Google's CalDAV support is almost complete. Known issues with CalDAV: http://www.google.com/support/calendar/bin/answer.py?answer=99360 > We've already been telling people to stop using it, yet we still ship it and > it's displayed prominently as an option for new calendars. Most users won't > know to choose CalDAV when they see Google explicitly listed. My suggestion would be to keep the google-account-setup plugin in evo, and have it produce CalDAV URIs instead of Google Calendar ones. The Google Calendar backend can then be dropped. > What about address books? Is the Google backend still the only way to reach > Google Contacts? (Google has no plans for LDAP support as of last month, from > what I'm seeing.) If so, libgdata integration would definitely be welcome > there. As far as I know, it is the only way. The patch I have locally converts the Contacts backend to use libgdata, but I think there are still a few places where we don't quite map evo's data fields to the available Google Contacts ones perfectly (the Google Contacts API doesn't expose as many fields as evo supports).
(In reply to comment #5) > I'm also curious if there's any advantage over CalDAV for Google calendars. If > not, maybe we should just drop the Google calendar backend altogether > (providing automated Google -> CalDAV migration). > > We've already been telling people to stop using it, yet we still ship it and > it's displayed prominently as an option for new calendars. Most users won't > know to choose CalDAV when they see Google explicitly listed. Done since 2.28.0, both. The backend shouldn't be used these days, as the transition is done, and the plugin just allows easier setup for google calendars, but internally is using CalDAV. Definitely keep it. The same plugin also manages google address books, even it's the other source file, but same binary (most likely known). The only piece in plugin, which is using libgdata, is retrieving of a list of user's calendars. It was easier with libgdata than with CalDAV. As mentioned above, only address book code is using libgdata on eds side.
(In reply to comment #7) > Done since 2.28.0, both. The backend shouldn't be used these days, as the > transition is done, and the plugin just allows easier setup for google > calendars, but internally is using CalDAV. Definitely keep it. The same plugin > also manages google address books, even it's the other source file, but same > binary (most likely known). Why is the code still in the e-d-s tree then? http://git.gnome.org/browse/evolution-data-server/tree/calendar/backends/google > The only piece in plugin, which is using libgdata, is retrieving of a list of > user's calendars. It was easier with libgdata than with CalDAV. That's bug #583742.
(In reply to comment #8) > Why is the code still in the e-d-s tree then? > http://git.gnome.org/browse/evolution-data-server/tree/calendar/backends/google I'm guessing because we don't have migration in place for users that still have google-backend calendars. We still get bug reports about it. Should be fairly easy to migrate, I would think.
Rather because wasn't any need to remove it from sources fully. This is there from times of 2.28.0 (and slightly earlier): http://git.gnome.org/browse/evolution/tree/plugins/google-account-setup/google-source.c#n807
Created attachment 156583 [details] [review] Port e-d-s to external libgdata Here it is! A working, finished patch! Yay! This converts the Google addressbook and calendar backends to use external libgdata (version >= 0.6.3, see below). The calendar backend port is from a few months ago, and I haven't tested or polished it for this patch (i.e. it compiles against libgdata 0.6.3, but that's about all I can guarantee), as it really should just die; people should use CalDAV instead. I don't know how we can convert existing users of the Google backend to use CalDAV, though. Modifications to the Evolution google-account-setup plugin, perhaps? The contacts backend port is in a good state, and fairly well tested. I've extended it to support structured names and addresses, but there are still things like categories, photos and other attributes which support could be added for. That can wait until this has landed and stabilised, since they should be a lot easier to do. A few of the things in the contacts backend now depend on some changes which only just landed in libgdata, and which will make the 0.6.3 minor release, which I hope to push out by the end of the weekend. Note that this will require external dependency bump for e-d-s. As far as patch review goes, whoever reviews it can pretty much ignore all the changes to the calendar backend, as I said above. Most of the changes in the contacts backend are a fairly simple rewrite from libgdata-1.2 to the external libgdata. The stuff which needs a closer look is in util.c, since that's where the nitty-gritty of the mapping between EVCard and GDataContactsContact happens. The rest of the patch is just the (satisfying) deletion of servers/google/*. If this can be committed really early in the cycle, that'll give plenty of opportunity for more testing. :-)
Just realised I modified the debug macro in util.h, which would need to be reverted in the next iteration of the patch/before it's committed.
Patch builds cleanly with some minor adjustments and seems to work for my Google contacts, though I didn't test extensively. Patch itself looks well-written, though the surrounding backend code has some serious whitespace and indentation issues. Still, I think it's ready to commit but I'd like a second opinion from Chen.
My only minor concern here is the libgdata requirement is rather bleeding edge and mandatory. Are distros going to be shipping libgdata 0.6.3 alongside GNOME 2.30? Fedora 13 only has 0.6.1 at the moment.
(In reply to comment #13) > Patch builds cleanly with some minor adjustments and seems to work for my > Google contacts, though I didn't test extensively. Patch itself looks > well-written, though the surrounding backend code has some serious whitespace > and indentation issues. Still, I think it's ready to commit but I'd like a > second opinion from Chen. If I get some time in my remaining week of holiday, I'd like to blitz the surrounding code, restructure it (why does it need to be in three files‽) and sort out the whitespace. That's for another bug report, though. (In reply to comment #14) > My only minor concern here is the libgdata requirement is rather bleeding edge > and mandatory. Are distros going to be shipping libgdata 0.6.3 alongside GNOME > 2.30? Fedora 13 only has 0.6.1 at the moment. I hope they will. It includes some important fixes which the e-d-s stuff depends on. Anyway, this is for 2.32 (or 3.0, or whatever we're calling it).
Right, I just want to make sure Evolution 2.31 is buildable on GNOME 2.30. But since libgdata is an external dependency I guess there's some wiggle room here.
(In reply to comment #13) > Still, I think it's ready to commit but I'd like a > second opinion from Chen. Ping?
We're still early in the 3.0 development cycle. As long as you're not going to disappear on us in the next few months, go ahead and commit and proceed with the cleanup/restructuring work you mentioned. You might also peruse our other Google-related bugs and see if there's anything that libgdata already addresses or could easily solve. http://tinyurl.com/evolution-google
commit 28897beab5dcd4aab3586322a96280be92585db6 Author: Philip Withnall <philip@tecnocode.co.uk> Date: Sun Apr 19 08:37:27 2009 +0100 Bug 580021 — Port to external libgdata Convert Google calendar and contacts backends to use the external libgdata, depending on version >= 0.6.3. The e-d-s internal libgdata-1.2 has been dropped completely. The conversion of the Google calendar backend is untested and probably not working, but since people should be using CalDAV to access their Google Calendars, this shouldn't be a problem. The conversion of the Google contacts backend is fairly well tested, and has had support for a few new properties added. Most notably, addresses and names are now stored on Google's servers in a structured format which more closely maps to e-d-s' vCard representation than the previous flat string. Closes: bgo#580021 addressbook/backends/google/Makefile.am | 12 +- .../backends/google/e-book-backend-google.c | 171 ++- addressbook/backends/google/google-book.c | 420 ++--- addressbook/backends/google/google-book.h | 15 - addressbook/backends/google/util.c | 647 ++++--- addressbook/backends/google/util.h | 6 +- calendar/backends/google/Makefile.am | 12 +- .../backends/google/e-cal-backend-google-utils.c | 874 +++++--- .../backends/google/e-cal-backend-google-utils.h | 22 +- calendar/backends/google/e-cal-backend-google.c | 226 +-- calendar/backends/google/e-cal-backend-google.h | 23 +- configure.ac | 29 +- servers/Makefile.am | 2 +- servers/google/Makefile.am | 3 - servers/google/libgdata-google/Makefile.am | 38 - .../google/libgdata-google/gdata-google-service.c | 689 ------- .../google/libgdata-google/gdata-google-service.h | 73 - .../google/libgdata-google/libgdata-google.pc.in | 16 - servers/google/libgdata/Makefile.am | 42 - servers/google/libgdata/gdata-entry.c | 2155 -------------------- servers/google/libgdata/gdata-entry.h | 265 --- servers/google/libgdata/gdata-feed.c | 680 ------ servers/google/libgdata/gdata-feed.h | 74 - servers/google/libgdata/gdata-service-iface.c | 113 - servers/google/libgdata/gdata-service-iface.h | 82 - servers/google/libgdata/libgdata.pc.in | 15 - 26 files changed, 1347 insertions(+), 5357 deletions(-)
(In reply to comment #9) > (In reply to comment #8) > > Why is the code still in the e-d-s tree then? > > http://git.gnome.org/browse/evolution-data-server/tree/calendar/backends/google > > I'm guessing because we don't have migration in place for users that still have > google-backend calendars. We still get bug reports about it. Should be fairly > easy to migrate, I would think. What would be the best way to implement migration?
Comment #7 implies the migration is in place already, at least for Evolution users. Need to double check with Milan.
Yup, it's there, though I'm wondering whether it works in 2.30.x, as maybe the e_source_peek_absolute_uri can return uri there, or is the function called at all? I'm not sure, it'll need a bit of testing. Could you try it, Philip, please? http://git.gnome.org/browse/evolution/tree/plugins/google-account-setup/google-source.c?h=gnome-2-28#n808
(In reply to comment #22) > Yup, it's there, though I'm wondering whether it works in 2.30.x, as maybe the > e_source_peek_absolute_uri can return uri there, or is the function called at > all? I'm not sure, it'll need a bit of testing. Could you try it, Philip, > please? > > http://git.gnome.org/browse/evolution/tree/plugins/google-account-setup/google-source.c?h=gnome-2-28#n808 From some quick testing, it doesn't look like it's doing the migration at all. I'll see if I can take a closer look/cook a patch for it tomorrow.
The function itself is run only when running newer version on the older data. This is determined by /apps/evolution/last_version and /apps/evolution/version keys in GConf. It makes sense to not run migration every start. Try to set both keys to 2.28.0 and run 2.30.0, in that case the migration code should be run. Note the new evolution will rewrite these keys. I see that the google's .eplug file has load-on-startup="false", but system_plugin="true". I wonder what it does, maybe it's prevented from its migration, because the plugin isn't loaded.
(In reply to comment #24) > I see that the google's .eplug file has load-on-startup="false", but > system_plugin="true". I wonder what it does, maybe it's prevented from its > migration, because the plugin isn't loaded. Even when the plugin's loading, it doesn't work, because the event hook specification in its eplug file is wrong. It should be: <event target="module" id="module.migration" handle="e_calendar_google_migrate"/> rather than: <event target="component" id="component.migration" handle="e_calendar_google_migrate"/> With load-on-startup="true" and that change, migration works fine. I'm just testing to see if it still works with load-on-startup="false".
Filed as bug #616400.
(In reply to comment #25) > With load-on-startup="true" and that change, migration works fine. I'm just > testing to see if it still works with load-on-startup="false". It does! As far as I understand it, then, we can remove the Google Calendar backend from e-d-s as soon as the patch in bug #616400 is committed.
Created attachment 159266 [details] [review] Remove Google Calendar backend
Review of attachment 159266 [details] [review]: Please commit it to master.
Comment on attachment 159266 [details] [review] Remove Google Calendar backend commit 7d7178ce0c74c8327cf9343ca722847af14d8a5e Author: Philip Withnall <philip@tecnocode.co.uk> Date: Wed Apr 21 16:53:52 2010 +0100 Remove Google Calendar backend Remove the Google Calendar calendar backend completely. Google Calendar can, and should, be accessed through CalDAV, and the google-account-setup plugin in Evolution will manage this (and migrate old sources to use CalDAV instead of the Google Calendar backend). Closes: bgo#580021 calendar/backends/Makefile.am | 2 +- calendar/backends/google/Makefile.am | 35 - .../backends/google/e-cal-backend-google-factory.c | 169 -- .../backends/google/e-cal-backend-google-factory.h | 39 - .../backends/google/e-cal-backend-google-utils.c | 1099 ------------- .../backends/google/e-cal-backend-google-utils.h | 45 - calendar/backends/google/e-cal-backend-google.c | 1607 -------------------- calendar/backends/google/e-cal-backend-google.h | 80 - configure.ac | 1 - po/POTFILES.in | 2 - 10 files changed, 1 insertions(+), 3078 deletions(-)
Backend successfully killed!