On mié, 2009-02-11 at 15:15 +0100, Philip Van Hoof wrote: > On Wed, 2009-02-11 at 15:09 +0100, Carlos Garnacho wrote: > > > Overall, great patch :), makes things quite more generic. A few comments > > about it: > > > > * What about using GTypeModule and GInterfaces for the modules? you > > could have a single module that implements one interface for the indexer > > and other interface for trackerd. > > I tried this, I found it too complicated and more difficult for higher > languages to integrate with. For example this way it's relatively easy > to write a push module in Vala. > > There were also a few things that were not possible, or quite difficult > at least. I had started the refactor to make them similar to how your > module-sys works. But that was not possible for a couple of reasons. > > I decided to just use GModule (it's only about a few methods anyway). > > > * Do we really have to expose TrackerIndexer? I'd rather keep that as an > > internal detail, and this wouldn't work as is for external modules, we'd > > have to install a bunch of headers first... If we went through > > GTypeModule, modules could trigger signals that the indexer would > > listen, without any need to expose it to modules. > > I think TrackerIndexer can be left out for the current modules. I added > it because for future ones it might be necessary to have it (for example > to pause and continue it). > > Pausing, pre-committing and continuing it something I think will > eventually be necessary to add (for example a AddMany that adds really a > lot of triples, should pause+pre-commit, import, and then continue the > indexer. > > Kinda like freeze/thaw.
Hmm, I see... but I still think it's TrackerIndexer which should decide upon external factors, and not let 3rd party modules fiddle with tracker-indexer behavior, also it sounds to me like push modules developers have to be quite aware of tracker-indexer inner workings to make their module work sensibly. > > > * in src/tracker-indexer/tracker-push.c (load_modules): there's no good > > reason for gotos if it's just used in a single place, just make that > > return immediately if the directory couldn't be opened. > > Ok, I'll replace that. > > > * in src/tracker-indexer/tracker-push.h: the .c file doesn't implement > > tracker_push_module_[create|shutdown] > > Ah, oeps. > > > * in src/tracker-indexer/Makefile.am and src/trackerd/Makefile.am: No > > need to discern between "pushi" and "pushd", since there's no naming > > collision. > > Okay, I'll just use "push" everywhere then. > > Thanks! > > > On mar, 2009-02-03 at 11:52 +0100, Philip Van Hoof wrote: > > > This is a new patch but with the indentation right for the two new files > > > this time > > > > > > On Tue, 2009-02-03 at 00:40 +0100, Philip Van Hoof wrote: > > > > Hi there, > > > > > > > > This long E-mail is also a request for a review of the attached patch, > > > > Martyn. > > > > > > > > You can call me crazy for working on this until 24 'O clock, but I guess > > > > I wanted to have it done today. > > > > > > > > Because Ivan Frade told me he likes the concept of letting applications > > > > push in data into Tracker for many situations, we decided to refactor > > > > the basis of the Evolution support that I recently finished [1]. > > > > > > > > [1] http://live.gnome.org/Evolution/Metadata (note that this is now a > > > > desktop neutral standard) > > > > > > > > I made this part GModule based. Meaning that you can add more such > > > > "Evolution like" modules. That is also the plan. For example one module > > > > for KMail and very different one for an RSS web grabber will be made > > > > very soon. Perhaps we'll make other ones for other kinds of apps too. > > > > > > > > How it works is simple, but can get a little bit confusing to read in C. > > > > I recommend reading this Vala example, as it does the same thing [2]. > > > > > > > > [2] http://live.gnome.org/Vala/DBusClientSamples/Waiting > > > > > > > > In Tracker a few more things got added, like the GModule support. > > > > > > > > tracker-push.c in trackerd is more or less a GModule loader and at the > > > > same time what the Vala example does [2] (go read it, in code it's much > > > > more easy to grasp than reading this gibberish explanation!): > > > > > > > > -> Do a ListNames on DBus initially and if our service is among the ones > > > > we want to consume, we more or less launch the GModule's > > > > capabilities. > > > > > > > > -> Then we start listening for NameOwnerChanged to know when we either > > > > need to shutdown the GModule's capabilities, or when we need to > > > > launch it. > > > > > > > > Use-cases: > > > > > > > > o. Evolution is not running, trackerd starts. User now starts Evolution. > > > > The NameOwnerChanged will detect this event and will launch the > > > > GModule for Evolution. > > > > > > > > o. Evolution is running, trackerd starts. ListNames will find the > > > > service for us and our handler will therefore launch the GModule. > > > > > > > > o. We were once registered with Evolution, we're still running. But now > > > > Evolution shuts down. The NameOwnerChanged will detect this event and > > > > will disable the capabilities of the GModule for Evolution. > > > > > > > > Now replace Evolution with KMail if you prefer the KDE desktop. Same > > > > story. > > > > > > > > We decided to refactor this because we want to enable Vala, Python, C#, > > > > Perl, D, C++, YodaYodi++#1^^ developers to develop modules. > > > > > > > > This means that it's all GObject & GModule based now. > > > > > > > > You install a GModule that has a function that returns a GObject that > > > > implements some methods in a directory A, and one in a directory B. > > > > > > > > Why two? Well one for the indexer and one for the daemon. The deamon is > > > > for your 'things' that need to be kept alive for a long time. The > > > > indexer is for your 'things' that will do a lot of DB transactions and > > > > insertions. > > > > > > > > For for example the Evolution one the deamon one proxies RDF triples to > > > > the indexer one for this reason. > > > > > > > > You can take a look at the existing Evolution one's Makefile.am to see > > > > how to deal with that, where to install it, etc. > > > > > > > > Anyway, here's the patch. Please review asap as we'll need this for > > > > followup tasks soon (like the RSS and the KMail support). > > > > > > > > > > > > Thanks! > > > > > > > _______________________________________________ > > > tracker-list mailing list > > > [email protected] > > > http://mail.gnome.org/mailman/listinfo/tracker-list > > > > _______________________________________________ > > tracker-list mailing list > > [email protected] > > http://mail.gnome.org/mailman/listinfo/tracker-list > _______________________________________________ tracker-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/tracker-list
