Fix parse_commit to properly dereference annotated tags

Previously, checking out an annotated tag would fail with a KeyError
because parse_commit() didn't dereference tag objects to their target
commits. This fix adds tag dereferencing to all code paths in
parse_commit(), ensuring that annotated tags are properly resolved
to their target commits.

Fixes #1638

NEWS

@@ -4,6 +4,11 @@
    and make this the default.
    (Jelmer Vernooij, #835)
 
+ * Fix ``parse_commit`` to properly dereference annotated tags when
+   checking out tags. Previously, checking out an annotated tag would
+   fail with a KeyError.
+   (Jelmer Vernooij, #1638)
+
  * Fix KeyError when pulling from a shallow clone. Handle missing commits
    gracefully in graph traversal operations for shallow repositories.
    (Jelmer Vernooij, #813)

dulwich/objectspec.py

@@ -24,7 +24,7 @@
 from collections.abc import Iterator
 from typing import TYPE_CHECKING, Optional, Union
 
-from .objects import Commit, ShaFile, Tree
+from .objects import Commit, ShaFile, Tag, Tree
 
 if TYPE_CHECKING:
     from .refs import Ref, RefsContainer
@@ -245,15 +245,33 @@ def parse_commit(repo: "Repo", committish: Union[str, bytes]) -> "Commit":
       KeyError: When the reference commits can not be found
       ValueError: If the range can not be parsed
     """
+
+    def dereference_tag(obj):
+        """Follow tag references until we reach a non-tag object."""
+        while isinstance(obj, Tag):
+            obj_type, obj_sha = obj.object
+            try:
+                obj = repo.object_store[obj_sha]
+            except KeyError:
+                # Tag points to a missing object
+                raise KeyError(obj_sha)
+        if not isinstance(obj, Commit):
+            raise ValueError(f"Expected commit, got {obj.type_name}")
+        return obj
+
     committish = to_bytes(committish)
     try:
-        return repo[committish]
+        obj = repo[committish]
     except KeyError:
         pass
+    else:
+        return dereference_tag(obj)
     try:
-        return repo[parse_ref(repo, committish)]
+        obj = repo[parse_ref(repo, committish)]
     except KeyError:
         pass
+    else:
+        return dereference_tag(obj)
     if len(committish) >= 4 and len(committish) < 40:
         try:
             int(committish, 16)
@@ -261,9 +279,11 @@ def parse_commit(repo: "Repo", committish: Union[str, bytes]) -> "Commit":
             pass
         else:
             try:
-                return scan_for_short_id(repo.object_store, committish, Commit)
+                obj = scan_for_short_id(repo.object_store, committish, Commit)
             except KeyError:
                 pass
+            else:
+                return dereference_tag(obj)
     raise KeyError(committish)
 
 

tests/test_objectspec.py

@@ -23,7 +23,7 @@
 
 # TODO: Round-trip parse-serialize-parse and serialize-parse-serialize tests.
 
-from dulwich.objects import Blob
+from dulwich.objects import Blob, Commit, Tag
 from dulwich.objectspec import (
     parse_commit,
     parse_commit_range,
@@ -84,6 +84,82 @@ class ParseCommitTests(TestCase):
         [c1] = build_commit_graph(r.object_store, [[1]])
         self.assertEqual(c1, parse_commit(r, c1.id[:10]))
 
+    def test_annotated_tag(self) -> None:
+        r = MemoryRepo()
+        [c1] = build_commit_graph(r.object_store, [[1]])
+        # Create an annotated tag pointing to the commit
+        tag = Tag()
+        tag.name = b"v1.0"
+        tag.message = b"Test tag"
+        tag.tag_time = 1234567890
+        tag.tag_timezone = 0
+        tag.object = (Commit, c1.id)
+        tag.tagger = b"Test Tagger <test@example.com>"
+        r.object_store.add_object(tag)
+        # parse_commit should follow the tag to the commit
+        self.assertEqual(c1, parse_commit(r, tag.id))
+
+    def test_nested_tags(self) -> None:
+        r = MemoryRepo()
+        [c1] = build_commit_graph(r.object_store, [[1]])
+        # Create an annotated tag pointing to the commit
+        tag1 = Tag()
+        tag1.name = b"v1.0"
+        tag1.message = b"Test tag"
+        tag1.tag_time = 1234567890
+        tag1.tag_timezone = 0
+        tag1.object = (Commit, c1.id)
+        tag1.tagger = b"Test Tagger <test@example.com>"
+        r.object_store.add_object(tag1)
+
+        # Create another tag pointing to the first tag
+        tag2 = Tag()
+        tag2.name = b"v1.0-release"
+        tag2.message = b"Release tag"
+        tag2.tag_time = 1234567900
+        tag2.tag_timezone = 0
+        tag2.object = (Tag, tag1.id)
+        tag2.tagger = b"Test Tagger <test@example.com>"
+        r.object_store.add_object(tag2)
+
+        # parse_commit should follow both tags to the commit
+        self.assertEqual(c1, parse_commit(r, tag2.id))
+
+    def test_tag_to_missing_commit(self) -> None:
+        r = MemoryRepo()
+        # Create a tag pointing to a non-existent commit
+        missing_sha = b"1234567890123456789012345678901234567890"
+        tag = Tag()
+        tag.name = b"v1.0"
+        tag.message = b"Test tag"
+        tag.tag_time = 1234567890
+        tag.tag_timezone = 0
+        tag.object = (Commit, missing_sha)
+        tag.tagger = b"Test Tagger <test@example.com>"
+        r.object_store.add_object(tag)
+
+        # Should raise KeyError for missing commit
+        self.assertRaises(KeyError, parse_commit, r, tag.id)
+
+    def test_tag_to_blob(self) -> None:
+        r = MemoryRepo()
+        # Create a blob
+        blob = Blob.from_string(b"Test content")
+        r.object_store.add_object(blob)
+
+        # Create a tag pointing to the blob
+        tag = Tag()
+        tag.name = b"blob-tag"
+        tag.message = b"Tag pointing to blob"
+        tag.tag_time = 1234567890
+        tag.tag_timezone = 0
+        tag.object = (Blob, blob.id)
+        tag.tagger = b"Test Tagger <test@example.com>"
+        r.object_store.add_object(tag)
+
+        # Should raise ValueError as it's not a commit
+        self.assertRaises(ValueError, parse_commit, r, tag.id)
+
 
 class ParseRefTests(TestCase):
     def test_nonexistent(self) -> None: