Add a GitFile class that uses the same locking protocol for writes as git.

Change-Id: Id9c6f8b5880a73e0e714cbc61e954d0ecae6103a

dulwich/file.py

@@ -0,0 +1,106 @@
+# file.py -- Safe access to git files
+# Copyright (C) 2010 Google, Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; version 2
+# of the License or (at your option) a later version of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
+
+
+"""Safe access to git files."""
+
+
+import errno
+import os
+
+
+def GitFile(filename, mode='r', bufsize=-1):
+    if 'a' in mode:
+        raise IOError('append mode not supported for Git files')
+    if 'w' in mode:
+        return _GitFile(filename, mode, bufsize)
+    else:
+        return file(filename, mode, bufsize)
+
+
+class _GitFile(object):
+    """File that follows the git locking protocol for writes.
+
+    All writes to a file foo will be written into foo.lock in the same
+    directory, and the lockfile will be renamed to overwrite the original file
+    on close. The lockfile is automatically removed upon filesystem error.
+
+    :note: You *must* call close() or abort() on a _GitFile for the lock to be
+        released. Typically this will happen in a finally block.
+    """
+
+    PROXY_PROPERTIES = set(['closed', 'encoding', 'errors', 'mode', 'name',
+                            'newlines', 'softspace'])
+    PROXY_METHODS = ('__iter__', 'flush', 'fileno', 'isatty', 'next', 'read',
+                     'readline', 'readlines', 'xreadlines', 'seek', 'tell',
+                     'truncate', 'write', 'writelines')
+    def __init__(self, filename, mode, bufsize):
+        self._filename = filename
+        self._lockfilename = '%s.lock' % self._filename
+        fd = os.open(self._lockfilename, os.O_RDWR | os.O_CREAT | os.O_EXCL)
+        self._file = os.fdopen(fd, mode, bufsize)
+
+        for method in self.PROXY_METHODS:
+            setattr(self, method,
+                    self._safe_method(getattr(self._file, method)))
+
+    def _safe_method(self, file_method):
+        # note that built-in file methods have no kwargs
+        def do_safe_method(*args):
+            try:
+                return file_method(*args)
+            except (OSError, IOError):
+                self.abort()
+                raise
+        return do_safe_method
+
+    def abort(self):
+        """Close and discard the lockfile without overwriting the target.
+
+        If the file is already closed, this is a no-op.
+        """
+        self._file.close()
+        try:
+            os.remove(self._lockfilename)
+        except OSError, e:
+            # The file may have been removed already, which is ok.
+            if e.errno != errno.ENOENT:
+                raise
+
+    def close(self):
+        """Close this file, saving the lockfile over the original.
+
+        :note: If this method fails, it will attempt to delete the lockfile.
+            However, it is not guaranteed to do so (e.g. if a filesystem becomes
+            suddenly read-only), which will prevent future writes to this file
+            until the lockfile is removed manually.
+        :raises OSError: if the original file could not be overwritten. The lock
+            file is still closed, so further attempts to write to the same file
+            object will raise ValueError.
+        """
+        self._file.close()
+        try:
+            os.rename(self._lockfilename, self._filename)
+        finally:
+            self.abort()
+
+    def __getattr__(self, name):
+        """Proxy property calls to the underlying file."""
+        if name in self.PROXY_PROPERTIES:
+            return getattr(self._file, name)
+        raise AttributeError(name)

dulwich/index.py

@@ -22,6 +22,7 @@ import os
 import stat
 import struct
 
