Merge pull request #1125 from jelmer/peeled-tags

Refactor peeled tag handling

dulwich/client.py

@@ -126,7 +126,7 @@ from dulwich.pack import (
 )
 from dulwich.refs import (
     read_info_refs,
-    ANNOTATED_TAG_SUFFIX,
+    PEELED_TAG_SUFFIX,
     _import_remote_refs,
 )
 from dulwich.repo import Repo
@@ -992,7 +992,7 @@ def check_wants(wants, refs):
 
     """
     missing = set(wants) - {
-        v for (k, v) in refs.items() if not k.endswith(ANNOTATED_TAG_SUFFIX)
+        v for (k, v) in refs.items() if not k.endswith(PEELED_TAG_SUFFIX)
     }
     if missing:
         raise InvalidWants(missing)

dulwich/object_store.py

@@ -77,7 +77,7 @@ from dulwich.pack import (
     PACK_SPOOL_FILE_MAX_SIZE,
 )
 from dulwich.protocol import DEPTH_INFINITE
-from dulwich.refs import ANNOTATED_TAG_SUFFIX, Ref
+from dulwich.refs import PEELED_TAG_SUFFIX, Ref
 
 INFODIR = "info"
 PACKDIR = "pack"
@@ -115,7 +115,7 @@ class BaseObjectStore:
             sha
             for (ref, sha) in refs.items()
             if (sha not in self or _want_deepen(sha))
-            and not ref.endswith(ANNOTATED_TAG_SUFFIX)
+            and not ref.endswith(PEELED_TAG_SUFFIX)
             and not sha == ZERO_SHA
         ]
 
@@ -277,7 +277,7 @@ class BaseObjectStore:
         warnings.warn(
             "Please use dulwich.object_store.peel_sha()",
             DeprecationWarning, stacklevel=2)
-        return peel_sha(self, sha)
+        return peel_sha(self, sha)[1]
 
     def _get_depth(
         self, head, get_parents=lambda commit: commit.parents, max_depth=None,
@@ -1642,7 +1642,7 @@ def iter_tree_contents(
             yield entry
 
 
-def peel_sha(store: ObjectContainer, sha: bytes) -> ShaFile:
+def peel_sha(store: ObjectContainer, sha: bytes) -> Tuple[ShaFile, ShaFile]:
     """Peel all tags from a SHA.
 
     Args:
