From b04096de25fe9549f8f96e9a610a4936aa6b4a92 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sat, 19 May 2012 11:52:53 -0700
Subject: [PATCH] do not preserve metadata during copy-move (GC-383)

The shutil.move() function attempts to copy metadata (e.g., permissions and
mtime) when copying a file across filesystems. This always fails on Samba shares
because the utime() call is never permitted by normal users. We don't care about
preserving mtimes across moves, though, so this commit eschews shutil and
reimplements the move algorithm.
---
 beets/util/__init__.py | 36 +++++++++++++++++++++++-------------
 docs/changelog.rst     |  1 +
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/beets/util/__init__.py b/beets/util/__init__.py
index 49df5684e..d93f9f8a6 100644
--- a/beets/util/__init__.py
+++ b/beets/util/__init__.py
@@ -89,7 +89,7 @@ class FilesystemError(HumanReadableException):
 
     def get_message(self):
         # Use a nicer English phrasing for some specific verbs.
-        if self.verb in ('move', 'copy'):
+        if self.verb in ('move', 'copy', 'rename'):
             clause = 'while {0} {1} to {2}'.format(
                 self._gerund(), repr(self.paths[0]), repr(self.paths[1])
             )
@@ -314,10 +314,10 @@ def remove(path, soft=True):
         raise FilesystemError(exc, 'delete', (path,), traceback.format_exc())
 
 def copy(path, dest, replace=False, pathmod=os.path):
-    """Copy a plain file. Permissions are not copied. If dest already
-    exists, raises an OSError unless replace is True. Has no effect if
-    path is the same as dest. Paths are translated to system paths
-    before the syscall.
+    """Copy a plain file. Permissions are not copied. If `dest` already
+    exists, raises a FilesystemError unless `replace` is True. Has no
+    effect if `path` is the same as `dest`. Paths are translated to
+    system paths before the syscall.
     """
     if samefile(path, dest):
         return
@@ -332,22 +332,32 @@ def copy(path, dest, replace=False, pathmod=os.path):
                               traceback.format_exc())
 
 def move(path, dest, replace=False, pathmod=os.path):
-    """Rename a file. dest may not be a directory. If dest already
-    exists, raises an OSError unless replace is True. Hos no effect if
-    path is the same as dest. Paths are translated to system paths.
+    """Rename a file. `dest` may not be a directory. If `dest` already
+    exists, raises an OSError unless `replace` is True. Has no effect if
+    `path` is the same as `dest`. If the paths are on different
+    filesystems (or the rename otherwise fails), a copy is attempted
+    instead, in which case metadata will *not* be preserved. Paths are
+    translated to system paths.
     """
     if samefile(path, dest):
         return
     path = syspath(path)
     dest = syspath(dest)
     if pathmod.exists(dest):
-        raise FilesystemError('file exists', 'move', (path, dest),
+        raise FilesystemError('file exists', 'rename', (path, dest),
                               traceback.format_exc())
+
+    # First, try renaming the file.
     try:
-        shutil.move(path, dest)
-    except (OSError, IOError) as exc:
-        raise FilesystemError(exc, 'move', (path, dest),
-                              traceback.format_exc())
+        os.rename(path, dest)
+    except OSError:
+        # Otherwise, copy and delete the original.
+        try:
+            shutil.copyfile(path, dest)
+            os.remove(path)
+        except (OSError, IOError) as exc:
+            raise FilesystemError(exc, 'move', (path, dest),
+                                  traceback.format_exc())
 
 def unique_path(path):
     """Returns a version of ``path`` that does not exist on the
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 718b5970c..1ed9ea2b4 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -19,6 +19,7 @@ Changelog
   action assigned.
 * New plugin event: ``library_opened`` is called when beets starts up and
   opens the library database.
+* Fix a crash when moving files to a Samba share.
 * :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