GNOME Bugzilla – Bug 641115
grilo plugins do not store private data in right places
Last modified: 2012-04-25 14:38:28 UTC
Some plugins, like podcasts, bookmarks or metadata-store creates their own files to store information. These files are saved in $HOME. Freedesktop has a directory specification (XDG) telling where files of this sort should be saved. Plugins should follow this spec and store their files in the appropriate places.
Created attachment 203439 [details] [review] Patch for bookmarks plugin Follow XDG spec to save data files.
Created attachment 203440 [details] [review] Metadata store patch
Created attachment 203441 [details] [review] Podcasts plugin patch
Hi, any plans to apply this patches?
(In reply to comment #4) > Hi, any plans to apply this patches? Yes, I'm looking into them now, but they seem to be fine, so expect them to be applied quite soon :-)
Simon, if you had glancing it, could you update the review status of patches to proper state?
I got a server error when trying to mark them as "Reviewed - Commit now". The patches are fine: they do what they claim to do. I just wonder if we will need a way to save these databases in an application-specific place. Otherwise, you would share them between all apps using Grilo, right?
I've pushed them to master. As for 0.1.x, I'm not sure what to do: applications would stop loading their databases if they get this change, unless some update script is used to move the old databases to the new location.
(In reply to comment #7) > I just wonder if we will need a way to save these databases in an > application-specific place. Otherwise, you would share them between all apps > using Grilo, right? That's a good point. Right now it is done so all bookmarks are shared among all applications. Maybe we can use GrlConfig to specify a private place for each application. For instance, something like passing a "token" string to create the DB in a token-based place, or something like that.
*** Bug 674687 has been marked as a duplicate of this bug. ***
Created attachment 212674 [details] [review] Bookmarks plugin patch (0.1.x) Save the data files in the directory specified by the XDG Base Directory Specification. If previous data files exist in $HOME, move them to the new place. The patch is based in the ones attached by Juan, in addition, it preserves the old data moving the existing database in $HOME (if any) to the XDG directory.
Created attachment 212675 [details] [review] Podcast plugin patch (0.1.x) Save the data files in the directory specified by the XDG Base Directory Specification. If previous data files exist in $HOME, move them to the new place. The patch is based in the ones attached by Juan, in addition, it preserves the old data moving the existing database in $HOME (if any) to the XDG directory.
Created attachment 212676 [details] [review] Metadata-store plugin patch (0.1.x) Save the data files in the directory specified by the XDG Base Directory Specification. If previous data files exist in $HOME, move them to the new place. The patch is based in the ones attached by Juan, in addition, it preserves the old data moving the existing database in $HOME (if any) to the XDG directory.
Review of attachment 212674 [details] [review]: ::: src/media/bookmarks/grl-bookmarks.c @@ +264,3 @@ + db_path = g_strconcat (path, G_DIR_SEPARATOR_S, GRL_SQL_DB, NULL); + if (!g_file_test (db_path, G_FILE_TEST_EXISTS)) { + GRL_DEBUG ("Moving the previous database (if exists) to $XDG_DATA_HOME"); Two points here: - I would only print the message if we are actually going to move the old DB (which would be in case it exists) - I wouldn't print "moving to $XDG_DATA_HOME", because actually we aren't moving the file to that directory, but to a place inside that directory. @@ +265,3 @@ + if (!g_file_test (db_path, G_FILE_TEST_EXISTS)) { + GRL_DEBUG ("Moving the previous database (if exists) to $XDG_DATA_HOME"); + home = g_getenv ("HOME"); Better let's use g_get_home_dir() function
Review of attachment 212675 [details] [review]: Same issues as in the case of bookmarks patch
Review of attachment 212676 [details] [review]: Same issues as in the case of bookmarks patch
Created attachment 212760 [details] [review] Bookmarks plugin patch (0.1.x) v2 Updated patch according to comments in comment 4
(In reply to comment #17) > Created an attachment (id=212760) [details] [review] > Bookmarks plugin patch (0.1.x) v2 > > Updated patch according to comments in comment 4 ^^^ comment 14
Created attachment 212762 [details] [review] Podcast plugin patch (0.1.x) v2 Updated patch according to comments in comment 14
Created attachment 212763 [details] [review] Metadata store patch (0.1.x) v2 Updated patch according to comments in comment 14
Review of attachment 212760 [details] [review]: Patch looks good.
Review of attachment 212762 [details] [review]: Patch looks good.
Review of attachment 212763 [details] [review]: Patch looks good
Comment on attachment 212760 [details] [review] Bookmarks plugin patch (0.1.x) v2 commit 53ca72b8f00812b540a8bbb129377c6f5e28e975 Author: Antía Puentes <apuentes@igalia.com> Date: Tue Apr 24 11:38:11 2012 +0200 bookmarks: Save data in proper place Save the data files in the directory specified by the XDG Base Directory Specification. If previous data files exist in $HOME, move them to the new place. Partially fixes https://bugzilla.gnome.org/show_bug.cgi?id=641115 src/media/bookmarks/grl-bookmarks.c | 41 +++++++++++++++++++++++++++------- 1 files changed, 32 insertions(+), 9 deletions(-)
Comment on attachment 212762 [details] [review] Podcast plugin patch (0.1.x) v2 commit 096ee2ec4801c781075cd6824e5ecbe2e40f86b1 Author: Antía Puentes <apuentes@igalia.com> Date: Tue Apr 24 11:41:53 2012 +0200 podcasts: Save data in proper place Save the data files in the directory specified by the XDG Base Directory Specification. If previous data files exist in $HOME, move them to the new place. Partially fixes https://bugzilla.gnome.org/show_bug.cgi?id=641115 src/media/podcasts/grl-podcasts.c | 41 ++++++++++++++++++++++++++++-------- 1 files changed, 32 insertions(+), 9 deletions(-)
Comment on attachment 212763 [details] [review] Metadata store patch (0.1.x) v2 commit 308491a4aa79fd3f23ef0845fabbb08017e37392 Author: Antía Puentes <apuentes@igalia.com> Date: Tue Apr 24 11:42:53 2012 +0200 metadata-store: Save data in proper place Save the data files in the directory specified by the XDG Base Directory Specification. If previous data files exist in $HOME, move them to the new place. Partially fixes https://bugzilla.gnome.org/show_bug.cgi?id=641115 src/metadata/metadata-store/grl-metadata-store.c | 43 +++++++++++++++++----- 1 files changed, 33 insertions(+), 10 deletions(-)