Merge support for side-band-64k.

NEWS

@@ -35,6 +35,12 @@
   * ObjectStore.iter_tree_contents can optionally yield tree objects as well.
     (Dave Borowitz).
 
+  * Add side-band-64k support to ReceivePackHandler. (Dave Borowitz)
+
+  * Change server capabilities methods to classmethods. (Dave Borowitz)
+
+  * Tweak server handler injection. (Dave Borowitz)
+
 
 0.6.1	2010-07-22
 
@@ -69,6 +75,10 @@
 
   * Quiet logging output from web tests. (Dave Borowitz)
 
+  * More flexible version checking for compat tests. (Dave Borowitz)
+
+  * Compat tests for servers with and without side-band-64k. (Dave Borowitz)
+
  CLEANUP
 
   * Clean up file headers. (Dave Borowitz)
@@ -87,6 +97,10 @@
 
   * Move reference WSGI handler to web.py. (Dave Borowitz)
 
+  * Factor out _report_status in ReceivePackHandler. (Dave Borowitz)
+
+  * Factor out a function to convert a line to a pkt-line. (Dave Borowitz)
+
 
 0.6.0	2010-05-22
 

dulwich/protocol.py

@@ -57,6 +57,18 @@ class ProtocolFile(object):
         pass
 
 
+def pkt_line(data):
+    """Wrap data in a pkt-line.
+
+    :param data: The data to wrap, as a str or None.
+    :return: The data prefixed with its length in pkt-line format; if data was
+        None, returns the flush-pkt ('0000')
+    """
+    if data is None:
+        return '0000'
+    return '%04x%s' % (len(data) + 4, data)
+
+
 class Protocol(object):
 
     def __init__(self, read, write, report_activity=None):
