dulwich-users team mailing list archive

[dborowitz@xxxxxxxxxx]

From: Dave Borowitz <dborowitz@xxxxxxxxxx>

The previous behavior failed with an AttributeError that was being
silently swallowed by the server code. It still passed tests because the
pipe was getting closed, which was what the client expected anyway.

Now that this error is no longer swallowed, we needed to actually fix
the behavior.

Change-Id: I386e2e2995ab64d2fac4fc79db2f6de0afbac276
---
 NEWS                         |    2 ++
 dulwich/repo.py              |    2 --
 dulwich/server.py            |   27 ++++++++++++++++++++++++++-
 dulwich/tests/test_server.py |    2 +-
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index c4f5f0b..404a49e 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,8 @@
 
   * Correctly return a tuple from MemoryObjectStore.get_raw. (Dave Borowitz)
 
+  * Fix short-circuit behavior for no-op fetches in the server. (Dave Borowitz)
+
  API CHANGES
 
   * write_pack no longer takes the num_objects argument and requires an object
diff --git a/dulwich/repo.py b/dulwich/repo.py
index dea3a8e..0de83bb 100644
--- a/dulwich/repo.py
+++ b/dulwich/repo.py
@@ -835,8 +835,6 @@ class BaseRepo(object):
         """
         wants = determine_wants(self.get_refs())
         if wants is None:
-            # TODO(dborowitz): find a way to short-circuit that doesn't change
-            # this interface.
             return None
         haves = self.object_store.find_common_revisions(graph_walker)
         return self.object_store.iter_shas(
diff --git a/dulwich/server.py b/dulwich/server.py
index 1c05a16..99aea42 100644
--- a/dulwich/server.py
+++ b/dulwich/server.py
@@ -381,7 +381,12 @@ class ProtocolGraphWalker(object):
         # Now client will sending want want want commands
         want = self.proto.read_pkt_line()
         if not want:
-            return []
+            # The client closes the pipe immediately on a no-op fetch, so we
+            # won't even need to send a pack.
+            self.set_ack_type(_NO_OP_ACK)
+            self.handler.set_client_capabilities(
+              self.handler.required_capabilities())
+            return None
         line, caps = extract_want_line_capabilities(want)
         self.handler.set_client_capabilities(caps)
         self.set_ack_type(ack_type(caps))
@@ -490,11 +495,31 @@ class ProtocolGraphWalker(object):
           MULTI_ACK: MultiAckGraphWalkerImpl,
           MULTI_ACK_DETAILED: MultiAckDetailedGraphWalkerImpl,
           SINGLE_ACK: SingleAckGraphWalkerImpl,
+          _NO_OP_ACK: NoOpGraphWalkerImpl,
           }
         self._impl = impl_classes[ack_type](self)
 
 
 _GRAPH_WALKER_COMMANDS = ('have', 'done', None)
+_NO_OP_ACK = 'no_op_ack'
+
+
+class NoOpGraphWalkerImpl(object):
+    """Graph walker implementation for no-op fetches.
+
+    The git client just sends a single flush-pkt with no capabilities when it
+    wants a no-op fetch, so we can't even choose a Single or
+    MultiAckGraphWalkerImpl.
+    """
+
+    def __init__(self, walker):
+        pass
+
+    def ack(self, have_ref):
+        pass
+
+    def next(self):
+        return None
 
 
 class SingleAckGraphWalkerImpl(object):
diff --git a/dulwich/tests/test_server.py b/dulwich/tests/test_server.py
index a630489..1ed4bdc 100644
--- a/dulwich/tests/test_server.py
+++ b/dulwich/tests/test_server.py
@@ -286,7 +286,7 @@ class ProtocolGraphWalkerTestCase(TestCase):
         self.assertRaises(GitProtocolError, self._walker.determine_wants, heads)
 
         self._walker.proto.set_output([])
-        self.assertEquals([], self._walker.determine_wants(heads))
+        self.assertEquals(None, self._walker.determine_wants(heads))
 
         self._walker.proto.set_output(['want %s multi_ack' % ONE, 'foo'])
         self.assertRaises(GitProtocolError, self._walker.determine_wants, heads)
-- 
1.7.3.1