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

["Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>]

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. A similar situation occurs when the
sequencer runs "git commit" or "git merge". If the user generates SIGINT
while editing the commit message then the git processes creating the
commit will ignore it but the git process running the rebase will be
killed.

Fix this by ignoring SIGINT and SIGQUIT when forking "exec" commands,
"git commit" and "git merge". This matches what git already does when
running the user's editor and matches the behavior of the standard
library's system() function.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
    rebase -i: ignore signals when forking subprocesses
    
    Having written this I started thinking about what happens when we fork
    hooks, merge strategies and merge drivers. I now wonder if it would be
    better to change run_command() instead - are there any cases where we
    actually want git to be killed when the user interrupts a child process?

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1581%2Fphillipwood%2Fsequencer-subprocesses-ignore-sigint-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1581/phillipwood/sequencer-subprocesses-ignore-sigint-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1581

 sequencer.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index a66dcf8ab26..26d70f68454 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1059,6 +1059,7 @@ static int run_git_commit(const char *defmsg,
 			  unsigned int flags)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
+	int res;
 
 	if ((flags & CLEANUP_MSG) && (flags & VERBATIM_MSG))
 		BUG("CLEANUP_MSG and VERBATIM_MSG are mutually exclusive");
@@ -1116,10 +1117,16 @@ static int run_git_commit(const char *defmsg,
 	if (!(flags & EDIT_MSG))
 		strvec_push(&cmd.args, "--allow-empty-message");
 
+	sigchain_push(SIGINT, SIG_IGN);
+	sigchain_push(SIGQUIT, SIG_IGN);
 	if (is_rebase_i(opts) && !(flags & EDIT_MSG))
-		return run_command_silent_on_success(&cmd);
+		res = run_command_silent_on_success(&cmd);
 	else
-		return run_command(&cmd);
+		res = run_command(&cmd);
+	sigchain_pop(SIGINT);
+	sigchain_pop(SIGQUIT);
+
+	return res;
 }
 
 static int rest_is_empty(const struct strbuf *sb, int start)
@@ -3628,10 +3635,14 @@ static int do_exec(struct repository *r, const char *command_line)
 	struct child_process cmd = CHILD_PROCESS_INIT;
 	int dirty, status;
 
+	sigchain_push(SIGINT, SIG_IGN);
+	sigchain_push(SIGQUIT, SIG_IGN);
 	fprintf(stderr, _("Executing: %s\n"), command_line);
 	cmd.use_shell = 1;
 	strvec_push(&cmd.args, command_line);
 	status = run_command(&cmd);
+	sigchain_pop(SIGINT);
+	sigchain_pop(SIGQUIT);
 
 	/* force re-reading of the cache */
 	discard_index(r->index);
@@ -4111,7 +4122,11 @@ static int do_merge(struct repository *r,
 				NULL, 0);
 		rollback_lock_file(&lock);
 
+		sigchain_push(SIGINT, SIG_IGN);
+		sigchain_push(SIGQUIT, SIG_IGN);
 		ret = run_command(&cmd);
+		sigchain_pop(SIGINT);
+		sigchain_pop(SIGQUIT);
 
 		/* force re-reading of the cache */
 		if (!ret) {

base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3
-- 
gitgitgadget