After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 518589 - Rhythmbox does not adhere to freedesktop.org's XDG Base Directory Specification
Rhythmbox does not adhere to freedesktop.org's XDG Base Directory Specification
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: general
0.11.x
Other All
: Normal enhancement
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks: 523057 560824
 
 
Reported: 2008-02-25 11:44 UTC by Tim Kersten
Modified: 2011-07-26 19:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Move rhythmdb.xml and playlists.xml to ~/.local/share/rhythmbox folder (3.44 KB, patch)
2008-11-13 05:43 UTC, John Daiker
needs-work Details | Review
Updated to use g_get_user_data_dir() (3.42 KB, patch)
2008-11-14 17:04 UTC, John Daiker
needs-work Details | Review
Use g_file_copy, fix some leaks, more consistent coding style (4.44 KB, patch)
2008-11-15 00:55 UTC, John Daiker
needs-work Details | Review
Changes as per comment 15. (5.55 KB, patch)
2008-11-18 17:36 UTC, John Daiker
none Details | Review
Handle the playlists.xml in construct_sources instead of rb_shell_new (6.74 KB, patch)
2008-11-19 02:39 UTC, John Daiker
none Details | Review

Description Tim Kersten 2008-02-25 11:44:41 UTC
The specification can be found at: http://standards.freedesktop.org/basedir-spec/latest/index.html

From http://www.aigarius.com/blog/2007/01/10/fhs-extension-for-user-home-folders/ :
Currently there is a huge mess of files and folders that start with a “.” in any users home folder. There is no structure or policy on how applications should choose file and folder names for data that needs to be stored in users home directory. Additionally there is no established consistency between Gnome, KDE and most other applications. Gnome application have part of their configuration information in gconf folder and other part in a gnome subfolder. KDE applications have a complex structure under .kde/. And most other applications either have one file directly in users home folder or have their own dot-folder there.

There is a specification that deals with this!

From the specification:

##########################################
There is a single base directory relative to which user-specific data files should be written. This directory is defined by the environment variable $XDG_DATA_HOME.

There is a single base directory relative to which user-specific configuration files should be written. This directory is defined by the environment variable $XDG_CONFIG_HOME.

$XDG_DATA_HOME defines the base directory relative to which user specific data files should be stored. If $XDG_DATA_HOME is either not set or empty, a default equal to $HOME/.local/share should be used.

$XDG_CONFIG_HOME defines the base directory relative to which user specific configuration files should be stored. If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.
##########################################

The reasoning behind this is so that it's easy to remove config files while not affecting any user data. This is especially important when a user upgrades their OS, i.e. Ubuntu 7.10 -> Ubuntu 8.04, and wants to start with the default settings for their applications. It will also help organise the ever so horrid mess in the home folder.

A simple rule might be:
Data for which there exists sane defaults, is config data. Data which should not be lost is user data.

i.e. settings such as whether or not the user wants tabs to be used in their IM windows are "CONFIG" values, where as usernames, passwords, and more importantly chat history are user "DATA".
Comment 1 Tim Kersten 2008-02-25 11:55:17 UTC
Oh sorry, I realised that I made an example of usernames and passwords etc. This was intended for pidgin. I still think it would be nice if all programs were to adhere to the spec though. The home folder is a mess and the only way it will get tidy is if all programs adhere to this. In the case of Rhythmbox, keeping song statistics in user "DATA", but the preferences of "which plugin's are enabled", or "which columns are to be shown" in the user "CONFIG" folders.
Comment 2 antistress 2008-03-16 03:03:07 UTC
i agree, this an important bug considering :
- present awful mess in $home/ 
- the inability for a Linux user to simply make a backup of its files since data and config files are mixed on its hard drive instead of being stored at the right place

see that article for an illustration : 
Cleaning user preferences, keeping user data
http://ploum.frimouvy.org/?184-cleaning-user-preferences-keeping-user-data

