Support the 'side-band-64k' capability in 'git-receive-pack'.

This still lacks tests for our behaviour when the server does not support
side-band-64k.

NEWS

@@ -8,6 +8,9 @@
 
   * New ``Repo.clone`` method. (Jelmer Vernooij, #725369)
 
+  * ``GitClient.send_pack`` now supports the 'side-band-64k' capability.
+    (Jelmer Vernooij)
+
  CHANGES
 
   * unittest2 or python >= 2.7 is now required for the testsuite.

dulwich/client.py

@@ -32,6 +32,7 @@ from dulwich.errors import (
     UpdateRefsError,
     )
 from dulwich.protocol import (
+    PktLineParser,
     Protocol,
     TCP_GIT_PORT,
     ZERO_SHA,
@@ -52,8 +53,8 @@ def _fileno_can_read(fileno):
     """Check if a file descriptor is readable."""
     return len(select.select([fileno], [], [], 0)[0]) > 0
 
-COMMON_CAPABILITIES = ['ofs-delta']
-FETCH_CAPABILITIES = ['multi_ack', 'side-band-64k'] + COMMON_CAPABILITIES
+COMMON_CAPABILITIES = ['ofs-delta', 'side-band-64k']
+FETCH_CAPABILITIES = ['multi_ack'] + COMMON_CAPABILITIES
 SEND_CAPABILITIES = ['report-status'] + COMMON_CAPABILITIES
 
 
@@ -117,7 +118,7 @@ class ReportStatusParser(object):
 
 # TODO(durin42): this doesn't correctly degrade if the server doesn't
 # support some capabilities. This should work properly with servers
-# that don't support side-band-64k and multi_ack.
+# that don't support multi_ack.
 class GitClient(object):
     """Git smart server client.
 
@@ -163,19 +164,14 @@ class GitClient(object):
             refs[ref] = sha
         return refs, server_capabilities
 
-    def _parse_status_report(self, proto):
-        report_status_parser = ReportStatusParser()
-        for pkt in proto.read_pkt_seq():
-            report_status_parser.handle_packet(pkt)
-        report_status_parser.check()
-
-    # TODO(durin42): add side-band-64k capability support here and advertise it
-    def send_pack(self, path, determine_wants, generate_pack_contents):
+    def send_pack(self, path, determine_wants, generate_pack_contents,
+                  progress=None):
         """Upload a pack to a remote repository.
 
         :param path: Repository path
         :param generate_pack_contents: Function that can return a sequence of the
             shas of the objects to upload.
+        :param progress: Optional callback called with progress updates
 
         :raises SendPackError: if server rejects the pack data
         :raises UpdateRefsError: if the server supports report-status
@@ -183,8 +179,9 @@ class GitClient(object):
         """
         proto, unused_can_read = self._connect('receive-pack', path)
         old_refs, server_capabilities = self._read_refs(proto)
+        negotiated_capabilities = list(self._send_capabilities)
         if 'report-status' not in server_capabilities:
-            self._send_capabilities.remove('report-status')
+            negotiated_capabilities.remove('report-status')
         new_refs = determine_wants(old_refs)
         if not new_refs:
             proto.write_pkt_line(None)
@@ -202,7 +199,7 @@ class GitClient(object):
                 else:
                     proto.write_pkt_line(
                       '%s %s %s\0%s' % (old_sha1, new_sha1, refname,
-                                        ' '.join(self._send_capabilities)))
+                                        ' '.join(negotiated_capabilities)))
                     sent_capabilities = True
             if new_sha1 not in have and new_sha1 != ZERO_SHA:
                 want.append(new_sha1)
@@ -211,9 +208,22 @@ class GitClient(object):
             return new_refs
         objects = generate_pack_contents(have, want)
         entries, sha = write_pack_objects(proto.write_file(), objects)
-
-        if 'report-status' in self._send_capabilities:
-            self._parse_status_report(proto)
+        if 'report-status' in negotiated_capabilities:
+            report_status_parser = ReportStatusParser()
+        else:
+            report_status_parser = None
+        if "side-band-64k" in negotiated_capabilities:
+            channel_callbacks = { 2: progress }
+            if 'report-status' in negotiated_capabilities:
+                channel_callbacks[1] = PktLineParser(
+                    report_status_parser.handle_packet).parse
+            self._read_side_band64k_data(proto, channel_callbacks)
+        else:
+            if 'report-status':
+                for pkt in proto.read_pkt_seq():
+                    report_status_parser.handle_packet(pkt)
+        if report_status_parser is not None:
+            report_status_parser.check()
         # wait for EOF before returning
         data = proto.read()
         if data:
@@ -240,7 +250,7 @@ class GitClient(object):
             commit()
 
     def fetch_pack(self, path, determine_wants, graph_walker, pack_data,
-                   progress):
+                   progress=None):
         """Retrieve a pack from a git smart server.
 
         :param determine_wants: Callback that returns list of commits to fetch
@@ -250,13 +260,14 @@ class GitClient(object):
         """
         proto, can_read = self._connect('upload-pack', path)
         (refs, server_capabilities) = self._read_refs(proto)
+        negotiated_capabilities = list(self._fetch_capabilities)
         wants = determine_wants(refs)
         if not wants:
             proto.write_pkt_line(None)
             return refs
         assert isinstance(wants, list) and type(wants[0]) == str
         proto.write_pkt_line('want %s %s\n' % (
-            wants[0], ' '.join(self._fetch_capabilities)))
+            wants[0], ' '.join(negotiated_capabilities)))
         for want in wants[1:]:
             proto.write_pkt_line('want %s\n' % want)
         proto.write_pkt_line(None)
@@ -279,19 +290,36 @@ class GitClient(object):
             if len(parts) < 3 or parts[2] != 'continue':
                 break
             pkt = proto.read_pkt_line()
-        # TODO(durin42): this is broken if the server didn't support the
-        # side-band-64k capability.
+        if "side-band-64k" in negotiated_capabilities:
+            self._read_side_band64k_data(proto, {1: pack_data, 2: progress})
+            # wait for EOF before returning
+            data = proto.read()
+            if data:
+                raise Exception('Unexpected response %r' % data)
+        else:
+            # FIXME: Buffering?
+            pack_data(self.read())
+        return refs
+
+    def _read_side_band64k_data(self, proto, channel_callbacks):
+        """Read per-channel data.
+
+        This requires the side-band-64k capability.
+
+        :param proto: Protocol object to read from
+        :param channel_callbacks: Dictionary mapping channels to packet
+            handlers to use. None for a callback discards channel data.
+        """
         for pkt in proto.read_pkt_seq():
             channel = ord(pkt[0])
             pkt = pkt[1:]
-            if channel == 1:
-                pack_data(pkt)
-            elif channel == 2:
-                if progress is not None:
-                    progress(pkt)
-            else:
+            try:
+                cb = channel_callbacks[channel]
+            except KeyError:
                 raise AssertionError('Invalid sideband channel %d' % channel)
-        return refs
+            else:
+                if cb is not None:
+                    cb(pkt)
 
 
 class TCPGitClient(GitClient):

dulwich/tests/test_client.py

@@ -63,7 +63,7 @@ class GitClientTests(TestCase):
         self.assertEquals(set(['multi_ack', 'side-band-64k', 'ofs-delta',
                                'thin-pack']),
                           set(self.client._fetch_capabilities))
-        self.assertEquals(set(['ofs-delta', 'report-status']),
+        self.assertEquals(set(['ofs-delta', 'report-status', 'side-band-64k']),
                           set(self.client._send_capabilities))
 
     def test_fetch_pack_none(self):