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: 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) 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 diff --git a/beets/importer.py b/beets/importer.py index 6a10f4c97..bbe152cd4 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,19 +660,19 @@ 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 \ - 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. @@ -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..f6cd488d6 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 @@ -477,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.' @@ -500,6 +500,30 @@ 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 + + if os.path.exists(syspath(dest)) and not replace: + raise FilesystemError(u'file exists', 'rename', (path, dest)) + try: + os.link(syspath(path), syspath(dest)) + except NotImplementedError: + raise FilesystemError(u'OS does not support hard links.' + 'link', (path, dest), traceback.format_exc()) + except OSError as 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): """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/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']: diff --git a/docs/changelog.rst b/docs/changelog.rst index 0c3779c3c..236d174ab 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,7 +28,10 @@ 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` - * :doc:`/plugins/embedart` by default now asks for confirmation before +* A new :ref:`hardlink` config option instructs the importer to create hard + links on filesystems that support them. Thanks to :user:`jacobwgillespie`. + :bug:`2445` +* :doc:`/plugins/embedart` by default now asks for confirmation before embedding art into music files. Thanks to :user:`Stunner`. :bug:`1999` Fixes: 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..094462d2f 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,19 @@ 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``. + +As with symbolic links (see :ref:`link`, above), this will not work on Windows +and you will want to set ``write`` to ``no``. Otherwise, metadata on the +original file will be modified. + 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..834d3391c 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -141,6 +141,27 @@ 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) + 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): + 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..26dec3de8 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))