From e00f15175a10275ce069b3edfff4827aed685036 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 30 May 2012 16:39:47 -0700 Subject: [PATCH] prune is a no-op when file exists i.e., when moving a file that's already at its destination. --- beets/importer.py | 22 +++++++++------------- docs/changelog.rst | 2 ++ test/test_importer.py | 10 ++++++++++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index b2fff0399..68713c88c 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -461,13 +461,13 @@ class ImportTask(object): # Utilities. def prune(self, filename): - """Prune any empty directories above the given file, which must - not exist. If this task has no `toppath` or the file path - provided is not within the `toppath`, then this function has no - effect. + """Prune any empty directories above the given file. If this + task has no `toppath` or the file path provided is not within + the `toppath`, then this function has no effect. Similarly, if + the file still exists, no pruning is performed, so it's safe to + call when the file in question may not have been removed. """ - assert not os.path.exists(filename) - if self.toppath: + if self.toppath and not os.path.exists(filename): util.prune_dirs(os.path.dirname(filename), self.toppath) @@ -760,9 +760,7 @@ def manipulate_files(config): # Just move the file. old_path = item.path lib.move(item, False) - # Clean up empty parent directory. - if task.toppath: - task.prune(old_path) + task.prune(old_path) elif config.copy: # If it's a reimport, move in-library files and copy # out-of-library files. Otherwise, copy and keep track @@ -810,7 +808,7 @@ def fetch_art(config): album = lib.get_album(task.album_id) album.set_art(artpath, not (config.delete or config.move)) - if (config.delete or config.move) and task.toppath: + if config.delete or config.move: task.prune(artpath) def finalize(config): @@ -845,9 +843,7 @@ def finalize(config): # Only delete files that were actually copied. if old_path not in new_paths: util.remove(syspath(old_path), False) - # Clean up directory if it is emptied. - if task.toppath: - task.prune(old_path) + task.prune(old_path) # Update progress. if config.resume is not False: diff --git a/docs/changelog.rst b/docs/changelog.rst index 08340dac6..c619f8338 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -36,6 +36,8 @@ Changelog and give more context to "failed fingerprint generation" errors. * Interactive prompts are sent to stdout instead of stderr. * :doc:`/plugins/embedart`: Fix crash when audio files are unreadable. +* Fix an assertion failure while importing with moving enabled when the file was + already at its destination. .. _artist credits: http://wiki.musicbrainz.org/Artist_Credit diff --git a/test/test_importer.py b/test/test_importer.py index 2bd6039c8..27ba87e16 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -344,6 +344,16 @@ class ImportApplyTest(unittest.TestCase, _common.ExtraAsserts): _call_stages(config, [self.i], self.info, toppath=self.srcdir) self.assertNotExists(os.path.dirname(self.srcpath)) + def test_manipulate_files_with_null_move(self): + """It should be possible to "move" a file even when the file is + already at the destination. + """ + config = _common.iconfig(self.lib, move=True) + self.lib.move(self.i) # Already at destination. + _call_stages(config, [self.i], self.info, toppath=self.srcdir, + stages=[importer.manipulate_files]) + self.assertExists(self.i.path) + class AsIsApplyTest(unittest.TestCase): def setUp(self): self.dbpath = os.path.join(_common.RSRC, 'templib.blb')