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
........................................
fviard lacie com
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:spam pvanhoof be]
Date: mar. 26/01/2010 17:31
À: Florent Viard
Cc: tracker-list gnome org
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