diff --git a/docs/changelog.rst b/docs/changelog.rst index c8007e7b..741956b9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,6 +14,27 @@ Bug fix release. backends with a library provider. Previously, it wrongly worked for all backends with a playlists provider. (Fixes: :issue:`1257`) +- Core: Respect :confval:`core/cache_dir` and :confval:`core/data_dir` config + values added in 1.1.0 when creating the dirs Mopidy need to store data. This + should not change the behavior for desktop users running Mopidy. When running + Mopidy as a system service installed from a package which sets the core dir + configs properly (e.g. Debian and Arch packages), this fix avoids the + creation of a couple of directories that should not be used, typically + :file:`/var/lib/mopidy/.local` and :file:`/var/lib/mopidy/.cache`. (Fixes: + :issue:`1259`, PR: :issue:`1266`) + +- Local: Deprecate :confval:`local/data_dir` and respect + :confval:`core/data_dir` instead. This does not change the defaults for + desktop users, only system services installed from packages that properly set + :confval:`core/data_dir`, like the Debian and Arch packages. (Fixes: + :issue:`1259`, PR: :issue:`1266`) + +- M3U: Changed default for the :confval:`m3u/playlists_dir` from + ``$XDG_DATA_DIR/mopidy/m3u`` to unset, which now means the extension's data + dir. This does not change the defaults for desktop users, only system + services installed from packages that properly set :confval:`core/data_dir`, + like the Debian and Arch pakages. (Fixes: :issue:`1259`, PR: :issue:`1266`) + - Stream: If "file" is present in the :confval:`stream/protocols` config value and the :ref:`ext-file` extension is enabled, we exited with an error because two extensions claimed the same URI scheme. We now log a warning recommending diff --git a/docs/ext/local.rst b/docs/ext/local.rst index 18f66adc..d6e3b56a 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -47,8 +47,8 @@ active at a time. To create a new library provider you must create class that implements the :class:`mopidy.local.Library` interface and install it in the extension registry under ``local:library``. Any data that the library needs to store on -disc should be stored in :confval:`local/data_dir` using the library name as -part of the filename or directory to avoid any conflicts. +disc should be stored in the extension's data dir, as returned by +:meth:`~mopidy.ext.Extension.get_data_dir`. Configuration diff --git a/docs/ext/m3u.rst b/docs/ext/m3u.rst index d05f88f1..2b86b73a 100644 --- a/docs/ext/m3u.rst +++ b/docs/ext/m3u.rst @@ -52,4 +52,5 @@ See :ref:`config` for general help on configuring Mopidy. .. confval:: m3u/playlists_dir - Path to directory with M3U files. + Path to directory with M3U files. Unset by default, in which case the + extension's data dir is used to store playlists. diff --git a/mopidy/__main__.py b/mopidy/__main__.py index ee359268..208d2ff1 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -75,15 +75,16 @@ def main(): args = root_cmd.parse(mopidy_args) - create_file_structures_and_config(args, extensions_data) - check_old_locations() - config, config_errors = config_lib.load( args.config_files, [d.config_schema for d in extensions_data], [d.config_defaults for d in extensions_data], args.config_overrides) + create_core_dirs(config) + create_initial_config_file(args, extensions_data) + check_old_locations() + verbosity_level = args.base_verbosity_level if args.verbosity_level: verbosity_level += args.verbosity_level @@ -166,12 +167,17 @@ def main(): raise -def create_file_structures_and_config(args, extensions_data): - path.get_or_create_dir(b'$XDG_DATA_DIR/mopidy') - path.get_or_create_dir(b'$XDG_CONFIG_DIR/mopidy') +def create_core_dirs(config): + path.get_or_create_dir(config['core']['cache_dir']) + path.get_or_create_dir(config['core']['config_dir']) + path.get_or_create_dir(config['core']['data_dir']) + + +def create_initial_config_file(args, extensions_data): + """Initialize whatever the last config file is with defaults""" - # Initialize whatever the last config file is with defaults config_file = args.config_files[-1] + if os.path.exists(path.expand_path(config_file)): return diff --git a/mopidy/local/__init__.py b/mopidy/local/__init__.py index ff61c17c..3ee2703e 100644 --- a/mopidy/local/__init__.py +++ b/mopidy/local/__init__.py @@ -23,7 +23,7 @@ class Extension(ext.Extension): schema = super(Extension, self).get_config_schema() schema['library'] = config.String() schema['media_dir'] = config.Path() - schema['data_dir'] = config.Path() + schema['data_dir'] = config.Deprecated() schema['playlists_dir'] = config.Deprecated() schema['tag_cache_file'] = config.Deprecated() schema['scan_timeout'] = config.Integer( diff --git a/mopidy/local/ext.conf b/mopidy/local/ext.conf index ebd7962f..6dd29147 100644 --- a/mopidy/local/ext.conf +++ b/mopidy/local/ext.conf @@ -2,7 +2,6 @@ enabled = true library = json media_dir = $XDG_MUSIC_DIR -data_dir = $XDG_DATA_DIR/mopidy/local scan_timeout = 1000 scan_flush_threshold = 1000 scan_follow_symlinks = false diff --git a/mopidy/local/json.py b/mopidy/local/json.py index 0be5e99e..501990ee 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -12,7 +12,7 @@ import tempfile import mopidy from mopidy import compat, local, models from mopidy.internal import encoding, timer -from mopidy.local import search, storage, translator +from mopidy.local import Extension, search, storage, translator logger = logging.getLogger(__name__) @@ -116,7 +116,7 @@ class JsonLibrary(local.Library): self._browse_cache = None self._media_dir = config['local']['media_dir'] self._json_file = os.path.join( - config['local']['data_dir'], b'library.json.gz') + Extension().get_data_dir(config), b'library.json.gz') storage.check_dirs_and_files(config) diff --git a/mopidy/local/storage.py b/mopidy/local/storage.py index 1808c4a2..aaa74fba 100644 --- a/mopidy/local/storage.py +++ b/mopidy/local/storage.py @@ -3,8 +3,6 @@ from __future__ import absolute_import, unicode_literals import logging import os -from mopidy.internal import encoding, path - logger = logging.getLogger(__name__) @@ -13,10 +11,3 @@ def check_dirs_and_files(config): logger.warning( 'Local media dir %s does not exist.' % config['local']['media_dir']) - - try: - path.get_or_create_dir(config['local']['data_dir']) - except EnvironmentError as error: - logger.warning( - 'Could not create local data dir: %s', - encoding.locale_decode(error)) diff --git a/mopidy/m3u/__init__.py b/mopidy/m3u/__init__.py index e0fcf305..06825932 100644 --- a/mopidy/m3u/__init__.py +++ b/mopidy/m3u/__init__.py @@ -21,7 +21,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() - schema['playlists_dir'] = config.Path() + schema['playlists_dir'] = config.Path(optional=True) return schema def setup(self, registry): diff --git a/mopidy/m3u/actor.py b/mopidy/m3u/actor.py index fe959d86..fc4734a2 100644 --- a/mopidy/m3u/actor.py +++ b/mopidy/m3u/actor.py @@ -4,7 +4,7 @@ import logging import pykka -from mopidy import backend +from mopidy import backend, m3u from mopidy.internal import encoding, path from mopidy.m3u.library import M3ULibraryProvider from mopidy.m3u.playlists import M3UPlaylistsProvider @@ -21,12 +21,16 @@ class M3UBackend(pykka.ThreadingActor, backend.Backend): self._config = config - try: - path.get_or_create_dir(config['m3u']['playlists_dir']) - except EnvironmentError as error: - logger.warning( - 'Could not create M3U playlists dir: %s', - encoding.locale_decode(error)) + if config['m3u']['playlists_dir'] is not None: + self._playlists_dir = config['m3u']['playlists_dir'] + try: + path.get_or_create_dir(self._playlists_dir) + except EnvironmentError as error: + logger.warning( + 'Could not create M3U playlists dir: %s', + encoding.locale_decode(error)) + else: + self._playlists_dir = m3u.Extension().get_data_dir(config) self.playlists = M3UPlaylistsProvider(backend=self) self.library = M3ULibraryProvider(backend=self) diff --git a/mopidy/m3u/ext.conf b/mopidy/m3u/ext.conf index 0e828b1b..adc0d00a 100644 --- a/mopidy/m3u/ext.conf +++ b/mopidy/m3u/ext.conf @@ -1,3 +1,3 @@ [m3u] enabled = true -playlists_dir = $XDG_DATA_DIR/mopidy/m3u +playlists_dir = diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index af92e062..3567f8aa 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -23,7 +23,7 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): def __init__(self, *args, **kwargs): super(M3UPlaylistsProvider, self).__init__(*args, **kwargs) - self._playlists_dir = self.backend._config['m3u']['playlists_dir'] + self._playlists_dir = self.backend._playlists_dir self._playlists = {} self.refresh() diff --git a/tests/data/library.json.gz b/tests/data/local/library.json.gz similarity index 100% rename from tests/data/library.json.gz rename to tests/data/local/library.json.gz diff --git a/tests/local/test_json.py b/tests/local/test_json.py index 520287ad..169ba743 100644 --- a/tests/local/test_json.py +++ b/tests/local/test_json.py @@ -45,10 +45,11 @@ class BrowseCacheTest(unittest.TestCase): class JsonLibraryTest(unittest.TestCase): config = { + 'core': { + 'data_dir': path_to_data_dir(''), + }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), - 'playlists_dir': b'', 'library': 'json', }, } diff --git a/tests/local/test_library.py b/tests/local/test_library.py index 7763057f..4142d6c3 100644 --- a/tests/local/test_library.py +++ b/tests/local/test_library.py @@ -65,10 +65,11 @@ class LocalLibraryProviderTest(unittest.TestCase): ] config = { + 'core': { + 'data_dir': path_to_data_dir(''), + }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), - 'playlists_dir': b'', 'library': 'json', }, } @@ -105,11 +106,15 @@ class LocalLibraryProviderTest(unittest.TestCase): tmpdir = tempfile.mkdtemp() try: - tmplib = os.path.join(tmpdir, 'library.json.gz') - shutil.copy(path_to_data_dir('library.json.gz'), tmplib) + tmpdir_local = os.path.join(tmpdir, 'local') + shutil.copytree(path_to_data_dir('local'), tmpdir_local) - config = {'local': self.config['local'].copy()} - config['local']['data_dir'] = tmpdir + config = { + 'core': { + 'data_dir': tmpdir, + }, + 'local': self.config['local'], + } backend = actor.LocalBackend(config=config, audio=None) # Sanity check that value is in the library @@ -117,6 +122,7 @@ class LocalLibraryProviderTest(unittest.TestCase): self.assertEqual(result, self.tracks[0:1]) # Clear and refresh. + tmplib = os.path.join(tmpdir_local, 'library.json.gz') open(tmplib, 'w').close() backend.library.refresh() diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index bab70847..b99f8508 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -23,12 +23,11 @@ from tests.local import generate_song, populate_tracklist class LocalPlaybackProviderTest(unittest.TestCase): config = { 'core': { + 'data_dir': path_to_data_dir(''), 'max_tracklist_length': 10000, }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), - 'playlists_dir': b'', 'library': 'json', } } diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index 72da3f13..b7ed7dcb 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -18,11 +18,11 @@ from tests.local import generate_song, populate_tracklist class LocalTracklistProviderTest(unittest.TestCase): config = { 'core': { + 'data_dir': path_to_data_dir(''), 'max_tracklist_length': 10000 }, 'local': { 'media_dir': path_to_data_dir(''), - 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', 'library': 'json', }