Comment 3 Jonathan Matthew 2008-03-16 03:28:15 UTC
How would this help?  Moving rhythmdb.xml and playlists.xml from ~/.gnome2/rhythmbox/ to ~/.local/share/rhythmbox/ doesn't sound like much of a win to me.  Where are we mixing configuration and user data?
Comment 4 Lionel Dricot 2008-03-16 11:44:29 UTC
Jonathan : it would be a huge win because when you install a new computer or you recover from backup, you will know that you just have to restore your files and your ~/.local/.

Currently, restoring the whole .gnome2 folder can be a very painful experience (I experience this currently with fonts horribly broken in my whole gnome desktop since I upgraded from 2.20 to 2.22). It's especially the case if you explicitelly don't want to keep your old settings and just want to recover your data.
Comment 5 antistress 2008-03-16 17:03:36 UTC
besides, it may be hard for a global application to make a system backup, with possibility to choose data and/or config, if there's no standard rule for storing these files.

Standards are good for the desktop, i think
Comment 6 John Daiker 2008-11-13 05:43:32 UTC
Created attachment 122544 [details] [review]
Move rhythmdb.xml and playlists.xml to ~/.local/share/rhythmbox folder

I finally decided that this is a worthy bug to try and fix.  I recently upgraded from Hardy to Intrepid and had to deal with moving data from one /home to another.  I did this for lots of apps, include rhythmbox and xchat, among others.
Comment 7 antistress 2008-11-13 09:32:22 UTC
what about covers ?
Comment 8 Jonathan Matthew 2008-11-13 12:11:14 UTC
This should use g_get_user_data_dir() rather than g_get_home_dir() + /.local/share/.

