fix off-by-one depth bug in serving shallow clones

dulwich/server.py

@@ -391,7 +391,8 @@ def _find_shallow(store, heads, depth):
 
     :param store: An ObjectStore for looking up objects.
     :param heads: Iterable of head SHAs to start walking from.
-    :param depth: The depth of ancestors to include.
+    :param depth: The depth of ancestors to include. A depth of one includes
+        only the heads themselves.
     :return: A tuple of (shallow, not_shallow), sets of SHAs that should be
         considered shallow and unshallow according to the arguments. Note that
         these sets may overlap if a commit is reachable along multiple paths.
@@ -408,7 +409,7 @@ def _find_shallow(store, heads, depth):
     for head_sha in heads:
         obj = store.peel_sha(head_sha)
         if isinstance(obj, Commit):
-            todo.append((obj.id, 0))
+            todo.append((obj.id, 1))
 
     not_shallow = set()
     shallow = set()

dulwich/tests/compat/server_utils.py

@@ -171,11 +171,32 @@ class ServerTests(object):
         run_git_or_fail(['clone', '--mirror', '--depth=1', '--no-single-branch',
                         self.url(port), self._stub_repo.path])
         clone = self._stub_repo = Repo(self._stub_repo.path)
-        expected_shallow = [b'94de09a530df27ac3bb613aaecdd539e0a0655e1',
-                            b'da5cd81e1883c62a25bb37c4d1f8ad965b29bf8d']
+        expected_shallow = [b'35e0b59e187dd72a0af294aedffc213eaa4d03ff',
+                            b'514dc6d3fbfe77361bcaef320c4d21b72bc10be9']
         self.assertEqual(expected_shallow, _get_shallow(clone))
         self.assertReposNotEqual(clone, self._source_repo)
 
+    def test_shallow_clone_from_git_is_identical(self):
+        require_git_version(self.min_single_branch_version)
+        self._source_repo = self.import_repo('server_new.export')
+        self._stub_repo_git = _StubRepo('shallow-git')
+        self.addCleanup(tear_down_repo, self._stub_repo_git)
+        self._stub_repo_dw = _StubRepo('shallow-dw')
+        self.addCleanup(tear_down_repo, self._stub_repo_dw)
+
+        # shallow clone using stock git, then using dulwich
+        run_git_or_fail(['clone', '--mirror', '--depth=1', '--no-single-branch',
+                         'file://' + self._source_repo.path,
+                         self._stub_repo_git.path])
+
+        port = self._start_server(self._source_repo)
+        run_git_or_fail(['clone', '--mirror', '--depth=1', '--no-single-branch',
+                        self.url(port), self._stub_repo_dw.path])
+
+        # compare the two clones; they should be equal
+        self.assertReposEqual(Repo(self._stub_repo_git.path),
+                              Repo(self._stub_repo_dw.path))
+
     def test_fetch_same_depth_into_shallow_clone_from_dulwich(self):
         require_git_version(self.min_single_branch_version)
         self._source_repo = self.import_repo('server_new.export')
@@ -183,14 +204,14 @@ class ServerTests(object):
         self.addCleanup(tear_down_repo, self._stub_repo)
         port = self._start_server(self._source_repo)
 
