[PATCH V3] commands: 'listening at' line when port specified (issue3976)

[Mads Kiilerich]

On 03/24/2014 07:12 PM, Chinmay Joshi wrote:
> # HG changeset patch
> # User Chinmay Joshi <c at chinmayjoshi.com>
> # Date 1395682982 -19800
> #      Mon Mar 24 23:13:02 2014 +0530
> # Node ID d3fe362ba10816cc6e2b3934437147c0eb2d67a7
> # Parent  3d1d16b19e7dd5e96e242daed86512429bc1d3f6
> commands: 'listening at' line when port specified (issue3976)
>
> When hg serve is executed with -p <port>,  there output with
> "listening at host ...." shown.

The issue tracker has a brief discussion of the problem but do not 
discuss in detail how it should be solved.

I agree that it would be fine if it showed a status line when running in 
the foreground so the user knows that Mercurial is running. I am not 
convinced that it also should do that when running as a daemon. Whoever 
started it that way must have a clear expectation of what it will do and 
has no use of a status message and I think it should stay quiet in that 
case. (That would also make the changes to the test suite less invasive.)

Others might have other opinions ;-)

> diff -r 3d1d16b19e7d -r d3fe362ba108 mercurial/commands.py
> --- a/mercurial/commands.py Fri Feb 28 02:09:00 2014 +0100
> +++ b/mercurial/commands.py Mon Mar 24 23:13:02 2014 +0530
> @@ -5237,14 +5237,15 @@
>           self.httpd = hgweb_server.create_server(self.ui, self.app)
>
>           if self.opts['port'] and not self.ui.verbose:
> -            return
> +            port = ":%d" % self.opts['port']
> +        else:
> +            port = ':%d' % self.httpd.port
>
>           if self.httpd.prefix:
>               prefix = self.httpd.prefix.strip('/') + '/'
>           else:
>               prefix = ''
>
> -        port = ':%d' % self.httpd.port
>           if port == ':80':
>               port = ''
>
> diff -r 3d1d16b19e7d -r d3fe362ba108 tests/run-tests.py
> --- a/tests/run-tests.py Fri Feb 28 02:09:00 2014 +0100
> +++ b/tests/run-tests.py Mon Mar 24 23:13:02 2014 +0530
> @@ -57,6 +57,7 @@
>   import threading
>   import killdaemons as killmod
>   import Queue as queue
> +import socket
>
>   processlock = threading.Lock()
>
> @@ -359,6 +360,7 @@
>       env["HGPORT"] = str(port)
>       env["HGPORT1"] = str(port + 1)
>       env["HGPORT2"] = str(port + 2)
> +    env["HOSTNAME"] = socket.getfqdn()

Is this actually used? Or is it just to make $HOSTNAME more consistent?

>       env["HGRCPATH"] = os.path.join(threadtmp, '.hgrc')
>       env["DAEMON_PIDS"] = os.path.join(threadtmp, 'daemon.pids')
>       env["HGEDITOR"] = sys.executable + ' -c "import sys; sys.exit(0)"'
> @@ -961,6 +963,7 @@
>           (r':%s\b' % port, ':$HGPORT'),
>           (r':%s\b' % (port + 1), ':$HGPORT1'),
>           (r':%s\b' % (port + 2), ':$HGPORT2'),
> +        (r'%s\b' % socket.getfqdn(), '$HOSTNAME'),

I am a bit concerned that getfqdn actually calls out to "the os" and 
possibly the network twice for each test, no matter if it use web stuff 
or not. That could increase the test execution time on badly configured 
machines. It is probably not significant considering all the other 
things that happens, but I think it would be slightly more safe to have 
a global 'fqdn = socket.getfqdn()'.

A bigger concern is that this not exactly is the algorithm Mercurial 
uses to determine what to print out. The tests might thus fail on 
machines with odd (but not necessarily incorrect) configurations.

The actual hostname is not relevant for this change so I think I would 
suggest just using * globbing for the hostname in the tests (even though 
it will be a bit confusing when the lines already have $HGPORT 
substitutions and contain a *).

A follow-up patch could add a check for the globbing to check-code.py .

/Mads