(In reply to comment #7)
> what about covers ?

bug 502006.

Comment 9 John Daiker 2008-11-14 17:04:14 UTC
Created attachment 122672 [details] [review]
Updated to use g_get_user_data_dir()
Comment 10 John Daiker 2008-11-14 17:04:29 UTC
Adding me to CC... forgot last time.
Comment 11 Lionel Dricot 2008-11-14 20:07:06 UTC
If covers were automatically retrieved on the Internet, they should belong to .cache. Removing it is not a problem (and might be a good thing when upgrading RB and that the cover finder algo is improved). 

Proof : if you are connected to the Internet, removing the covers is completely invisible for the user.
Comment 12 Lionel Dricot 2008-11-14 20:26:09 UTC
Sorry, didn't see the comment of Jonathan
Comment 13 Jonathan Matthew 2008-11-14 21:56:38 UTC
+	} else if (!g_file_query_exists(g_file_new_for_path(g_build_filename (rb_xdg_dot_dir (), "playlists.xml", NULL)), NULL)) {

This is ugly and it leaks both the filename and the file object.

+	if (shell->priv->rhythmdb_file) {
+		rb_debug("Using rhythmdb_file for database");

That's not a very helpful piece of debug output.

+	/* Move the file to the new XDG folder */
+	g_file_move(src, dest, G_FILE_COPY_BACKUP, NULL, NULL, NULL, &error);

Why are you using G_FILE_COPY_BACKUP here?
What happens if an error occurs during the move operation?

Also, please pay attention to code style details (spacing, placement of brackets etc.)
Comment 14 John Daiker 2008-11-15 00:55:20 UTC
Created attachment 122706 [details] [review]
Use g_file_copy, fix some leaks, more consistent coding style

This still leaks ~100 bytes regardless of whether files are in the new and old locations or not.
rb-shell.c:977
xdg_file = g_file_new_for_path (pathname);

I do this same thing on line 1022, also, but don't see a leak there.  No ideas what I'm doing differently.  I literally copy/pasted the code from construct_db to rb_shell_new (with changes to playlists.xml instead of rhythmdb.xml).

I'll leave it up to brighter minds than mine.
Comment 15 Jonathan Matthew 2008-11-15 03:15:04 UTC
I wasn't saying that you should use g_file_copy rather than g_file_move, I was wondering why you were passing that specific flag.  g_file_move seems to be the right thing to do, as I don't think we want to leave the old file around.

It looks like this doesn't handle the case where neither the xdg file nor the ~/.gnome2/rhythmbox/ file exists - it appears that it would end up using the ~/.gnome2/ path, which is wrong.

For one time operations like this, we should report the full error (as returned by g_file_move) to the user.

Since the code for dealing with playlists.xml and rhythmdb.xml is effectively identical, it should be factored out into a new function, or rb_dot_to_xdg_dot should do it.

rb_dot_to_xdg_dot needs a better name, and it needs to be documented.  Perhaps:

/**
 * rb_find_user_data_file:
 * @name: name of file to find
 * @override: user-specified path name (or NULL)
 * @move_from_gnome_dir: if %TRUE, move the file from .gnome2/rhythmbox
 *  if it doesn't exist in the user data directory
 * @error: returns error information
 *
 *  .. some details ..
 *
 * Returns: allocated string containing the location of the file to use
 */
char *
rb_find_user_data_file (const char *name, const char *override, gboolean move_from_gnome_dir, GError **error)
{
  .. etc. ..
}

Comment 16 John Daiker 2008-11-18 17:36:17 UTC
Created attachment 122964 [details] [review]
Changes as per comment 15.

g_file_move is now used instead of g_file_copy.

The xdg folder _should_ be used now.  I tested this once and it seemed to work fine.

The error is pushed upward, but not all the way to the user.  rb_debug output is present, however.

The new rb_find_user_data_file function takes care of the file moving and determining the path to use for different files.

Documentation has been added.

I _think_ it doesn't leak anymore.  Two (quick) valgrind sessions didn't know anything from my new functions.
Comment 17 John Daiker 2008-11-19 02:39:45 UTC
Created attachment 122999 [details] [review]
Handle the playlists.xml in construct_sources instead of rb_shell_new
Comment 18 John Daiker 2008-12-17 00:28:03 UTC
ping?
Comment 19 Jonathan Matthew 2009-02-09 03:04:08 UTC
I'm going to commit this once I've finished fixing the following:

- the documentation isn't very useful; it should describe what the function does, not what it's used for
- it needs to check if the file exists in the user data dir first and only try moving from the .gnome2 dir if it doesn't
- if move_from_gnome_dir is FALSE, it should only check the user data dir
- the error dialog displayed when the move fails should include source and destination paths, or at least some idea of what they are
- some formatting things too
Comment 20 Jonathan Matthew 2009-02-16 08:55:28 UTC
OK, done and tested and whatever.

I'm closing this bug because everything except the catalog files for magnatune and jamendo are taken care of now, and those are covered in bug 502006.  I summarily decided that trying to move icons and plugins from ~/.gnome2/rhythmbox/ to the user data dir is too much effort.  For those, the old path will continue to work indefinitely, but the new path is preferred and should be used in any documentation.

2009-02-16  Jonathan Matthew  <jonathan@d14n.org>

        based on a patch by: John Daiker  <daikerjohn@gmail.com>

        * lib/rb-file-helpers.c: (rb_dot_dir), (rb_user_data_dir),
        (rb_user_cache_dir), (rb_find_user_data_file),
        (rb_file_helpers_shutdown):             
        * lib/rb-file-helpers.h:
        Add helpers to get our user data and cache directories, and for
        handling the migration of files from ~/.gnome2/rhythmbox/
        intelligently.
        
        * shell/rb-shell.c: (rb_shell_init), (rb_shell_new),
        (construct_db), (construct_sources):
        Use rb_find_user_data_file to migrate rhythmdb.xml and playlists.xml
        to the user data directory.

        * lib/rb-stock-icons.c: (rb_stock_icons_init):
        * plugins/rb-plugin.c: (rb_get_plugin_paths):
        Add user data dir search paths for icons and plugins

        * plugins/audioscrobbler/rb-audioscrobbler.c:
        (rb_audioscrobbler_load_queue), (rb_audioscrobbler_save_queue):
        Store audioscrobbler cache in user data dir.

        * tests/bench-rhythmdb-load.c: (main):
        Use rb_user_data_dir to find rhythmdb.xml
        
        * bindings/python/Makefile.am:
        * bindings/python/rb.defs:
        Add bindings for rb_user_data_dir and rb_user_cache_dir
        
        * plugins/artdisplay/artdisplay/CoverArtDatabase.py:
        Use rb_user_cache_dir to find the cover cache path.
        
        Fixes #518589 and #560824.