Forráskód Böngészése

Fix LocalGitClient assertion error with MemoryRepo (#1179)

Fix MemoryObjectStore.add_pack_data to properly handle delta objects
by routing through add_pack() which resolves deltas correctly.

Previously, when fetching externally cloned repositories into MemoryRepo,
the code would fail with AssertionError when trying to call sha_file()
on unresolved delta objects in unpacked_objects.

The fix changes add_pack_data to use the existing pack infrastructure
that properly handles delta resolution instead of trying to directly
add unresolved delta objects.

Adds comprehensive tests covering both the direct scenario and the
real-world use case of fetching from git-cloned repositories.
Jelmer Vernooij 1 hónapja
szülő
commit
e0635a7626
4 módosított fájl, 107 hozzáadás és 2 törlés
  1. 5 0
      NEWS
  2. 19 2
      dulwich/object_store.py
  3. 36 0
      tests/test_object_store.py
  4. 47 0
      tests/test_repository.py

+ 5 - 0
NEWS

@@ -1,5 +1,10 @@
 0.22.9	UNRELEASED
 
+ * Fix ``LocalGitClient`` assertion error when fetching externally cloned repositories
+   into ``MemoryRepo``. Previously, the client would fail with an AssertionError
+   when trying to process pack data from repositories that were cloned externally.
+   (Jelmer Vernooij, #1179)
+
  * Add support for ``format`` argument to ``Repo.init()`` and ``Repo.init_bare()``
    to specify repository format version (0 or 1). This allows creating repositories
    with different format versions by setting the ``core.repositoryformatversion``

+ 19 - 2
dulwich/object_store.py

@@ -1273,8 +1273,25 @@ class MemoryObjectStore(BaseObjectStore):
         Args:
           count: Number of items to add
         """
-        for unpacked_object in unpacked_objects:
-            self.add_object(unpacked_object.sha_file())
+        if count == 0:
+            return
+
+        # Since MemoryObjectStore doesn't support pack files, we need to
+        # extract individual objects. To handle deltas properly, we write
+        # to a temporary pack and then use PackInflater to resolve them.
+        f, commit, abort = self.add_pack()
+        try:
+            write_pack_data(
+                f.write,
+                unpacked_objects,
+                num_records=count,
+                progress=progress,
+            )
+        except BaseException:
+            abort()
+            raise
+        else:
+            commit()
 
     def add_thin_pack(self, read_all, read_some, progress=None) -> None:
         """Add a new thin pack to this object store.

+ 36 - 0
tests/test_object_store.py

@@ -114,6 +114,42 @@ class MemoryObjectStoreTests(ObjectStoreTests, TestCase):
         self.assertEqual([], entries)
         o.add_thin_pack(f.read, None)
 
+    def test_add_pack_data_with_deltas(self) -> None:
+        """Test that add_pack_data properly handles delta objects.
+
+        This test verifies that MemoryObjectStore.add_pack_data can handle
+        pack data containing delta objects. Before the fix for issue #1179,
+        this would fail with AssertionError when trying to call sha_file()
+        on unresolved delta objects.
+
+        The fix routes through add_pack() which properly resolves deltas.
+        """
+        o1 = MemoryObjectStore()
+        o2 = MemoryObjectStore()
+        base_blob = make_object(Blob, data=b"base data")
+        o1.add_object(base_blob)
+
+        # Create a pack with a delta object
+        f = BytesIO()
+        entries = build_pack(
+            f,
+            [
+                (REF_DELTA, (base_blob.id, b"more data")),
+            ],
+            store=o1,
+        )
+
+        # Use add_thin_pack which internally calls add_pack_data
+        # This demonstrates the scenario where delta resolution is needed
+        f.seek(0)
+        o2.add_object(base_blob)  # Need base object for thin pack
+        o2.add_thin_pack(f.read, None)
+
+        # Verify the delta object was properly resolved and added
+        packed_blob_sha = sha_to_hex(entries[0][3])
+        self.assertIn(packed_blob_sha, o2)
+        self.assertEqual((Blob.type_num, b"more data"), o2.get_raw(packed_blob_sha))
+
 
 class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
     def setUp(self) -> None:

+ 47 - 0
tests/test_repository.py

@@ -136,6 +136,53 @@ class MemoryRepoTests(TestCase):
         self.addCleanup(tear_down_repo, repo)
         repo.fetch(r)
 
+    def test_fetch_from_git_cloned_repo(self) -> None:
+        """Test fetching from a git-cloned repo into MemoryRepo (issue #1179)."""
+        import tempfile
+
+        from dulwich.client import LocalGitClient
+
+        with tempfile.TemporaryDirectory() as tmpdir:
+            # Create initial repo using dulwich
+            initial_path = os.path.join(tmpdir, "initial")
+            initial_repo = Repo.init(initial_path, mkdir=True)
+
+            # Create some content
+            test_file = os.path.join(initial_path, "test.txt")
+            with open(test_file, "w") as f:
+                f.write("test content\n")
+
+            # Stage and commit using dulwich
+            initial_repo.stage(["test.txt"])
+            initial_repo.do_commit(
+                b"Initial commit\n",
+                committer=b"Test Committer <test@example.com>",
+                author=b"Test Author <test@example.com>",
+            )
+
+            # Clone using dulwich
+            cloned_path = os.path.join(tmpdir, "cloned")
+            cloned_repo = initial_repo.clone(cloned_path, mkdir=True)
+
+            initial_repo.close()
+            cloned_repo.close()
+
+            # Fetch from the cloned repo into MemoryRepo
+            memory_repo = MemoryRepo()
+            client = LocalGitClient()
+
+            # This should not raise AssertionError
+            result = client.fetch(cloned_path, memory_repo)
+
+            # Verify the fetch worked
+            self.assertIn(b"HEAD", result.refs)
+            self.assertIn(b"refs/heads/master", result.refs)
+
+            # Verify we can read the fetched objects
+            head_sha = result.refs[b"HEAD"]
+            commit = memory_repo[head_sha]
+            self.assertEqual(commit.message, b"Initial commit\n")
+
 
 class RepositoryRootTests(TestCase):
     def mkdtemp(self):