dulwich-users team mailing list archive

[dborowitz@xxxxxxxxxx]

From: Dave Borowitz <dborowitz@xxxxxxxxxx>

Git clients expect this to work with a client-side warning that the
repo they're cloning is empty. All the server needs to do in this case
is send a single flush-pkt during the refs advertisement.

Change-Id: Ia08b595be78b5f1a4ab8b617aa3f8cb85cc75702
---
 NEWS                                 |    2 ++
 dulwich/server.py                    |    4 +++-
 dulwich/tests/compat/server_utils.py |   22 ++++++++++++++++++++++
 dulwich/tests/test_server.py         |   16 ++++++----------
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/NEWS b/NEWS
index 6746e66..7a6efc6 100644
--- a/NEWS
+++ b/NEWS
@@ -40,6 +40,8 @@
     performance when wide revision graphs are involved.
     (Jelmer Vernooij, #818168)
 
+  * Teach the server how to serve a clone of an empty repo. (Dave Borowitz)
+
  API CHANGES
 
   * write_pack no longer takes the num_objects argument and requires an object
diff --git a/dulwich/server.py b/dulwich/server.py
index dcb5695..132b27f 100644
--- a/dulwich/server.py
+++ b/dulwich/server.py
@@ -332,7 +332,9 @@ class ProtocolGraphWalker(object):
         :return: a list of SHA1s requested by the client
         """
         if not heads:
-            raise GitProtocolError('No heads found')
+            # The repo is empty, so short-circuit the whole process.
+            self.proto.write_pkt_line(None)
+            return None
         values = set(heads.itervalues())
         if self.advertise_refs or not self.http_req:
             for i, (ref, sha) in enumerate(heads.iteritems()):
diff --git a/dulwich/tests/compat/server_utils.py b/dulwich/tests/compat/server_utils.py
index a62a793..59dc585 100644
--- a/dulwich/tests/compat/server_utils.py
+++ b/dulwich/tests/compat/server_utils.py
@@ -20,10 +20,14 @@
 """Utilities for testing git server compatibility."""
 
 
+import os
 import select
+import shutil
 import socket
+import tempfile
 import threading
 
+from dulwich.repo import Repo
 from dulwich.server import (
     ReceivePackHandler,
     )
@@ -100,6 +104,24 @@ class ServerTests(object):
         self._old_repo.object_store._pack_cache = None
         self.assertReposEqual(self._old_repo, self._new_repo)
 
+    def test_clone_from_dulwich_empty(self):
+        old_repo_dir = os.path.join(tempfile.mkdtemp(), 'empty_old')
+        run_git_or_fail(['init', '--quiet', '--bare', old_repo_dir])
+        self._old_repo = Repo(old_repo_dir)
+        port = self._start_server(self._old_repo)
+
+        new_repo_base_dir = tempfile.mkdtemp()
+        try:
+            new_repo_dir = os.path.join(new_repo_base_dir, 'empty_new')
+            run_git_or_fail(['clone', self.url(port), new_repo_dir],
+                            cwd=new_repo_base_dir)
+            new_repo = Repo(new_repo_dir)
+            self.assertReposEqual(self._old_repo, new_repo)
+        finally:
+            # We don't create a Repo from new_repo_dir until after some errors
+            # may have occurred, so don't depend on tearDown to clean it up.
+            shutil.rmtree(new_repo_base_dir)
+
 
 class ShutdownServerMixIn:
     """Mixin that allows serve_forever to be shut down.
diff --git a/dulwich/tests/test_server.py b/dulwich/tests/test_server.py
index 3e5d34a..1737050 100644
--- a/dulwich/tests/test_server.py
+++ b/dulwich/tests/test_server.py
@@ -78,16 +78,11 @@ class TestProto(object):
         self._received[band].append(data)
 
     def write_pkt_line(self, data):
-        if data is None:
-            data = 'None'
         self._received[0].append(data)
 
     def get_received_line(self, band=0):
         lines = self._received[band]
-        if lines:
-            return lines.pop(0)
-        else:
-            return None
+        return lines.pop(0)
 
 
 class TestGenericHandler(Handler):
@@ -164,14 +159,14 @@ class UploadPackHandlerTestCase(TestCase):
                          self._handler.proto.get_received_line(2))
         self.assertEqual('second message',
                          self._handler.proto.get_received_line(2))
-        self.assertEqual(None, self._handler.proto.get_received_line(2))
+        self.assertRaises(IndexError, self._handler.proto.get_received_line, 2)
 
     def test_no_progress(self):
         caps = list(self._handler.required_capabilities()) + ['no-progress']
         self._handler.set_client_capabilities(caps)
         self._handler.progress('first message')
         self._handler.progress('second message')
-        self.assertEqual(None, self._handler.proto.get_received_line(2))
+        self.assertRaises(IndexError, self._handler.proto.get_received_line, 2)
 
     def test_get_tagged(self):
         refs = {
@@ -268,7 +263,8 @@ class ProtocolGraphWalkerTestCase(TestCase):
         self.assertEquals((None, None), _split_proto_line('', allowed))
 
     def test_determine_wants(self):
-        self.assertRaises(GitProtocolError, self._walker.determine_wants, {})
+        self.assertEqual(None, self._walker.determine_wants({}))
+        self.assertEqual(None, self._walker.proto.get_received_line())
 
         self._walker.proto.set_output([
           'want %s multi_ack' % ONE,
@@ -309,7 +305,7 @@ class ProtocolGraphWalkerTestCase(TestCase):
         lines = []
         while True:
             line = self._walker.proto.get_received_line()
-            if line == 'None':
+            if line is None:
                 break
             # strip capabilities list if present
             if '\x00' in line:
-- 
1.7.3.1