Re: [PATCH 3/3] execv_dashed_external: wait for child on signal death

[Jeff King <peff@peff.net>]

On Sat, Jan 07, 2017 at 08:28:22AM +0100, Johannes Sixt wrote:

> > But when you have a dashed external (or an alias pointing to
> > a builtin, which will re-exec git for the builtin), there's
> > an extra process in the mix. For instance, running:
> > 
> >   git -c alias.l=log log
> 
> This should be
> 
>   git -c alias.l=log l

Yeah, it should be.

> For the complete series:
> 
> Acked-by: Johannes Sixt <j6t@kdbg.org>

Thanks.

> What should we add to Documentation/technical/api-run-command.txt about the
> new flag? "Do not use?" "Understand the commit message of <this commit>
> before setting the flag to true?"

I think it can be explained pretty easily as "after killing all children
marked for clean_on_exit, do a blocking waitpid() on any child that is
also marked wait_after_clean". But I notice we do not actually document
clean_on_exit, either, nor most of the options in "struct
child_process".

IMHO it would be an improvement to merge the contents of the
technical/api-run-command.txt documentation into the header file, and
document each option with a comment above its definition. That makes it
a lot easier to have the will to document any newly-added options,
because if you don't they look bad next to the others. :)

Mechanics aside, I am not sure if we need a "do not use" or "here are
the caveats". I think if we explain what it does, we can rely on list
discussion and review to catch any obviously-stupid uses (both of it and
of clean_on_exit).

-Peff