[bundle buggy] Bundle downloads have DOS line endings?

Andrew Bennetts andrew at canonical.com
Tue Mar 27 02:30:00 BST 2007


On
<http://bundlebuggy.aaronbentley.com/request/%3C1174815629.28932.5.camel@localhost.localdomain%3E>,
the "Download patch" link to
<http://bundlebuggy.aaronbentley.com/download_patch/%3C1174815629.28932.5.camel@localhost.localdomain%3E>
gives me a bundle that doesn't work with "bzr merge":

    $ ~/code/bzr/bzr merge ~/Desktop/hpss-hooks.patch 
    bzr: ERROR: End of line marker was not \n in bzr revision-bundle

Vim reports that the fileformat is "dos".  Doing "set fileformat=unix" and
resaving seems to unbreak the bundle.

I'm attaching the broken bundle from bundlebuggy, and the corrected one that
worked for me, in case it helps diagnose the problem.

-Andrew.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: hpss-hooks.patch
Type: text/x-diff
Size: 15616 bytes
Desc: broken.bundle
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070327/35ead4ba/attachment-0001.bin 
-------------- next part --------------
# Bazaar revision bundle v0.9
#
# message:
#   New SmartServer hooks facility. There are two initial hooks documented
#   in bzrlib.transport.smart.SmartServerHooks. The two initial hooks allow
#   plugins to execute code upon server startup and shutdown.
#   (Robert Collins).
#   
#   SmartServer in standalone mode will now close its listening socket
#   when it stops, rather than waiting for garbage collection. This primarily
#   fixes test suite hangs when a test tries to connect to a shutdown server.
#   It may also help improve behaviour when dealing with a server running
#   on a specific port (rather than dynamically assigned ports).
#   (Robert Collins)
#   
# committer: Robert Collins <robertc at robertcollins.net>
# date: Sun 2007-03-25 18:59:56.763999939 +1000

=== added file bzrlib/hooks.py // file-id:hooks.py-20070325015548-ix4np2q0kd845
... 2au-1
--- /dev/null
+++ bzrlib/hooks.py
@@ -0,0 +1,46 @@
+# Copyright (C) 2007 Canonical Ltd
+#
+# 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+
+"""Support for plugin hooking logic."""
+from bzrlib.lazy_import import lazy_import
+lazy_import(globals(), """
+from bzrlib import (
+        errors,
+        )
+""")
+
+
+class Hooks(dict):
+    """A dictionary mapping hook name to a list of callables.
+    
+    e.g. ['FOO'] Is the list of items to be called when the
+    FOO hook is triggered.
+    """
+
+    def install_hook(self, hook_name, a_callable):
+        """Install a_callable in to the hook hook_name.
+
+        :param hook_name: A hook name. See the __init__ method of BranchHooks
+            for the complete list of hooks.
+        :param a_callable: The callable to be invoked when the hook triggers.
+            The exact signature will depend on the hook - see the __init__ 
+            method of BranchHooks for details on each hook.
+        """
+        try:
+            self[hook_name].append(a_callable)
+        except KeyError:
+            raise errors.UnknownHook(self.__class__.__name__, hook_name)

=== modified file NEWS
--- NEWS
+++ NEWS
@@ -1,5 +1,20 @@
 IN DEVELOPMENT
 
+  INTERNALS:
+
+    * New SmartServer hooks facility. There are two initial hooks documented
+      in bzrlib.transport.smart.SmartServerHooks. The two initial hooks allow
+      plugins to execute code upon server startup and shutdown.
+      (Robert Collins).
+
+   * SmartServer in standalone mode will now close its listening socket
+     when it stops, rather than waiting for garbage collection. This primarily
+     fixes test suite hangs when a test tries to connect to a shutdown server.
+     It may also help improve behaviour when dealing with a server running
+     on a specific port (rather than dynamically assigned ports).
+     (Robert Collins)
+  
+
 bzr 0.15 (not finalised)
 
   INTERNALS:

