# HG changeset patch # User anatoly techtonik # Date 1271397653 -10800 # Node ID 29863266525cc128124c3ec3b6f73d10b32f20dc # Parent 9035f880dfb7cd65f4b8db3096bb42836bc01770 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 -r 9035f880dfb7 -r 29863266525c dulwich/file.py --- a/dulwich/file.py Fri Apr 16 03:35:57 2010 +0200 +++ b/dulwich/file.py Fri Apr 16 09:00:53 2010 +0300 @@ -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 @@ 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 @@ 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 -r 9035f880dfb7 -r 29863266525c dulwich/tests/test_file.py --- a/dulwich/tests/test_file.py Fri Apr 16 03:35:57 2010 +0200 +++ b/dulwich/tests/test_file.py Fri Apr 16 09:00:53 2010 +0300 @@ -23,7 +23,60 @@ 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):