+from dulwich.file import GitFile
 from dulwich.objects import (
     S_IFGITLINK,
     S_ISGITLINK,
@@ -173,7 +174,7 @@ class Index(object):
 
     def write(self):
         """Write current contents of index to disk."""
-        f = open(self._filename, 'wb')
+        f = GitFile(self._filename, 'wb')
         try:
             f = SHA1Writer(f)
             write_index_dict(f, self._byname)
@@ -182,7 +183,7 @@ class Index(object):
 
     def read(self):
         """Read current contents of index from disk."""
-        f = open(self._filename, 'rb')
+        f = GitFile(self._filename, 'rb')
         try:
             f = SHA1Reader(f)
             for x in read_index(f):

dulwich/object_store.py

@@ -29,6 +29,7 @@ import urllib2
 from dulwich.errors import (
     NotTreeError,
     )
+from dulwich.file import GitFile
 from dulwich.objects import (
     Commit,
     ShaFile,
@@ -294,7 +295,7 @@ class DiskObjectStore(BaseObjectStore):
         path = os.path.join(dir, sha[2:])
         if os.path.exists(path):
             return # Already there, no need to write again
-        f = open(path, 'w+')
+        f = GitFile(path, 'wb')
         try:
             f.write(o.as_legacy_object())
         finally:

dulwich/objects.py

@@ -36,6 +36,7 @@ from dulwich.errors import (
     NotCommitError,
     NotTreeError,
     )
+from dulwich.file import GitFile
 from dulwich.misc import (
     make_sha,
     )
@@ -184,7 +185,7 @@ class ShaFile(object):
     def from_file(cls, filename):
         """Get the contents of a SHA file on disk"""
         size = os.path.getsize(filename)
-        f = open(filename, 'rb')
+        f = GitFile(filename, 'rb')
         try:
             map = mmap.mmap(f.fileno(), size, access=mmap.ACCESS_READ)
             shafile = cls._parse_file(map)
@@ -637,4 +638,3 @@ try:
     from dulwich._objects import parse_tree
 except ImportError:
     pass
-

dulwich/pack.py

@@ -55,6 +55,7 @@ from dulwich.errors import (
     ApplyDeltaError,
     ChecksumMismatch,
     )
+from dulwich.file import GitFile
 from dulwich.lru_cache import (
     LRUSizeCache,
     )
@@ -150,7 +151,7 @@ def load_pack_index(filename):
 
     :param filename: Path to the index file
     """
-    f = open(filename, 'rb')
+    f = GitFile(filename, 'rb')
     if f.read(4) == '\377tOc':
         version = struct.unpack(">L", f.read(4))[0]
         if version == 2:
@@ -211,7 +212,7 @@ class PackIndex(object):
         # ensure that it hasn't changed.
         self._size = os.path.getsize(filename)
         if file is None:
-            self._file = open(filename, 'rb')
+            self._file = GitFile(filename, 'rb')
         else:
             self._file = file
         self._contents, map_offset = simple_mmap(self._file, 0, self._size)
@@ -497,7 +498,7 @@ class PackData(object):
         self._size = os.path.getsize(filename)
         self._header_size = 12
         assert self._size >= self._header_size, "%s is too small for a packfile (%d < %d)" % (filename, self._size, self._header_size)
-        self._file = open(self._filename, 'rb')
+        self._file = GitFile(self._filename, 'rb')
         self._read_header()
         self._offset_cache = LRUSizeCache(1024*1024*20, 
             compute_size=_compute_object_size)
@@ -809,7 +810,7 @@ def write_pack(filename, objects, num_objects):
     :param objects: Iterable over (object, path) tuples to write
     :param num_objects: Number of objects to write
     """
-    f = open(filename + ".pack", 'wb')
+    f = GitFile(filename + ".pack", 'wb')
     try:
         entries, data_sum = write_pack_data(f, objects, num_objects)
     finally:
@@ -873,7 +874,7 @@ def write_pack_index_v1(filename, entries, pack_checksum):
             crc32_checksum.
     :param pack_checksum: Checksum of the pack file.
     """
-    f = open(filename, 'wb')
+    f = GitFile(filename, 'wb')
     f = SHA1Writer(f)
     fan_out_table = defaultdict(lambda: 0)
     for (name, offset, entry_checksum) in entries:
@@ -1021,7 +1022,7 @@ def write_pack_index_v2(filename, entries, pack_checksum):
             crc32_checksum.
     :param pack_checksum: Checksum of the pack file.
     """
-    f = open(filename, 'wb')
+    f = GitFile(filename, 'wb')
     f = SHA1Writer(f)
     f.write('\377tOc') # Magic!
     f.write(struct.pack(">L", 2))

dulwich/repo.py

@@ -32,6 +32,7 @@ from dulwich.errors import (
     NotGitRepository,
     NotTreeError, 
     )
+from dulwich.file import GitFile
 from dulwich.object_store import (
     DiskObjectStore,
     )
@@ -161,7 +162,7 @@ class DiskRefsContainer(RefsContainer):
         file = self.refpath(name)
         if not os.path.exists(file):
             raise KeyError(name)
-        f = open(file, 'rb')
+        f = GitFile(file, 'rb')
         try:
             return f.read().strip("\n")
         finally:
@@ -172,7 +173,7 @@ class DiskRefsContainer(RefsContainer):
         dirpath = os.path.dirname(file)
         if not os.path.exists(dirpath):
             os.makedirs(dirpath)
-        f = open(file, 'wb')
+        f = GitFile(file, 'wb')
         try:
             f.write(ref+"\n")
         finally:
@@ -310,7 +311,7 @@ class Repo(object):
         if not os.path.exists(path):
             return {}
         ret = {}
-        f = open(path, 'rb')
+        f = GitFile(path, 'rb')
         try:
             for entry in read_packed_refs(f):
                 ret[entry[1]] = entry[0]
@@ -482,15 +483,25 @@ class Repo(object):
             os.mkdir(os.path.join(path, *d))
         ret = cls(path)
         ret.refs.set_ref("HEAD", "refs/heads/master")
-        open(os.path.join(path, 'description'), 'wb').write("Unnamed repository")
-        open(os.path.join(path, 'info', 'excludes'), 'wb').write("")
-        open(os.path.join(path, 'config'), 'wb').write("""[core]
+        f = GitFile(os.path.join(path, 'description'), 'wb')
+        try:
+            f.write("Unnamed repository")
+        finally:
+            f.close()
+
+        f = GitFile(os.path.join(path, 'config'), 'wb')
+        try:
+            f.write("""[core]
     repositoryformatversion = 0
     filemode = true
     bare = false
     logallrefupdates = true
 """)
+        finally:
+            f.close()
+
+        f = GitFile(os.path.join(path, 'info', 'excludes'), 'wb')
+        f.close()
         return ret
 
     create = init_bare
-

dulwich/tests/test_file.py

@@ -0,0 +1,133 @@
+# test_file.py -- Test for git files
+# Copyright (C) 2010 Google, Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; version 2
+# of the License or (at your option) a later version of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
+
+
+import errno
+import os
+import shutil
+import tempfile
+import unittest
+
+from dulwich.file import GitFile
+
+class GitFileTests(unittest.TestCase):
+    def setUp(self):
+        self._tempdir = tempfile.mkdtemp()
+        f = open(self.path('foo'), 'wb')
+        f.write('foo contents')
+        f.close()
+
+    def tearDown(self):
+        shutil.rmtree(self._tempdir)
+
+    def path(self, filename):
+        return os.path.join(self._tempdir, filename)
+
+    def test_readonly(self):
+        f = GitFile(self.path('foo'), 'rb')
+        self.assertTrue(isinstance(f, file))
+        self.assertEquals('foo contents', f.read())
+        self.assertEquals('', f.read())
+        f.seek(4)
+        self.assertEquals('contents', f.read())
+        f.close()
+
+    def test_write(self):
+        foo = self.path('foo')
+        foo_lock = '%s.lock' % foo
+
+        orig_f = open(foo, 'rb')
+        self.assertEquals(orig_f.read(), 'foo contents')
+        orig_f.close()
+
+        self.assertFalse(os.path.exists(foo_lock))
+        f = GitFile(foo, 'wb')
+        self.assertFalse(f.closed)
+        self.assertRaises(AttributeError, getattr, f, 'not_a_file_property')
+
+        self.assertTrue(os.path.exists(foo_lock))
+        f.write('new stuff')
+        f.seek(4)
+        f.write('contents')
+        f.close()
+        self.assertFalse(os.path.exists(foo_lock))
+
+        new_f = open(foo, 'rb')
+        self.assertEquals('new contents', new_f.read())
+        new_f.close()
+
+    def test_open_twice(self):
+        foo = self.path('foo')
+        f1 = GitFile(foo, 'wb')
+        f1.write('new')
+        try:
+            f2 = GitFile(foo, 'wb')
+            fail()
+        except OSError, e:
+            self.assertEquals(errno.EEXIST, e.errno)
+        f1.write(' contents')
+        f1.close()
+
+        # Ensure trying to open twice doesn't affect original.
+        f = open(foo, 'rb')
+        self.assertEquals('new contents', f.read())
+        f.close()
+
+    def test_abort(self):
+        foo = self.path('foo')
+        foo_lock = '%s.lock' % foo
+
+        orig_f = open(foo, 'rb')
+        self.assertEquals(orig_f.read(), 'foo contents')
+        orig_f.close()
+
+        f = GitFile(foo, 'wb')
+        f.write('new contents')
+        f.abort()
+        self.assertTrue(f.closed)
+        self.assertFalse(os.path.exists(foo_lock))
+
+        new_orig_f = open(foo, 'rb')
+        self.assertEquals(new_orig_f.read(), 'foo contents')
+        new_orig_f.close()
+
+    def test_safe_method(self):
+        foo = self.path('foo')
+        foo_lock = '%s.lock' % foo
+
+        f = GitFile(foo, 'wb')
+        f.write('new contents')
+
+        def error_method(x):
+            f._test = x
+            raise IOError('fake IO error')
+
+        try:
+            f._safe_method(error_method)('test value')
+            fail()
+        except IOError, e:
+            # error is re-raised
+            self.assertEquals('fake IO error', e.message)
+
+        # method got correct args
+        self.assertEquals('test value', f._test)
+        self.assertFalse(os.path.exists(foo_lock))
+
+        new_orig_f = open(foo, 'rb')
+        self.assertEquals(new_orig_f.read(), 'foo contents')
+        new_orig_f.close()