reimporting with copying: copy external files

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()

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)
 ---------------------

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)