Rev 2635: (robertc) Reinstate the accidentally backed out external_url patch. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Fri Jul 20 04:55:47 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2635
revision-id: pqm at pqm.ubuntu.com-20070720035545-uh4yjypen2ux6z8s
parent: pqm at pqm.ubuntu.com-20070720015347-eaeqmggngaemmbde
parent: robertc at robertcollins.net-20070720032020-xiftpb5gqeebo861
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2007-07-20 04:55:45 +0100
message:
  (robertc) Reinstate the accidentally backed out external_url patch.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/smart/server.py         server.py-20061110062051-chzu10y32vx8gvur-1
  bzrlib/tests/test_errors.py    test_errors.py-20060210110251-41aba2deddf936a8
  bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/transport/chroot.py     chroot.py-20061011104729-0us9mgm97z378vnt-1
  bzrlib/transport/decorator.py  decorator.py-20060402223305-e913a0f25319ab42
  bzrlib/transport/ftp.py        ftp.py-20051116161804-58dc9506548c2a53
  bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
  bzrlib/transport/local.py      local_transport.py-20050711165921-9b1f142bfe480c24
  bzrlib/transport/memory.py     memory.py-20051016101338-cd008dbdf69f04fc
  bzrlib/transport/remote.py     ssh.py-20060608202016-c25gvf1ob7ypbus6-1
  bzrlib/transport/sftp.py       sftp.py-20051019050329-ab48ce71b7e32dfe
    ------------------------------------------------------------
    revno: 2634.1.1
    merged: robertc at robertcollins.net-20070720032020-xiftpb5gqeebo861
    parent: pqm at pqm.ubuntu.com-20070720015347-eaeqmggngaemmbde
    parent: pqm at pqm.ubuntu.com-20070705224207-7pslqt12ofh4vnzx
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: integration
    timestamp: Fri 2007-07-20 13:20:20 +1000
    message:
      (robertc) Reinstate the accidentally backed out external_url patch.
=== modified file 'NEWS'
--- a/NEWS	2007-07-20 00:58:41 +0000
+++ b/NEWS	2007-07-20 03:20:20 +0000
@@ -247,6 +247,13 @@
 
     * ``LockDir.wait`` removed.  (Martin Pool)
 
+    * The ``SmartServer`` hooks API has changed for the ``server_started`` and
+      ``server_stopped`` hooks. The first parameter is now an iterable of
+      backing URLs rather than a single URL. This is to reflect that many
+      URLs may map to the external URL of the server. E.g. the server interally
+      may have a chrooted URL but also the local file:// URL will be at the 
+      same location. (Robert Collins)
+
   INTERNALS:
 
     * New SMTPConnection class to unify email handling.  (Adeodato Simó)

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-07-17 13:27:14 +0000
+++ b/bzrlib/errors.py	2007-07-20 03:20:20 +0000
@@ -198,6 +198,15 @@
         self.current = current
 
 
+class InProcessTransport(BzrError):
+
+    _fmt = "The transport '%(transport)s' is only accessible within this " \
+        "process."
+
+    def __init__(self, transport):
+        self.transport = transport
+
+
 class InvalidEntryName(BzrError):
     
     _fmt = "Invalid entry name: %(name)s"

=== modified file 'bzrlib/smart/server.py'
--- a/bzrlib/smart/server.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/smart/server.py	2007-07-20 03:20:20 +0000
@@ -22,6 +22,7 @@
 
 from bzrlib.hooks import Hooks
 from bzrlib import (
+    errors,
     trace,
     transport,
 )
@@ -65,8 +66,30 @@
 
     def serve(self):
         self._should_terminate = False
+        # for hooks we are letting code know that a server has started (and
+        # later stopped).
+        # There are three interesting urls:
+        # The URL the server can be contacted on. (e.g. bzr://host/)
+        # The URL that a commit done on the same machine as the server will
+        # have within the servers space. (e.g. file:///home/user/source)
+        # The URL that will be given to other hooks in the same process -
+        # the URL of the backing transport itself. (e.g. chroot+:///)
+        # We need all three because:
+        #  * other machines see the first
+        #  * local commits on this machine should be able to be mapped to
+        #    this server 
+        #  * commits the server does itself need to be mapped across to this
+        #    server.
+        # The latter two urls are different aliases to the servers url,
+        # so we group those in a list - as there might be more aliases 
+        # in the future.
+        backing_urls = [self.backing_transport.base]
+        try:
+            backing_urls.append(self.backing_transport.external_url())
+        except errors.InProcessTransport:
+            pass
         for hook in SmartTCPServer.hooks['server_started']:
-            hook(self.backing_transport.base, self.get_url())
+            hook(backing_urls, self.get_url())
         self._started.set()
         try:
             try:
@@ -100,7 +123,7 @@
                 # ignore errors on close
                 pass
             for hook in SmartTCPServer.hooks['server_stopped']:
-                hook(self.backing_transport.base, self.get_url())
+                hook(backing_urls, self.get_url())
 
     def get_url(self):
         """Return the url of the server"""
@@ -163,11 +186,11 @@
         Hooks.__init__(self)
         # Introduced in 0.16:
         # invoked whenever the server starts serving a directory.
