From 0a4709f7ef231ff6499f8ef24aeaa5017ce8d07c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 13 Feb 2017 16:54:56 -0500 Subject: [PATCH 01/12] lyrics: Tolerate empty Google response (#2437) --- beetsplug/lyrics.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index c020a93ae..6714b2fee 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -564,11 +564,17 @@ class Google(Backend): urllib.parse.quote(query.encode('utf-8'))) data = self.fetch_url(url) - data = json.loads(data) + if not data: + self._log.debug(u'google backend returned no data') + return None + try: + data = json.loads(data) + except ValueError as exc: + self._log.debug(u'google backend returned malformed JSON: {}', exc) if 'error' in data: reason = data['error']['errors'][0]['reason'] - self._log.debug(u'google lyrics backend error: {0}', reason) - return + self._log.debug(u'google backend error: {0}', reason) + return None if 'items' in data.keys(): for item in data['items']: From b4efecb709598b5a8bd362d9a1bdf8d9f2e272a5 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 15:56:13 -0600 Subject: [PATCH 02/12] Add option to hardlink when importing --- beets/importer.py | 17 ++++++++++++----- beets/library.py | 26 +++++++++++++++++--------- beets/util/__init__.py | 26 ++++++++++++++++++++++++++ beetsplug/importadded.py | 3 ++- docs/dev/plugins.rst | 4 ++++ docs/reference/config.rst | 19 +++++++++++++++++-- test/_common.py | 1 + test/test_files.py | 17 +++++++++++++++++ test/test_importadded.py | 1 + test/test_importer.py | 23 ++++++++++++++++++++++- test/test_mediafile_edge.py | 9 +++++++++ 11 files changed, 128 insertions(+), 18 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 6a10f4c97..275e889f6 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -220,13 +220,19 @@ class ImportSession(object): iconfig['resume'] = False iconfig['incremental'] = False - # Copy, move, and link are mutually exclusive. + # Copy, move, link, and hardlink are mutually exclusive. if iconfig['move']: iconfig['copy'] = False iconfig['link'] = False + iconfig['hardlink'] = False elif iconfig['link']: iconfig['copy'] = False iconfig['move'] = False + iconfig['hardlink'] = False + elif iconfig['hardlink']: + iconfig['copy'] = False + iconfig['move'] = False + iconfig['link'] = False # Only delete when copying. if not iconfig['copy']: @@ -654,18 +660,18 @@ class ImportTask(BaseImportTask): item.update(changes) def manipulate_files(self, move=False, copy=False, write=False, - link=False, session=None): + link=False, hardlink=False, session=None): items = self.imported_items() # Save the original paths of all items for deletion and pruning # in the next step (finalization). self.old_paths = [item.path for item in items] for item in items: - if move or copy or link: + if move or copy or link or hardlink: # In copy and link modes, treat re-imports specially: # move in-library files. (Out-of-library files are # copied/moved as usual). old_path = item.path - if (copy or link) and self.replaced_items[item] and \ + if (copy or link or hardlink) and self.replaced_items[item] and \ session.lib.directory in util.ancestry(old_path): item.move() # We moved the item, so remove the @@ -674,7 +680,7 @@ class ImportTask(BaseImportTask): else: # A normal import. Just copy files and keep track of # old paths. - item.move(copy, link) + item.move(copy, link, hardlink) if write and (self.apply or self.choice_flag == action.RETAG): item.try_write() @@ -1412,6 +1418,7 @@ def manipulate_files(session, task): copy=session.config['copy'], write=session.config['write'], link=session.config['link'], + hardlink=session.config['hardlink'], session=session, ) diff --git a/beets/library.py b/beets/library.py index 4e5d9ccf6..b263ecd64 100644 --- a/beets/library.py +++ b/beets/library.py @@ -663,7 +663,7 @@ class Item(LibModel): # Files themselves. - def move_file(self, dest, copy=False, link=False): + def move_file(self, dest, copy=False, link=False, hardlink=False): """Moves or copies the item's file, updating the path value if the move succeeds. If a file exists at ``dest``, then it is slightly modified to be unique. @@ -678,6 +678,10 @@ class Item(LibModel): util.link(self.path, dest) plugins.send("item_linked", item=self, source=self.path, destination=dest) + elif hardlink: + util.hardlink(self.path, dest) + plugins.send("item_hardlinked", item=self, source=self.path, + destination=dest) else: plugins.send("before_item_moved", item=self, source=self.path, destination=dest) @@ -730,15 +734,16 @@ class Item(LibModel): self._db._memotable = {} - def move(self, copy=False, link=False, basedir=None, with_album=True, - store=True): + def move(self, copy=False, link=False, hardlink=False, basedir=None, + with_album=True, store=True): """Move the item to its designated location within the library directory (provided by destination()). Subdirectories are created as needed. If the operation succeeds, the item's path field is updated to reflect the new location. If `copy` is true, moving the file is copied rather than moved. - Similarly, `link` creates a symlink instead. + Similarly, `link` creates a symlink instead, and `hardlink` + creates a hardlink. basedir overrides the library base directory for the destination. @@ -761,7 +766,7 @@ class Item(LibModel): # Perform the move and store the change. old_path = self.path - self.move_file(dest, copy, link) + self.move_file(dest, copy, link, hardlink) if store: self.store() @@ -979,7 +984,7 @@ class Album(LibModel): for item in self.items(): item.remove(delete, False) - def move_art(self, copy=False, link=False): + def move_art(self, copy=False, link=False, hardlink=False): """Move or copy any existing album art so that it remains in the same directory as the items. """ @@ -999,6 +1004,8 @@ class Album(LibModel): util.copy(old_art, new_art) elif link: util.link(old_art, new_art) + elif hardlink: + util.hardlink(old_art, new_art) else: util.move(old_art, new_art) self.artpath = new_art @@ -1008,7 +1015,8 @@ class Album(LibModel): util.prune_dirs(os.path.dirname(old_art), self._db.directory) - def move(self, copy=False, link=False, basedir=None, store=True): + def move(self, copy=False, link=False, hardlink=False, basedir=None, + store=True): """Moves (or copies) all items to their destination. Any album art moves along with them. basedir overrides the library base directory for the destination. By default, the album is stored to the @@ -1026,11 +1034,11 @@ class Album(LibModel): # Move items. items = list(self.items()) for item in items: - item.move(copy, link, basedir=basedir, with_album=False, + item.move(copy, link, hardlink, basedir=basedir, with_album=False, store=store) # Move art. - self.move_art(copy, link) + self.move_art(copy, link, hardlink) if store: self.store() diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 18d89ddd9..ba78dfa30 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -500,6 +500,32 @@ def link(path, dest, replace=False): traceback.format_exc()) +def hardlink(path, dest, replace=False): + """Create a hard link from path to `dest`. Raises an OSError if + `dest` already exists, unless `replace` is True. Does nothing if + `path` == `dest`.""" + if (samefile(path, dest)): + return + + path = syspath(path) + dest = syspath(dest) + if os.path.exists(dest) and not replace: + raise FilesystemError(u'file exists', 'rename', (path, dest)) + try: + os.link(path, dest) + except NotImplementedError: + # raised on python >= 3.2 and Windows versions before Vista + raise FilesystemError(u'OS does not support hard links.' + 'link', (path, dest), traceback.format_exc()) + except OSError as exc: + # TODO: Windows version checks can be removed for python 3 + if hasattr('sys', 'getwindowsversion'): + if sys.getwindowsversion()[0] < 6: # is before Vista + exc = u'OS does not support hard links.' + raise FilesystemError(exc, 'link', (path, dest), + traceback.format_exc()) + + def unique_path(path): """Returns a version of ``path`` that does not exist on the filesystem. Specifically, if ``path` itself already exists, then diff --git a/beetsplug/importadded.py b/beetsplug/importadded.py index 07434ee72..c1838884b 100644 --- a/beetsplug/importadded.py +++ b/beetsplug/importadded.py @@ -36,6 +36,7 @@ class ImportAddedPlugin(BeetsPlugin): register('before_item_moved', self.record_import_mtime) register('item_copied', self.record_import_mtime) register('item_linked', self.record_import_mtime) + register('item_hardlinked', self.record_import_mtime) register('album_imported', self.update_album_times) register('item_imported', self.update_item_times) register('after_write', self.update_after_write_time) @@ -51,7 +52,7 @@ class ImportAddedPlugin(BeetsPlugin): def record_if_inplace(self, task, session): if not (session.config['copy'] or session.config['move'] or - session.config['link']): + session.config['link'] or session.config['hardlink']): self._log.debug(u"In place import detected, recording mtimes from " u"source paths") items = [task.item] \ diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index fb063aee0..4d41c8971 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -161,6 +161,10 @@ The events currently available are: for a file. Parameters: ``item``, ``source`` path, ``destination`` path +* `item_hardlinked`: called with an ``Item`` object whenever a hardlink is + created for a file. + Parameters: ``item``, ``source`` path, ``destination`` path + * `item_removed`: called with an ``Item`` object every time an item (singleton or album's part) is removed from the library (even when its file is not deleted from disk). diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 779e920d2..679b036f6 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -433,8 +433,8 @@ link ~~~~ Either ``yes`` or ``no``, indicating whether to use symbolic links instead of -moving or copying files. (It conflicts with the ``move`` and ``copy`` -options.) Defaults to ``no``. +moving or copying files. (It conflicts with the ``move``, ``copy`` and +``hardlink`` options.) Defaults to ``no``. This option only works on platforms that support symbolic links: i.e., Unixes. It will fail on Windows. @@ -442,6 +442,21 @@ It will fail on Windows. It's likely that you'll also want to set ``write`` to ``no`` if you use this option to preserve the metadata on the linked files. +.. _hardlink: + +hardlink +~~~~ + +Either ``yes`` or ``no``, indicating whether to use hard links instead of +moving or copying or symlinking files. (It conflicts with the ``move``, +``copy``, and ``link`` options.) Defaults to ``no``. + +This option only works on platforms that support hardlinks: i.e., Unixes. +It will fail on Windows. + +It's likely that you'll also want to set ``write`` to ``no`` if you use this +option to preserve the metadata on the linked files. + resume ~~~~~~ diff --git a/test/_common.py b/test/_common.py index 2e7418516..f3213ec31 100644 --- a/test/_common.py +++ b/test/_common.py @@ -54,6 +54,7 @@ _item_ident = 0 # OS feature test. HAVE_SYMLINK = sys.platform != 'win32' +HAVE_HARDLINK = sys.platform != 'win32' def item(lib=None): diff --git a/test/test_files.py b/test/test_files.py index b566f363e..4a81b8c20 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -141,6 +141,23 @@ class MoveTest(_common.TestCase): self.i.move(link=True) self.assertEqual(self.i.path, util.normpath(self.dest)) + @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") + def test_hardlink_arrives(self): + self.i.move(hardlink=True) + self.assertExists(self.dest) + self.assertTrue(os.path.islink(self.dest)) + self.assertEqual(os.readlink(self.dest), self.path) + + @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") + def test_hardlink_does_not_depart(self): + self.i.move(hardlink=True) + self.assertExists(self.path) + + @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") + def test_hardlink_changes_path(self): + self.i.move(hardlink=True) + self.assertEqual(self.i.path, util.normpath(self.dest)) + class HelperTest(_common.TestCase): def test_ancestry_works_on_file(self): diff --git a/test/test_importadded.py b/test/test_importadded.py index 52aa26756..dd933c3c8 100644 --- a/test/test_importadded.py +++ b/test/test_importadded.py @@ -93,6 +93,7 @@ class ImportAddedTest(unittest.TestCase, ImportHelper): self.config['import']['copy'] = False self.config['import']['move'] = False self.config['import']['link'] = False + self.config['import']['hardlink'] = False self.assertAlbumImport() def test_import_album_with_preserved_mtimes(self): diff --git a/test/test_importer.py b/test/test_importer.py index 87724f8da..c372d04da 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -22,6 +22,7 @@ import re import shutil import unicodedata import sys +import stat from six import StringIO from tempfile import mkstemp from zipfile import ZipFile @@ -209,7 +210,8 @@ class ImportHelper(TestHelper): def _setup_import_session(self, import_dir=None, delete=False, threaded=False, copy=True, singletons=False, - move=False, autotag=True, link=False): + move=False, autotag=True, link=False, + hardlink=False): config['import']['copy'] = copy config['import']['delete'] = delete config['import']['timid'] = True @@ -219,6 +221,7 @@ class ImportHelper(TestHelper): config['import']['autotag'] = autotag config['import']['resume'] = False config['import']['link'] = link + config['import']['hardlink'] = hardlink self.importer = TestImportSession( self.lib, loghandler=None, query=None, @@ -353,6 +356,24 @@ class NonAutotaggedImportTest(_common.TestCase, ImportHelper): mediafile.path ) + @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") + def test_import_hardlink_arrives(self): + config['import']['hardlink'] = True + self.importer.run() + for mediafile in self.import_media: + filename = os.path.join( + self.libdir, + b'Tag Artist', b'Tag Album', + util.bytestring_path('{0}.mp3'.format(mediafile.title)) + ) + self.assertExists(filename) + s1 = os.stat(mediafile.path) + s2 = os.stat(filename) + self.assertTrue( + (s1[stat.ST_INO], s1[stat.ST_DEV]) == \ + (s2[stat.ST_INO], s2[stat.ST_DEV]) + ) + def create_archive(session): (handle, path) = mkstemp(dir=py3_path(session.temp_dir)) diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py index 657ca455d..8b28890be 100644 --- a/test/test_mediafile_edge.py +++ b/test/test_mediafile_edge.py @@ -197,6 +197,15 @@ class SafetyTest(unittest.TestCase, _common.TempDirMixin): finally: os.unlink(fn) + @unittest.skipUnless(_common.HAVE_HARDLINK, u'platform lacks hardlink') + def test_broken_hardlink(self): + fn = os.path.join(_common.RSRC, b'brokenlink') + os.link('does_not_exist', fn) + try: + self.assertRaises(mediafile.UnreadableFileError, + mediafile.MediaFile, fn) + finally: + os.unlink(fn) class SideEffectsTest(unittest.TestCase): def setUp(self): From 076c753943e764b2e2a5707a9f9322cc73dfee61 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 16:54:11 -0600 Subject: [PATCH 03/12] Remove windows hardlink checks --- beets/util/__init__.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index ba78dfa30..83f35f563 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -514,14 +514,9 @@ def hardlink(path, dest, replace=False): try: os.link(path, dest) except NotImplementedError: - # raised on python >= 3.2 and Windows versions before Vista raise FilesystemError(u'OS does not support hard links.' 'link', (path, dest), traceback.format_exc()) except OSError as exc: - # TODO: Windows version checks can be removed for python 3 - if hasattr('sys', 'getwindowsversion'): - if sys.getwindowsversion()[0] < 6: # is before Vista - exc = u'OS does not support hard links.' raise FilesystemError(exc, 'link', (path, dest), traceback.format_exc()) From e5c93b6b16c0160ca86552d4a1ea07cfd295f9b5 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 16:57:16 -0600 Subject: [PATCH 04/12] Add hardlink: no to default config --- beets/config_default.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index fa77a82dc..3b0377966 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -6,6 +6,7 @@ import: copy: yes move: no link: no + hardlink: no delete: no resume: ask incremental: no From 0c836f9369646abf781e90fcc85d951377fd1944 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 17:12:47 -0600 Subject: [PATCH 05/12] Add a user-friendly error for cross-device hardlinks --- beets/util/__init__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 83f35f563..d7d45941c 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -18,6 +18,7 @@ from __future__ import division, absolute_import, print_function import os import sys +import errno import locale import re import shutil @@ -517,8 +518,12 @@ def hardlink(path, dest, replace=False): raise FilesystemError(u'OS does not support hard links.' 'link', (path, dest), traceback.format_exc()) except OSError as exc: - raise FilesystemError(exc, 'link', (path, dest), - traceback.format_exc()) + if exc.errno == errno.EXDEV: + raise FilesystemError(u'Cannot hard link across devices.' + 'link', (path, dest), traceback.format_exc()) + else: + raise FilesystemError(exc, 'link', (path, dest), + traceback.format_exc()) def unique_path(path): From ccd0f5d12933903a479edcd35205a7c79e5eb7a2 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 17:19:42 -0600 Subject: [PATCH 06/12] Remove not-found hardlink test (the OS prevents this from happening) --- test/test_mediafile_edge.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py index 8b28890be..c631cdecf 100644 --- a/test/test_mediafile_edge.py +++ b/test/test_mediafile_edge.py @@ -197,16 +197,6 @@ class SafetyTest(unittest.TestCase, _common.TempDirMixin): finally: os.unlink(fn) - @unittest.skipUnless(_common.HAVE_HARDLINK, u'platform lacks hardlink') - def test_broken_hardlink(self): - fn = os.path.join(_common.RSRC, b'brokenlink') - os.link('does_not_exist', fn) - try: - self.assertRaises(mediafile.UnreadableFileError, - mediafile.MediaFile, fn) - finally: - os.unlink(fn) - class SideEffectsTest(unittest.TestCase): def setUp(self): self.empty = os.path.join(_common.RSRC, b'empty.mp3') From 902b955696e1a4fbd214802f29f7af32d12d200c Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 17:22:01 -0600 Subject: [PATCH 07/12] Fix test_hardlink_arrives --- test/test_files.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/test_files.py b/test/test_files.py index 4a81b8c20..38689bbd9 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -145,8 +145,12 @@ class MoveTest(_common.TestCase): def test_hardlink_arrives(self): self.i.move(hardlink=True) self.assertExists(self.dest) - self.assertTrue(os.path.islink(self.dest)) - self.assertEqual(os.readlink(self.dest), self.path) + s1 = os.stat(self.path) + s2 = os.stat(self.dest) + self.assertTrue( + (s1[stat.ST_INO], s1[stat.ST_DEV]) == \ + (s2[stat.ST_INO], s2[stat.ST_DEV]) + ) @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_hardlink_does_not_depart(self): From 1fd22604fb50d8c53e9eb733ddba624e887225b0 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 17:33:26 -0600 Subject: [PATCH 08/12] Fix linter issues --- beets/importer.py | 4 ++-- docs/reference/config.rst | 2 +- test/test_files.py | 4 ++-- test/test_importer.py | 4 ++-- test/test_mediafile_edge.py | 1 + 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 275e889f6..bbe152cd4 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -671,8 +671,8 @@ class ImportTask(BaseImportTask): # move in-library files. (Out-of-library files are # copied/moved as usual). old_path = item.path - if (copy or link or hardlink) and self.replaced_items[item] and \ - session.lib.directory in util.ancestry(old_path): + if (copy or link or hardlink) and self.replaced_items[item] \ + and session.lib.directory in util.ancestry(old_path): item.move() # We moved the item, so remove the # now-nonexistent file from old_paths. diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 679b036f6..bf3d89b2e 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -445,7 +445,7 @@ option to preserve the metadata on the linked files. .. _hardlink: hardlink -~~~~ +~~~~~~~~ Either ``yes`` or ``no``, indicating whether to use hard links instead of moving or copying or symlinking files. (It conflicts with the ``move``, diff --git a/test/test_files.py b/test/test_files.py index 38689bbd9..834d3391c 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -148,8 +148,8 @@ class MoveTest(_common.TestCase): s1 = os.stat(self.path) s2 = os.stat(self.dest) self.assertTrue( - (s1[stat.ST_INO], s1[stat.ST_DEV]) == \ - (s2[stat.ST_INO], s2[stat.ST_DEV]) + (s1[stat.ST_INO], s1[stat.ST_DEV]) == + (s2[stat.ST_INO], s2[stat.ST_DEV]) ) @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") diff --git a/test/test_importer.py b/test/test_importer.py index c372d04da..26dec3de8 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -370,8 +370,8 @@ class NonAutotaggedImportTest(_common.TestCase, ImportHelper): s1 = os.stat(mediafile.path) s2 = os.stat(filename) self.assertTrue( - (s1[stat.ST_INO], s1[stat.ST_DEV]) == \ - (s2[stat.ST_INO], s2[stat.ST_DEV]) + (s1[stat.ST_INO], s1[stat.ST_DEV]) == + (s2[stat.ST_INO], s2[stat.ST_DEV]) ) diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py index c631cdecf..657ca455d 100644 --- a/test/test_mediafile_edge.py +++ b/test/test_mediafile_edge.py @@ -197,6 +197,7 @@ class SafetyTest(unittest.TestCase, _common.TempDirMixin): finally: os.unlink(fn) + class SideEffectsTest(unittest.TestCase): def setUp(self): self.empty = os.path.join(_common.RSRC, b'empty.mp3') From 4e77ef44844810e0749f86fd0b6c6c8e7ceb5178 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 20:48:34 -0600 Subject: [PATCH 09/12] Address review comments --- beets/util/__init__.py | 22 ++++++++++------------ docs/reference/config.rst | 8 +++----- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index d7d45941c..f6cd488d6 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -478,16 +478,15 @@ def move(path, dest, replace=False): def link(path, dest, replace=False): """Create a symbolic link from path to `dest`. Raises an OSError if `dest` already exists, unless `replace` is True. Does nothing if - `path` == `dest`.""" - if (samefile(path, dest)): + `path` == `dest`. + """ + if samefile(path, dest): return - path = syspath(path) - dest = syspath(dest) - if os.path.exists(dest) and not replace: + if os.path.exists(syspath(dest)) and not replace: raise FilesystemError(u'file exists', 'rename', (path, dest)) try: - os.symlink(path, dest) + os.symlink(syspath(path), syspath(dest)) except NotImplementedError: # raised on python >= 3.2 and Windows versions before Vista raise FilesystemError(u'OS does not support symbolic links.' @@ -504,16 +503,15 @@ def link(path, dest, replace=False): def hardlink(path, dest, replace=False): """Create a hard link from path to `dest`. Raises an OSError if `dest` already exists, unless `replace` is True. Does nothing if - `path` == `dest`.""" - if (samefile(path, dest)): + `path` == `dest`. + """ + if samefile(path, dest): return - path = syspath(path) - dest = syspath(dest) - if os.path.exists(dest) and not replace: + if os.path.exists(syspath(dest)) and not replace: raise FilesystemError(u'file exists', 'rename', (path, dest)) try: - os.link(path, dest) + os.link(syspath(path), syspath(dest)) except NotImplementedError: raise FilesystemError(u'OS does not support hard links.' 'link', (path, dest), traceback.format_exc()) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index bf3d89b2e..40f8e9185 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -451,11 +451,9 @@ Either ``yes`` or ``no``, indicating whether to use hard links instead of moving or copying or symlinking files. (It conflicts with the ``move``, ``copy``, and ``link`` options.) Defaults to ``no``. -This option only works on platforms that support hardlinks: i.e., Unixes. -It will fail on Windows. - -It's likely that you'll also want to set ``write`` to ``no`` if you use this -option to preserve the metadata on the linked files. +As with symbolic links (see :ref:`link`, above), this will not work on Windows +and you will want to set ``write`` to ``no``. Otherwise meatadata on the +original file will be modified. resume ~~~~~~ From 26408dd58da631673c23c2ffd2e5dbf660bdae1f Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 19 Feb 2017 22:44:28 -0500 Subject: [PATCH 10/12] Add a little more logging about matching process This could help debug #2446. It will help, at least, narrow down when the Munkres algorithm is taking too long. --- beets/autotag/match.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index 71d80e821..71b62adb7 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -103,7 +103,9 @@ def assign_items(items, tracks): costs.append(row) # Find a minimum-cost bipartite matching. + log.debug('Computing track assignment...') matching = Munkres().compute(costs) + log.debug('...done.') # Produce the output matching. mapping = dict((items[i], tracks[j]) for (i, j) in matching) @@ -349,7 +351,8 @@ def _add_candidate(items, results, info): checking the track count, ordering the items, checking for duplicates, and calculating the distance. """ - log.debug(u'Candidate: {0} - {1}', info.artist, info.album) + log.debug(u'Candidate: {0} - {1} ({2})', + info.artist, info.album, info.album_id) # Discard albums with zero tracks. if not info.tracks: From 8ccdb1c77e2becc851e2996b182eecedf40c16ba Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 19 Feb 2017 22:54:14 -0500 Subject: [PATCH 11/12] More logging for MusicBrainz requests Even more performance-isolating logging to help debug #2446. --- beets/autotag/mb.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py index a0d4dc33a..a6133adb1 100644 --- a/beets/autotag/mb.py +++ b/beets/autotag/mb.py @@ -374,6 +374,7 @@ def match_album(artist, album, tracks=None): return try: + log.debug(u'Searching for MusicBrainz releases with: {!r}', criteria) res = musicbrainzngs.search_releases( limit=config['musicbrainz']['searchlimit'].get(int), **criteria) except musicbrainzngs.MusicBrainzError as exc: @@ -424,6 +425,7 @@ def album_for_id(releaseid): object or None if the album is not found. May raise a MusicBrainzAPIError. """ + log.debug(u'Requesting MusicBrainz release {}', releaseid) albumid = _parse_id(releaseid) if not albumid: log.debug(u'Invalid MBID ({0}).', releaseid) From d10bfa1af6309e50a7e0954cba21e6491206bcf0 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 20 Feb 2017 22:11:34 -0500 Subject: [PATCH 12/12] Changelog & thanks for #2445 --- docs/changelog.rst | 3 +++ docs/reference/config.rst | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0774722c8..4734ba05a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,9 @@ New features: * :doc:`/plugins/badfiles`: Added a ``--verbose`` or ``-v`` option. Results are now displayed only for corrupted files by default and for all the files when the verbose option is set. :bug:`1654` :bug:`2434` +* A new :ref:`hardlink` config option instructs the importer to create hard + links on filesystems that support them. Thanks to :user:`jacobwgillespie`. + :bug:`2445` Fixes: diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 40f8e9185..094462d2f 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -452,7 +452,7 @@ moving or copying or symlinking files. (It conflicts with the ``move``, ``copy``, and ``link`` options.) Defaults to ``no``. As with symbolic links (see :ref:`link`, above), this will not work on Windows -and you will want to set ``write`` to ``no``. Otherwise meatadata on the +and you will want to set ``write`` to ``no``. Otherwise, metadata on the original file will be modified. resume