GNOME Bugzilla – Bug 565091
Bug for tracking progression on the Evolution metadata support
Last modified: 2009-01-15 12:59:35 UTC
This is a bug for tracking the progression of the support of (x) in Tracker. #x http://live.gnome.org/Evolution/Metadata
Created attachment 125000 [details] [review] Unfinished patch This is an unfinished patch (don't apply and commit it). It illustrates the (current) skeleton of the project that will add this support.
Created attachment 125191 [details] [review] Newer version, equally untested
Created attachment 125272 [details] [review] Newer version, some things are actually working now
Created attachment 125330 [details] [review] Same patch, most things in the EPlugin are working now
Please find a client-sample here: http://live.gnome.org/Evolution/Metadata?action=AttachFile&do=view&target=valaclientsample.vala tracker-evolution-registrar.c is of course the client-side Evolution DBus Registrar that tracker-indexer will get. It's unfinished, but I'm testing with valaclientsample.vala (url above). ps. valac --Xcc=-ggdb --pkg dbus-glib-1 -o valaclientsample valaclientsample.vala Then start Evolution after having done "make install" in tracker-evolution-plugin/ and then do "Edit->Plugins" (there's a bug in Evolution trunk about some plugins not being initialized unless you first go to that menu). Then start ./valaclientsample, and enjoy the "setmany %d" prints that will start happening, and will continue happening when you change flags, delete E-mails, etc etc in Evolution. The same events will be received by tracker-evolution-registrar.c, and will trigger the necessary actions at Tracker's side (for storing, deleting and updating the E-mail data being notified to the registrar).
Created attachment 125477 [details] [review] Same patch, integrating it into tracker-indexer
Created attachment 125537 [details] [review] Enabling, disabling should now work. Little bit of refactoring
Created attachment 125570 [details] [review] Further integrating the plugin The registrar now runs in trackerd, which will proxy all requests to a D-Bus object running in tracker-indexer that implements the same DBus interface. This ensures a more stable life cycle of the Registrar and it avoids needing direct access to the indexer's DBus interfaces (which might change over time, as that part of the DBus API ain't part of the public API of Tracker). With this patch you get: org.gnome.evolution.metadata.Manager: Manager interface org.gnome.evolution.metadata.Registrar: Registrar interface /org/freedesktop/Tracker/Evolution/Registrar : path in trackerd implements Registrar. This is the one that gets registered to the Manager /org/freedesktop/Tracker/Indexer/Evolution/Registrar : path in indexer implements Registrar. This one is known by the actual Registrar and proxies all requests to the actual one /org/gnome/evolution/metadata/Manager: Evolution's manager's path
Created attachment 125571 [details] [review] Bugfixes for activating and deactivating
Created attachment 125797 [details] [review] Added a bunch of predicates, more data is being passed now
Created attachment 125801 [details] [review] Several smaller bugfixes against last patch
Created attachment 125854 [details] [review] More bugfixes, more cleaning up, more glitch-removals
Created attachment 125855 [details] [review] Wrong file attached
Created attachment 125937 [details] [review] Preparing official proposal for review 2009-01-07 Philip Van Hoof <philip@codeminded.be> * src/tracker-indexer/modules/Makefile.am * src/tracker-indexer/modules/evolution.c: Selecting between the old and the new Evolution support, depending on availability of the right version of EDS * data/services/email.metadata: * data/db/sqlite-tracker.sql: * configure.ac * src/tracker-indexer/Makefile.am * src/trackerd/Makefile.am * src/Makefile.am * src/tracker-evolution-plugin/tracker-evolution-plugin.h * src/tracker-evolution-plugin/tracker-evolution-indexer.h * src/tracker-evolution-plugin/tracker-evolution.h * src/tracker-evolution-plugin/tracker-evolution-registrar.h * src/tracker-evolution-plugin/tracker-evolution-plugin.xml * src/tracker-evolution-plugin/liborg-freedesktop-Tracker-evolution-plugin.eplug.xml * src/tracker-evolution-plugin/tracker-evolution-common.h * src/tracker-evolution-plugin/Makefile.am * src/tracker-evolution-plugin/tracker-evolution-plugin.c * src/tracker-evolution-plugin/tracker-evolution-indexer.c * src/tracker-evolution-plugin/tracker-evolution-registrar.xml * src/tracker-evolution-plugin/tracker-evolution.c * src/tracker-evolution-plugin/tracker-evolution-registrar.c: Implementation of the new Evolution support * src/trackerd/tracker-main.c * src/tracker-indexer/tracker-main.c:
Created attachment 126108 [details] [review] Please review Final patch, please review. ChangeLog entry: 2009-01-09 Philip Van Hoof <philip@codeminded.be> * src/tracker-indexer/modules/Makefile.am * src/tracker-indexer/modules/evolution.c: Selecting between the old and the new Evolution support, depending on availability of the right version of EDS * data/services/email.metadata: * data/db/sqlite-tracker.sql: * configure.ac * src/tracker-indexer/Makefile.am * src/trackerd/Makefile.am * src/Makefile.am * src/tracker-evolution-plugin/tracker-evolution-plugin.h * src/tracker-evolution-plugin/tracker-evolution-indexer.h * src/tracker-evolution-plugin/tracker-evolution.h * src/tracker-evolution-plugin/tracker-evolution-registrar.h * src/tracker-evolution-plugin/tracker-evolution-plugin.xml * src/tracker-evolution-plugin/org-freedesktop-Tracker-evolution- plugin.eplug.xml * src/tracker-evolution-plugin/tracker-evolution-common.h * src/tracker-evolution-plugin/Makefile.am * src/tracker-evolution-plugin/tracker-evolution-plugin.c * src/tracker-evolution-plugin/tracker-evolution-indexer.c * src/tracker-evolution-plugin/tracker-evolution-registrar.xml * src/tracker-evolution-plugin/tracker-evolution.c * src/tracker-evolution-plugin/tracker-evolution-registrar.c: Implementation of the new Evolution support * src/trackerd/tracker-main.c
Some notes on the patch: - While the EPlugin is quite tied to Evolution, I don't see any reason for having the code in the Tracker side to be Evolution dependent, it reminds me to the email modules in the old tracker. - The EPlugin seems to handle only IMAP, and the TrackerIndexerModule seems to be still necessary for POP, sounds like extra maintenance pain to me... - The EPlugin still accesses to the raw DB contents, I somehow expected something more tied to Evolution internals - Things like: (flags & CAMEL_MESSAGE_ANSWERED)?"True":"False") Are missing some spaces to follow code style: (flags & CAMEL_MESSAGE_ANSWERED) ? "True" : "False") - Function declarations like: static void on_folder_summary_changed (CamelFolder *folder, CamelFolderChangeInfo *changes, gpointer user_data) Should have the arguments in different lines to conform to code style - if (FALSE && ...) Please consider #if 0 there :P - fd = g_open (path, O_RDONLY /*| O_NOATIME*/, S_IRUSR | S_IWUSR); You should probably restore O_NOATIME there... - I don't see attachments being handled anywhere, am I missing something? - Why does this have to be enabled in trackerd? is there anything special after having the emails indexed?
> - While the EPlugin is quite tied to Evolution, I don't see any reason for > having the code in the Tracker side to be Evolution dependent, it reminds me to > the email modules in the old tracker. Which of the code that runs in Evolution is still dependent on Evolution? > - The EPlugin still accesses to the raw DB contents, I somehow expected > something more tied to Evolution internals The EPlugin runs in Evolution's process. This is how you access Evolution's data from Evolution's process. Notice the #include <camel/camel-db.h>, it's using the API in camel-db.h, which is part of Evolution's mailer component (ie. Evolution's process). > - I don't see attachments being handled anywhere, am I missing something? This is todo > Why does this have to be enabled in trackerd? is there anything special after > having the emails indexed? Of course. Whenever anything about the E-mails change (like for example their "Seen" status) will trackerd get notified by Evolution. Whenever new E-mails arrive, is trackerd notified of this. Etcetera.
Created attachment 126283 [details] [review] First iteration after martyn's review > > - While the EPlugin is quite tied to Evolution, I don't see any reason for > > having the code in the Tracker side to be Evolution dependent, it reminds me > > to the email modules in the old tracker. > Which of the code that runs in Evolution is still dependent on Evolution? I of course mean, "which of the code that doesn't run in Evolution ..." I attached a new patch that addresses the concerns of martyn's first review (check mailing list). Please review.
> - I don't see attachments being handled anywhere, am I missing something? Note that this is disabled because most versions of GMime <= 2.2 crash on certain of my test E-mails. As discussed on IRC I disabled this to avoid crashes of tracker-indexer caused by these instabilities in those versions of GMime. I'm awaiting the migration of your Evolution support code to GMIme >= 2.4 to activate this. After that, to regain the support for attachments would be as easy as uncommenting the currently commented-out code and adding the filename extraction of the attachment to the metadata hashtable at the extract_mime_parts function of tracker-evolution-indexer.c
Created attachment 126289 [details] [review] Please review, both martyn's and carlo's remarks are addressed in this one > - The EPlugin seems to handle only IMAP, and the TrackerIndexerModule seems to > be still necessary for POP, sounds like extra maintenance pain to me... All types of accounts should work, as also the root account of Evolution is scanned as an account (this is the account that Evolution uses to deliver POP E-mail to). If not, the fix is to add this one to the EAccounts. I will investigate this but afaik is this account already included and handled. > - Things like: > > (flags & CAMEL_MESSAGE_ANSWERED)?"True":"False") > > Are missing some spaces to follow code style: > > (flags & CAMEL_MESSAGE_ANSWERED) ? "True" : "False") Fixed > - Function declarations like: > > static void > on_folder_summary_changed (CamelFolder *folder, CamelFolderChangeInfo > *changes, gpointer user_data) > > Should have the arguments in different lines to conform to code style Fixed > - if (FALSE && ...) > Please consider #if 0 there :P Done > - fd = g_open (path, O_RDONLY /*| O_NOATIME*/, S_IRUSR | S_IWUSR); > You should probably restore O_NOATIME there... Done > - I don't see attachments being handled anywhere, am I missing something? This is implemented but under the #if 0 #endif which you mentioned above, so not activated atm. The reason for this is, as stated before, the fact that GMime <= 2.2 is unstable and seems to crash on various of my E-mails. I'm awaiting your migration to GMime >= 2.4 for your Evolution support code to activate this part.
(In reply to comment #17) > Which of the code that doesn't run in Evolution is still dependent on Evolution? I mean, all the Evolution:: namespace, etc... that makes the whole patch look unextensible to other mail clients, to some extent I think it's bad to support just one part of Evolution emails as a complete stranger to the rest of other ways Tracker can be extended > > The EPlugin runs in Evolution's process. This is how you access Evolution's > data from Evolution's process. > > Notice the #include <camel/camel-db.h>, it's using the API in camel-db.h, which > is part of Evolution's mailer component (ie. Evolution's process). Really? I somehow expected the evolution APIs to be more comprehensive to avoid direct access to databases > Of course. Whenever anything about the E-mails change (like for example their > "Seen" status) will trackerd get notified by Evolution. Whenever new E-mails > arrive, is trackerd notified of this. Etcetera. > Oh, indeed, makes much sense, thanks for clarifying
(In reply to comment #20) > > - The EPlugin seems to handle only IMAP, and the TrackerIndexerModule seems to > > be still necessary for POP, sounds like extra maintenance pain to me... > > All types of accounts should work, as also the root account of Evolution is > scanned as an account (this is the account that Evolution uses to deliver POP > E-mail to). If not, the fix is to add this one to the EAccounts. I will > investigate this but afaik is this account already included and handled. Oh, I guessed that from the NO_EVOPLUG conditions, you only disable the IMAP sqlite storage in that case, if your code is going to handle POP as well, perhaps you should disable compiling the whole indexer module altogether.
Comment #22: The POP flow in Evolution is that Evolution has a POP account, yes, but only uses it to fetch the messages from. After fetching it will filter (copy) the E-mails to the local account using its filtering mechanisms. That local account is therefore where the E-mails of all POP accounts will end up in. If the filters instruct Evolution to copy those E-mails into different folders then those folders will still be part of the local Evolution account. I investigated whether or not my EPlugin is scanning this local account too. It seems to do this indeed. What I did to investigate this is setting up a all new user 'test'. The Evolution that I started for this user had no accounts at first launch. Thus I saw the "Create new account" wizard upon first startup. I configured only one POP account, a GMail POP account. Then I shut Evolution down and I've put a breakpoint on introduce_accounts_to which is a function that introduces an account to a registrar. The flow and print of the pointer should illustrate that indeed the local account is found and added. (gdb) break introduce_accounts_to Function "introduce_accounts_to" not defined. Make breakpoint pending on future shared library load? (y or [n]) y Breakpoint 1 (introduce_accounts_to) pending. (gdb) run Starting program: /opt/evolution/bin/evolution [Thread debugging using libthread_db enabled] [New Thread 0xb6624940 (LWP 16256)] ** (evolution:16256): DEBUG: mailto URL command: evolution %s ** (evolution:16256): DEBUG: mailto URL program: evolution [New Thread 0xb5ff8b90 (LWP 16266)] [Thread 0xb5ff8b90 (LWP 16266) exited] (evolution:16256): Gtk-CRITICAL **: gtk_icon_info_free: assertion `icon_info != NULL' failed [New Thread 0xb5ff8b90 (LWP 16267)] [Thread 0xb5ff8b90 (LWP 16267) exited] [New Thread 0xb572cb90 (LWP 16268)] [Thread 0xb572cb90 (LWP 16268) exited] [New Thread 0xb572cb90 (LWP 16269)] [Thread 0xb572cb90 (LWP 16269) exited] [Switching to Thread 0xb6624940 (LWP 16256)] Breakpoint 1, introduce_accounts_to (self=0x85d1e60, info=0x8435150) at tracker-evolution-plugin.c:1062 1062 tracker-evolution-plugin.c: No such file or directory. in tracker-evolution-plugin.c (gdb) break introduce_account_to Breakpoint 2 at 0xb4994e29: file tracker-evolution-plugin.c, line 1006. (gdb) cont Continuing. Breakpoint 2, introduce_account_to (self=0x85d1e60, account=0x8070460, info=0x8435150) at tracker-evolution-plugin.c:1006 1006 in tracker-evolution-plugin.c (gdb) print *account $1 = {parent_object = {g_type_instance = {g_class = 0x80fbca0}, ref_count = 1, qdata = 0x812d340}, name = 0x811c628 "test@test", uid = 0x811b9e0 "1231845650.15836.0@tinc", enabled = 1, id = 0x811c7d8, source = 0x811f8f8, transport = 0x811c5c8, drafts_folder_uri = 0x811bd00 "mbox:/home/test/.evolution/mail/local#Drafts", sent_folder_uri = 0x811c708 "mbox:/home/test/.evolution/mail/local#Sent", templates_folder_uri = 0x0, always_cc = 0, cc_addrs = 0x811c6b0 "", always_bcc = 0, bcc_addrs = 0x811bd38 "", receipt_policy = E_ACCOUNT_RECEIPT_NEVER, pgp_key = 0x0, pgp_encrypt_to_self = 0, pgp_always_sign = 0, pgp_no_imip_sign = 0, pgp_always_trust = 0, parent_uid = 0x0, smime_sign_key = 0x0, smime_encrypt_key = 0x0, smime_sign_default = 0, smime_encrypt_to_self = 0, smime_encrypt_default = 0} (gdb)
Created attachment 126438 [details] [review] New version of the patch after fixing martyn's second series of remarks More info here: http://mail.gnome.org/archives/tracker-list/2009-January/msg00030.html
The patch looks fine, my main concern is still having something so specific built into tracker, but I guess we could sort out the predicate names, etc... afterwards, since we still get to maintain all the code...
Committed as 2794