[MERGE][0.91][Bug 133965] PathNotChild, port mismatch with "bzr info" for bzr:// smartserver

[Vincent Ladeuil]

>>>>> "Andrew" == Andrew Bennetts <andrew at canonical.com> writes:

<snip/>

    Andrew> It turns out that the Registry allows associating an
    Andrew> arbitrary “info” object as well as a value when you
    Andrew> register something.  This was unused by the transport
    Andrew> registry before, so my updated patch stores the
    Andrew> default_port there now, rather than creating a
    Andrew> separate dictionary to maintain this in parallel with
    Andrew> the transport registry.

    Andrew> The transport registry already has separate concepts
    Andrew> for “register a protocol” vs. “register an
    Andrew> implementation of a protocol”, so this fits quite
    Andrew> neatly.  The code size shrinks a bit as a result.
    Andrew> I've also moved _get_default_port to a public method
    Andrew> on TransportListRegistry as a result.

    Andrew> I've also updated this patch to set default port
    Andrew> numbers for all protocols where it makes sense, not
    Andrew> just bzr://.

That looks perfect except for the comments below.

<snip/>

    Andrew> === modified file 'bzrlib/transport/__init__.py'
    Andrew> --- bzrlib/transport/__init__.py	2007-09-10 09:27:54 +0000
    Andrew> +++ bzrlib/transport/__init__.py	2007-09-12 02:05:23 +0000
    Andrew> @@ -128,19 +128,26 @@
    Andrew>          self.get(key).insert(0, 
    Andrew>                  registry._LazyObjectGetter(module_name, member_name))
 
    Andrew> -    def register_transport(self, key, help=None, info=None):
    Andrew> -        self.register(key, [], help, info)
    Andrew> +    def register_transport(self, key, help=None, default_port=None):
    Andrew> +        self.register(key, [], help, default_port)
 
    Andrew>      def set_default_transport(self, key=None):
    Andrew>          """Return either 'key' or the default key if key is None"""
    Andrew>          self._default_key = key
 
    Andrew> +    def get_default_port(self, scheme):
    Andrew> +        """Return the registered default port for this protocol scheme."""
    Andrew> +        try:
    Andrew> +            return self.get_info(scheme + '://')
    Andrew> +        except LookupError:
    Andrew> +            return None
    Andrew> +
 
    Andrew>  transport_list_registry = TransportListRegistry( )
 
 
    Andrew> -def register_transport_proto(prefix, help=None, info=None):
    Andrew> -    transport_list_registry.register_transport(prefix, help, info)
    Andrew> +def register_transport_proto(prefix, help=None, info=None, default_port=None):
    Andrew> +    transport_list_registry.register_transport(prefix, help, default_port)
 
 
Using transport_list_registry is not clean:
- what if ConnectedTransport need to be used without it ?
- at least use _get_protocol_handlers which is the API to access it.

    Andrew>  def register_lazy_transport(prefix, module, classname):
    Andrew> @@ -1255,6 +1262,10 @@
    Andrew>          host = urllib.unquote(host)
    Andrew>          path = urllib.unquote(path)
 
    Andrew> +        if port is None:
    Andrew> +            # The port isn't explicitly specified, so return the default (if
    Andrew> +            # there is one).
    Andrew> +            port = transport_list_registry.get_default_port(scheme)
    Andrew>          return (scheme, user, password, host, port, path)
 
Same here.

    Andrew>      @staticmethod
    Andrew> @@ -1285,7 +1296,10 @@
    Andrew>              # have one so that it doesn't get accidentally
    Andrew>              # exposed.
    Andrew>              netloc = '%s@%s' % (urllib.quote(user), netloc)
    Andrew> -        if port is not None:
    Andrew> +        if (port is not None and 
    Andrew> +            port != transport_list_registry.get_default_port(scheme)):

And here.

I realized you didn't write some of these uses, but there are some
inconsistencies here.

Oh and thanks for writing that patch :)

bb:tweak

        Vincent