Re: [Tracker] Review of the new Evolution/Generic E-mail metadata support for Tracker
On 14/01/09 18:04, Philip Van Hoof wrote:
On Wed, 2009-01-14 at 15:39 +, Martyn Russell wrote:
Thanks Philip.
OK, so I have finished commenting on the rest of the patch. I haven't
been able to build it yet because I am still building the evolution
development environment. So any further comments will be appended here
later.
Just want to say, that this is a huge patch and amount of work and you
have really done well to coordinate with the Evolution developers, the
rest of the Tracker developers and keep things up to date in Bugzilla
and on the mailing lists. So kudos to you Philip and thanks for doing
the work!
Thanks a lot for all those nice words
;)
#5, Just wondering why you did this?
+ tracker_module_config_init ();
+
tracker_turtle_init ();
tracker_thumbnailer_init (config);
- tracker_module_config_init ();
-
Ah, tracker_thumbnailer_init is getting a config passed that ain't
initialized.
You know that TrackerConfig isn't the same as TrackerModuleConfig right?
I don't think the module config needs to be intiated first.
#10, About these #defines, there are ways to specify the namespace used
by DBus from the xml files and this could avoid the need for these
defines. Did you know/try this?
I did, problem is that the XML is used twice. Once where the namespace
matches the C API, and one where it doesn't. This is for the one where
it doesn't.
I figured that this was more easy and more nice than having two XML
files.
Ah I see.
#19, I am a bit worried about the type mismatch here. I wonder if it
makes sense to have gint with the number of days not a time - it depends
on the granularity you want here I suppose.
+static guint
+get_stored_last_checkout (void)
+{
+ return (guint) tracker_data_manager_get_db_option_int
("EvolutionLastCheckout");
+}
No, in-seconds is the required granularity.
OK.
#27 I think we should collect all crack like this
(tracker-evolution-plugin.c) and keep them together. Also, I checked,
the DBus API is in my documentation. It looks like we just aren't
including the dbus-glib includes we need :)
+int e_plugin_lib_enable (EPluginLib *ep, int enable);
+/* Not in the public headers of DBus? */
+gchar* dbus_g_method_get_sender (DBusGMethodInvocation *context);
I had this fixed already. For future reference, the correct fix was:
#include
OK.
#30, Again, you have a lot of while (copy) { ...; next_in_list; } in
this code (tracker-evolution-plugin.c) - which would be better as a for
loop so the conditions are all clear at the top.
Leaving as is. At least for now.
#31, Can we avoid setting a local variable and testing the condition all
in one place like this:
+ if (!account->enabled ||!(uri = account->source->url))
+ return;
This is Evolution's structures. I have to check for both for
correctness. It's also Evolution who changes the values of those
instance's struct members, not just us.
Remember that this code runs in Evolution's process.
OK, as far as I could see, it was just being set to a variable in the
local scope (called uri) and used in that function intermittently as was
account->source->url - which is a bit confusing. But it is no big issue.
It isn't very clear. It makes more sense to me to just use
account->source->url the whole time, it is rarely used as url. Also
there are some spacing issues in that line above of course ;)
#32, There are a couple of char* cases which should be gchar* in this file.
+ char *uri = reg_info->uri;
Fixed that one, but note that some are Camel API matching. For example
Camel doesn't use gpointer but void*. Although the two are exactly the
same I prefer to reflect what the library's .h wants.
I agree. I suspected that might be the case.
#34, I see you use:
+ guint latest = smallest;
But guint can be 32 bits depending on architecture. It makes sense to
make this guint64 like the smallest variable is defined:
+ guint64 smallest = (guint64) time (NULL);
Agree, just forgot about that one while I was converting all those to
guint64. Thanks for spotting this, it would have been a horrible bug.
NP ;)
#35, Does it make more sense here to reference the connection
(tracker-evolution-registrar.c):
+ priv->connection = connection; /* weak */
DBusGConnection is not a GObject afaik.
We can use dbus_g_connection_ref() and dbus_g_connection_unref().
--
OK, from my point of view, we can commit this patch. Carlos are you
happy with this too or do you have anything further to add?
--
Regards,
Martyn
___
tracker-list mailing list
tracker-list@gnome.org
http://mail.gnome.org/mailman/listinfo/tracker-list
[Tracker] Releasing Tracker 0.6.90
Hi Jamie, So we have been in a state of bug fixing on Tracker trunk now for a good few months ironing out issues we have found and preparing for a release. We would like to make this release at the end of next week if possible. How do you feel about this? Would you like me to do the release or would you like to do it? Are there any issues/questions you have before releasing? -- Regards, Martyn ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
Re: [Tracker] Review of the new Evolution/Generic E-mail metadata support for Tracker
On Thu, 2009-01-15 at 11:20 +, Martyn Russell wrote: > On 14/01/09 18:04, Philip Van Hoof wrote: > OK, from my point of view, we can commit this patch. Carlos are you > happy with this too or do you have anything further to add? Carlos's final review happened on the bug. Committed as 2794 and closed the bug as fixed. Thanks for all the reviews! -- Philip Van Hoof, freelance software developer home: me at pvanhoof dot be gnome: pvanhoof at gnome dot org http://pvanhoof.be/blog http://codeminded.be ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
Re: [Tracker] Releasing Tracker 0.6.90
On Thu, 2009-01-15 at 11:36 +, Martyn Russell wrote: > Hi Jamie, > > So we have been in a state of bug fixing on Tracker trunk now for a good > few months ironing out issues we have found and preparing for a release. > > We would like to make this release at the end of next week if possible. > > How do you feel about this? > > Would you like me to do the release or would you like to do it? > I have not had time to keep up with all the changes can you schedule the release for beginning of next week so I can have a look/test over the weekend Im happy to delegate releasing to yourself - thanks for volunteering :) jamie ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
Re: [Tracker] Releasing Tracker 0.6.90
On 15/01/09 13:58, Jamie McCracken wrote: On Thu, 2009-01-15 at 11:36 +, Martyn Russell wrote: Hi Jamie, So we have been in a state of bug fixing on Tracker trunk now for a good few months ironing out issues we have found and preparing for a release. We would like to make this release at the end of next week if possible. How do you feel about this? Would you like me to do the release or would you like to do it? I have not had time to keep up with all the changes can you schedule the release for beginning of next week so I can have a look/test over the weekend I have asked the team, and the end of next week is when we would like to do this. It should give you a bit more time to review TRUNK. Im happy to delegate releasing to yourself - thanks for volunteering :) That's fine ;) -- Regards, Martyn ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
[Tracker] Crashes with tracker-indexer
Hi!
I see tracker-indexer crashing at the same point but not always with the
same files:
...
Tracker-Message: <--- [33] DBus request to check 1000 files
Tracker-Message: ---> [33] Success, no error given
(tracker-indexer:10627): Tracker-DEBUG: Parent
path:'/local/laguerre/FretsOnFire' exists in cache, should index
(tracker-indexer:10627): Tracker-DEBUG: Updating item
'/local/laguerre/FretsOnFire/pygame.mouse.so'
(tracker-indexer:10627): Tracker-DEBUG: Parent
path:'/local/laguerre/FretsOnFire' exists in cache, should index
(tracker-indexer:10627): Tracker-DEBUG: Updating item
'/local/laguerre/FretsOnFire/_random.so'
(tracker-indexer:10627): Tracker-DEBUG: Transaction commit
(tracker-indexer:10627): Tracker-DEBUG: Flushing index with 18 items in
cache
Program received signal SIGSEGV, Segmentation fault.
0x77bbf78d in indexer_update_word (indez=0x7518d0,
word=0x11e51e0 "_random", new_hits=0x115c580) at tracker-db-index.c:611
611 if (new_hit->id > previous_hits[center].id) {
Missing separate debuginfos, use: debuginfo-install
e2fsprogs-libs-1.41.3-2.fc10.x86_64 gamin-0.1.9-6.fc10.x86_64
gvfs-1.0.3-4.fc10.x86_64 hal-libs-0.5.12-12.20081027git.fc10.x86_64
keyutils-libs-1.2-3.fc9.x86_64 krb5-libs-1.6.3-16.fc10.x86_64
libcap-2.10-2.fc10.x86_64 libcurl-7.18.2-9.fc10.x86_64
libidn-0.6.14-8.x86_64 libselinux-2.0.73-1.fc10.x86_64
libssh2-0.18-7.fc9.x86_64 libxslt-1.1.24-2.fc10.x86_64
nspr-4.7.3-2.fc10.x86_64 nss-3.12.2.0-3.fc10.x86_64
openldap-2.4.12-1.fc10.x86_64 openssl-0.9.8g-12.fc10.x86_64
raptor-1.4.18-1.fc10.x86_64
(gdb) bt
#0 0x77bbf78d in indexer_update_word (indez=0x7518d0,
word=0x11e51e0 "_random", new_hits=0x115c580) at tracker-db-index.c:611
#1 0x77bbfa4a in cache_flush_item (key=0x11e51e0,
value=0x115c580, user_data=0x7518d0) at tracker-db-index.c:724
#2 0x77bc00bb in tracker_db_index_flush (indez=0x6840a0) at
tracker-db-index.c:878
#3 0x00408f49 in flush_data (indexer=0x778820) at
tracker-indexer.c:350
#4 0x003af9837f5b in g_timeout_dispatch (source=0x801bc0,
callback=0x7fffd44b, user_data=0x0) at gmain.c:3589
#5 0x003af983779b in g_main_dispatch () at gmain.c:2144
#6 IA__g_main_context_dispatch (context=0x6315a0) at gmain.c:2697
#7 0x003af983af6d in g_main_context_iterate (context=0x6315a0,
block=1, dispatch=1, self=) at gmain.c:2778
#8 0x003af983b49d in IA__g_main_loop_run (loop=0x799cc0) at
gmain.c:2986
#9 0x0040faf5 in main (argc=1, argv=0x7fffd928) at
tracker-main.c:396
(gdb) print center
$1 = 2147483647
(gdb) print right
$2 = 4294967295
(gdb) print left
$3 = 0
(gdb) print old_hit_count
$4 = 5
(gdb) print word
$5 = (const gchar *) 0x11e51e0 "_random"
(gdb) print tsiz
$6 = 40
Any ideas?
I tried to reindex all my documents but without any success.
Laurent.
signature.asc
Description: Ceci est une partie de message numériquement signée
___
tracker-list mailing list
tracker-list@gnome.org
http://mail.gnome.org/mailman/listinfo/tracker-list
Re: [Tracker] Releasing Tracker 0.6.90
Why the version number is 0.6.90, not 0.6.7? Hi Jamie, So we have been in a state of bug fixing on Tracker trunk now for a good few months ironing out issues we have found and preparing for a release. We would like to make this release at the end of next week if possible. How do you feel about this? Would you like me to do the release or would you like to do it? Are there any issues/questions you have before releasing? ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
Re: [Tracker] Releasing Tracker 0.6.90
On Fri, Jan 16, 2009 at 7:26 AM, Jerry Tan wrote: > Why the version number is 0.6.90, not 0.6.7? I personally would prefer next release to be 0.7 (2 major things happened: there was a serious refactoring and the indexer is split from the daemon, and too much else). Successive releases can be 0.7.x (FTS, etc) while Xesam (forever stalled) can be 0.8. -- my place on the web: floss-and-misc.blogspot.com ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
Re: [Tracker] Releasing Tracker 0.6.90
On Thu, Jan 15, 2009 at 1:36 PM, Martyn Russell wrote: > Hi Jamie, > > So we have been in a state of bug fixing on Tracker trunk now for a good few > months ironing out issues we have found and preparing for a release. > > We would like to make this release at the end of next week if possible. > > How do you feel about this? > > Would you like me to do the release or would you like to do it? > > Are there any issues/questions you have before releasing? I'm not Jamie, but how about reviewing the docs before releasing... -- my place on the web: floss-and-misc.blogspot.com ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
Re: [Tracker] Releasing Tracker 0.6.90
Hi all, El vie, 16-01-2009 a las 08:44 +0200, ext Tshepang Lekhonkhobe escribió: > On Fri, Jan 16, 2009 at 7:26 AM, Jerry Tan wrote: > > Why the version number is 0.6.90, not 0.6.7? We plan to break quite seriously the APIs and the internals of tracker in the next releases, so this release is "the last one with this API" (maybe we have some bugfixing release as 0.6.91 or 0.6.9.1) > I personally would prefer next release to be 0.7 (2 major things > happened: there was a serious refactoring and the indexer is split > from the daemon, and too much else). Big changes indeed, but the API and overall idea of tracker is the same. We rewrote the code, but still kept a lot of things (e.g. the DB schema and libtracker). > Successive releases can be 0.7.x (FTS, etc) while Xesam (forever > stalled) can be 0.8. Updating plans: For 0.7 series the plan is to move to FTS (new index) and to a decomposed tables approach (new DB schema), with a new query language (SparQL) and support for a "real" ontology (probably nepomuk). We should also think about a new libtracker with a better API for application developers. These are _really big_ changes :) Currently there is some work ongoing for this 0.7 series in a git repository (http://live.gnome.org/Tracker/Decomposed) that would be eventually moved to trunk after Jamie's review. Just mention that in 0.6.90 we have put a lot of work, and it is working pretty well; so don't forget it just because 0.7 is going to be _even better_. Regards, Ivan ___ tracker-list mailing list tracker-list@gnome.org http://mail.gnome.org/mailman/listinfo/tracker-list