-        # Fetch at depth 1
-        run_git_or_fail(['clone', '--mirror', '--depth=1', '--no-single-branch',
+        # Fetch at depth 2
+        run_git_or_fail(['clone', '--mirror', '--depth=2', '--no-single-branch',
                         self.url(port), self._stub_repo.path])
         clone = self._stub_repo = Repo(self._stub_repo.path)
 
         # Fetching at the same depth is a no-op.
         run_git_or_fail(
-          ['fetch', '--depth=1', self.url(port)] + self.branch_args(),
+          ['fetch', '--depth=2', self.url(port)] + self.branch_args(),
           cwd=self._stub_repo.path)
         expected_shallow = [b'94de09a530df27ac3bb613aaecdd539e0a0655e1',
                             b'da5cd81e1883c62a25bb37c4d1f8ad965b29bf8d']
@@ -204,19 +225,19 @@ class ServerTests(object):
         self.addCleanup(tear_down_repo, self._stub_repo)
         port = self._start_server(self._source_repo)
 
-        # Fetch at depth 1
-        run_git_or_fail(['clone', '--mirror', '--depth=1', '--no-single-branch',
+        # Fetch at depth 2
+        run_git_or_fail(['clone', '--mirror', '--depth=2', '--no-single-branch',
                         self.url(port), self._stub_repo.path])
         clone = self._stub_repo = Repo(self._stub_repo.path)
 
         # Fetching at the same depth is a no-op.
         run_git_or_fail(
-          ['fetch', '--depth=1', self.url(port)] + self.branch_args(),
+          ['fetch', '--depth=2', self.url(port)] + self.branch_args(),
           cwd=self._stub_repo.path)
 
-        # The whole repo only has depth 3, so it should equal server_new.
+        # The whole repo only has depth 4, so it should equal server_new.
         run_git_or_fail(
-          ['fetch', '--depth=3', self.url(port)] + self.branch_args(),
+          ['fetch', '--depth=4', self.url(port)] + self.branch_args(),
           cwd=self._stub_repo.path)
         self.assertEqual([], _get_shallow(clone))
         self.assertReposEqual(clone, self._source_repo)

dulwich/tests/compat/test_web.py

@@ -187,6 +187,11 @@ class DumbWebTestCase(WebTests, CompatTestCase):
         # clones.
         raise SkipTest('Dumb web shallow cloning not supported.')
 
+    def test_shallow_clone_from_git_is_identical(self):
+        # Note: remove this if C git and dulwich implement dumb web shallow
+        # clones.
+        raise SkipTest('Dumb web shallow cloning not supported.')
+
     def test_fetch_same_depth_into_shallow_clone_from_dulwich(self):
         # Note: remove this if C git and dulwich implement dumb web shallow
         # clones.

dulwich/tests/test_server.py

@@ -235,13 +235,13 @@ class FindShallowTests(TestCase):
         c1, c2, c3 = self.make_linear_commits(3)
 
         self.assertEqual((set([c3.id]), set([])),
-                         _find_shallow(self._store, [c3.id], 0))
-        self.assertEqual((set([c2.id]), set([c3.id])),
                          _find_shallow(self._store, [c3.id], 1))
-        self.assertEqual((set([c1.id]), set([c2.id, c3.id])),
+        self.assertEqual((set([c2.id]), set([c3.id])),
                          _find_shallow(self._store, [c3.id], 2))
-        self.assertEqual((set([]), set([c1.id, c2.id, c3.id])),
+        self.assertEqual((set([c1.id]), set([c2.id, c3.id])),
                          _find_shallow(self._store, [c3.id], 3))
+        self.assertEqual((set([]), set([c1.id, c2.id, c3.id])),
+                         _find_shallow(self._store, [c3.id], 4))
 
     def test_multiple_independent(self):
         a = self.make_linear_commits(2, message=b'a')
@@ -250,7 +250,7 @@ class FindShallowTests(TestCase):
         heads = [a[1].id, b[1].id, c[1].id]
 
         self.assertEqual((set([a[0].id, b[0].id, c[0].id]), set(heads)),
-                         _find_shallow(self._store, heads, 1))
+                         _find_shallow(self._store, heads, 2))
 
     def test_multiple_overlapping(self):
         # Create the following commit tree:
@@ -263,7 +263,7 @@ class FindShallowTests(TestCase):
 
         # 1 is shallow along the path from 4, but not along the path from 2.
         self.assertEqual((set([c1.id]), set([c1.id, c2.id, c3.id, c4.id])),
-                         _find_shallow(self._store, [c2.id, c4.id], 2))
+                         _find_shallow(self._store, [c2.id, c4.id], 3))
 
     def test_merge(self):
         c1 = self.make_commit()
@@ -271,7 +271,7 @@ class FindShallowTests(TestCase):
         c3 = self.make_commit(parents=[c1.id, c2.id])
 
         self.assertEqual((set([c1.id, c2.id]), set([c3.id])),
-                         _find_shallow(self._store, [c3.id], 1))
+                         _find_shallow(self._store, [c3.id], 2))
 
     def test_tag(self):
         c1, c2 = self.make_linear_commits(2)
@@ -279,7 +279,7 @@ class FindShallowTests(TestCase):
         self._store.add_object(tag)
 
         self.assertEqual((set([c1.id]), set([c2.id])),
-                         _find_shallow(self._store, [tag.id], 1))
+                         _find_shallow(self._store, [tag.id], 2))
 
 
 class TestUploadPackHandler(UploadPackHandler):
@@ -479,7 +479,7 @@ class ProtocolGraphWalkerTestCase(TestCase):
           expected, list(iter(self._walker.proto.get_received_line, None)))
 
     def test_handle_shallow_request_no_client_shallows(self):
-        self._handle_shallow_request([b'deepen 1\n'], [FOUR, FIVE])
+        self._handle_shallow_request([b'deepen 2\n'], [FOUR, FIVE])
         self.assertEqual(set([TWO, THREE]), self._walker.shallow)
         self.assertReceived([
           b'shallow ' + TWO,
@@ -490,7 +490,7 @@ class ProtocolGraphWalkerTestCase(TestCase):
         lines = [
           b'shallow ' + TWO + b'\n',
           b'shallow ' + THREE + b'\n',
-          b'deepen 1\n',
+          b'deepen 2\n',
           ]
         self._handle_shallow_request(lines, [FOUR, FIVE])
         self.assertEqual(set([TWO, THREE]), self._walker.shallow)
@@ -499,7 +499,7 @@ class ProtocolGraphWalkerTestCase(TestCase):
     def test_handle_shallow_request_unshallows(self):
         lines = [
           b'shallow ' + TWO + b'\n',
-          b'deepen 2\n',
+          b'deepen 3\n',
           ]
         self._handle_shallow_request(lines, [FOUR, FIVE])
         self.assertEqual(set([ONE]), self._walker.shallow)