[MERGE] Default port for bzr:// (bug 86897)

[Martin Pool]

Andrew Bennetts wrote:
> The attached bundle adds a default port for bzr:// (see
> https://launchpad.net/bugs/86897).

Thanks, it'd be good to get this in for 0.15.

It should have a news entry.

> I'm not sure how to test it though.  Ideas welcome.

We could monkeypatch socket to observe how it is called, but the
complexity of testing it doesn't seem to me to be justified compared to
the simplicity of the patch.  So I think it would be ok to just put it
in and check it manually.



> 
> -Andrew.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> # Bazaar revision bundle v0.9
> #
> # message:
> #   Give bzr:// a default port of 4155.
> # committer: Andrew Bennetts <andrew.bennetts at canonical.com>
> # date: Mon 2007-02-26 20:20:26.141000032 +1100
> 
> === modified file bzrlib/builtins.py
> --- bzrlib/builtins.py
> +++ bzrlib/builtins.py
> @@ -3128,7 +3128,8 @@
>          Option('port',
>                 help='listen for connections on nominated port of the form '
>                      '[hostname:]portnumber. Passing 0 as the port number will '
> -                    'result in a dynamically allocated port.',
> +                    'result in a dynamically allocated port. Default port is '
> +                    '4155.',
>                 type=str),
>          Option('directory',
>                 help='serve contents of directory',

It looks like if you don't give a hostname it defaults to 127.0.0.1.
This is good for security but might be confusing.  (Separate issue I
guess but should be fixed.)

> @@ -3151,16 +3152,20 @@
>          t = get_transport(url)
>          if inet:
>              server = smart.SmartServerPipeStreamMedium(sys.stdin, sys.stdout, t)
> -        elif port is not None:
> -            if ':' in port:
> -                host, port = port.split(':')
> -            else:
> +        else:
> +            if port is None:
> +                # port 4155 is the default port for bzr, registered with IANA.
> +                port = 4155
>                  host = '127.0.0.1'
> -            server = smart.SmartTCPServer(t, host=host, port=int(port))
> +            else:
> +                if ':' in port:
> +                    host, port = port.split(':')
> +                else:
> +                    host = '127.0.0.1'
> +                port = int(port)
> +            server = smart.SmartTCPServer(t, host=host, port=port)
>              print 'listening on port: ', server.port
>              sys.stdout.flush()

Also it'd be good to show the ip address if any.

> -        else:
> -            raise errors.BzrCommandError("bzr serve requires one of --inet or --port")
>          server.serve()
>  
>  
> 
> === modified file bzrlib/tests/blackbox/test_serve.py
> --- bzrlib/tests/blackbox/test_serve.py
> +++ bzrlib/tests/blackbox/test_serve.py
> @@ -126,11 +126,6 @@
>  
>          self.assertServerFinishesCleanly(process)
>  
> -    def test_bzr_serve_no_args(self):
> -        """'bzr serve' with no arguments or options should not traceback."""
> -        out, err = self.run_bzr_error(
> -            ['bzr serve requires one of --inet or --port'], 'serve')
> -
>      def test_bzr_connect_to_bzr_ssh(self):
>          """User acceptance that get_transport of a bzr+ssh:// behaves correctly.
>  
> 
> === modified file bzrlib/transport/smart.py
> --- bzrlib/transport/smart.py
> +++ bzrlib/transport/smart.py
> @@ -1731,10 +1731,14 @@
>      def __init__(self, url):
>          _scheme, _username, _password, _host, _port, _path = \
>              transport.split_url(url)
> -        try:
> -            _port = int(_port)
> -        except (ValueError, TypeError), e:
> -            raise errors.InvalidURL(path=url, extra="invalid port %s" % _port)
> +        if _port is None:
> +            _port = 4155
> +        else:
> +            try:
> +                _port = int(_port)
> +            except (ValueError, TypeError), e:
> +                raise errors.InvalidURL(
> +                    path=url, extra="invalid port %s" % _port)
>          medium = SmartTCPClientMedium(_host, _port)
>          super(SmartTCPTransport, self).__init__(url, medium=medium)

So +1 but you should fix those two.

-- 
Martin