-        # The api signature is (backing url, public url).
+        # The api signature is (backing urls, 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).
+        # The api signature is (backing urls, public url).
         self['server_stopped'] = []
 
 SmartTCPServer.hooks = SmartServerHooks()

=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py	2007-07-17 13:27:14 +0000
+++ b/bzrlib/tests/test_errors.py	2007-07-20 03:20:20 +0000
@@ -53,6 +53,12 @@
             'It supports versions "(4, 5, 6)" to "(7, 8, 9)".',
             str(error))
 
+    def test_in_process_transport(self):
+        error = errors.InProcessTransport('fpp')
+        self.assertEqualDiff(
+            "The transport 'fpp' is only accessible within this process.",
+            str(error))
+
     def test_inventory_modified(self):
         error = errors.InventoryModified("a tree to be repred")
         self.assertEqualDiff("The current inventory for the tree 'a tree to "

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2007-07-20 03:20:20 +0000
@@ -834,6 +834,8 @@
         self._captureVar('BZR_NO_SMART_VFS', None)
         class FlakyTransport(object):
             base = 'a_url'
+            def external_url(self):
+                return self.base
             def get_bytes(self, path):
                 raise Exception("some random exception from inside server")
         smart_server = server.SmartTCPServer(backing_transport=FlakyTransport())
@@ -860,12 +862,15 @@
     the server is obtained by calling self.setUpServer(readonly=False).
     """
 
-    def setUpServer(self, readonly=False):
+    def setUpServer(self, readonly=False, backing_transport=None):
         """Setup the server.
 
         :param readonly: Create a readonly server.
         """
-        self.backing_transport = memory.MemoryTransport()
+        if not backing_transport:
+            self.backing_transport = memory.MemoryTransport()
+        else:
+            self.backing_transport = backing_transport
         if readonly:
             self.real_backing_transport = self.backing_transport
             self.backing_transport = get_transport("readonly+" + self.backing_transport.abspath('.'))
@@ -998,29 +1003,66 @@
 
 class TestServerHooks(SmartTCPTests):
 
-    def capture_server_call(self, backing_url, public_url):
+    def capture_server_call(self, backing_urls, 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 = []
-        server.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 = []
-        server.SmartTCPServer.hooks.install_hook('server_stopped',
-            self.capture_server_call)
-        self.setUpServer()
-        result = [(self.backing_transport.base, self.transport.base)]
+        self.hook_calls.append((backing_urls, public_url))
+
+    def test_server_started_hook_memory(self):
+        """The server_started hook fires when the server is started."""
+        self.hook_calls = []
+        server.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('.')
+        # The default test server uses MemoryTransport and that has no external
+        # url:
+        self.assertEqual([([self.backing_transport.base], self.transport.base)],
+            self.hook_calls)
+
+    def test_server_started_hook_file(self):
+        """The server_started hook fires when the server is started."""
+        self.hook_calls = []
+        server.SmartTCPServer.hooks.install_hook('server_started',
+            self.capture_server_call)
+        self.setUpServer(backing_transport=get_transport("."))
+        # 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('.')
+        # The default test server uses MemoryTransport and that has no external
+        # url:
+        self.assertEqual([([
+            self.backing_transport.base, self.backing_transport.external_url()],
+             self.transport.base)],
+            self.hook_calls)
+
+    def test_server_stopped_hook_simple_memory(self):
+        """The server_stopped hook fires when the server is stopped."""
+        self.hook_calls = []
+        server.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.
+        self.assertEqual([], self.hook_calls)
+        # nor after a single message
+        self.transport.has('.')
+        self.assertEqual([], self.hook_calls)
+        # clean up the server
+        self.tearDownServer()
+        # now it should have fired.
+        self.assertEqual(result, self.hook_calls)
+
+    def test_server_stopped_hook_simple_file(self):
+        """The server_stopped hook fires when the server is stopped."""
+        self.hook_calls = []
+        server.SmartTCPServer.hooks.install_hook('server_stopped',
+            self.capture_server_call)
+        self.setUpServer(backing_transport=get_transport("."))
+        result = [(
+            [self.backing_transport.base, self.backing_transport.external_url()]
+            , self.transport.base)]
         # check the stopping message isn't emitted up front.
         self.assertEqual([], self.hook_calls)
         # nor after a single message

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2007-07-20 03:20:20 +0000
@@ -135,6 +135,14 @@
         t_b = t_a.clone('b')
         self.assertRaises(NoSuchFile, t_b.ensure_base)
 
+    def test_external_url(self):
+        """.external_url either works or raises InProcessTransport."""
+        t = self.get_transport()
+        try:
+            t.external_url()
+        except errors.InProcessTransport:
+            pass
+
     def test_has(self):
         t = self.get_transport()
 

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-07-19 06:39:13 +0000
+++ b/bzrlib/transport/__init__.py	2007-07-20 03:20:20 +0000
@@ -317,6 +317,28 @@
         else:
             return True
 
+    def external_url(self):
+        """Return a URL for self that can be given to an external process.
+
+        There is no guarantee that the URL can be accessed from a different
+        machine - e.g. file:/// urls are only usable on the local machine,
+        sftp:/// urls when the server is only bound to localhost are only
+        usable from localhost etc.
+
+        NOTE: This method may remove security wrappers (e.g. on chroot
+        transports) and thus should *only* be used when the result will not
+        be used to obtain a new transport within bzrlib. Ideally chroot
+        transports would know enough to cause the external url to be the exact
+        one used that caused the chrooting in the first place, but that is not
+        currently the case.
+
+        :return: A URL that can be given to another process.
+        :raises InProcessTransport: If the transport is one that cannot be
+            accessed out of the current process (e.g. a MemoryTransport)
+            then InProcessTransport is raised.
+        """
+        raise NotImplementedError(self.external_url)
+
     def should_cache(self):
         """Return True if the data pulled across should be cached locally.
         """

=== modified file 'bzrlib/transport/chroot.py'
--- a/bzrlib/transport/chroot.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/chroot.py	2007-07-20 03:20:20 +0000
@@ -98,6 +98,15 @@
     def delete_tree(self, relpath):
         return self._call('delete_tree', relpath)
 
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # Chroots, like MemoryTransport depend on in-process
+        # state and thus the base cannot simply be handed out.
+        # See the base class docstring for more details and
+        # possible directions. For now we return the chrooted
+        # url. 
+        return self.server.backing_transport.external_url()
+
     def get(self, relpath):
         return self._call('get', relpath)
 

=== modified file 'bzrlib/transport/decorator.py'
--- a/bzrlib/transport/decorator.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/decorator.py	2007-07-20 03:20:20 +0000
@@ -79,6 +79,13 @@
         """See Transport.delete_tree()."""
         return self._decorated.delete_tree(relpath)
 
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # while decorators are in-process only, they
+        # can be handed back into bzrlib safely, so
+        # its just the base.
+        return self.base
+
     @classmethod
     def _get_url_prefix(self):
         """Return the URL prefix of this decorator."""

=== modified file 'bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/ftp.py	2007-07-20 03:20:20 +0000
@@ -489,6 +489,11 @@
             self._translate_perm_error(e, abspath, 'error deleting',
                 unknown_exc=errors.NoSuchFile)
 
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # FTP URL's are externally usable.
+        return self.base
+
     def listable(self):
         """See Transport.listable."""
         return True

=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/http/__init__.py	2007-07-20 03:20:20 +0000
@@ -439,6 +439,11 @@
         """Delete the item at relpath"""
         raise errors.TransportNotPossible('http does not support delete()')
 
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # HTTP URL's are externally usable.
+        return self.base
+
     def is_readonly(self):
         """See Transport.is_readonly."""
         return True

=== modified file 'bzrlib/transport/local.py'
--- a/bzrlib/transport/local.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/local.py	2007-07-20 03:20:20 +0000
@@ -393,6 +393,11 @@
         except (IOError, OSError),e:
             self._translate_error(e, path)
 
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # File URL's are externally usable.
+        return self.base
+
     def copy_to(self, relpaths, other, mode=None, pb=None):
         """Copy a set of entries from self into another Transport.
 

=== modified file 'bzrlib/transport/memory.py'
--- a/bzrlib/transport/memory.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/memory.py	2007-07-20 03:20:20 +0000
@@ -27,7 +27,13 @@
 from cStringIO import StringIO
 import warnings
 
-from bzrlib.errors import TransportError, NoSuchFile, FileExists, LockError
+from bzrlib.errors import (
+    FileExists,
+    LockError,
+    InProcessTransport,
+    NoSuchFile,
+    TransportError,
+    )
 from bzrlib.trace import mutter
 from bzrlib.transport import (
     LateReadError,
@@ -122,6 +128,12 @@
             raise NoSuchFile(relpath)
         del self._files[_abspath]
 
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # MemoryTransport's are only accessible in-process
+        # so we raise here
+        raise InProcessTransport(self)
+
     def get(self, relpath):
         """See Transport.get()."""
         _abspath = self._abspath(relpath)

=== modified file 'bzrlib/transport/remote.py'
--- a/bzrlib/transport/remote.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/remote.py	2007-07-20 03:20:20 +0000
@@ -291,6 +291,11 @@
         resp = self._call2('delete', self._remote_path(relpath))
         self._translate_error(resp)
 
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # the external path for RemoteTransports is the base
+        return self.base
+
     def readv(self, relpath, offsets):
         if not offsets:
             return

=== modified file 'bzrlib/transport/sftp.py'
--- a/bzrlib/transport/sftp.py	2007-07-13 02:23:34 +0000
+++ b/bzrlib/transport/sftp.py	2007-07-20 03:20:20 +0000
@@ -764,6 +764,11 @@
         except (IOError, paramiko.SSHException), e:
             self._translate_io_exception(e, path, ': unable to delete')
             
+    def external_url(self):
+        """See bzrlib.transport.Transport.external_url."""
+        # the external path for SFTP is the base
+        return self.base
+
     def listable(self):
         """Return True if this store supports listing."""
         return True




More information about the bazaar-commits mailing list