From a6c1ad22356aa80fe1e6fe25800088e8a7c408fa Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Thu, 17 May 2012 11:42:58 -0700 Subject: [PATCH] reimporting with copying: copy external files --- beets/importer.py | 42 ++++++++++++++++++++++++----------------- docs/changelog.rst | 5 +++++ test/test_importer.py | 44 ++++++++++++++++++++++++++++++++++++------- 3 files changed, 67 insertions(+), 24 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 0691985b6..34a905b63 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -741,24 +741,32 @@ def apply_choices(config): # Move/copy files. task.old_paths = [item.path for item in items] # For deletion. for item in items: - if config.copy or config.move: - if config.move: - # Just move the file. - old_path = item.path - lib.move(item, False) - # Clean up empty parent directory. - if task.toppath: - task.prune(old_path) - else: - # If it's a reimport, move the file. Otherwise, copy - # and keep track of the old path. - old_path = item.path - do_copy = not bool(replaced_items[item]) - lib.move(item, do_copy) - if not do_copy: - # If we moved the item, remove the now-nonexistent - # file from old_paths. + if config.move: + # Just move the file. + old_path = item.path + lib.move(item, False) + # Clean up empty parent directory. + if task.toppath: + 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 + # of the old path. + old_path = item.path + if replaced_items[item]: + # This is a reimport. Move in-library files and copy + # out-of-library files. + if lib.directory in util.ancestry(old_path): + lib.move(item, False) + # We moved the item, so remove the + # now-nonexistent file from old_paths. task.old_paths.remove(old_path) + else: + lib.move(item, True) + else: + # A normal import. Just copy files and keep track of + # old paths. + lib.move(item, True) if config.write and task.should_write_tags(): item.write() diff --git a/docs/changelog.rst b/docs/changelog.rst index 0a0ccd76b..7b8b99c74 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,11 @@ Changelog * New plugin event: ``library_opened`` is called when beets starts up and opens the library database. * :doc:`/plugins/mpdupdate`: Fix TypeError crash (thanks to Philippe Mongeau). +* When re-importing files with ``import_copy`` enabled, only files inside the + library directory are moved. Files outside the library directory are still + copied. This solves a problem (introduced in 1.0b14) where beets could crash + after adding files to the library but before finishing copying them; during + the next import, the (external) files would be moved instead of copied. 1.0b14 (May 12, 2012) --------------------- diff --git a/test/test_importer.py b/test/test_importer.py index dd091a316..9f2868bc5 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -289,7 +289,37 @@ class ImportApplyTest(unittest.TestCase, _common.ExtraAsserts): task = _call_apply(coro, [self.i], self.info) self.assertEqual(task.old_paths, [self.srcpath]) - def test_reimport_moves_file_and_does_not_add_to_old_paths(self): + def test_reimport_inside_file_moves_and_does_not_add_to_old_paths(self): + """Reimporting a file *inside* the library directory should + *move* the file. + """ + # Add the item to the library while inside the library directory. + internal_srcpath = os.path.join(self.libdir, 'source.mp3') + shutil.move(self.srcpath, internal_srcpath) + temp_item = library.Item.from_path(internal_srcpath) + self.lib.add(temp_item) + self.lib.conn.commit() + + self.i = library.Item.from_path(internal_srcpath) + self.i.comp = False + + # Then, re-import the same file. + coro = importer.apply_choices(_common.iconfig(self.lib)) + coro.next() + task = _call_apply(coro, [self.i], self.info) + + # Old file should be gone. + self.assertNotExists(internal_srcpath) + # New file should be present. + self.assertExists(os.path.join(self.libdir, 'one.mp3')) + # Also, the old file should not be in old_paths because it does + # not exist. + self.assertEqual(task.old_paths, []) + + def test_reimport_outside_file_copies(self): + """Reimporting a file *outside* the library directory should + *copy* the file (when copying is enabled). + """ # First, add the item to the library. temp_item = library.Item.from_path(self.srcpath) self.lib.add(temp_item) @@ -300,13 +330,13 @@ class ImportApplyTest(unittest.TestCase, _common.ExtraAsserts): coro.next() task = _call_apply(coro, [self.i], self.info) - # Old file should be gone. - self.assertNotExists(self.srcpath) - # New file should be present. + # Old file should still exist. + self.assertExists(self.srcpath) + # New file should also be present. self.assertExists(os.path.join(self.libdir, 'one.mp3')) - # Also, the old file should not be in old_paths because it does - # not exist. - self.assertEqual(task.old_paths, []) + # The old (copy-source) file should be marked for possible + # deletion. + self.assertEqual(task.old_paths, [self.srcpath]) def test_apply_with_move(self): config = _common.iconfig(self.lib, move=True)