Re: [PATCH] start_command: reset disposition of all signals in child

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 08/09/2023 16:42, Junio C Hamano wrote:
>> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> 
>>> [3] This is really a work-around for not moving the child into its own
>>>      process group and changing the foreground process group of the
>>>      controlling terminal.
>> I am puzzled, as I somehow thought that "does the user conceive a
>> subprocess as external and different-from-git entity, or is it
>> merely an implementation detail?  many use of subprocesses in our
>> codebase, it is the latter." from Peff was a good argument against
>> such isolation between spawning "git" and spawned subprocesses.
>
> It is and in those cases we do not ignore SIGINT and SIGQUIT in the
> parent when we fork the subprocess. What I was trying to say is that
> in the few cases where we do ignore SIGINT and SIGQUIT in the parent
> when we fork a subprocess we're working round the child being in the
> same process group at the parent.

Hmph, but picking a few grep hits for 'sigchain_push.*SIG_IGN' at
random, the typical pattern seem to be (this example was taken from
builtin/receive-pack.c):

	code = start_command(&proc);
	if (code) {
		...
		return code;
	}
	sigchain_push(SIGPIPE, SIG_IGN);
	while (1) {
		...
	}
	close(proc.in);
	sigchain_pop(SIGPIPE);
	return finish_command(&proc);

The way we spawn an editor in editor.c looks the same:

		p.use_shell = 1;
		p.trace2_child_class = "editor";
		if (start_command(&p) < 0) {
			strbuf_release(&realpath);
			return error("unable to start editor '%s'", editor);
		}

		sigchain_push(SIGINT, SIG_IGN);
		sigchain_push(SIGQUIT, SIG_IGN);
		ret = finish_command(&p);

IOW, we do not ignore then spawn.  We spawn and ignore only in the
parent, so there shouldn't be any reason to worry about our child
inheriting the "we the parent git process do not want to be killed
by \C-c" settings, should there?

I have a vague recollection that the "propagate what was already
ignored to be ignored down to the child, too" was not about signals
we ignored, but inherited from the end-user who started git with
certain signals ignored, but it is so old a piece of code that the
details of the rationale escapes me.