=== modified file bzrlib/branch.py
--- bzrlib/branch.py
+++ bzrlib/branch.py
@@ -54,6 +54,7 @@
                            NotBranchError, UninitializableFormat,
                            UnlistableStore, UnlistableBranch,
                            )
+from bzrlib.hooks import Hooks
 from bzrlib.symbol_versioning import (deprecated_function,
                                       deprecated_method,
                                       DEPRECATED_PARAMETER,
@@ -884,7 +885,7 @@
             control_files.unlock()
 
 
-class BranchHooks(dict):
+class BranchHooks(Hooks):
     """A dictionary mapping hook name to a list of callables for branch hooks.
     
     e.g. ['set_rh'] Is the list of items to be called when the
@@ -897,7 +898,7 @@
         These are all empty initially, because by default nothing should get
         notified.
         """
-        dict.__init__(self)
+        Hooks.__init__(self)
         # Introduced in 0.15:
         # invoked whenever the revision history has been set
         # with set_revision_history. The api signature is
@@ -936,20 +937,6 @@
         # and an empty branch recieves new_revno of 0, new_revid of None.
         self['post_uncommit'] = []
 
-    def install_hook(self, hook_name, a_callable):
-        """Install a_callable in to the hook hook_name.
-
-        :param hook_name: A hook name. See the __init__ method of BranchHooks
-            for the complete list of hooks.
-        :param a_callable: The callable to be invoked when the hook triggers.
-            The exact signature will depend on the hook - see the __init__ 
-            method of BranchHooks for details on each hook.
-        """
-        try:
-            self[hook_name].append(a_callable)
-        except KeyError:
-            raise errors.UnknownHook('branch', hook_name)
-
 
 # install the default hooks into the Branch class.
 Branch.hooks = BranchHooks()

=== modified file bzrlib/tests/__init__.py
--- bzrlib/tests/__init__.py
+++ bzrlib/tests/__init__.py
@@ -680,7 +680,10 @@
         self._benchcalls = []
         self._benchtime = None
         # prevent hooks affecting tests
-        self._preserved_hooks = bzrlib.branch.Branch.hooks
+        self._preserved_hooks = {
+            bzrlib.branch.Branch:bzrlib.branch.Branch.hooks,
+            bzrlib.transport.smart.SmartTCPServer:bzrlib.transport.smart.SmartTCPServer.hooks,
+            }
         self.addCleanup(self._restoreHooks)
         # this list of hooks must be kept in sync with the defaults
         # in branch.py
@@ -968,7 +971,8 @@
             osutils.set_or_unset_env(name, value)
 
     def _restoreHooks(self):
-        bzrlib.branch.Branch.hooks = self._preserved_hooks
+        for klass, hooks in self._preserved_hooks.items():
+            setattr(klass, 'hooks', hooks)
 
     def tearDown(self):
         self._runCleanups()

=== modified file bzrlib/tests/test_selftest.py
--- bzrlib/tests/test_selftest.py
+++ bzrlib/tests/test_selftest.py
@@ -903,6 +903,8 @@
         """The bzrlib hooks should be sanitised by setUp."""
         self.assertEqual(bzrlib.branch.BranchHooks(),
             bzrlib.branch.Branch.hooks)
+        self.assertEqual(bzrlib.transport.smart.SmartServerHooks(),
+            bzrlib.transport.smart.SmartTCPServer.hooks)
 
     def test__gather_lsprof_in_benchmarks(self):
         """When _gather_lsprof_in_benchmarks is on, accumulate profile data.

=== modified file bzrlib/tests/test_smart_transport.py
--- bzrlib/tests/test_smart_transport.py
+++ bzrlib/tests/test_smart_transport.py
@@ -808,14 +808,46 @@
         self.server = smart.SmartTCPServer(self.backing_transport)
         self.server.start_background_thread()
         self.transport = smart.SmartTCPTransport(self.server.get_url())
+        self.addCleanup(self.tearDownServer)
 
-    def tearDown(self):
+    def tearDownServer(self):
         if getattr(self, 'transport', None):
             self.transport.disconnect()
+            del self.transport
         if getattr(self, 'server', None):
             self.server.stop_background_thread()
-        super(SmartTCPTests, self).tearDown()
-        
+            del self.server
+
+
+class TestServerSocketUsage(SmartTCPTests):
+
+    def test_server_closes_listening_sock_on_shutdown(self):
+        """The server should close its listening socket when its stopped."""
+        self.setUpServer()
+        # clean up the server and initial transport (which wont have connected)
+        server = self.server
+        # force a connection, which uses the listening socket to synchronise
+        # with the server thread, so that when we shut it down it has already
+        # executed the 'self._should_terminate = False' line in the server
+        # method.
+        self.transport.has('.')
+        self.tearDownServer()
+        # make a new connection to break out the inner loop in the server.
+        transport = smart.SmartTCPTransport(server.get_url())
+        # force the connection
+        transport.has('.')
+        # and close it.
+        transport.disconnect()
+        del transport
+        while server._server_thread.isAlive():
+            # this is fugly: we should have an event for the server we can
+            # wait for.
+            import time; time.sleep(0.001)
+        # if the listening socket has closed, we should get a BADFD error
+        # when connecting, rather than a hang.
+        transport = smart.SmartTCPTransport(server.get_url())
+        self.assertRaises(errors.ConnectionError, transport.has, '.')
+
 
 class WritableEndToEndTests(SmartTCPTests):
     """Client to server tests that require a writable transport."""
@@ -901,7 +933,56 @@
         self.setUpServer(readonly=True)
         self.assertRaises(errors.TransportNotPossible, self.transport.mkdir,
             'foo')
-        
+
+
+class TestServerHooks(SmartTCPTests):
+
+    def capture_server_call(self, backing_url, public_url):
+        """Record a server_started|stopped hook firing."""
+        self.hook_calls.append((backing_url, public_url))
+
+    def test_server_started_hook(self):
+        """The server_started hook fires when the server is started."""
+        self.hook_calls = []
+        smart.SmartTCPServer.hooks.install_hook('server_started',
+            self.capture_server_call)
+        self.setUpServer()
+        # at this point, the server will be starting a thread up.
+        # there is no indicator at the moment, so bodge it by doing a request.
+        self.transport.has('.')
+        self.assertEqual([(self.backing_transport.base, self.transport.base)],
+            self.hook_calls)
+
+    def test_server_stopped_hook_simple(self):
+        """The server_stopped hook fires when the server is stopped."""
+        self.hook_calls = []
+        smart.SmartTCPServer.hooks.install_hook('server_stopped',
+            self.capture_server_call)
+        self.setUpServer()
+        result = [(self.backing_transport.base, self.transport.base)]
+        # check the stopping message isn't emitted up front, this also
+        # has the effect of synchronising with the server, so that
+        # when we shut it down it has already executed the 
+        # 'self._should_terminate = False' line in the server method.
+        self.transport.has('.')
+        self.assertEqual([], self.hook_calls)
+        # clean up the server
+        server = self.server
+        self.tearDownServer()
+        # make a new connection to break out the inner loop in the server.
+        transport = smart.SmartTCPTransport(result[0][1])
+        transport.has('.')
+        transport.disconnect()
+        del transport
+        while server._server_thread.isAlive():
+            # this is fugly: we should have an event for the server we can
+            # wait for.
+            import time; time.sleep(0.001)
+        self.assertEqual(result, self.hook_calls)
+
+# TODO: test that when the server suffers an exception that it calls the
+# server-stopped hook.
+
 
 class SmartServerRequestHandlerTests(tests.TestCaseWithTransport):
     """Test that call directly into the handler logic, bypassing the network."""

=== modified file bzrlib/transport/smart.py
--- bzrlib/transport/smart.py
+++ bzrlib/transport/smart.py
@@ -212,6 +212,7 @@
     urlutils,
     )
 from bzrlib.bundle.serializer import write_bundle
+from bzrlib.hooks import Hooks
 try:
     from bzrlib.transport import ssh
 except errors.ParamikoNotPresent:
@@ -820,7 +821,10 @@
 
 
 class SmartTCPServer(object):
-    """Listens on a TCP socket and accepts connections from smart clients"""
+    """Listens on a TCP socket and accepts connections from smart clients
+
+    hooks: An instance of SmartServerHooks.
+    """
 
     def __init__(self, backing_transport, host='127.0.0.1', port=0):
         """Construct a new server.
@@ -833,7 +837,8 @@
         """
         self._server_socket = socket.socket()
         self._server_socket.bind((host, port))
-        self.port = self._server_socket.getsockname()[1]
+        self._sockname = self._server_socket.getsockname()
+        self.port = self._sockname[1]
         self._server_socket.listen(1)
         self._server_socket.settimeout(1)
         self.backing_transport = backing_transport
@@ -845,19 +850,30 @@
         from socket import timeout as socket_timeout
         from socket import error as socket_error
         self._should_terminate = False
-        while not self._should_terminate:
+        for hook in SmartTCPServer.hooks['server_started']:
+            hook(self.backing_transport.base, self.get_url())
+        try:
+            while not self._should_terminate:
+                try:
+                    self.accept_and_serve()
+                except socket_timeout:
+                    # just check if we're asked to stop
+                    pass
+                except socket_error, e:
+                    trace.warning("client disconnected: %s", e)
+                    pass
+        finally:
             try:
-                self.accept_and_serve()
-            except socket_timeout:
-                # just check if we're asked to stop
-                pass
-            except socket_error, e:
-                trace.warning("client disconnected: %s", e)
-                pass
+                self._server_socket.close()
+            except socket_error:
+                # ignore errors on close
+                pass
+            for hook in SmartTCPServer.hooks['server_stopped']:
+                hook(self.backing_transport.base, self.get_url())
 
     def get_url(self):
         """Return the url of the server"""
-        return "bzr://%s:%d/" % self._server_socket.getsockname()
+        return "bzr://%s:%d/" % self._sockname
 
     def accept_and_serve(self):
         conn, client_addr = self._server_socket.accept()
@@ -887,6 +903,28 @@
         ## self._server_thread.join()
 
 
+class SmartServerHooks(Hooks):
+    """Hooks for the smart server."""
+
+    def __init__(self):
+        """Create the default hooks.
+
+        These are all empty initially, because by default nothing should get
+        notified.
+        """
+        Hooks.__init__(self)
+        # Introduced in 0.16:
+        # invoked whenever the server starts serving a directory.
+        # The api signature is (backing url, public url).
+        self['server_started'] = []
+        # Introduced in 0.16:
+        # invoked whenever the server stops serving a directory.
+        # The api signature is (backing url, public url).
+        self['server_stopped'] = []
+
+SmartTCPServer.hooks = SmartServerHooks()
+
+
 class SmartTCPServer_for_testing(SmartTCPServer):
     """Server suitable for use by transport tests.
     

=== modified directory  // last-changed:robertc at robertcollins.net-2007032508595
... 6-my8jv7cifqzyltyz
# revision id: robertc at robertcollins.net-20070325085956-my8jv7cifqzyltyz
# sha1: 1b42b958d741d1e3a9ffa40dbf0a87d7d7fcca49
# inventory sha1: a718fe25860c2e6a2822ebf7a40a9874d76b1129
# parent ids:
#   pqm at pqm.ubuntu.com-20070321071219-55447700ec71371f
# base id: pqm at pqm.ubuntu.com-20070321071219-55447700ec71371f
# properties:
#   branch-nick: hpss-hooks



More information about the bazaar mailing list