@@ -98,14 +110,10 @@ class Protocol(object):
         :param line: A string containing the data to send
         """
         try:
-            if line is None:
-                self.write("0000")
-                if self.report_activity:
-                    self.report_activity(4, 'write')
-            else:
-                self.write("%04x%s" % (len(line)+4, line))
-                if self.report_activity:
-                    self.report_activity(4+len(line), 'write')
+            line = pkt_line(line)
+            self.write(line)
+            if self.report_activity:
+                self.report_activity(len(line), 'write')
         except socket.error, e:
             raise GitProtocolError(e)
 
@@ -310,3 +318,46 @@ def ack_type(capabilities):
     elif 'multi_ack' in capabilities:
         return MULTI_ACK
     return SINGLE_ACK
+
+
+class BufferedPktLineWriter(object):
+    """Writer that wraps its data in pkt-lines and has an independent buffer.
+
+    Consecutive calls to write() wrap the data in a pkt-line and then buffers it
+    until enough lines have been written such that their total length (including
+    length prefix) reach the buffer size.
+    """
+
+    def __init__(self, write, bufsize=65515):
+        """Initialize the BufferedPktLineWriter.
+
+        :param write: A write callback for the underlying writer.
+        :param bufsize: The internal buffer size, including length prefixes.
+        """
+        self._write = write
+        self._bufsize = bufsize
+        self._wbuf = StringIO()
+        self._buflen = 0
+
+    def write(self, data):
+        """Write data, wrapping it in a pkt-line."""
+        line = pkt_line(data)
+        line_len = len(line)
+        over = self._buflen + line_len - self._bufsize
+        if over >= 0:
+            start = line_len - over
+            self._wbuf.write(line[:start])
+            self.flush()
+        else:
+            start = 0
+        saved = line[start:]
+        self._wbuf.write(saved)
+        self._buflen += len(saved)
+
+    def flush(self):
+        """Flush all data from the buffer."""
+        data = self._wbuf.getvalue()
+        if data:
+            self._write(data)
+        self._len = 0
+        self._wbuf = StringIO()

dulwich/server.py

@@ -57,6 +57,7 @@ from dulwich.protocol import (
     ack_type,
     extract_capabilities,
     extract_want_line_capabilities,
+    BufferedPktLineWriter,
     )
 from dulwich.repo import (
     Repo,
@@ -161,16 +162,20 @@ class Handler(object):
         self.proto = proto
         self._client_capabilities = None
 
-    def capability_line(self):
-        return " ".join(self.capabilities())
+    @classmethod
+    def capability_line(cls):
+        return " ".join(cls.capabilities())
 
-    def capabilities(self):
-        raise NotImplementedError(self.capabilities)
+    @classmethod
+    def capabilities(cls):
+        raise NotImplementedError(cls.capabilities)
 
-    def innocuous_capabilities(self):
+    @classmethod
+    def innocuous_capabilities(cls):
         return ("include-tag", "thin-pack", "no-progress", "ofs-delta")
 
-    def required_capabilities(self):
+    @classmethod
+    def required_capabilities(cls):
         """Return a list of capabilities that we require the client to have."""
         return []
 
@@ -206,11 +211,13 @@ class UploadPackHandler(Handler):
         self.stateless_rpc = stateless_rpc
         self.advertise_refs = advertise_refs
 
-    def capabilities(self):
+    @classmethod
+    def capabilities(cls):
         return ("multi_ack_detailed", "multi_ack", "side-band-64k", "thin-pack",
                 "ofs-delta", "no-progress", "include-tag")
 
-    def required_capabilities(self):
+    @classmethod
+    def required_capabilities(cls):
         return ("side-band-64k", "thin-pack", "ofs-delta")
 
     def progress(self, message):
@@ -569,8 +576,9 @@ class ReceivePackHandler(Handler):
         self.stateless_rpc = stateless_rpc
         self.advertise_refs = advertise_refs
 
-    def capabilities(self):
-        return ("report-status", "delete-refs")
+    @classmethod
+    def capabilities(cls):
+        return ("report-status", "delete-refs", "side-band-64k")
 
     def _apply_pack(self, refs):
         f, commit = self.repo.object_store.add_thin_pack()
@@ -614,6 +622,29 @@ class ReceivePackHandler(Handler):
 
         return status
 
+    def _report_status(self, status):
+        if self.has_capability('side-band-64k'):
+            writer = BufferedPktLineWriter(
+              lambda d: self.proto.write_sideband(1, d))
+            write = writer.write
+
+            def flush():
+                writer.flush()
+                self.proto.write_pkt_line(None)
+        else:
+            write = self.proto.write_pkt_line
+            flush = lambda: None
+
+        for name, msg in status:
+            if name == 'unpack':
+                write('unpack %s\n' % msg)
+            elif msg == 'ok':
+                write('ok %s\n' % name)
+            else:
+                write('ng %s %s\n' % (name, msg))
+        write(None)
+        flush()
+
     def handle(self):
         refs = self.repo.get_refs().items()
 
@@ -654,14 +685,7 @@ class ReceivePackHandler(Handler):
         # when we have read all the pack from the client, send a status report
         # if the client asked for it
         if self.has_capability('report-status'):
-            for name, msg in status:
-                if name == 'unpack':
-                    self.proto.write_pkt_line('unpack %s\n' % msg)
-                elif msg == 'ok':
-                    self.proto.write_pkt_line('ok %s\n' % name)
-                else:
-                    self.proto.write_pkt_line('ng %s %s\n' % (name, msg))
-            self.proto.write_pkt_line(None)
+            self._report_status(status)
 
 
 # Default handler classes for git services.
@@ -674,7 +698,7 @@ DEFAULT_HANDLERS = {
 class TCPGitRequestHandler(SocketServer.StreamRequestHandler):
 
     def __init__(self, handlers, *args, **kwargs):
-        self.handlers = handlers and handlers or DEFAULT_HANDLERS
+        self.handlers = handlers
         SocketServer.StreamRequestHandler.__init__(self, *args, **kwargs)
 
     def handle(self):
@@ -698,8 +722,10 @@ class TCPGitServer(SocketServer.TCPServer):
         return TCPGitRequestHandler(self.handlers, *args, **kwargs)
 
     def __init__(self, backend, listen_addr, port=TCP_GIT_PORT, handlers=None):
+        self.handlers = dict(DEFAULT_HANDLERS)
+        if handlers is not None:
+            self.handlers.update(handlers)
         self.backend = backend
-        self.handlers = handlers
         logger.info('Listening for TCP connections on %s:%d', listen_addr, port)
         SocketServer.TCPServer.__init__(self, (listen_addr, port),
                                         self._make_handler)

dulwich/tests/compat/server_utils.py

@@ -24,6 +24,9 @@ import select
 import socket
 import threading
 
+from dulwich.server import (
+    ReceivePackHandler,
+    )
 from dulwich.tests.utils import (
     tear_down_repo,
     )
@@ -155,3 +158,15 @@ class ShutdownServerMixIn:
             except:
                 self.handle_error(request, client_address)
                 self.close_request(request)
+
+
+# TODO(dborowitz): Come up with a better way of testing various permutations of
+# capabilities. The only reason it is the way it is now is that side-band-64k
+# was only recently introduced into git-receive-pack.
+class NoSideBand64kReceivePackHandler(ReceivePackHandler):
+    """ReceivePackHandler that does not support side-band-64k."""
+
+    @classmethod
+    def capabilities(cls):
+        return tuple(c for c in ReceivePackHandler.capabilities()
+                     if c != 'side-band-64k')

dulwich/tests/compat/test_server.py

@@ -29,10 +29,12 @@ import threading
 from dulwich.server import (
     DictBackend,
     TCPGitServer,
+    ReceivePackHandler,
     )
 from server_utils import (
     ServerTests,
     ShutdownServerMixIn,
+    NoSideBand64kReceivePackHandler,
     )
 from utils import (
     CompatTestCase,
@@ -54,7 +56,10 @@ if not getattr(TCPGitServer, 'shutdown', None):
 
 
 class GitServerTestCase(ServerTests, CompatTestCase):
-    """Tests for client/server compatibility."""
+    """Tests for client/server compatibility.
+
+    This server test case does not use side-band-64k in git-receive-pack.
+    """
 
     protocol = 'git'
 
@@ -66,10 +71,35 @@ class GitServerTestCase(ServerTests, CompatTestCase):
         ServerTests.tearDown(self)
         CompatTestCase.tearDown(self)
 
+    def _handlers(self):
+        return {'git-receive-pack': NoSideBand64kReceivePackHandler}
+
+    def _check_server(self, dul_server):
+        receive_pack_handler_cls = dul_server.handlers['git-receive-pack']
+        caps = receive_pack_handler_cls.capabilities()
+        self.assertFalse('side-band-64k' in caps)
+
     def _start_server(self, repo):
         backend = DictBackend({'/': repo})
-        dul_server = TCPGitServer(backend, 'localhost', 0)
+        dul_server = TCPGitServer(backend, 'localhost', 0,
+                                  handlers=self._handlers())
+        self._check_server(dul_server)
         threading.Thread(target=dul_server.serve).start()
         self._server = dul_server
         _, port = self._server.socket.getsockname()
         return port
+
+
+class GitServerSideBand64kTestCase(GitServerTestCase):
+    """Tests for client/server compatibility with side-band-64k support."""
+
+    # side-band-64k in git-receive-pack was introduced in git 1.7.0.2
+    min_git_version = (1, 7, 0, 2)
+
+    def _handlers(self):
+        return None  # default handlers include side-band-64k
+
+    def _check_server(self, server):
+        receive_pack_handler_cls = server.handlers['git-receive-pack']
+        caps = receive_pack_handler_cls.capabilities()
+        self.assertTrue('side-band-64k' in caps)

dulwich/tests/compat/test_utils.py

@@ -0,0 +1,91 @@
+# test_utils.py -- Tests for git compatibility utilities
+# Copyright (C) 2010 Google, Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor,
+# Boston, MA  02110-1301, USA.
+
+"""Tests for git compatibility utilities."""
+
+from unittest import TestCase
+
+from dulwich.tests import (
+    TestSkipped,
+    )
+import utils
+
+
+class GitVersionTests(TestCase):
+
+    def setUp(self):
+        self._orig_run_git = utils.run_git
+        self._version_str = None  # tests can override to set stub version
+
+        def run_git(args, **unused_kwargs):
+            self.assertEqual(['--version'], args)
+            return 0, self._version_str
+        utils.run_git = run_git
+
+    def tearDown(self):
+        utils.run_git = self._orig_run_git
+
+    def test_git_version_none(self):
+        self._version_str = 'not a git version'
+        self.assertEqual(None, utils.git_version())
+
+    def test_git_version_3(self):
+        self._version_str = 'git version 1.6.6'
+        self.assertEqual((1, 6, 6, 0), utils.git_version())
+
+    def test_git_version_4(self):
+        self._version_str = 'git version 1.7.0.2'
+        self.assertEqual((1, 7, 0, 2), utils.git_version())
+
+    def test_git_version_extra(self):
+        self._version_str = 'git version 1.7.0.3.295.gd8fa2'
+        self.assertEqual((1, 7, 0, 3), utils.git_version())
+
+    def assertRequireSucceeds(self, required_version):
+        try:
+            utils.require_git_version(required_version)
+        except TestSkipped:
+            self.fail()
+
+    def assertRequireFails(self, required_version):
+        self.assertRaises(TestSkipped, utils.require_git_version,
+                          required_version)
+
+    def test_require_git_version(self):
+        try:
+            self._version_str = 'git version 1.6.6'
+            self.assertRequireSucceeds((1, 6, 6))
+            self.assertRequireSucceeds((1, 6, 6, 0))
+            self.assertRequireSucceeds((1, 6, 5))
+            self.assertRequireSucceeds((1, 6, 5, 99))
+            self.assertRequireFails((1, 7, 0))
+            self.assertRequireFails((1, 7, 0, 2))
+            self.assertRaises(ValueError, utils.require_git_version,
+                              (1, 6, 6, 0, 0))
+
+            self._version_str = 'git version 1.7.0.2'
+            self.assertRequireSucceeds((1, 6, 6))
+            self.assertRequireSucceeds((1, 6, 6, 0))
+            self.assertRequireSucceeds((1, 7, 0))
+            self.assertRequireSucceeds((1, 7, 0, 2))
+            self.assertRequireFails((1, 7, 0, 3))
+            self.assertRequireFails((1, 7, 1))
+        except TestSkipped, e:
+            # This test is designed to catch all TestSkipped exceptions.
+            self.fail('Test unexpectedly skipped: %s' % e)

dulwich/tests/compat/test_web.py

@@ -42,6 +42,7 @@ from dulwich.web import (
 from server_utils import (
     ServerTests,
     ShutdownServerMixIn,
+    NoSideBand64kReceivePackHandler,
     )
 from utils import (
     CompatTestCase,
@@ -84,7 +85,10 @@ class WebTests(ServerTests):
 
 
 class SmartWebTestCase(WebTests, CompatTestCase):
-    """Test cases for smart HTTP server."""
+    """Test cases for smart HTTP server.
+
+    This server test case does not use side-band-64k in git-receive-pack.
+    """
 
     min_git_version = (1, 6, 6)
 
@@ -96,8 +100,33 @@ class SmartWebTestCase(WebTests, CompatTestCase):
         WebTests.tearDown(self)
         CompatTestCase.tearDown(self)
 
+    def _handlers(self):
+        return {'git-receive-pack': NoSideBand64kReceivePackHandler}
+
+    def _check_app(self, app):
+        receive_pack_handler_cls = app.handlers['git-receive-pack']
+        caps = receive_pack_handler_cls.capabilities()
+        self.assertFalse('side-band-64k' in caps)
+
     def _make_app(self, backend):
-        return HTTPGitApplication(backend)
+        app = HTTPGitApplication(backend, handlers=self._handlers())
+        self._check_app(app)
+        return app
+
+
+class SmartWebSideBand64kTestCase(SmartWebTestCase):
+    """Test cases for smart HTTP server with side-band-64k support."""
+
+    # side-band-64k in git-receive-pack was introduced in git 1.7.0.2
+    min_git_version = (1, 7, 0, 2)
+
+    def _handlers(self):
+        return None  # default handlers include side-band-64k
+
+    def _check_app(self, app):
+        receive_pack_handler_cls = app.handlers['git-receive-pack']
+        caps = receive_pack_handler_cls.capabilities()
+        self.assertTrue('side-band-64k' in caps)
 
 
 class DumbWebTestCase(WebTests, CompatTestCase):

dulwich/tests/compat/utils.py

@@ -35,6 +35,7 @@ from dulwich.tests import (
     )
 
 _DEFAULT_GIT = 'git'
+_VERSION_LEN = 4
 
 
 def git_version(git_path=_DEFAULT_GIT):
@@ -42,8 +43,8 @@ def git_version(git_path=_DEFAULT_GIT):
 
     :param git_path: Path to the git executable; defaults to the version in
         the system path.
-    :return: A tuple of ints of the form (major, minor, point), or None if no
-        git installation was found.
+    :return: A tuple of ints of the form (major, minor, point, sub-point), or
+        None if no git installation was found.
     """
     try:
         _, output = run_git(['--version'], git_path=git_path,
@@ -53,21 +54,40 @@ def git_version(git_path=_DEFAULT_GIT):
     version_prefix = 'git version '
     if not output.startswith(version_prefix):
         return None
-    output = output[len(version_prefix):]
-    nums = output.split('.')
-    if len(nums) == 2:
-        nums.add('0')
-    else:
-        nums = nums[:3]
-    try:
-        return tuple(int(x) for x in nums)
-    except ValueError:
-        return None
+
+    parts = output[len(version_prefix):].split('.')
+    nums = []
+    for part in parts:
+        try:
+            nums.append(int(part))
+        except ValueError:
+            break
+
+    while len(nums) < _VERSION_LEN:
+        nums.append(0)
+    return tuple(nums[:_VERSION_LEN])
 
 
 def require_git_version(required_version, git_path=_DEFAULT_GIT):
-    """Require git version >= version, or skip the calling test."""
+    """Require git version >= version, or skip the calling test.
+
+    :param required_version: A tuple of ints of the form (major, minor, point,
+        sub-point); ommitted components default to 0.
+    :param git_path: Path to the git executable; defaults to the version in
+        the system path.
+    :raise ValueError: if the required version tuple has too many parts.
+    :raise TestSkipped: if no suitable git version was found at the given path.
+    """
     found_version = git_version(git_path=git_path)
+    if len(required_version) > _VERSION_LEN:
+        raise ValueError('Invalid version tuple %s, expected %i parts' %
+                         (required_version, _VERSION_LEN))
+
+    required_version = list(required_version)
+    while len(found_version) < len(required_version):
+        required_version.append(0)
+    required_version = tuple(required_version)
+
     if found_version < required_version:
         required_version = '.'.join(map(str, required_version))
         found_version = '.'.join(map(str, found_version))

dulwich/tests/test_protocol.py

@@ -30,6 +30,7 @@ from dulwich.protocol import (
     SINGLE_ACK,
     MULTI_ACK,
     MULTI_ACK_DETAILED,
+    BufferedPktLineWriter,
     )
 from dulwich.tests import TestCase
 
@@ -192,3 +193,57 @@ class CapabilitiesTestCase(TestCase):
         self.assertEquals(MULTI_ACK_DETAILED,
                           ack_type(['foo', 'bar', 'multi_ack',
                                     'multi_ack_detailed']))
+
+
+class BufferedPktLineWriterTests(TestCase):
+
+    def setUp(self):
+        self._output = StringIO()
+        self._writer = BufferedPktLineWriter(self._output.write, bufsize=16)
+
+    def assertOutputEquals(self, expected):
+        self.assertEquals(expected, self._output.getvalue())
+
+    def _truncate(self):
+        self._output.seek(0)
+        self._output.truncate()
+
+    def test_write(self):
+        self._writer.write('foo')
+        self.assertOutputEquals('')
+        self._writer.flush()
+        self.assertOutputEquals('0007foo')
+
+    def test_write_none(self):
+        self._writer.write(None)
+        self.assertOutputEquals('')
+        self._writer.flush()
+        self.assertOutputEquals('0000')
+
+    def test_flush_empty(self):
+        self._writer.flush()
+        self.assertOutputEquals('')
+
+    def test_write_multiple(self):
+        self._writer.write('foo')
+        self._writer.write('bar')
+        self.assertOutputEquals('')
+        self._writer.flush()
+        self.assertOutputEquals('0007foo0007bar')
+
+    def test_write_across_boundary(self):
+        self._writer.write('foo')
+        self._writer.write('barbaz')
+        self.assertOutputEquals('0007foo000abarba')
+        self._truncate()
+        self._writer.flush()
+        self.assertOutputEquals('z')
+
+    def test_write_to_boundary(self):
+        self._writer.write('foo')
+        self._writer.write('barba')
+        self.assertOutputEquals('0007foo0009barba')
+        self._truncate()
+        self._writer.write('z')
+        self._writer.flush()
+        self.assertOutputEquals('0005z')

dulwich/tests/test_server.py

@@ -76,13 +76,24 @@ class TestProto(object):
             return None
 
 
+class TestGenericHandler(Handler):
+
+    def __init__(self):
+        Handler.__init__(self, Backend(), None)
+
+    @classmethod
+    def capabilities(cls):
+        return ('cap1', 'cap2', 'cap3')
+
+    @classmethod
+    def required_capabilities(cls):
+        return ('cap2',)
+
+
 class HandlerTestCase(TestCase):
 
     def setUp(self):
-        super(HandlerTestCase, self).setUp()
-        self._handler = Handler(Backend(), None)
-        self._handler.capabilities = lambda: ('cap1', 'cap2', 'cap3')
-        self._handler.required_capabilities = lambda: ('cap2',)
+        self._handler = TestGenericHandler()
 
     def assertSucceeds(self, func, *args, **kwargs):
         try:
@@ -208,7 +219,8 @@ class TestUploadPackHandler(Handler):
         self.stateless_rpc = False
         self.advertise_refs = False
 
-    def capabilities(self):
+    @classmethod
+    def capabilities(cls):
         return ('multi_ack',)
 
 

dulwich/web.py

@@ -259,7 +259,7 @@ class HTTPGitRequest(object):
     def __init__(self, environ, start_response, dumb=False, handlers=None):
         self.environ = environ
         self.dumb = dumb
-        self.handlers = handlers and handlers or DEFAULT_HANDLERS
+        self.handlers = handlers
         self._start_response = start_response
         self._cache_headers = []
         self._headers = []
@@ -340,7 +340,9 @@ class HTTPGitApplication(object):
     def __init__(self, backend, dumb=False, handlers=None):
         self.backend = backend
         self.dumb = dumb
-        self.handlers = handlers
+        self.handlers = dict(DEFAULT_HANDLERS)
+        if handlers is not None:
+            self.handlers.update(handlers)
 
     def __call__(self, environ, start_response):
         path = environ['PATH_INFO']