2
0
Эх сурвалжийг харах

Merge Dave, highlights:

* fix thin pack handling
* more object checking
* test cleanups
* code cleanups
Jelmer Vernooij 15 жил өмнө
parent
commit
176a4f572a

+ 8 - 1
dulwich/errors.py

@@ -19,15 +19,22 @@
 
 """Dulwich-related exception classes and utility functions."""
 
+import binascii
+
+
 class ChecksumMismatch(Exception):
     """A checksum didn't match the expected contents."""
 
     def __init__(self, expected, got, extra=None):
+        if len(expected) == 20:
+            expected = binascii.hexlify(expected)
+        if len(got) == 20:
+            got = binascii.hexlify(got)
         self.expected = expected
         self.got = got
         self.extra = extra
         if self.extra is None:
-            Exception.__init__(self, 
+            Exception.__init__(self,
                 "Checksum mismatch: Expected %s, got %s" % (expected, got))
         else:
             Exception.__init__(self,

+ 18 - 9
dulwich/object_store.py

@@ -45,6 +45,7 @@ from dulwich.objects import (
 from dulwich.pack import (
     Pack,
     PackData,
+    ThinPackData,
     iter_sha1,
     load_pack_index,
     write_pack,
@@ -314,13 +315,14 @@ class PackBasedObjectStore(BaseObjectStore):
         """Add a set of objects to this object store.
 
         :param objects: Iterable over objects, should support __len__.
+        :return: Pack object of the objects written.
         """
         if len(objects) == 0:
             # Don't bother writing an empty pack file
             return
         f, commit = self.add_pack()
         write_pack_data(f, objects, len(objects))
-        commit()
+        return commit()
 
 
 class DiskObjectStore(PackBasedObjectStore):
@@ -390,24 +392,25 @@ class DiskObjectStore(PackBasedObjectStore):
 
         :param path: Path to the pack file.
         """
-        data = PackData(path)
+        data = ThinPackData(self, path)
 
         # Write index for the thin pack (do we really need this?)
         temppath = os.path.join(self.pack_dir, 
             sha_to_hex(urllib2.randombytes(20))+".tempidx")
-        data.create_index_v2(temppath, self.get_raw)
+        data.create_index_v2(temppath)
         p = Pack.from_objects(data, load_pack_index(temppath))
 
         # Write a full pack version
         temppath = os.path.join(self.pack_dir, 
             sha_to_hex(urllib2.randombytes(20))+".temppack")
-        write_pack(temppath, ((o, None) for o in p.iterobjects(self.get_raw)), 
-                len(p))
+        write_pack(temppath, ((o, None) for o in p.iterobjects()), len(p))
         pack_sha = load_pack_index(temppath+".idx").objects_sha1()
         newbasename = os.path.join(self.pack_dir, "pack-%s" % pack_sha)
         os.rename(temppath+".pack", newbasename+".pack")
         os.rename(temppath+".idx", newbasename+".idx")
-        self._add_known_pack(Pack(newbasename))
+        final_pack = Pack(newbasename)
+        self._add_known_pack(final_pack)
+        return final_pack
 
     def move_in_pack(self, path):
         """Move a specific file containing a pack into the pack directory.
@@ -424,7 +427,9 @@ class DiskObjectStore(PackBasedObjectStore):
         write_pack_index_v2(basename+".idx", entries, p.get_stored_checksum())
         p.close()
         os.rename(path, basename + ".pack")
-        self._add_known_pack(Pack(basename))
+        final_pack = Pack(basename)
+        self._add_known_pack(final_pack)
+        return final_pack
 
     def add_thin_pack(self):
         """Add a new thin pack to this object store.
@@ -438,7 +443,9 @@ class DiskObjectStore(PackBasedObjectStore):
             os.fsync(fd)
             f.close()
             if os.path.getsize(path) > 0:
-                self.move_in_thin_pack(path)
+                return self.move_in_thin_pack(path)
+            else:
+                return None
         return f, commit
 
     def add_pack(self):
@@ -453,7 +460,9 @@ class DiskObjectStore(PackBasedObjectStore):
             os.fsync(fd)
             f.close()
             if os.path.getsize(path) > 0:
-                self.move_in_pack(path)
+                return self.move_in_pack(path)
+            else:
+                return None
         return f, commit
 
     def add_object(self, obj):

+ 6 - 1
dulwich/objects.py

@@ -106,6 +106,11 @@ def filename_to_hex(filename):
     return hex
 
 
+def object_header(num_type, length):
+    """Return an object header for the given numeric type and text length."""
+    return "%s %d\0" % (object_class(num_type).type_name, length)
+
+
 def serializable_property(name, docstring=None):
     def set(obj, value):
         obj._ensure_parsed()
@@ -390,7 +395,7 @@ class ShaFile(object):
             raise ChecksumMismatch(new_sha, old_sha)
 
     def _header(self):
-        return "%s %lu\0" % (self.type_name, self.raw_length())
+        return object_header(self.type, self.raw_length())
 
     def raw_length(self):
         """Returns the length of the raw string of this object."""

+ 157 - 96
dulwich/pack.py

@@ -69,6 +69,7 @@ from dulwich.objects import (
     ShaFile,
     hex_to_sha,
     sha_to_hex,
+    object_header,
     )
 from dulwich.misc import (
     make_sha,
@@ -273,7 +274,8 @@ class PackIndex(object):
         """Unpack the i-th entry in the index file.
 
         :return: Tuple with object name (SHA), offset in pack file and CRC32
-            checksum (if known)."""
+            checksum (if known).
+        """
         raise NotImplementedError(self._unpack_entry)
 
     def _unpack_name(self, i):
@@ -306,8 +308,7 @@ class PackIndex(object):
     def iterentries(self):
         """Iterate over the entries in this pack index.
 
-        Will yield tuples with object name, offset in packfile and crc32
-        checksum.
+        :yields: tuples with object name, offset in packfile and crc32 checksum.
         """
         for i in range(len(self)):
             yield self._unpack_entry(i)
@@ -321,8 +322,10 @@ class PackIndex(object):
 
     def check(self):
         """Check that the stored checksum matches the actual checksum."""
-        # TODO: Check pack contents, too
-        return self.calculate_checksum() == self.get_stored_checksum()
+        actual = self.calculate_checksum()
+        stored = self.get_stored_checksum()
+        if actual != stored:
+            raise ChecksumMismatch(stored, actual)
 
     def calculate_checksum(self):
         """Calculate the SHA1 checksum over this pack index.
@@ -636,6 +639,14 @@ class PackObjectIterator(object):
         self.i+=1
         return ret
 
+def obj_sha(type, chunks):
+    """Compute the SHA for a numeric type and object chunks."""
+    sha = make_sha()
+    sha.update(object_header(type, chunks_length(chunks)))
+    for chunk in chunks:
+        sha.update(chunk)
+    return sha.digest()
+
 
 class PackData(object):
     """The data contained in a packfile.
@@ -667,8 +678,8 @@ class PackData(object):
     def __init__(self, filename, file=None, size=None):
         """Create a PackData object representing the pack in the given filename.
 
-        The file must exist and stay readable until the object is disposed of.
-        It must also stay the same size. It will be mapped whenever needed.
+        The file must exist and stay readable until the object is disposed of. It
+        must also stay the same size. It will be mapped whenever needed.
 
         Currently there is a restriction on the size of the pack as the python
         mmap implementation is flawed.
@@ -683,6 +694,7 @@ class PackData(object):
         (version, self._num_objects) = read_pack_header(self._file.read)
         self._offset_cache = LRUSizeCache(1024*1024*20,
             compute_size=_compute_object_size)
+        self.pack = None
 
     @classmethod
     def from_file(cls, file, size):
@@ -726,7 +738,18 @@ class PackData(object):
             todo -= len(x)
         return s.digest()
 
-    def resolve_object(self, offset, type, obj, get_ref, get_offset=None):
+    def get_ref(self, sha):
+        """Get the object for a ref SHA, only looking in this pack."""
+        # TODO: cache these results
+        if self.pack is None:
+            raise KeyError(sha)
+        offset = self.pack.index.object_index(sha)
+        if not offset:
+            raise KeyError(sha)
+        type, obj = self.get_object_at(offset)
+        return offset, type, obj
+
+    def resolve_object(self, offset, type, obj, get_ref=None):
         """Resolve an object, possibly resolving deltas when necessary.
 
         :return: Tuple with object type and contents.
@@ -734,124 +757,88 @@ class PackData(object):
         if type not in DELTA_TYPES:
             return type, obj
 
-        if get_offset is None:
-            get_offset = self.get_object_at
-
+        if get_ref is None:
+            get_ref = self.get_ref
         if type == OFS_DELTA:
             (delta_offset, delta) = obj
+            # TODO: clean up asserts and replace with nicer error messages
+            assert isinstance(offset, int)
             assert isinstance(delta_offset, int)
             base_offset = offset-delta_offset
-            type, base_obj = get_offset(base_offset)
+            type, base_obj = self.get_object_at(base_offset)
             assert isinstance(type, int)
         elif type == REF_DELTA:
             (basename, delta) = obj
             assert isinstance(basename, str) and len(basename) == 20
-            type, base_obj = get_ref(basename)
+            base_offset, type, base_obj = get_ref(basename)
             assert isinstance(type, int)
-            # Can't be a ofs delta, as we wouldn't know the base offset
-            assert type != OFS_DELTA
-            base_offset = None
-        type, base_chunks = self.resolve_object(base_offset, type, base_obj,
-                                                get_ref)
-        if base_offset is not None:
-            self._offset_cache[base_offset] = type, base_chunks
-        return (type, apply_delta(base_chunks, delta))
+        type, base_chunks = self.resolve_object(base_offset, type, base_obj)
+        chunks = apply_delta(base_chunks, delta)
+        # TODO(dborowitz): This can result in poor performance if large base
+        # objects are separated from deltas in the pack. We should reorganize
+        # so that we apply deltas to all objects in a chain one after the other
+        # to optimize cache performance.
+        if offset is not None:
+            self._offset_cache[offset] = type, chunks
+        return type, chunks
 
     def iterobjects(self, progress=None):
         return PackObjectIterator(self, progress)
 
-    def iterentries(self, ext_resolve_ref=None, progress=None):
+    def iterentries(self, progress=None):
         """Yield entries summarizing the contents of this pack.
 
-        :param ext_resolve_ref: Optional function to resolve base
-            objects (in case this is a thin pack)
-        :param progress: Progress function, called with current and
-            total object count.
-
-        This will yield tuples with (sha, offset, crc32)
+        :param progress: Progress function, called with current and total object
+            count.
+        :yields: tuples with (sha, offset, crc32)
         """
-        found = {}
-        postponed = defaultdict(list)
-        class Postpone(Exception):
-            """Raised to postpone delta resolving."""
-
-        def get_ref_text(sha):
-            assert len(sha) == 20
-            if sha in found:
-                return self.get_object_at(found[sha])
-            if ext_resolve_ref:
-                try:
-                    return ext_resolve_ref(sha)
-                except KeyError:
-                    pass
-            raise Postpone, (sha, )
-        extra = []
-        todo = chain(self.iterobjects(progress=progress), extra)
-        for (offset, type, obj, crc32) in todo:
+        for offset, type, obj, crc32 in self.iterobjects(progress=progress):
             assert isinstance(offset, int)
             assert isinstance(type, int)
-            try:
-                type, obj = self.resolve_object(offset, type, obj,
-                    get_ref_text)
-            except Postpone, (sha, ):
-                postponed[sha].append((offset, type, obj))
-            else:
-                shafile = ShaFile.from_raw_chunks(type, obj)
-                sha = shafile.sha().digest()
-                found[sha] = offset
-                yield sha, offset, crc32
-                extra.extend(postponed.get(sha, []))
-        if postponed:
-            raise KeyError([sha_to_hex(h) for h in postponed.keys()])
+            assert isinstance(obj, list) or isinstance(obj, tuple)
+            type, obj = self.resolve_object(offset, type, obj)
+            yield obj_sha(type, obj), offset, crc32
 
-    def sorted_entries(self, resolve_ext_ref=None, progress=None):
+    def sorted_entries(self, progress=None):
         """Return entries in this pack, sorted by SHA.
 
-        :param resolve_ext_ref: Optional function to resolve base
-            objects (in case this is a thin pack)
-        :param progress: Progress function, called with current and
-            total object count
+        :param progress: Progress function, called with current and total object
+            count
         :return: List of tuples with (sha, offset, crc32)
         """
-        ret = list(self.iterentries(resolve_ext_ref, progress=progress))
+        ret = list(self.iterentries(progress=progress))
         ret.sort()
         return ret
 
-    def create_index_v1(self, filename, resolve_ext_ref=None, progress=None):
+    def create_index_v1(self, filename, progress=None):
         """Create a version 1 file for this data file.
 
         :param filename: Index filename.
-        :param resolve_ext_ref: Function to use for resolving externally
-            referenced SHA1s (for thin packs)
         :param progress: Progress report function
         """
-        entries = self.sorted_entries(resolve_ext_ref, progress=progress)
+        entries = self.sorted_entries(progress=progress)
         write_pack_index_v1(filename, entries, self.calculate_checksum())
 
-    def create_index_v2(self, filename, resolve_ext_ref=None, progress=None):
+    def create_index_v2(self, filename, progress=None):
         """Create a version 2 index file for this data file.
 
         :param filename: Index filename.
-        :param resolve_ext_ref: Function to use for resolving externally
-            referenced SHA1s (for thin packs)
         :param progress: Progress report function
         """
-        entries = self.sorted_entries(resolve_ext_ref, progress=progress)
+        entries = self.sorted_entries(progress=progress)
         write_pack_index_v2(filename, entries, self.calculate_checksum())
 
-    def create_index(self, filename, resolve_ext_ref=None, progress=None,
+    def create_index(self, filename, progress=None,
                      version=2):
         """Create an  index file for this data file.
 
         :param filename: Index filename.
-        :param resolve_ext_ref: Function to use for resolving externally
-            referenced SHA1s (for thin packs)
         :param progress: Progress report function
         """
         if version == 1:
-            self.create_index_v1(filename, resolve_ext_ref, progress)
+            self.create_index_v1(filename, progress)
         elif version == 2:
-            self.create_index_v2(filename, resolve_ext_ref, progress)
+            self.create_index_v2(filename, progress)
         else:
             raise ValueError("unknown index format %d" % version)
 
@@ -862,7 +849,10 @@ class PackData(object):
 
     def check(self):
         """Check the consistency of this pack."""
-        return (self.calculate_checksum() == self.get_stored_checksum())
+        actual = self.calculate_checksum()
+        stored = self.get_stored_checksum()
+        if actual != stored:
+            raise ChecksumMismatch(stored, actual)
 
     def get_object_at(self, offset):
         """Given an offset in to the packfile return the object that is there.
@@ -880,6 +870,77 @@ class PackData(object):
         return unpack_object(self._file.read)[:2]
 
 
+class ThinPackData(PackData):
+    """PackData for thin packs, which require an ObjectStore for resolving."""
+
+    def __init__(self, store, *args, **kwargs):
+        super(ThinPackData, self).__init__(*args, **kwargs)
+        self.store = store
+
+    def get_ref(self, sha):
+        """Resolve a reference looking in both this pack and the store."""
+        try:
+            # As part of completing a pack we create a Pack object with a
+            # ThinPackData and a full PackIndex, so check in the index first if
+            # possible.
+            # TODO(dborowitz): reevaluate this when the pack completion code is
+            # rewritten.
+            return super(ThinPackData, self).get_ref(sha)
+        except KeyError:
+            type, obj = self.store.get_raw(sha)
+            return None, type, obj
+
+    def iterentries(self, progress=None):
+        """Yield entries summarizing the contents of this pack.
+
+        :param progress: Progress function, called with current and
+            total object count.
+
+        This will yield tuples with (sha, offset, crc32)
+        """
+        found = {}
+        postponed = defaultdict(list)
+
+        class Postpone(Exception):
+            """Raised to postpone delta resolving."""
+
+            def __init__(self, sha):
+                self.sha = sha
+
+        def get_ref_text(sha):
+            assert len(sha) == 20
+            if sha in found:
+                offset = found[sha]
+                type, obj = self.get_object_at(offset)
+                return offset, type, obj
+            try:
+                return self.get_ref(sha)
+            except KeyError:
+                raise Postpone(sha)
+
+        extra = []
+        todo = chain(self.iterobjects(progress=progress), extra)
+        for (offset, type, obj, crc32) in todo:
+            assert isinstance(offset, int)
+            if obj is None:
+                # Inflate postponed delta
+                obj, type = self.get_object_at(offset)
+            assert isinstance(type, int)
+            assert isinstance(obj, list) or isinstance(obj, tuple)
+            try:
+                type, obj = self.resolve_object(offset, type, obj, get_ref_text)
+            except Postpone, e:
+                # Save memory by not storing the inflated obj in postponed
+                postponed[e.sha].append((offset, type, None, crc32))
+            else:
+                sha = obj_sha(type, obj)
+                found[sha] = offset
+                yield sha, offset, crc32
+                extra.extend(postponed.pop(sha, []))
+        if postponed:
+            raise KeyError([sha_to_hex(h) for h in postponed.keys()])
+
+
 class SHA1Reader(object):
     """Wrapper around a file-like object that remembers the SHA1 of its data."""
 
@@ -1232,6 +1293,7 @@ class Pack(object):
         ret = Pack("")
         ret._data = data
         ret._idx = idx
+        data.pack = ret
         return ret
 
     def name(self):
@@ -1243,6 +1305,7 @@ class Pack(object):
         """The pack data object being used."""
         if self._data is None:
             self._data = PackData(self._data_path)
+            self._data.pack = self
             assert len(self.index) == len(self._data)
             idx_stored_checksum = self.index.get_pack_checksum()
             data_stored_checksum = self._data.get_stored_checksum()
@@ -1281,12 +1344,15 @@ class Pack(object):
         return iter(self.index)
 
     def check(self):
-        """Check the integrity of this pack."""
-        if not self.index.check():
-            return False
-        if not self.data.check():
-            return False
-        return True
+        """Check the integrity of this pack.
+
+        :raise ChecksumMismatch: if a checksum for the index or data is wrong
+        """
+        self.index.check()
+        self.data.check()
+        for obj in self.iterobjects():
+            obj.check()
+        # TODO: object connectivity checks
 
     def get_stored_checksum(self):
         return self.data.get_stored_checksum()
@@ -1299,30 +1365,25 @@ class Pack(object):
         except KeyError:
             return False
 
-    def get_raw(self, sha1, resolve_ref=None):
+    def get_raw(self, sha1):
         offset = self.index.object_index(sha1)
         obj_type, obj = self.data.get_object_at(offset)
         if type(offset) is long:
           offset = int(offset)
-        if resolve_ref is None:
-            resolve_ref = self.get_raw
-        kind, chunks = self.data.resolve_object(offset, obj_type, obj,
-            resolve_ref)
-        return kind, "".join(chunks)
+        type_num, chunks = self.data.resolve_object(offset, obj_type, obj)
+        return type_num, "".join(chunks)
 
     def __getitem__(self, sha1):
         """Retrieve the specified SHA1."""
         type, uncomp = self.get_raw(sha1)
         return ShaFile.from_raw_string(type, uncomp)
 
-    def iterobjects(self, get_raw=None):
+    def iterobjects(self):
         """Iterate over the objects in this pack."""
-        if get_raw is None:
-            get_raw = self.get_raw
         for offset, type, obj, crc32 in self.data.iterobjects():
             assert isinstance(offset, int)
-            type, obj = self.data.resolve_object(offset, type, obj, get_raw)
-            yield ShaFile.from_raw_chunks(type, obj)
+            yield ShaFile.from_raw_chunks(
+                    *self.data.resolve_object(offset, type, obj))
 
 
 try:

+ 24 - 24
dulwich/server.py

@@ -27,12 +27,15 @@ Documentation/technical directory in the cgit distribution, and in particular:
 
 
 import collections
+import socket
+import zlib
 import SocketServer
 
 from dulwich.errors import (
     ApplyDeltaError,
     ChecksumMismatch,
     GitProtocolError,
+    ObjectFormatException,
     )
 from dulwich.objects import (
     hex_to_sha,
@@ -49,9 +52,9 @@ from dulwich.protocol import (
     SINGLE_ACK,
     TCP_GIT_PORT,
     ZERO_SHA,
+    ack_type,
     extract_capabilities,
     extract_want_line_capabilities,
-    ack_type,
     )
 
 
@@ -123,9 +126,10 @@ class PackStreamCopier(PackStreamReader):
     def verify(self):
         """Verify a pack stream and write it to the output file.
 
-        See PackStreamReader.iterobjects for a list of exceptions this may throw.
+        See PackStreamReader.iterobjects for a list of exceptions this may
+        throw.
         """
-        for _, _, _, _ in self.iterobjects():
+        for _, _, _ in self.read_objects():
             pass
 
 
@@ -560,48 +564,44 @@ class ReceivePackHandler(Handler):
 
     def _apply_pack(self, refs):
         f, commit = self.repo.object_store.add_thin_pack()
-        all_exceptions = (IOError, OSError, ChecksumMismatch, ApplyDeltaError)
+        all_exceptions = (IOError, OSError, ChecksumMismatch, ApplyDeltaError,
+                          AssertionError, socket.error, zlib.error,
+                          ObjectFormatException)
         status = []
         unpack_error = None
         # TODO: more informative error messages than just the exception string
         try:
             PackStreamCopier(self.proto.read, self.proto.recv, f).verify()
-        except all_exceptions, e:
-            unpack_error = str(e).replace('\n', '')
-        try:
-            commit()
-        except all_exceptions, e:
-            if not unpack_error:
-                unpack_error = str(e).replace('\n', '')
-
-        if unpack_error:
-            status.append(('unpack', unpack_error))
-        else:
+            p = commit()
+            if not p:
+                raise IOError('Failed to write pack')
+            p.check()
             status.append(('unpack', 'ok'))
+        except all_exceptions, e:
+            status.append(('unpack', str(e).replace('\n', '')))
+            # The pack may still have been moved in, but it may contain broken
+            # objects. We trust a later GC to clean it up.
 
         for oldsha, sha, ref in refs:
-            ref_error = None
+            ref_status = 'ok'
             try:
                 if sha == ZERO_SHA:
-                    if not self.has_capability('delete-refs'):
+                    if not 'delete-refs' in self.capabilities():
                         raise GitProtocolError(
                           'Attempted to delete refs without delete-refs '
                           'capability.')
                     try:
                         del self.repo.refs[ref]
                     except all_exceptions:
-                        ref_error = 'failed to delete'
+                        ref_status = 'failed to delete'
                 else:
                     try:
                         self.repo.refs[ref] = sha
                     except all_exceptions:
-                        ref_error = 'failed to write'
+                        ref_status = 'failed to write'
             except KeyError, e:
-                ref_error = 'bad ref'
-            if ref_error:
-                status.append((ref, ref_error))
-            else:
-                status.append((ref, 'ok'))
+                ref_status = 'bad ref'
+            status.append((ref, ref_status))
 
         return status
 

+ 2 - 1
dulwich/tests/compat/server_utils.py

@@ -53,7 +53,8 @@ class ServerTests(object):
 
     def assertReposEqual(self, repo1, repo2):
         self.assertEqual(repo1.get_refs(), repo2.get_refs())
-        self.assertEqual(set(repo1.object_store), set(repo2.object_store))
+        self.assertEqual(sorted(set(repo1.object_store)),
+                         sorted(set(repo2.object_store)))
 
     def assertReposNotEqual(self, repo1, repo2):
         refs1 = repo1.get_refs()

+ 1 - 1
dulwich/tests/compat/test_pack.py

@@ -52,7 +52,7 @@ class TestPack(PackTests):
 
     def test_copy(self):
         origpack = self.get_pack(pack1_sha)
-        self.assertEquals(True, origpack.index.check())
+        self.assertSucceeds(origpack.index.check)
         pack_path = os.path.join(self._tempdir, "Elch")
         write_pack(pack_path, [(x, "") for x in origpack.iterobjects()],
                    len(origpack))

+ 0 - 4
dulwich/tests/compat/test_server.py

@@ -76,7 +76,3 @@ class GitServerTestCase(ServerTests, CompatTestCase):
         self._server = dul_server
         _, port = self._server.socket.getsockname()
         return port
-
-    def test_push_to_dulwich(self):
-        # TODO(dborowitz): enable after merging thin pack fixes.
-        raise TestSkipped('Skipping push test due to known deadlock bug.')

+ 0 - 4
dulwich/tests/compat/test_web.py

@@ -96,10 +96,6 @@ class SmartWebTestCase(WebTests, CompatTestCase):
     def _make_app(self, backend):
         return HTTPGitApplication(backend)
 
-    def test_push_to_dulwich(self):
-        # TODO(dborowitz): enable after merging thin pack fixes.
-        raise TestSkipped('Skipping push test due to known pack bug.')
-
 
 class DumbWebTestCase(WebTests, CompatTestCase):
     """Test cases for dumb HTTP server."""

+ 26 - 11
dulwich/tests/test_index.py

@@ -1,16 +1,16 @@
 # test_index.py -- Tests for the git index
 # Copyright (C) 2008-2009 Jelmer Vernooij <jelmer@samba.org>
-# 
+#
 # 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
 # or (at your option) any 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,
@@ -24,8 +24,10 @@ from cStringIO import (
     StringIO,
     )
 import os
+import shutil
 import stat
 import struct
+import tempfile
 from unittest import TestCase
 
 from dulwich.index import (
@@ -43,6 +45,7 @@ from dulwich.objects import (
     Blob,
     )
 
+
 class IndexTestCase(TestCase):
 
     datadir = os.path.join(os.path.dirname(__file__), 'data/indexes')
@@ -51,7 +54,7 @@ class IndexTestCase(TestCase):
         return Index(os.path.join(self.datadir, name))
 
 
-class SimpleIndexTestcase(IndexTestCase):
+class SimpleIndexTestCase(IndexTestCase):
 
     def test_len(self):
         self.assertEquals(1, len(self.get_simple_index("index")))
@@ -60,21 +63,33 @@ class SimpleIndexTestcase(IndexTestCase):
         self.assertEquals(['bla'], list(self.get_simple_index("index")))
 
     def test_getitem(self):
-        self.assertEquals( ((1230680220, 0), (1230680220, 0), 2050, 3761020, 33188, 1000, 1000, 0, 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', 0)
-            , 
-                self.get_simple_index("index")["bla"])
+        self.assertEquals(((1230680220, 0), (1230680220, 0), 2050, 3761020,
+                           33188, 1000, 1000, 0,
+                           'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', 0),
+                          self.get_simple_index("index")["bla"])
 
 
 class SimpleIndexWriterTestCase(IndexTestCase):
 
+    def setUp(self):
+        IndexTestCase.setUp(self)
+        self.tempdir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        IndexTestCase.tearDown(self)
+        shutil.rmtree(self.tempdir)
+
     def test_simple_write(self):
-        entries = [('barbla', (1230680220, 0), (1230680220, 0), 2050, 3761020, 33188, 1000, 1000, 0, 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', 0)]
-        x = open('test-simple-write-index', 'w+')
+        entries = [('barbla', (1230680220, 0), (1230680220, 0), 2050, 3761020,
+                    33188, 1000, 1000, 0,
+                    'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', 0)]
+        filename = os.path.join(self.tempdir, 'test-simple-write-index')
+        x = open(filename, 'w+')
         try:
             write_index(x, entries)
         finally:
             x.close()
-        x = open('test-simple-write-index', 'r')
+        x = open(filename, 'r')
         try:
             self.assertEquals(entries, list(read_index(x)))
         finally:
@@ -108,7 +123,7 @@ class CommitTreeTests(TestCase):
         self.assertEquals(dirid, "c1a1deb9788150829579a8b4efa6311e7b638650")
         self.assertEquals((stat.S_IFDIR, dirid), self.store[rootid]["bla"])
         self.assertEquals((stat.S_IFREG, blob.id), self.store[dirid]["bar"])
-        self.assertEquals(set([rootid, dirid, blob.id]), 
+        self.assertEquals(set([rootid, dirid, blob.id]),
                           set(self.store._data.keys()))
 
 

+ 19 - 27
dulwich/tests/test_object_store.py

@@ -1,16 +1,16 @@
 # test_object_store.py -- tests for object_store.py
 # Copyright (C) 2008 Jelmer Vernooij <jelmer@samba.org>
-# 
+#
 # 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
 # or (at your option) any 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,
@@ -20,6 +20,8 @@
 """Tests for the object store interface."""
 
 
+import shutil
+import tempfile
 from unittest import TestCase
 
 from dulwich.objects import (
@@ -38,34 +40,16 @@ testobject = Blob()
 testobject.data = "yummy data"
 
 
-class SpecificDiskObjectStoreTests(TestCase):
-
-    def setUp(self):
-        self.store_dir = tempfile.mkdtemp()
-
-    def tearDown(self):
-        shutil.rmtree(self.store_dir)
-
-    def test_pack_dir(self):
-        o = DiskObjectStore(self.store_dir)
-        self.assertEquals(os.path.join(self.store_dir, "pack"), o.pack_dir)
-
-    def test_empty_packs(self):
-        o = DiskObjectStore(self.store_dir)
-        self.assertEquals([], o.packs)
-
-
-
 class ObjectStoreTests(object):
 
     def test_iter(self):
         self.assertEquals([], list(self.store))
 
     def test_get_nonexistant(self):
-        self.assertRaises(KeyError, self.store.__getitem__, "a" * 40)
+        self.assertRaises(KeyError, lambda: self.store["a" * 40])
 
     def test_contains_nonexistant(self):
-        self.assertFalse(self.store.__contains__("a" * 40))
+        self.assertFalse(("a" * 40) in self.store)
 
     def test_add_objects_empty(self):
         self.store.add_objects([])
@@ -78,7 +62,7 @@ class ObjectStoreTests(object):
     def test_add_object(self):
         self.store.add_object(testobject)
         self.assertEquals(set([testobject.id]), set(self.store))
-        self.assertTrue(self.store.__contains__(testobject.id))
+        self.assertTrue(testobject.id in self.store)
         r = self.store[testobject.id]
         self.assertEquals(r, testobject)
 
@@ -86,19 +70,19 @@ class ObjectStoreTests(object):
         data = [(testobject, "mypath")]
         self.store.add_objects(data)
         self.assertEquals(set([testobject.id]), set(self.store))
-        self.assertTrue(self.store.__contains__(testobject.id))
+        self.assertTrue(testobject.id in self.store)
         r = self.store[testobject.id]
         self.assertEquals(r, testobject)
 
 
-class MemoryObjectStoreTests(ObjectStoreTests,TestCase):
+class MemoryObjectStoreTests(ObjectStoreTests, TestCase):
 
     def setUp(self):
         TestCase.setUp(self)
         self.store = MemoryObjectStore()
 
 
-class DiskObjectStoreTests(ObjectStoreTests,TestCase):
+class DiskObjectStoreTests(ObjectStoreTests, TestCase):
 
     def setUp(self):
         TestCase.setUp(self)
@@ -109,5 +93,13 @@ class DiskObjectStoreTests(ObjectStoreTests,TestCase):
         TestCase.tearDown(self)
         shutil.rmtree(self.store_dir)
 
+    def test_pack_dir(self):
+        o = DiskObjectStore(self.store_dir)
+        self.assertEquals(os.path.join(self.store_dir, "pack"), o.pack_dir)
+
+    def test_empty_packs(self):
+        o = DiskObjectStore(self.store_dir)
+        self.assertEquals([], o.packs)
+
 
 # TODO: MissingObjectFinderTests

+ 10 - 0
dulwich/tests/test_objects.py

@@ -39,6 +39,7 @@ from dulwich.objects import (
     Tag,
     format_timezone,
     hex_to_sha,
+    sha_to_hex,
     hex_to_filename,
     check_hexsha,
     check_identity,
@@ -89,6 +90,15 @@ except ImportError:
                 return
 
 
+class TestHexToSha(unittest.TestCase):
+
+    def test_simple(self):
+        self.assertEquals("\xab\xcd" * 10, hex_to_sha("abcd" * 10))
+
+    def test_reverse(self):
+        self.assertEquals("abcd" * 10, sha_to_hex("\xab\xcd" * 10))
+
+
 class BlobReadTests(unittest.TestCase):
     """Test decompression of blobs"""
 

+ 148 - 78
dulwich/tests/test_pack.py

@@ -1,17 +1,17 @@
 # test_pack.py -- Tests for the handling of git packs.
 # Copyright (C) 2007 James Westby <jw+debian@jameswestby.net>
 # Copyright (C) 2008 Jelmer Vernooij <jelmer@samba.org>
-# 
+#
 # 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) any 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,
@@ -23,10 +23,17 @@
 
 from cStringIO import StringIO
 import os
+import shutil
+import tempfile
 import unittest
 import zlib
 
+from dulwich.errors import (
+    ChecksumMismatch,
+    )
 from dulwich.objects import (
+    hex_to_sha,
+    sha_to_hex,
     Tree,
     )
 from dulwich.pack import (
@@ -49,26 +56,39 @@ a_sha = '6f670c0fb53f9463760b7295fbb814e965fb20c8'
 tree_sha = 'b2a2766a2879c209ab1176e7e778b81ae422eeaa'
 commit_sha = 'f18faa16531ac570a3fdc8c7ca16682548dafd12'
 
+
 class PackTests(unittest.TestCase):
     """Base class for testing packs"""
-  
+
+    def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.tempdir)
+
     datadir = os.path.join(os.path.dirname(__file__), 'data/packs')
-  
+
     def get_pack_index(self, sha):
         """Returns a PackIndex from the datadir with the given sha"""
         return load_pack_index(os.path.join(self.datadir, 'pack-%s.idx' % sha))
-  
+
     def get_pack_data(self, sha):
         """Returns a PackData object from the datadir with the given sha"""
         return PackData(os.path.join(self.datadir, 'pack-%s.pack' % sha))
-  
+
     def get_pack(self, sha):
         return Pack(os.path.join(self.datadir, 'pack-%s' % sha))
 
+    def assertSucceeds(self, func, *args, **kwargs):
+        try:
+            func(*args, **kwargs)
+        except ChecksumMismatch, e:
+            self.fail(e)
+
 
 class PackIndexTests(PackTests):
     """Class that tests the index of packfiles"""
-  
+
     def test_object_index(self):
         """Tests that the correct object offset is returned from the index."""
         p = self.get_pack_index(pack1_sha)
@@ -76,88 +96,118 @@ class PackIndexTests(PackTests):
         self.assertEqual(p.object_index(a_sha), 178)
         self.assertEqual(p.object_index(tree_sha), 138)
         self.assertEqual(p.object_index(commit_sha), 12)
-  
+
     def test_index_len(self):
         p = self.get_pack_index(pack1_sha)
         self.assertEquals(3, len(p))
-  
+
     def test_get_stored_checksum(self):
         p = self.get_pack_index(pack1_sha)
-        self.assertEquals("\xf2\x84\x8e*\xd1o2\x9a\xe1\xc9.;\x95\xe9\x18\x88\xda\xa5\xbd\x01", str(p.get_stored_checksum()))
-        self.assertEquals( 'r\x19\x80\xe8f\xaf\x9a_\x93\xadgAD\xe1E\x9b\x8b\xa3\xe7\xb7' , str(p.get_pack_checksum()))
-  
+        self.assertEquals('f2848e2ad16f329ae1c92e3b95e91888daa5bd01',
+                          sha_to_hex(p.get_stored_checksum()))
+        self.assertEquals('721980e866af9a5f93ad674144e1459b8ba3e7b7',
+                          sha_to_hex(p.get_pack_checksum()))
+
     def test_index_check(self):
         p = self.get_pack_index(pack1_sha)
-        self.assertEquals(True, p.check())
-  
+        self.assertSucceeds(p.check)
+
     def test_iterentries(self):
         p = self.get_pack_index(pack1_sha)
-        self.assertEquals([('og\x0c\x0f\xb5?\x94cv\x0br\x95\xfb\xb8\x14\xe9e\xfb \xc8', 178, None), ('\xb2\xa2vj(y\xc2\t\xab\x11v\xe7\xe7x\xb8\x1a\xe4"\xee\xaa', 138, None), ('\xf1\x8f\xaa\x16S\x1a\xc5p\xa3\xfd\xc8\xc7\xca\x16h%H\xda\xfd\x12', 12, None)], list(p.iterentries()))
-  
+        entries = [(sha_to_hex(s), o, c) for s, o, c in p.iterentries()]
+        self.assertEquals([
+          ('6f670c0fb53f9463760b7295fbb814e965fb20c8', 178, None),
+          ('b2a2766a2879c209ab1176e7e778b81ae422eeaa', 138, None),
+          ('f18faa16531ac570a3fdc8c7ca16682548dafd12', 12, None)
+          ], entries)
+
     def test_iter(self):
         p = self.get_pack_index(pack1_sha)
         self.assertEquals(set([tree_sha, commit_sha, a_sha]), set(p))
-  
+
 
 class TestPackDeltas(unittest.TestCase):
-  
-    test_string1 = "The answer was flailing in the wind"
-    test_string2 = "The answer was falling down the pipe"
-    test_string3 = "zzzzz"
-  
-    test_string_empty = ""
-    test_string_big = "Z" * 8192
-  
+
+    test_string1 = 'The answer was flailing in the wind'
+    test_string2 = 'The answer was falling down the pipe'
+    test_string3 = 'zzzzz'
+
+    test_string_empty = ''
+    test_string_big = 'Z' * 8192
+
     def _test_roundtrip(self, base, target):
         self.assertEquals([target],
-            apply_delta(base, create_delta(base, target)))
-  
+                          apply_delta(base, create_delta(base, target)))
+
     def test_nochange(self):
         self._test_roundtrip(self.test_string1, self.test_string1)
-  
+
     def test_change(self):
         self._test_roundtrip(self.test_string1, self.test_string2)
-  
+
     def test_rewrite(self):
         self._test_roundtrip(self.test_string1, self.test_string3)
-  
+
     def test_overflow(self):
         self._test_roundtrip(self.test_string_empty, self.test_string_big)
 
 
 class TestPackData(PackTests):
     """Tests getting the data from the packfile."""
-  
+
     def test_create_pack(self):
         p = self.get_pack_data(pack1_sha)
-  
+
     def test_pack_len(self):
         p = self.get_pack_data(pack1_sha)
         self.assertEquals(3, len(p))
-  
+
     def test_index_check(self):
         p = self.get_pack_data(pack1_sha)
-        self.assertEquals(True, p.check())
-  
+        self.assertSucceeds(p.check)
+
     def test_iterobjects(self):
         p = self.get_pack_data(pack1_sha)
-        self.assertEquals([(12, 1, 'tree b2a2766a2879c209ab1176e7e778b81ae422eeaa\nauthor James Westby <jw+debian@jameswestby.net> 1174945067 +0100\ncommitter James Westby <jw+debian@jameswestby.net> 1174945067 +0100\n\nTest commit\n', 3775879613L), (138, 2, '100644 a\x00og\x0c\x0f\xb5?\x94cv\x0br\x95\xfb\xb8\x14\xe9e\xfb \xc8', 912998690L), (178, 3, 'test 1\n', 1373561701L)], [(len, type, "".join(chunks), offset) for (len, type, chunks, offset) in p.iterobjects()])
-  
+        commit_data = ('tree b2a2766a2879c209ab1176e7e778b81ae422eeaa\n'
+                       'author James Westby <jw+debian@jameswestby.net> '
+                       '1174945067 +0100\n'
+                       'committer James Westby <jw+debian@jameswestby.net> '
+                       '1174945067 +0100\n'
+                       '\n'
+                       'Test commit\n')
+        blob_sha = '6f670c0fb53f9463760b7295fbb814e965fb20c8'
+        tree_data = '100644 a\0%s' % hex_to_sha(blob_sha)
+        actual = []
+        for offset, type, chunks, crc32 in p.iterobjects():
+            actual.append((offset, type, ''.join(chunks), crc32))
+        self.assertEquals([
+          (12, 1, commit_data, 3775879613L),
+          (138, 2, tree_data, 912998690L),
+          (178, 3, 'test 1\n', 1373561701L)
+          ], actual)
+
     def test_iterentries(self):
         p = self.get_pack_data(pack1_sha)
-        self.assertEquals(set([('og\x0c\x0f\xb5?\x94cv\x0br\x95\xfb\xb8\x14\xe9e\xfb \xc8', 178, 1373561701L), ('\xb2\xa2vj(y\xc2\t\xab\x11v\xe7\xe7x\xb8\x1a\xe4"\xee\xaa', 138, 912998690L), ('\xf1\x8f\xaa\x16S\x1a\xc5p\xa3\xfd\xc8\xc7\xca\x16h%H\xda\xfd\x12', 12, 3775879613L)]), set(p.iterentries()))
-  
+        entries = set((sha_to_hex(s), o, c) for s, o, c in p.iterentries())
+        self.assertEquals(set([
+          ('6f670c0fb53f9463760b7295fbb814e965fb20c8', 178, 1373561701L),
+          ('b2a2766a2879c209ab1176e7e778b81ae422eeaa', 138, 912998690L),
+          ('f18faa16531ac570a3fdc8c7ca16682548dafd12', 12, 3775879613L),
+          ]), entries)
+
     def test_create_index_v1(self):
         p = self.get_pack_data(pack1_sha)
-        p.create_index_v1("v1test.idx")
-        idx1 = load_pack_index("v1test.idx")
+        filename = os.path.join(self.tempdir, 'v1test.idx')
+        p.create_index_v1(filename)
+        idx1 = load_pack_index(filename)
         idx2 = self.get_pack_index(pack1_sha)
         self.assertEquals(idx1, idx2)
-  
+
     def test_create_index_v2(self):
         p = self.get_pack_data(pack1_sha)
-        p.create_index_v2("v2test.idx")
-        idx1 = load_pack_index("v2test.idx")
+        filename = os.path.join(self.tempdir, 'v2test.idx')
+        p.create_index_v2(filename)
+        idx1 = load_pack_index(filename)
         idx2 = self.get_pack_index(pack1_sha)
         self.assertEquals(idx1, idx2)
 
@@ -195,25 +245,27 @@ class TestPack(PackTests):
 
     def test_copy(self):
         origpack = self.get_pack(pack1_sha)
-        self.assertEquals(True, origpack.index.check())
-        write_pack("Elch", [(x, "") for x in origpack.iterobjects()], 
-            len(origpack))
-        newpack = Pack("Elch")
+        self.assertSucceeds(origpack.index.check)
+        basename = os.path.join(self.tempdir, 'Elch')
+        write_pack(basename, [(x, '') for x in origpack.iterobjects()],
+                   len(origpack))
+        newpack = Pack(basename)
         self.assertEquals(origpack, newpack)
-        self.assertEquals(True, newpack.index.check())
+        self.assertSucceeds(newpack.index.check)
         self.assertEquals(origpack.name(), newpack.name())
-        self.assertEquals(origpack.index.get_pack_checksum(), 
+        self.assertEquals(origpack.index.get_pack_checksum(),
                           newpack.index.get_pack_checksum())
-        
-        self.assertTrue(
-                (origpack.index.version != newpack.index.version) or
-                (origpack.index.get_stored_checksum() == newpack.index.get_stored_checksum()))
+
+        wrong_version = origpack.index.version != newpack.index.version
+        orig_checksum = origpack.index.get_stored_checksum()
+        new_checksum = newpack.index.get_stored_checksum()
+        self.assertTrue(wrong_version or orig_checksum == new_checksum)
 
     def test_commit_obj(self):
         p = self.get_pack(pack1_sha)
         commit = p[commit_sha]
-        self.assertEquals("James Westby <jw+debian@jameswestby.net>",
-            commit.author)
+        self.assertEquals('James Westby <jw+debian@jameswestby.net>',
+                          commit.author)
         self.assertEquals([], commit.parents)
 
     def test_name(self):
@@ -221,63 +273,81 @@ class TestPack(PackTests):
         self.assertEquals(pack1_sha, p.name())
 
 
-class TestHexToSha(unittest.TestCase):
+pack_checksum = hex_to_sha('721980e866af9a5f93ad674144e1459b8ba3e7b7')
+
 
-    def test_simple(self):
-        self.assertEquals('\xab\xcd' * 10, hex_to_sha("abcd" * 10))
+class BaseTestPackIndexWriting(object):
 
-    def test_reverse(self):
-        self.assertEquals("abcd" * 10, sha_to_hex('\xab\xcd' * 10))
+    def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
 
+    def tearDown(self):
+        shutil.rmtree(self.tempdir)
 
-class BaseTestPackIndexWriting(object):
+    def assertSucceeds(self, func, *args, **kwargs):
+        try:
+            func(*args, **kwargs)
+        except ChecksumMismatch, e:
+            self.fail(e)
 
     def test_empty(self):
-        pack_checksum = 'r\x19\x80\xe8f\xaf\x9a_\x93\xadgAD\xe1E\x9b\x8b\xa3\xe7\xb7'
-        self._write_fn("empty.idx", [], pack_checksum)
-        idx = load_pack_index("empty.idx")
-        self.assertTrue(idx.check())
+        filename = os.path.join(self.tempdir, 'empty.idx')
+        self._write_fn(filename, [], pack_checksum)
+        idx = load_pack_index(filename)
+        self.assertSucceeds(idx.check)
         self.assertEquals(idx.get_pack_checksum(), pack_checksum)
         self.assertEquals(0, len(idx))
 
     def test_single(self):
-        pack_checksum = 'r\x19\x80\xe8f\xaf\x9a_\x93\xadgAD\xe1E\x9b\x8b\xa3\xe7\xb7'
-        my_entries = [('og\x0c\x0f\xb5?\x94cv\x0br\x95\xfb\xb8\x14\xe9e\xfb \xc8', 178, 42)]
-        my_entries.sort()
-        self._write_fn("single.idx", my_entries, pack_checksum)
-        idx = load_pack_index("single.idx")
+        entry_sha = hex_to_sha('6f670c0fb53f9463760b7295fbb814e965fb20c8')
+        my_entries = [(entry_sha, 178, 42)]
+        filename = os.path.join(self.tempdir, 'single.idx')
+        self._write_fn(filename, my_entries, pack_checksum)
+        idx = load_pack_index(filename)
         self.assertEquals(idx.version, self._expected_version)
-        self.assertTrue(idx.check())
+        self.assertSucceeds(idx.check)
         self.assertEquals(idx.get_pack_checksum(), pack_checksum)
         self.assertEquals(1, len(idx))
         actual_entries = list(idx.iterentries())
         self.assertEquals(len(my_entries), len(actual_entries))
-        for a, b in zip(my_entries, actual_entries):
-            self.assertEquals(a[0], b[0])
-            self.assertEquals(a[1], b[1])
+        for mine, actual in zip(my_entries, actual_entries):
+            my_sha, my_offset, my_crc = mine
+            actual_sha, actual_offset, actual_crc = actual
+            self.assertEquals(my_sha, actual_sha)
+            self.assertEquals(my_offset, actual_offset)
             if self._has_crc32_checksum:
-                self.assertEquals(a[2], b[2])
+                self.assertEquals(my_crc, actual_crc)
             else:
-                self.assertTrue(b[2] is None)
+                self.assertTrue(actual_crc is None)
 
 
 class TestPackIndexWritingv1(unittest.TestCase, BaseTestPackIndexWriting):
 
     def setUp(self):
         unittest.TestCase.setUp(self)
+        BaseTestPackIndexWriting.setUp(self)
         self._has_crc32_checksum = False
         self._expected_version = 1
         self._write_fn = write_pack_index_v1
 
+    def tearDown(self):
+        unittest.TestCase.tearDown(self)
+        BaseTestPackIndexWriting.tearDown(self)
+
 
 class TestPackIndexWritingv2(unittest.TestCase, BaseTestPackIndexWriting):
 
     def setUp(self):
         unittest.TestCase.setUp(self)
+        BaseTestPackIndexWriting.setUp(self)
         self._has_crc32_checksum = True
         self._expected_version = 2
         self._write_fn = write_pack_index_v2
 
+    def tearDown(self):
+        unittest.TestCase.tearDown(self)
+        BaseTestPackIndexWriting.tearDown(self)
+
 
 class ReadZlibTests(unittest.TestCase):