[PATCH 3 of 3] killdaemons: read pids from file arguments if any

Mads Kiilerich mads at kiilerich.com
Sun Aug 19 17:39:14 CDT 2012


Patrick Mezard wrote, On 08/19/2012 09:22 PM:
> If this change is accepted, the plan is to:

I see that as indication that you expect some feedback ;-)

> # HG changeset patch
> # User Patrick Mezard <patrick at mezard.eu>
> # Date 1345403716 -7200
> # Node ID f474bf5159f481399e55485769d774183d7cf120
> # Parent  59dcbbaf61aee7f9b534454820ea00e1bf204c74
> killdaemons: read pids from file arguments if any

(What component should be specified for changes like this that obviously 
shouldn't be mentioned in the release note? Shouldn't it be 'tests' or 
something starting with 'test'?)

> With no arguments, fallback to the previous behaviour of reading from
> $DAEMON_PIDS.

I would rather drop the hardcoded handling of $DAEMON_PIDS and specify 
it explicitly on the command line where needed. Explicit is better than 
implicit.

> On Windows, MinGW kill implementation does not seem to use system
> process identifiers, and systematically fails when called in tests.
> killdaemons.py could be a suitable replacement.

For that reason I think we should move the killdaemons functionality 
into hg as something like 'hg serve --kill $DAEMON_PIDS'.

(That would however introduce a problem when we want to import this 
functionality from run-tests - just like we have a problem with 
importing hghave ... The best solution to both cases might be to do a 
simple cut'n'paste of the relevant lines of code.)

> test-http-branchmap.t is updated to be run by Windows test suite. The:
>
>    listening at http://*:$HGPORT1/
>
> line does not appear on Windows because the detached process can no
> longer write on its parent streams. Grepping hg serve stdout directly
> causes the parent process to never return and hang the test. This is a
> bug, but I have no simple solution and prefer to pay this small price
> and enable hg serve tests on Windows.

That should be a separate changeset.

(It seems like the reason 'listening' doesn't appear simply is
* hg serve output is written to a file and thus doesn't appear 
immediately when starting hg serve
* the line is grepped out when the content of that file is displayed.

It seems like there is a more important story to be told: First why we 
have to redirect stdout, and second why we have to grep.)

> - Check test-http-branchmap.t does not fail/hang on the buildbot

> - Convert all kill utility calls to killdaemons.py calls
> - Add a rule in check-code.py to forbid kill calls

'kill' is in some places used to kill sub processes where the pid come 
from $!. Shouldn't 'kill' still be used in these cases?

> - Possibly drop the 'server' rule from hghave.

Yes, it should have been named something like 'hg-serve-d-kill' ... and 
when we have a way to kill 'serve -d' processes on all platforms it can 
go away.

/Mads


More information about the Mercurial-devel mailing list