Hi Florent Viard, Jürg, I think, did the most investigating on this matter (on IRC) together with you, so I'll leave further reviews to Jürg.
Just FYI. Cheers, Philip On Wed, 2010-01-27 at 14:48 +0100, Florent Viard wrote: > Hi, > > Here is an updated version of the previous patch. > > Regards, > > Florent Viard > Software Development Engineer > ........................................ > [email protected] > Phone: +33 1 58 49 57 20 > Fax: +33 1 58 49 57 20 > 33, boulevard du Général Martial Valin > 75015 Paris - FRANCE > www.lacie.com > Please print only if necessary > > > > > -------- Message d'origine-------- > De: [email protected] de la part de Florent Viard > Date: mar. 26/01/2010 18:53 > À: [email protected] > Objet : [Tracker] RE : Little patch regarding the finding > configuration paths in libtracker and tracker-control. > > Hi, > > << gboolean path_has_write_access_or_was_created >> > was not a function that I created, i just removed its static flag and > also added its prototype to the header in order to use it from outside > file-utils.c (for tracker-control). I didn't pretend to rename your > functions :p but I think that you certainly could add a prefix to its > name. > > The first object of the patch was because XDG variable was used for > file-utils and HOME used for tracker-control, that could lead to an > inconsistency: in some case they wouldn't have looked for the config > files in the same place. > > Secondly, g_get_home_dir > (http://library.gnome.org/devel/glib/unstable/glib-Miscellaneous-Utility-Functions.html#g-get-home-dir) > looks at the passwd file to find the home dir. So it is better to look at it > only in a case of a missing HOME env var. (As it is easy to change the > tracker working dirs using env vars). Otherwise, it is better to stay > consistent in trying to use the XDG vars if they are defined. > > > > Florent Viard > Software Development Engineer > ........................................ > [email protected] > Phone: +33 1 58 49 57 20 > Fax: +33 1 58 49 57 20 > 33, boulevard du Général Martial Valin > 75015 Paris - FRANCE > www.lacie.com > Please print only if necessary > > > > > -------- Message d'origine-------- > De: Philip Van Hoof [mailto:[email protected]] > Date: mar. 26/01/2010 17:31 > À: Florent Viard > Cc: [email protected] > Objet : Re: [Tracker] Little patch regarding the finding configuration > paths in libtracker and tracker-control. > > On Tue, 2010-01-26 at 12:16 +0100, Florent Viard wrote: > > > -static gboolean > > +gboolean > > path_has_write_access_or_was_created (const gchar *path) > > { > > When you make a new public function, you have to prefix it with at > least > tracker_ > > > gboolean writable; > > @@ -673,8 +673,14 @@ > > return TRUE; > > } > > > > + user_data_dir = g_getenv ("HOME"); > > + > > + if (!user_data_dir) { > > + user_data_dir = g_get_home_dir (); > > + } > > Why isn't g_get_home_dir sufficient? Why the g_getenv? > > > /* Change environment, this is actually what we have on > Ubuntu. */ > > - new_dir = g_build_path (G_DIR_SEPARATOR_S, g_get_home_dir > (), ".local", "share", NULL); > > + new_dir = g_build_path (G_DIR_SEPARATOR_S, user_data_dir, > ".local", "share", NULL); > > > Index: source/src/libtracker-common/tracker-file-utils.h > > =================================================================== > > --- source/src/libtracker-common/tracker-file-utils.h (révision > 3508) > > +++ source/src/libtracker-common/tracker-file-utils.h (copie de > travail) > > @@ -49,6 +49,7 @@ > > const gchar > *basename_exception_prefix); > > gchar * tracker_path_evaluate_name (const gchar > *uri); > > > > +gboolean path_has_write_access_or_was_created (const gchar > *path); > > Use at least tracker_ as prefix, and come up with a better name if it > is > to become public. > > > crawler = tracker_crawler_new (); > > @@ -386,13 +386,24 @@ > > main_loop); > > > > /* Go through service files */ > > - home_dir = g_getenv ("HOME"); > > + > > > > - if (!home_dir) { > > - home_dir = g_get_home_dir (); > > + /* Check the default XDG_DATA_HOME location */ > > + home_conf_dir = g_getenv ("XDG_CONFIG_HOME"); > > Hmm, I thought g_get_home_dir checks for those environment variables > already. > > > + > > + if (home_conf_dir && > path_has_write_access_or_was_created (home_conf_dir)) { > > + path = g_build_path (G_DIR_SEPARATOR_S, > home_conf_dir, "tracker", NULL); > > + } else { > > + > > + home_conf_dir = g_getenv ("HOME"); > > What is wrong with just using g_get_home_dir ? > > > + if (!home_conf_dir) { > > + home_conf_dir = g_get_home_dir (); > > + } > > + path = g_build_path (G_DIR_SEPARATOR_S, > home_conf_dir, ".config", "tracker", NULL); > > } > > > > - path = g_build_path (G_DIR_SEPARATOR_S, home_dir, > ".config", "tracker", NULL); > > + > > file = g_file_new_for_path (path); > > g_free (path); > > > > -- > 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 > > > > > > > This e-mail and any attachment are confidential and intended solely > for the use of the individual to whom it is addressed. If you are not > the intended recipient, please telephone or email the sender and > delete this message and any attachment from your system. Unauthorized > publication, use, dissemination, forwarding, printing or copying of > this e-mail and its associated attachments is strictly prohibited. > > > > > > > This e-mail and any attachment are confidential and intended solely > for the use of the individual to whom it is addressed. If you are not > the intended recipient, please telephone or email the sender and > delete this message and any attachment from your system. Unauthorized > publication, use, dissemination, forwarding, printing or copying of > this e-mail and its associated attachments is strictly prohibited. > _______________________________________________ > 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