@@ -1651,10 +1651,10 @@ def peel_sha(store: ObjectContainer, sha: bytes) -> ShaFile:
         intermediate tags; if the original ref does not point to a tag,
         this will equal the original SHA1.
     """
-    obj = store[sha]
+    unpeeled = obj = store[sha]
     obj_class = object_class(obj.type_name)
     while obj_class is Tag:
         assert isinstance(obj, Tag)
         obj_class, sha = obj.object
         obj = store[sha]
-    return obj
+    return unpeeled, obj

dulwich/refs.py

@@ -25,6 +25,7 @@
 from contextlib import suppress
 import os
 from typing import Dict, Optional
+import warnings
 
 from dulwich.errors import (
     PackedRefsException,
@@ -50,7 +51,10 @@ SYMREF = b"ref: "
 LOCAL_BRANCH_PREFIX = b"refs/heads/"
 LOCAL_TAG_PREFIX = b"refs/tags/"
 BAD_REF_CHARS = set(b"\177 ~^:?*[")
-ANNOTATED_TAG_SUFFIX = b"^{}"
+PEELED_TAG_SUFFIX = b"^{}"
+
+# For backwards compatibility
+ANNOTATED_TAG_SUFFIX = PEELED_TAG_SUFFIX
 
 
 class SymrefLoop(Exception):
@@ -593,7 +597,7 @@ class InfoRefsContainer(RefsContainer):
         self._peeled = {}
         for line in f.readlines():
             sha, name = line.rstrip(b"\n").split(b"\t")
-            if name.endswith(ANNOTATED_TAG_SUFFIX):
+            if name.endswith(PEELED_TAG_SUFFIX):
                 name = name[:-3]
                 if not check_ref_format(name):
                     raise ValueError("invalid ref name %r" % name)
@@ -1153,7 +1157,7 @@ def read_info_refs(f):
 
 def write_info_refs(refs, store: ObjectContainer):
     """Generate info refs."""
-    # Avoid recursive import :(
+    # TODO: Avoid recursive import :(
     from dulwich.object_store import peel_sha
     for name, sha in sorted(refs.items()):
         # get_refs() includes HEAD as a special case, but we don't want to
@@ -1164,10 +1168,10 @@ def write_info_refs(refs, store: ObjectContainer):
             o = store[sha]
         except KeyError:
             continue
-        peeled = peel_sha(store, sha)
+        unpeeled, peeled = peel_sha(store, sha)
         yield o.id + b"\t" + name + b"\n"
         if o.id != peeled.id:
-            yield peeled.id + b"\t" + name + ANNOTATED_TAG_SUFFIX + b"\n"
+            yield peeled.id + b"\t" + name + PEELED_TAG_SUFFIX + b"\n"
 
 
 def is_local_branch(x):
@@ -1179,7 +1183,7 @@ def strip_peeled_refs(refs):
     return {
         ref: sha
         for (ref, sha) in refs.items()
-        if not ref.endswith(ANNOTATED_TAG_SUFFIX)
+        if not ref.endswith(PEELED_TAG_SUFFIX)
     }
 
 
@@ -1275,6 +1279,27 @@ def _import_remote_refs(
     tags = {
         n[len(LOCAL_TAG_PREFIX) :]: v
         for (n, v) in stripped_refs.items()
-        if n.startswith(LOCAL_TAG_PREFIX) and not n.endswith(ANNOTATED_TAG_SUFFIX)
+        if n.startswith(LOCAL_TAG_PREFIX) and not n.endswith(PEELED_TAG_SUFFIX)
     }
     refs_container.import_refs(LOCAL_TAG_PREFIX, tags, message=message, prune=prune_tags)
+
+
+def serialize_refs(store, refs):
+    # TODO: Avoid recursive import :(
+    from dulwich.object_store import peel_sha
+    ret = {}
+    for ref, sha in refs.items():
+        try:
+            unpeeled, peeled = peel_sha(store, sha)
+        except KeyError:
+            warnings.warn(
+                "ref %s points at non-present sha %s"
+                % (ref.decode("utf-8", "replace"), sha.decode("ascii")),
+                UserWarning,
+            )
+            continue
+        else:
+            if isinstance(unpeeled, Tag):
+                ret[ref + PEELED_TAG_SUFFIX] = peeled.id
+            ret[ref] = unpeeled.id
+    return ret

dulwich/repo.py

@@ -46,6 +46,8 @@ from typing import (
     Iterable,
     Set
 )
+import warnings
+
 
 if TYPE_CHECKING:
     # There are no circular imports here, but we try to defer imports as long
@@ -105,6 +107,7 @@ from dulwich.refs import (  # noqa: F401
     ANNOTATED_TAG_SUFFIX,
     LOCAL_BRANCH_PREFIX,
     LOCAL_TAG_PREFIX,
+    serialize_refs,
     check_ref_format,
     RefsContainer,
     DictRefsContainer,
@@ -120,9 +123,6 @@ from dulwich.refs import (  # noqa: F401
 )
 
 
-import warnings
-
-
 CONTROLDIR = ".git"
 OBJECTDIR = "objects"
 REFSDIR = "refs"
@@ -520,21 +520,7 @@ class BaseRepo:
         if depth not in (None, 0):
             raise NotImplementedError("depth not supported yet")
 
-        refs = {}
-        for ref, sha in self.get_refs().items():
-            try:
-                obj = self.object_store[sha]
-            except KeyError:
-                warnings.warn(
-                    "ref %s points at non-present sha %s"
-                    % (ref.decode("utf-8", "replace"), sha.decode("ascii")),
-                    UserWarning,
-                )
-                continue
-            else:
-                if isinstance(obj, Tag):
-                    refs[ref + ANNOTATED_TAG_SUFFIX] = obj.object[1]
-                refs[ref] = sha
+        refs = serialize_refs(self.object_store, self.get_refs())
 
         wants = determine_wants(refs)
         if not isinstance(wants, list):
@@ -772,7 +758,7 @@ class BaseRepo:
         cached = self.refs.get_peeled(ref)
         if cached is not None:
             return cached
-        return peel_sha(self.object_store, self.refs[ref]).id
+        return peel_sha(self.object_store, self.refs[ref])[1].id
 
     def get_walker(self, include: Optional[List[bytes]] = None,
                    *args, **kwargs):

dulwich/server.py

@@ -128,7 +128,7 @@ from dulwich.protocol import (
 )
 from dulwich.refs import (
     RefsContainer,
-    ANNOTATED_TAG_SUFFIX,
+    PEELED_TAG_SUFFIX,
     write_info_refs,
 )
 from dulwich.repo import (
@@ -499,9 +499,9 @@ def _find_shallow(store: ObjectContainer, heads, depth):
 
     todo = []  # stack of (sha, depth)
     for head_sha in heads:
-        obj = peel_sha(store, head_sha)
-        if isinstance(obj, Commit):
-            todo.append((obj.id, 1))
+        _unpeeled, peeled = peel_sha(store, head_sha)
+        if isinstance(peeled, Commit):
+            todo.append((peeled.id, 1))
 
     not_shallow = set()
     shallow = set()
@@ -635,7 +635,7 @@ class _ProtocolGraphWalker:
                 self.proto.write_pkt_line(line)
                 if peeled_sha != sha:
                     self.proto.write_pkt_line(
-                        format_ref_line(ref + ANNOTATED_TAG_SUFFIX, peeled_sha))
+                        format_ref_line(ref + PEELED_TAG_SUFFIX, peeled_sha))
 
             # i'm done..
             self.proto.write_pkt_line(None)

dulwich/tests/test_object_store.py

@@ -264,7 +264,7 @@ class ObjectStoreTests:
         tag2 = self.make_tag(b"2", testobject)
         tag3 = self.make_tag(b"3", testobject)
         for obj in [testobject, tag1, tag2, tag3]:
-            self.assertEqual(testobject, peel_sha(self.store, obj.id))
+            self.assertEqual((obj, testobject), peel_sha(self.store, obj.id))
 
     def test_get_raw(self):
         self.store.add_object(testobject)