From: anatoly techtonik Date: Fri, 16 Apr 2010 17:25:52 +0000 (+0200) Subject: fix bug #557585 GitFile breaks dulwich on Windows X-Git-Tag: dulwich-0.6.0~31 X-Git-Url: http://git.samba.org/samba.git/?p=jelmer%2Fdulwich-libgit2.git;a=commitdiff_plain;h=dd4a48c00d4b4dfa68e0d7727f94d21619cac113 fix bug #557585 GitFile breaks dulwich on Windows 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 --- diff --git a/dulwich/file.py b/dulwich/file.py index 034186e..3ba7229 100644 --- a/dulwich/file.py +++ b/dulwich/file.py @@ -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() diff --git a/dulwich/tests/test_file.py b/dulwich/tests/test_file.py index 3a72a0b..f71acdb 100644 --- a/dulwich/tests/test_file.py +++ b/dulwich/tests/test_file.py @@ -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):