After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 565091 - Bug for tracking progression on the Evolution metadata support
Bug for tracking progression on the Evolution metadata support
Status: RESOLVED FIXED
Product: tracker
Classification: Core
Component: General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Philip Van Hoof
Jamie McCracken
Depends on: 565082 565681 566279
Blocks:
 
 
Reported: 2008-12-19 13:29 UTC by Philip Van Hoof
Modified: 2009-01-15 12:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Unfinished patch (40.07 KB, patch)
2008-12-19 13:30 UTC, Philip Van Hoof
none Details | Review
Newer version, equally untested (58.54 KB, patch)
2008-12-23 12:17 UTC, Philip Van Hoof
none Details | Review
Newer version, some things are actually working now (57.20 KB, patch)
2008-12-24 16:21 UTC, Philip Van Hoof
none Details | Review
Same patch, most things in the EPlugin are working now (59.09 KB, patch)
2008-12-26 11:42 UTC, Philip Van Hoof
none Details | Review
Same patch, integrating it into tracker-indexer (69.41 KB, patch)
2008-12-29 14:24 UTC, Philip Van Hoof
none Details | Review
Enabling, disabling should now work. Little bit of refactoring (77.07 KB, patch)
2008-12-30 17:21 UTC, Philip Van Hoof
none Details | Review
Further integrating the plugin (90.71 KB, patch)
2008-12-31 11:21 UTC, Philip Van Hoof
none Details | Review
Bugfixes for activating and deactivating (91.36 KB, patch)
2008-12-31 11:44 UTC, Philip Van Hoof
none Details | Review
Added a bunch of predicates, more data is being passed now (103.18 KB, patch)
2009-01-05 18:02 UTC, Philip Van Hoof
none Details | Review
Several smaller bugfixes against last patch (104.23 KB, patch)
2009-01-05 19:08 UTC, Philip Van Hoof
none Details | Review
More bugfixes, more cleaning up, more glitch-removals (10.91 KB, patch)
2009-01-06 14:16 UTC, Philip Van Hoof
none Details | Review
Wrong file attached (105.70 KB, patch)
2009-01-06 14:18 UTC, Philip Van Hoof
none Details | Review
Preparing official proposal for review (108.16 KB, patch)
2009-01-07 16:21 UTC, Philip Van Hoof
none Details | Review
Please review (108.73 KB, patch)
2009-01-09 13:42 UTC, Philip Van Hoof
none Details | Review
First iteration after martyn's review (110.12 KB, patch)
2009-01-12 15:14 UTC, Philip Van Hoof
none Details | Review
Please review, both martyn's and carlo's remarks are addressed in this one (110.95 KB, patch)
2009-01-12 16:05 UTC, Philip Van Hoof
none Details | Review
New version of the patch after fixing martyn's second series of remarks (112.92 KB, patch)
2009-01-14 18:06 UTC, Philip Van Hoof
none Details | Review

Description Philip Van Hoof 2008-12-19 13:29:33 UTC
This is a bug for tracking the progression of the support of (x) in Tracker.

#x http://live.gnome.org/Evolution/Metadata
Comment 1 Philip Van Hoof 2008-12-19 13:30:30 UTC
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.
Comment 2 Philip Van Hoof 2008-12-23 12:17:55 UTC
Created attachment 125191 [details] [review]
Newer version, equally untested
Comment 3 Philip Van Hoof 2008-12-24 16:21:18 UTC
Created attachment 125272 [details] [review]
Newer version, some things are actually working now
Comment 4 Philip Van Hoof 2008-12-26 11:42:35 UTC
Created attachment 125330 [details] [review]
Same patch, most things in the EPlugin are working now
Comment 5 Philip Van Hoof 2008-12-26 11:46:47 UTC
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).
Comment 6 Philip Van Hoof 2008-12-29 14:24:11 UTC
Created attachment 125477 [details] [review]
Same patch, integrating it into tracker-indexer
Comment 7 Philip Van Hoof 2008-12-30 17:21:48 UTC
Created attachment 125537 [details] [review]
Enabling, disabling should now work. Little bit of refactoring
Comment 8 Philip Van Hoof 2008-12-31 11:21:36 UTC
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
Comment 9 Philip Van Hoof 2008-12-31 11:44:35 UTC
Created attachment 125571 [details] [review]
Bugfixes for activating and deactivating
Comment 10 Philip Van Hoof 2009-01-05 18:02:55 UTC
Created attachment 125797 [details] [review]
Added a bunch of predicates, more data is being passed now
Comment 11 Philip Van Hoof 2009-01-05 19:08:33 UTC
Created attachment 125801 [details] [review]
Several smaller bugfixes against last patch
Comment 12 Philip Van Hoof 2009-01-06 14:16:12 UTC
Created attachment 125854 [details] [review]
More bugfixes, more cleaning up, more glitch-removals
Comment 13 Philip Van Hoof 2009-01-06 14:18:45 UTC
Created attachment 125855 [details] [review]
Wrong file attached
Comment 14 Philip Van Hoof 2009-01-07 16:21:09 UTC
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:
Comment 15 Philip Van Hoof 2009-01-09 13:42:06 UTC
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
Comment 16 Carlos Garnacho 2009-01-12 14:08:58 UTC
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?
Comment 17 Philip Van Hoof 2009-01-12 14:59:58 UTC
> - 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.






Comment 18 Philip Van Hoof 2009-01-12 15:14:26 UTC
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.
Comment 19 Philip Van Hoof 2009-01-12 15:19:37 UTC
> - 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


Comment 20 Philip Van Hoof 2009-01-12 16:05:09 UTC
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.
Comment 21 Carlos Garnacho 2009-01-12 16:25:58 UTC
(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
Comment 22 Carlos Garnacho 2009-01-12 16:28:24 UTC
(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 23 Philip Van Hoof 2009-01-13 11:33:29 UTC
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) 
Comment 24 Philip Van Hoof 2009-01-14 18:06:41 UTC
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
Comment 25 Carlos Garnacho 2009-01-15 12:28:39 UTC
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...
Comment 26 Philip Van Hoof 2009-01-15 12:59:35 UTC
Committed as 2794