Re: [PATCH] rebase -i: ignore signals when forking subprocesses

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 7, 2023, 21:16 UTC
Message-ID: <xmqq5y4lk7l4.fsf@gitster.g>
In-Reply-To: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 8 quoted lines

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> If the user presses Ctrl-C to interrupt a program run by a rebase "exec"
> command then SIGINT will also be sent to the git process running the
> rebase resulting in it being killed. Fortunately the consequences of
> this are not severe as all the state necessary to continue the rebase is
> saved to disc but it would be better to avoid killing git and instead
> report that the command failed.

The above made me wonder if we can guarantee that the intention of the end-user is always to kill only the external program and not "git". But with or without this change, "git" will stop making progress after such a signal (in other words, it is not like killing "exec sleep 20" will make "git rebase -i -x 'sleep 20'" to move to the next step without complaining), so "ignore signals" is not all that harmful as the phrasing makes it sound like. With the patch, we just handle signals that will kill the external programs, and their consequences, a bit more gracefully.

But that makes me wonder what happens if the external program has good reasons to ignore the signal (that is, the program does not die when signaled, without misbehaving). If "git" dies in such a case, would it help the overall end-user experience, or would it even hurt? If the latter, then "git" that ignores these interactive interrupts would be improvement in both cases (i.e. external programs that dies with signals, and those that ignores them). If the former, however, "git" that ignores the signals would be a regression.

Other than that, the change is well reasoned, I would think.

Thanks.