From b4efecb709598b5a8bd362d9a1bdf8d9f2e272a5 Mon Sep 17 00:00:00 2001 From: Jacob Gillespie Date: Sun, 19 Feb 2017 15:56:13 -0600 Subject: [PATCH] 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):