fix bug #557585 GitFile breaks dulwich on Windows
authoranatoly techtonik <techtonik@gmail.com>
Fri, 16 Apr 2010 17:25:52 +0000 (19:25 +0200)
committerJelmer Vernooij <jelmer@samba.org>
Fri, 16 Apr 2010 17:25:52 +0000 (19:25 +0200)
fix non-atomic os.rename() behavior on Windows that fails if target exists
added file.fancy_rename() that preserves source file is case rename fails

dulwich/file.py
dulwich/tests/test_file.py

index 034186ede3701bf84e412235c1bdcfa673da8715..3ba72298fff86ee9600dc5fa1e1fa4c7f62c5269 100644 (file)
@@ -22,6 +22,7 @@
 
 import errno
 import os
+import tempfile
 
 def ensure_dir_exists(dirname):
     """Ensure a directory exists, creating if necessary."""
@@ -31,6 +32,36 @@ def ensure_dir_exists(dirname):
         if e.errno != errno.EEXIST:
             raise
 
+def fancy_rename(oldname, newname):
+    """Rename file with temporary backup file to rollback if rename fails"""
+    if not os.path.exists(newname):
+        try:
+            os.rename(oldname, newname)
+        except OSError, e:
+            raise
+        return
+
+    # destination file exists
+    try:
+        (fd, tmpfile) = tempfile.mkstemp(".tmp", prefix=oldname+".", dir=".")
+        os.close(fd)
+        os.remove(tmpfile)
+    except OSError, e:
+        # either file could not be created (e.g. permission problem)
+        # or could not be deleted (e.g. rude virus scanner)
+        raise
+    try:
+        os.rename(newname, tmpfile)
+    except OSError, e:
+        raise   # no rename occurred
+    try:
+        os.rename(oldname, newname)
+    except OSError, e:
+        os.rename(tmpfile, newname)
+        raise
+    os.remove(tmpfile)
+
+
 def GitFile(filename, mode='r', bufsize=-1):
     """Create a file object that obeys the git file locking protocol.
 
@@ -129,6 +160,11 @@ class _GitFile(object):
         self._file.close()
         try:
             os.rename(self._lockfilename, self._filename)
+        except OSError, e:
+            # Windows versions prior to Vista don't support atomic renames
+            if e.errno != errno.EEXIST:
+                raise
+            fancy_rename(self._lockfilename, self._filename)
         finally:
             self.abort()
 
index 3a72a0bc4a73cd97716f650fc5ba1e0687abfcfc..f71acdbb4d2436c518105d2e1168375e241bcdf7 100644 (file)
@@ -23,7 +23,60 @@ import shutil
 import tempfile
 import unittest
 
-from dulwich.file import GitFile
+from dulwich.file import GitFile, fancy_rename
+
+
+class FancyRenameTests(unittest.TestCase):
+
+    def setUp(self):
+        self._tempdir = tempfile.mkdtemp()
+        self.foo = self.path('foo')
+        self.bar = self.path('bar')
+        self.create(self.foo, 'foo contents')
+
+    def tearDown(self):
+        shutil.rmtree(self._tempdir)
+
+    def path(self, filename):
+        return os.path.join(self._tempdir, filename)
+
+    def create(self, path, contents):
+        f = open(path, 'wb')
+        f.write(contents)
+        f.close()
+
+    def test_no_dest_exists(self):
+        self.assertFalse(os.path.exists(self.bar))
+        fancy_rename(self.foo, self.bar)
+        self.assertFalse(os.path.exists(self.foo))
+
+        new_f = open(self.bar, 'rb')
+        self.assertEquals('foo contents', new_f.read())
+        new_f.close()
+         
+    def test_dest_exists(self):
+        self.create(self.bar, 'bar contents')
+        fancy_rename(self.foo, self.bar)
+        self.assertFalse(os.path.exists(self.foo))
+
+        new_f = open(self.bar, 'rb')
+        self.assertEquals('foo contents', new_f.read())
+        new_f.close()
+
+    def test_dest_opened(self):
+        self.create(self.bar, 'bar contents')
+        dest_f = open(self.bar, 'rb')
+        self.assertRaises(OSError, fancy_rename, self.foo, self.bar)
+        dest_f.close()
+        self.assertTrue(os.path.exists(self.path('foo')))
+
+        new_f = open(self.foo, 'rb')
+        self.assertEquals('foo contents', new_f.read())
+        new_f.close()
+
+        new_f = open(self.bar, 'rb')
+        self.assertEquals('bar contents', new_f.read())
+        new_f.close()
 
 
 class GitFileTests(unittest.TestCase):