Merge pull request #1127 from jelmer/pack-windows

Factor out common _complete_thin_pack

docs/tutorial/remote.txt

@@ -9,7 +9,8 @@ Most of the tests in this file require a Dulwich server, so let's start one:
     >>> cid = repo.do_commit(b"message", committer=b"Jelmer <jelmer@samba.org>")
     >>> backend = DictBackend({b'/': repo})
     >>> dul_server = TCPGitServer(backend, b'localhost', 0)
-    >>> threading.Thread(target=dul_server.serve).start()
+    >>> server_thread = threading.Thread(target=dul_server.serve)
+    >>> server_thread.start()
     >>> server_address, server_port=dul_server.socket.getsockname()
 
 Remote repositories
@@ -82,6 +83,7 @@ importing the received pack file into the local repository::
    >>> from dulwich.repo import Repo
    >>> local = Repo.init("local", mkdir=True)
    >>> remote_refs = client.fetch(b"/", local)
+   >>> local.close()
 
 Let's shut down the server now that all tests have been run::
 

dulwich/object_store.py

@@ -824,7 +824,13 @@ class DiskObjectStore(PackBasedObjectStore):
         pack_sha, extra_entries = extend_pack(
             f, indexer.ext_refs(), get_raw=self.get_raw, compression_level=self.pack_compression_level,
             progress=progress)
-
+        f.flush()
+        try:
+            fileno = f.fileno()
+        except AttributeError:
+            pass
+        else:
+            os.fsync(fileno)
         f.close()
 
         entries.extend(extra_entries)
@@ -832,16 +838,22 @@ class DiskObjectStore(PackBasedObjectStore):
         # Move the pack in.
         entries.sort()
         pack_base_name = self._get_pack_basepath(entries)
-        target_pack = pack_base_name + ".pack"
+
+        for pack in self.packs:
+            if pack._basename == pack_base_name:
+                return pack
+
+        target_pack_path = pack_base_name + ".pack"
+        target_index_path = pack_base_name + ".idx"
         if sys.platform == "win32":
             # Windows might have the target pack file lingering. Attempt
             # removal, silently passing if the target does not exist.
             with suppress(FileNotFoundError):
-                os.remove(target_pack)
-        os.rename(path, target_pack)
+                os.remove(target_pack_path)
+        os.rename(path, target_pack_path)
 
         # Write the index.
-        with GitFile(pack_base_name + ".idx", "wb", mask=PACK_MODE) as index_file:
+        with GitFile(target_index_path, "wb", mask=PACK_MODE) as index_file:
             write_pack_index(index_file, entries, pack_sha)
 
         # Add the pack to the store and return it.
@@ -884,34 +896,10 @@ class DiskObjectStore(PackBasedObjectStore):
         Args:
           path: Path to the pack file.
         """
-        f.flush()
-        try:
-            fileno = f.fileno()
-        except AttributeError:
-            pass
-        else:
-            os.fsync(fileno)
         f.seek(0)
-        with PackData(path, f) as p:
-            entries = p.sorted_entries()
-            basename = self._get_pack_basepath(entries)
-            index_name = basename + ".idx"
-            if not os.path.exists(index_name):
-                with GitFile(index_name, "wb", mask=PACK_MODE) as f:
-                    write_pack_index(f, entries, p.get_stored_checksum())
-        for pack in self.packs:
-            if pack._basename == basename:
-                return pack
-        target_pack = basename + ".pack"
-        if sys.platform == "win32":
-            # Windows might have the target pack file lingering. Attempt
-            # removal, silently passing if the target does not exist.
-            with suppress(FileNotFoundError):
-                os.remove(target_pack)
-        os.rename(path, target_pack)
-        final_pack = Pack(basename)
-        self._add_cached_pack(basename, final_pack)
-        return final_pack
+        with PackData(path, f) as pd:
+            indexer = PackIndexer.for_pack_data(pd, resolve_ext_ref=self.get_raw)
+            return self._complete_pack(f, path, len(pd), indexer)
 
     def add_pack(self):
         """Add a new pack to this object store.
@@ -1048,14 +1036,17 @@ class MemoryObjectStore(BaseObjectStore):
 
         def commit():
             size = f.tell()
-            f.seek(0)
-            p = PackData.from_file(f, size)
-            for obj in PackInflater.for_pack_data(p, self.get_raw):
-                self.add_object(obj)
-            p.close()
+            if size > 0:
+                f.seek(0)
+                p = PackData.from_file(f, size)
+                for obj in PackInflater.for_pack_data(p, self.get_raw):
+                    self.add_object(obj)
+                p.close()
+            else:
+                f.close()
 
         def abort():
-            pass
+            f.close()
 
         return f, commit, abort
 

dulwich/tests/test_client.py

@@ -865,6 +865,7 @@ class LocalGitClientTests(TestCase):
         target = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, target)
         t = Repo.init_bare(target)
+        self.addCleanup(t.close)
         s = open_repo("a.git")
         self.addCleanup(tear_down_repo, s)
         self.assertEqual(s.get_refs(), c.fetch(s.path, t).refs)

dulwich/tests/test_object_store.py

@@ -305,7 +305,7 @@ class MemoryObjectStoreTests(ObjectStoreTests, TestCase):
     def test_add_pack_emtpy(self):
         o = MemoryObjectStore()
         f, commit, abort = o.add_pack()
-        self.assertRaises(AssertionError, commit)
+        commit()
 
     def test_add_thin_pack(self):
         o = MemoryObjectStore()
@@ -525,6 +525,7 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
 
     def test_add_pack(self):
         o = DiskObjectStore(self.store_dir)
+        self.addCleanup(o.close)
         f, commit, abort = o.add_pack()
         try:
             b = make_object(Blob, data=b"more yummy data")

dulwich/tests/test_repository.py

@@ -413,9 +413,11 @@ class RepositoryRootTests(TestCase):
         dest_dir = os.path.join(temp_dir, "a.git")
         shutil.copytree(os.path.join(repo_dir, "a.git"), dest_dir, symlinks=True)
         r = Repo(dest_dir)
+        self.addCleanup(r.close)
         del r.refs[b"refs/heads/master"]
         del r.refs[b"HEAD"]
         t = r.clone(os.path.join(temp_dir, "b.git"), mkdir=True)
+        self.addCleanup(t.close)
         self.assertEqual(
             {
                 b"refs/tags/mytag": b"28237f4dc30d0d462658d6b937b08a0f0b6ef55a",