From f9d0cc5ebc15b30173617a72127378c5462dd781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Wed, 18 Jan 2023 03:13:47 +0000 Subject: [PATCH] Factor out a common _complete_pack. --- docs/tutorial/remote.txt | 4 +- dulwich/object_store.py | 67 +++++++++++++----------------- dulwich/tests/test_client.py | 1 + dulwich/tests/test_object_store.py | 3 +- dulwich/tests/test_repository.py | 2 + 5 files changed, 37 insertions(+), 40 deletions(-) diff --git a/docs/tutorial/remote.txt b/docs/tutorial/remote.txt index 65d0f03cd..cb74df094 100644 --- a/docs/tutorial/remote.txt +++ b/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 ") >>> 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:: diff --git a/dulwich/object_store.py b/dulwich/object_store.py index e60395572..dec12e5d4 100644 --- a/dulwich/object_store.py +++ b/dulwich/object_store.py @@ -824,7 +824,13 @@ def _complete_pack(self, f, path, num_objects, indexer, progress=None): 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 @@ def _complete_pack(self, f, path, num_objects, indexer, progress=None): # 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 @@ def _move_in_pack(self, path, f): 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 @@ def add_pack(self): 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 diff --git a/dulwich/tests/test_client.py b/dulwich/tests/test_client.py index dacf1b473..64fefbd8b 100644 --- a/dulwich/tests/test_client.py +++ b/dulwich/tests/test_client.py @@ -865,6 +865,7 @@ def test_fetch_into_empty(self): 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) diff --git a/dulwich/tests/test_object_store.py b/dulwich/tests/test_object_store.py index 54b4f5d3f..fc0abf091 100644 --- a/dulwich/tests/test_object_store.py +++ b/dulwich/tests/test_object_store.py @@ -305,7 +305,7 @@ def test_add_pack(self): 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 @@ def test_pack_dir(self): 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") diff --git a/dulwich/tests/test_repository.py b/dulwich/tests/test_repository.py index 3ac257e3c..73c1aae4a 100644 --- a/dulwich/tests/test_repository.py +++ b/dulwich/tests/test_repository.py @@ -413,9 +413,11 @@ def test_clone_no_head(self): 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",