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. > * 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 -- 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 [email protected] http://mail.gnome.org/mailman/listinfo/tracker-list
