From 13274526c1fc62fd0f15fe2bc188843982f38ec9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Nov 2012 17:40:50 -0500 Subject: [PATCH 1/6] run-command: drop silent_exec_failure arg from wait_or_whine We do not actually use this parameter; instead we complain from the child itself (for fork/exec) or from start_command (if we are using spawn on Windows). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/run-command.c b/run-command.c index 3b982e4d55..3aae270dd3 100644 --- a/run-command.c +++ b/run-command.c @@ -226,7 +226,7 @@ static inline void set_cloexec(int fd) fcntl(fd, F_SETFD, flags | FD_CLOEXEC); } -static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure) +static int wait_or_whine(pid_t pid, const char *argv0) { int status, code = -1; pid_t waiting; @@ -432,8 +432,7 @@ fail_pipe: * At this point we know that fork() succeeded, but execvp() * failed. Errors have been reported to our stderr. */ - wait_or_whine(cmd->pid, cmd->argv[0], - cmd->silent_exec_failure); + wait_or_whine(cmd->pid, cmd->argv[0]); failed_errno = errno; cmd->pid = -1; } @@ -538,7 +537,7 @@ fail_pipe: int finish_command(struct child_process *cmd) { - return wait_or_whine(cmd->pid, cmd->argv[0], cmd->silent_exec_failure); + return wait_or_whine(cmd->pid, cmd->argv[0]); } int run_command(struct child_process *cmd) From f42ca31d8d209a43342ea345a52fc0bd43d71cc8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Nov 2012 17:41:04 -0500 Subject: [PATCH 2/6] launch_editor: refactor to use start/finish_command The launch_editor function uses the convenient run_command_* interface. Let's use the more flexible start_command and finish_command functions, which will let us manipulate the parent state while we're waiting for the child to finish. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- editor.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/editor.c b/editor.c index d8340031d2..842f7829fc 100644 --- a/editor.c +++ b/editor.c @@ -37,8 +37,16 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; + struct child_process p; - if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env)) + memset(&p, 0, sizeof(p)); + p.argv = args; + p.env = env; + p.use_shell = 1; + if (start_command(&p) < 0) + return error("unable to start editor '%s'", editor); + + if (finish_command(&p)) return error("There was a problem with the editor '%s'.", editor); } From 913ef36093eac3ec78b5fb155cc2beb5843b1ce5 Mon Sep 17 00:00:00 2001 From: Paul Fox Date: Fri, 30 Nov 2012 17:41:26 -0500 Subject: [PATCH 3/6] launch_editor: ignore terminal signals while editor has control The user's editor likely catches SIGINT (ctrl-C). but if the user spawns a command from the editor and uses ctrl-C to kill that command, the SIGINT will likely also kill git itself (depending on the editor, this can leave the terminal in an unusable state). Let's ignore it while the editor is running, and do the same for SIGQUIT, which many editors also ignore. This matches the behavior if we were to use system(3) instead of run-command. Signed-off-by: Paul Fox Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- editor.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/editor.c b/editor.c index 842f7829fc..c892a81ae3 100644 --- a/editor.c +++ b/editor.c @@ -1,6 +1,7 @@ #include "cache.h" #include "strbuf.h" #include "run-command.h" +#include "sigchain.h" #ifndef DEFAULT_EDITOR #define DEFAULT_EDITOR "vi" @@ -38,6 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; struct child_process p; + int ret; memset(&p, 0, sizeof(p)); p.argv = args; @@ -46,7 +48,12 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (start_command(&p) < 0) return error("unable to start editor '%s'", editor); - if (finish_command(&p)) + sigchain_push(SIGINT, SIG_IGN); + sigchain_push(SIGQUIT, SIG_IGN); + ret = finish_command(&p); + sigchain_pop(SIGINT); + sigchain_pop(SIGQUIT); + if (ret) return error("There was a problem with the editor '%s'.", editor); } From a2767c5c91ebda3c083419c80a7f64248c5ec175 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Nov 2012 17:41:38 -0500 Subject: [PATCH 4/6] run-command: do not warn about child death from terminal SIGINT and SIGQUIT are not generally interesting signals to the user, since they are typically caused by them hitting "^C" or otherwise telling their terminal to send the signal. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/run-command.c b/run-command.c index 3aae270dd3..757f263cd6 100644 --- a/run-command.c +++ b/run-command.c @@ -242,7 +242,8 @@ static int wait_or_whine(pid_t pid, const char *argv0) error("waitpid is confused (%s)", argv0); } else if (WIFSIGNALED(status)) { code = WTERMSIG(status); - error("%s died of signal %d", argv0, code); + if (code != SIGINT && code != SIGQUIT) + error("%s died of signal %d", argv0, code); /* * This return value is chosen so that code & 0xff * mimics the exit code that a POSIX shell would report for From 1250857c6c2695020bce6669a4ff43d57a507d91 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Nov 2012 17:41:50 -0500 Subject: [PATCH 5/6] launch_editor: propagate signals from editor to git We block SIGINT and SIGQUIT while the editor runs so that git is not killed accidentally by a stray "^C" meant for the editor or its subprocesses. This works because most editors ignore SIGINT. However, some editor wrappers, like emacsclient, expect to die due to ^C. We detect the signal death in the editor and properly exit, but not before writing a useless error message to stderr. Instead, let's notice when the editor was killed by a terminal signal and just raise the signal on ourselves. This skips the message and looks to our parent like we received SIGINT ourselves. The end effect is that if the user's editor ignores SIGINT, we will, too. And if it does not, then we will behave as if we did not ignore it. That should make all users happy. Note that in the off chance that another part of git has ignored SIGINT while calling launch_editor, we will still properly detect and propagate the failed return code from the editor (i.e., the worst case is that we generate the useless error, not fail to notice the editor's death). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- editor.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/editor.c b/editor.c index c892a81ae3..065a7abf2f 100644 --- a/editor.c +++ b/editor.c @@ -39,7 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; struct child_process p; - int ret; + int ret, sig; memset(&p, 0, sizeof(p)); p.argv = args; @@ -51,8 +51,11 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en sigchain_push(SIGINT, SIG_IGN); sigchain_push(SIGQUIT, SIG_IGN); ret = finish_command(&p); + sig = ret + 128; sigchain_pop(SIGINT); sigchain_pop(SIGQUIT); + if (sig == SIGINT || sig == SIGQUIT) + raise(sig); if (ret) return error("There was a problem with the editor '%s'.", editor); From 0398fc349627a8a55ec4727bfb85fb7fad1f4ff0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 5 Jan 2013 09:52:29 -0500 Subject: [PATCH 6/6] fix compilation with NO_PTHREADS Commit 1327452 cleaned up an unused parameter from wait_or_whine, but forgot to update a caller that is inside "#ifdef NO_PTHREADS". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-command.c b/run-command.c index 757f263cd6..24eaad5c66 100644 --- a/run-command.c +++ b/run-command.c @@ -725,7 +725,7 @@ error: int finish_async(struct async *async) { #ifdef NO_PTHREADS - return wait_or_whine(async->pid, "child process", 0); + return wait_or_whine(async->pid, "child process"); #else void *ret = (void *)(intptr_t)(-1);