Add get_peeled to BaseRepo so HTTP and git servers use one call to peel tags.

This method needs to go in BaseRepo rather than ObjectStore so it can take
advantage of the cached peeled values in the packed-refs file, which belongs
to the RefsContainer. To this end, added a similar get_peeled method to
RefsContainer that accesses the peeled ref cache. Unlike BaseRepo.get_peeled,
RefsContainer.get_peeled returns None if peeled ref information is not cached
(since it does not have access to an ObjectStore to do the peeling itself).

Modified the TCP git server and dumb HTTP server to advertise peeled refs
consistently and correctly. Added tests for all new functionality.

Change-Id: I214ffee1a3459a746a7e34a1d04c0f527c5c8347

dulwich/errors.py

@@ -61,6 +61,12 @@ class NotTreeError(WrongObjectException):
     _type = 'tree'
 
 
+class NotTagError(WrongObjectException):
+    """Indicates that the sha requested does not point to a tag."""
+
+    _type = 'tag'
+
+
 class NotBlobError(WrongObjectException):
     """Indicates that the sha requested does not point to a blob."""
   

dulwich/repo.py

@@ -32,6 +32,7 @@ from dulwich.errors import (
     NotCommitError, 
     NotGitRepository,
     NotTreeError, 
+    NotTagError,
     PackedRefsException,
     )
 from dulwich.file import (
@@ -48,6 +49,7 @@ from dulwich.objects import (
     Tag,
     Tree,
     hex_to_sha,
+    num_type_map,
     )
 
 OBJECTDIR = 'objects'
@@ -131,6 +133,16 @@ class RefsContainer(object):
         """
         raise NotImplementedError(self.get_packed_refs)
 
+    def get_peeled(self, name):
+        """Return the cached peeled value of a ref, if available.
+
+        :param name: Name of the ref to peel
+        :return: The peeled value of the ref. If the ref is known not point to a
+            tag, this will be the SHA the ref refers to. If the ref may point to
+            a tag, but no cached information is available, None is returned.
+        """
+        return None
+
     def import_refs(self, base, other):
         for name, value in other.iteritems():
             self["%s/%s" % (base, name)] = value
@@ -245,7 +257,7 @@ class DiskRefsContainer(RefsContainer):
     def __init__(self, path):
         self.path = path
         self._packed_refs = None
-        self._peeled_refs = {}
+        self._peeled_refs = None
 
     def __repr__(self):
         return "%s(%r)" % (self.__class__.__name__, self.path)
@@ -310,6 +322,7 @@ class DiskRefsContainer(RefsContainer):
                 first_line = iter(f).next().rstrip()
                 if (first_line.startswith("# pack-refs") and " peeled" in
                         first_line):
+                    self._peeled_refs = {}
                     for sha, name, peeled in read_packed_refs_with_peeled(f):
                         self._packed_refs[name] = sha
                         if peeled:
@@ -322,6 +335,24 @@ class DiskRefsContainer(RefsContainer):
                 f.close()
         return self._packed_refs
 
+    def get_peeled(self, name):
+        """Return the cached peeled value of a ref, if available.
+
+        :param name: Name of the ref to peel
+        :return: The peeled value of the ref. If the ref is known not point to a
+            tag, this will be the SHA the ref refers to. If the ref may point to
+            a tag, but no cached information is available, None is returned.
+        """
+        self.get_packed_refs()
+        if self._peeled_refs is None or name not in self._packed_refs:
+            # No cache: no peeled refs were read, or this ref is loose
+            return None
+        if name in self._peeled_refs:
+            return self._peeled_refs[name]
+        else:
+            # Known not peelable
+            return self[name]
+
     def read_loose_ref(self, name):
         """Read a reference file and return its contents.
 
@@ -558,6 +589,7 @@ def write_packed_refs(f, packed_refs, peeled_refs=None):
 
     :param f: empty file-like object to write to
     :param packed_refs: dict of refname to sha of packed refs to write
+    :param peeled_refs: dict of refname to peeled value of sha
     """
     if peeled_refs is None:
         peeled_refs = {}
@@ -660,6 +692,8 @@ class BaseRepo(object):
                 raise NotBlobError(ret)
             elif cls is Tree:
                 raise NotTreeError(ret)
+            elif cls is Tag:
+                raise NotTagError(ret)
             else:
                 raise Exception("Type invalid: %r != %r" % (ret._type, cls._type))
         return ret
@@ -686,6 +720,24 @@ class BaseRepo(object):
     def tag(self, sha):
         return self._get_object(sha, Tag)
 
+    def get_peeled(self, ref):
+        """Get the peeled value of a ref.
+
+        :param ref: the refname to peel
+        :return: the fully-peeled SHA1 of a tag object, after peeling all
+            intermediate tags; if the original ref does not point to a tag, this
+            will equal the original SHA1.
+        """
+        cached = self.refs.get_peeled(ref)
+        if cached is not None:
+            return cached
+        obj = self[ref]
+        obj_type = num_type_map[obj.type]
+        while obj_type == Tag:
+            obj_type, sha = obj.object
+            obj = self.get_object(sha)
+        return obj.id
+
     def get_blob(self, sha):
         return self._get_object(sha, Blob)
 

dulwich/server.py

@@ -259,7 +259,10 @@ class ProtocolGraphWalker(object):
                 if not i:
                     line = "%s\x00%s" % (line, self.handler.capability_line())
                 self.proto.write_pkt_line("%s\n" % line)
-                # TODO: include peeled value of any tags
+                peeled_sha = self.handler.backend.repo.get_peeled(ref)
+                if peeled_sha != sha:
+                    self.proto.write_pkt_line('%s %s^{}\n' %
+                                              (peeled_sha, ref))
 
             # i'm done..
             self.proto.write_pkt_line(None)

dulwich/tests/data/repos/a.git/objects/28/237f4dc30d0d462658d6b937b08a0f0b6ef55a

@@ -0,0 +1,2 @@
+x
5ÌA
+Â0…a×9Å\@™¦i›�""ÁLÚ1T"uPêéMA7�oó~å•ó»î2(0á�íHˆ\uB\]ÛMÞN‚c+ÄH�Ñõ!0ä”&5Zi-»)Ê~	œó’ß“~ Ã�§˜sœåP~G¨lÛÖ®Á†`�јkéÌüÔ÷ÀN0—

dulwich/tests/data/repos/a.git/objects/b0/931cadc54336e78a1d980420e3268903b57a50

@@ -0,0 +1,3 @@
+x
-�[
+Β0ύΞ*ξ*IΜ��Έ7�Η5T[o©΅RWo†Γΐ™
+­wο�*Θ`eφ�/“Ωi­·7sΰΒjƒpΑθμ«Ϋ��h�†ΚjkL[c7‡τΐόΈ„αL½‡ϊ�>Η�<Ά2βΎέ� ¤1JrηtάqΞΨµεhΜ°βςθΙΎΦ¥2v

dulwich/tests/data/repos/a.git/packed-refs

@@ -0,0 +1,3 @@
+# pack-refs with: peeled 
+b0931cadc54336e78a1d980420e3268903b57a50 refs/tags/mytag-packed
+^2a72d929692c41d8554c07f6301757ba18a65d91

dulwich/tests/data/repos/a.git/refs/tags/mytag

@@ -0,0 +1 @@
+28237f4dc30d0d462658d6b937b08a0f0b6ef55a

dulwich/tests/data/repos/refs.git/objects/3e/c9c43c84ff242e3ef4a9fc5bc111fd780a76a8

@@ -0,0 +1,3 @@
+x
-�Q
+Â0DýÎ)ö-›mšVñ^ i6±Ò6’.ŠžÞ~ÍÌÇ{#Cm›]rwy´Î×u�=’uº5^[³o¸õ<¸®H<*y?Æ´,“()ŽÌa«°¦ßˆœá2<Î)§×$8x÷¯§˜Rœ¹.è4Ykˆt�Pa�¨Ôµ¨
+q?…À™W)'«”ÜÔǧ6

dulwich/tests/data/repos/refs.git/objects/cd/a609072918d7b70057b6bef9f4c2537843fcfe

@@ -0,0 +1,5 @@
+x
-ŤQ
+Â0DýÎ)öm7i’Vń^ i6±bIEOo
+~Íc`ŢđAví.ą;ŤZy´Îk×u�<*ë¤Ń^Z˝oÉx\×T4
+ţ<	Ć4Ď.ŽLam
+¬ÍFÖj«#e¸/‚sĘé=ńŢýńSŠŞ‹äRY«•Bc ÂQ�k‘–Ĺ
üeZ¸Ü-\r?)Y9Ţ

dulwich/tests/data/repos/refs.git/packed-refs

@@ -1,3 +1,4 @@
 # pack-refs with: peeled 
 df6800012397fb85c56e7418dd4eb9405dee075c refs/tags/refs-0.1
 ^42d06bd4b77fed026b154d16493e5deab78f02ec
+42d06bd4b77fed026b154d16493e5deab78f02ec refs/heads/packed

dulwich/tests/data/repos/refs.git/refs/tags/refs-0.2

@@ -0,0 +1 @@
+3ec9c43c84ff242e3ef4a9fc5bc111fd780a76a8

dulwich/tests/test_repository.py

@@ -27,6 +27,7 @@ import tempfile
 import unittest
 
 from dulwich import errors
+from dulwich import objects
 from dulwich.repo import (
     check_ref_format,
     Repo,
@@ -75,8 +76,10 @@ class RepositoryTests(unittest.TestCase):
     def test_get_refs(self):
         r = self._repo = open_repo('a.git')
         self.assertEqual({
-            'HEAD': 'a90fa2d900a17e99b433217e988c4eb4a2e9a097', 
-            'refs/heads/master': 'a90fa2d900a17e99b433217e988c4eb4a2e9a097'
+            'HEAD': 'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
+            'refs/heads/master': 'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
+            'refs/tags/mytag': '28237f4dc30d0d462658d6b937b08a0f0b6ef55a',
+            'refs/tags/mytag-packed': 'b0931cadc54336e78a1d980420e3268903b57a50',
             }, r.get_refs())
   
     def test_head(self):
@@ -112,7 +115,40 @@ class RepositoryTests(unittest.TestCase):
     def test_tree_not_tree(self):
         r = self._repo = open_repo('a.git')
         self.assertRaises(errors.NotTreeError, r.tree, r.head())
-  
+
+    def test_tag(self):
+        r = self._repo = open_repo('a.git')
+        tag_sha = '28237f4dc30d0d462658d6b937b08a0f0b6ef55a'
+        tag = r.tag(tag_sha)
+        self.assertEqual(tag._type, 'tag')
+        self.assertEqual(tag.sha().hexdigest(), tag_sha)
+        obj_type, obj_sha = tag.object
+        self.assertEqual(obj_type, objects.Commit)
+        self.assertEqual(obj_sha, r.head())
+
+    def test_tag_not_tag(self):
+        r = self._repo = open_repo('a.git')
+        self.assertRaises(errors.NotTagError, r.tag, r.head())
+
+    def test_get_peeled(self):
+        # unpacked ref
+        r = self._repo = open_repo('a.git')
+        tag_sha = '28237f4dc30d0d462658d6b937b08a0f0b6ef55a'
+        self.assertNotEqual(r[tag_sha].sha().hexdigest(), r.head())
+        self.assertEqual(r.get_peeled('refs/tags/mytag'), r.head())
+
+        # packed ref with cached peeled value
+        packed_tag_sha = 'b0931cadc54336e78a1d980420e3268903b57a50'
+        parent_sha = r[r.head()].parents[0]
+        self.assertNotEqual(r[packed_tag_sha].sha().hexdigest(), parent_sha)
+        self.assertEqual(r.get_peeled('refs/tags/mytag-packed'), parent_sha)
+
+        # TODO: add more corner cases to test repo
+
+    def test_get_peeled_not_tag(self):
+        r = self._repo = open_repo('a.git')
+        self.assertEqual(r.get_peeled('HEAD'), r.head())
+
     def test_get_blob(self):
         r = self._repo = open_repo('a.git')
         commit = r.commit(r.head())
@@ -255,27 +291,47 @@ class RefsContainerTests(unittest.TestCase):
         tear_down_repo(self._repo)
 
     def test_get_packed_refs(self):
-        self.assertEqual(
-            {'refs/tags/refs-0.1': 'df6800012397fb85c56e7418dd4eb9405dee075c'},
-            self._refs.get_packed_refs())
+        self.assertEqual({
+            'refs/heads/packed': '42d06bd4b77fed026b154d16493e5deab78f02ec',
+            'refs/tags/refs-0.1': 'df6800012397fb85c56e7418dd4eb9405dee075c',
+            }, self._refs.get_packed_refs())
+
+    def test_get_peeled_not_packed(self):
+        # not packed
+        self.assertEqual(None, self._refs.get_peeled('refs/tags/refs-0.2'))
+        self.assertEqual('3ec9c43c84ff242e3ef4a9fc5bc111fd780a76a8',
+                         self._refs['refs/tags/refs-0.2'])
+
+        # packed, known not peelable
+        self.assertEqual(self._refs['refs/heads/packed'],
+                         self._refs.get_peeled('refs/heads/packed'))
+
+        # packed, peeled
+        self.assertEqual('42d06bd4b77fed026b154d16493e5deab78f02ec',
+                         self._refs.get_peeled('refs/tags/refs-0.1'))
 
     def test_keys(self):
         self.assertEqual([
             'HEAD',
             'refs/heads/loop',
             'refs/heads/master',
+            'refs/heads/packed',
             'refs/tags/refs-0.1',
+            'refs/tags/refs-0.2',
             ], sorted(list(self._refs.keys())))
-        self.assertEqual(['loop', 'master'],
+        self.assertEqual(['loop', 'master', 'packed'],
                          sorted(self._refs.keys('refs/heads')))
-        self.assertEqual(['refs-0.1'], list(self._refs.keys('refs/tags')))
+        self.assertEqual(['refs-0.1', 'refs-0.2'],
+                         sorted(self._refs.keys('refs/tags')))
 
     def test_as_dict(self):
         # refs/heads/loop does not show up
         self.assertEqual({
             'HEAD': '42d06bd4b77fed026b154d16493e5deab78f02ec',
             'refs/heads/master': '42d06bd4b77fed026b154d16493e5deab78f02ec',
+            'refs/heads/packed': '42d06bd4b77fed026b154d16493e5deab78f02ec',
             'refs/tags/refs-0.1': 'df6800012397fb85c56e7418dd4eb9405dee075c',
+            'refs/tags/refs-0.2': '3ec9c43c84ff242e3ef4a9fc5bc111fd780a76a8',
             }, self._refs.as_dict())
 
     def test_setitem(self):

dulwich/tests/test_server.py

@@ -45,6 +45,7 @@ TWO = '2' * 40
 THREE = '3' * 40
 FOUR = '4' * 40
 FIVE = '5' * 40
+SIX = '6' * 40
 
 class TestProto(object):
     def __init__(self):
@@ -143,14 +144,23 @@ class TestCommit(object):
         return '%s(%s)' % (self.__class__.__name__, self._sha)
 
 
+class TestRepo(object):
+    def __init__(self):
+        self.peeled = {}
+
+    def get_peeled(self, name):
+        return self.peeled[name]
+
+
 class TestBackend(object):
-    def __init__(self, objects):
+    def __init__(self, repo, objects):
+        self.repo = repo
         self.object_store = objects
 
 
 class TestUploadPackHandler(Handler):
     def __init__(self, objects, proto):
-        self.backend = TestBackend(objects)
+        self.backend = TestBackend(TestRepo(), objects)
         self.proto = proto
         self.stateless_rpc = False
         self.advertise_refs = False
@@ -172,6 +182,7 @@ class ProtocolGraphWalkerTestCase(TestCase):
             FOUR: TestCommit(FOUR, [TWO], 444),
             FIVE: TestCommit(FIVE, [THREE], 555),
             }
+
         self._walker = ProtocolGraphWalker(
             TestUploadPackHandler(self._objects, TestProto()))
 
@@ -225,6 +236,7 @@ class ProtocolGraphWalkerTestCase(TestCase):
             'want %s' % TWO,
             ])
         heads = {'ref1': ONE, 'ref2': TWO, 'ref3': THREE}
+        self._walker.handler.backend.repo.peeled = heads
         self.assertEquals([ONE, TWO], self._walker.determine_wants(heads))
 
         self._walker.proto.set_output(['want %s multi_ack' % FOUR])
@@ -239,6 +251,35 @@ class ProtocolGraphWalkerTestCase(TestCase):
         self._walker.proto.set_output(['want %s multi_ack' % FOUR])
         self.assertRaises(GitProtocolError, self._walker.determine_wants, heads)
 
+    def test_determine_wants_advertisement(self):
+        self._walker.proto.set_output([])
+        # advertise branch tips plus tag
+        heads = {'ref4': FOUR, 'ref5': FIVE, 'tag6': SIX}
+        peeled = {'ref4': FOUR, 'ref5': FIVE, 'tag6': FIVE}
+        self._walker.handler.backend.repo.peeled = peeled
+        self._walker.determine_wants(heads)
+        lines = []
+        while True:
+            line = self._walker.proto.get_received_line()
+            if line == 'None':
+                break
+            # strip capabilities list if present
+            if '\x00' in line:
+                line = line[:line.index('\x00')]
+            lines.append(line.rstrip())
+
+        self.assertEquals([
+            '%s ref4' % FOUR,
+            '%s ref5' % FIVE,
+            '%s tag6^{}' % FIVE,
+            '%s tag6' % SIX,
+            ], sorted(lines))
+
+        # ensure peeled tag was advertised immediately following tag
+        for i, line in enumerate(lines):
+            if line.endswith(' tag6'):
+                self.assertEquals('%s tag6^{}' % FIVE, lines[i+1])
+
     # TODO: test commit time cutoff
 
 

dulwich/tests/test_web.py

@@ -113,13 +113,28 @@ class DumbHandlersTestCase(WebTestCase):
         blob2 = TestBlob('222')
         blob3 = TestBlob('333')
 
-        tag1 = TestTag('aaa', TestTag.type, 'bbb')
-        tag2 = TestTag('bbb', TestBlob.type, '222')
+        tag1 = TestTag('aaa', TestBlob.type, '222')
+
+        class TestRepo(object):
+            def __init__(self, objects, peeled):
+                self._objects = dict((o.sha(), o) for o in objects)
+                self._peeled = peeled
+
+            def get_peeled(self, sha):
+                return self._peeled[sha]
+
+            def __getitem__(self, sha):
+                return self._objects[sha]
 
         class TestBackend(object):
             def __init__(self):
-                objects = [blob1, blob2, blob3, tag1, tag2]
-                self.repo = dict((o.sha(), o) for o in objects)
+                objects = [blob1, blob2, blob3, tag1]
+                self.repo = TestRepo(objects, {
+                    'HEAD': '000',
+                    'refs/heads/master': blob1.sha(),
+                    'refs/tags/tag-tag': blob2.sha(),
+                    'refs/tags/blob-tag': blob3.sha(),
+                    })
 
             def get_refs(self):
                 return {

dulwich/web.py

@@ -24,10 +24,6 @@ import os
 import re
 import time
 
-from dulwich.objects import (
-    Tag,
-    num_type_map,
-    )
 from dulwich.repo import (
     Repo,
     )
@@ -151,15 +147,9 @@ def get_info_refs(req, backend, mat, services=None):
             if not o:
                 continue
             yield '%s\t%s\n' % (sha, name)
-            obj_type = num_type_map[o.type]
-            if obj_type == Tag:
-                while obj_type == Tag:
-                    num_type, sha = o.object
-                    obj_type = num_type_map[num_type]
-                    o = backend.repo[sha]
-                if not o:
-                    continue
-                yield '%s\t%s^{}\n' % (o.sha(), name)
+            peeled_sha = backend.repo.get_peeled(name)
+            if peeled_sha != sha:
+                yield '%s\t%s^{}\n' % (peeled_sha, name)
 
 
 def get_info_packs(req, backend, mat):