Add get_peeled to BaseRepo so HTTP and git servers use one call to pe…

…el 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
MrJohz · Mar 4, 2010 · 69ad474 · 69ad474
1 parent b457b06
commit 69ad474
Show file tree

Hide file tree

Showing 15 changed files with 211 additions and 29 deletions.
diff --git a/dulwich/errors.py b/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."""
 

diff --git a/dulwich/repo.py b/dulwich/repo.py
@@ -32,6 +32,7 @@
     NotCommitError, 
     NotGitRepository,
     NotTreeError, 
+    NotTagError,
     PackedRefsException,
     )
 from dulwich.file import (
@@ -48,6 +49,7 @@
     Tag,
     Tree,
     hex_to_sha,
+    num_type_map,
     )
 
 OBJECTDIR = 'objects'
@@ -131,6 +133,16 @@ def get_packed_refs(self):
         """
         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 @@ def get_packed_refs(self):
                 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 @@ def get_packed_refs(self):
                 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 @@ def _get_object(self, sha, cls):
                 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 @@ def tree(self, sha):
     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)
 

diff --git a/dulwich/server.py b/dulwich/server.py
@@ -259,7 +259,10 @@ def determine_wants(self, heads):
                 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)

diff --git a/dulwich/tests/data/repos/a.git/objects/28/237f4dc30d0d462658d6b937b08a0f0b6ef55a b/dulwich/tests/data/repos/a.git/objects/28/237f4dc30d0d462658d6b937b08a0f0b6ef55a
@@ -0,0 +1,2 @@
+x5�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�
diff --git a/dulwich/tests/data/repos/a.git/objects/b0/931cadc54336e78a1d980420e3268903b57a50 b/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
diff --git a/dulwich/tests/data/repos/a.git/packed-refs b/dulwich/tests/data/repos/a.git/packed-refs
@@ -0,0 +1,3 @@
+# pack-refs with: peeled 
+b0931cadc54336e78a1d980420e3268903b57a50 refs/tags/mytag-packed
+^2a72d929692c41d8554c07f6301757ba18a65d91
diff --git a/dulwich/tests/data/repos/a.git/refs/tags/mytag b/dulwich/tests/data/repos/a.git/refs/tags/mytag
@@ -0,0 +1 @@
+28237f4dc30d0d462658d6b937b08a0f0b6ef55a
diff --git a/dulwich/tests/data/repos/refs.git/objects/3e/c9c43c84ff242e3ef4a9fc5bc111fd780a76a8 b/dulwich/tests/data/repos/refs.git/objects/3e/c9c43c84ff242e3ef4a9fc5bc111fd780a76a8
@@ -0,0 +1,3 @@
+x-�Q
+�0D��)�-�m�V�^�i6��6�.���~���{#Cm�]rwy���u�=�u�5^[�o��<��H<*y?ƴ,�()��a���߈��2<�)��$8x����R��.�4Yk�t�Pa��Ե�
+q?���W)'����ǧ6
diff --git a/dulwich/tests/data/repos/refs.git/objects/cd/a609072918d7b70057b6bef9f4c2537843fcfe b/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�

diff --git a/dulwich/tests/data/repos/refs.git/packed-refs b/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
diff --git a/dulwich/tests/data/repos/refs.git/refs/tags/refs-0.2 b/dulwich/tests/data/repos/refs.git/refs/tags/refs-0.2
@@ -0,0 +1 @@
+3ec9c43c84ff242e3ef4a9fc5bc111fd780a76a8
diff --git a/dulwich/tests/test_repository.py b/dulwich/tests/test_repository.py
@@ -27,6 +27,7 @@
 import unittest
 
 from dulwich import errors
+from dulwich import objects
 from dulwich.repo import (
     check_ref_format,
     Repo,
@@ -75,8 +76,10 @@ def test_ref(self):
     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 @@ def test_tree(self):
     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 @@ def tearDown(self):
         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):

diff --git a/dulwich/tests/test_server.py b/dulwich/tests/test_server.py
@@ -45,6 +45,7 @@
 THREE = '3' * 40
 FOUR = '4' * 40
 FIVE = '5' * 40
+SIX = '6' * 40
 
 class TestProto(object):
     def __init__(self):
@@ -143,14 +144,23 @@ def __repr__(self):
         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 @@ def setUp(self):
             FOUR: TestCommit(FOUR, [TWO], 444),
             FIVE: TestCommit(FIVE, [THREE], 555),
             }
+
         self._walker = ProtocolGraphWalker(
             TestUploadPackHandler(self._objects, TestProto()))
 
@@ -225,6 +236,7 @@ def test_determine_wants(self):
             '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 @@ def test_determine_wants(self):
         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
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		x5�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