Re: [PATCH 5/5] launch_editor: propagate SIGINT from editor to git

2012-11-30 Thread Jeff King
On Sun, Nov 11, 2012 at 08:48:38PM +0100, Johannes Sixt wrote:

 Am 11.11.2012 17:57, schrieb Jeff King:
  @@ -51,6 +51,8 @@ int launch_editor(const char *path, struct strbuf 
  *buffer, const char *const *en
  sigchain_push(SIGINT, SIG_IGN);
  ret = finish_command(p);
  sigchain_pop(SIGINT);
  +   if (WIFSIGNALED(ret)  WTERMSIG(ret) == SIGINT)
  +   raise(SIGINT);
 
 The return value of finish_command() is already a digested version of
 waitpid's status value. According to
 Documentation/technical/api-run-command.txt:
 
 . If the program terminated due to a signal, then the return value is
 the signal number - 128, ...
 
 the correct condition would be
 
   if (ret == SIGINT - 128)

Yeah, that is the same thing as WTERMSIG (which uses ret  0x7f) for
the range of -127..-1. I do not mind changing it to match run-command's
stated output, but I am curious whether there are systems where WTERMSIG
is not defined in the same way, and the code would break.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/5] launch_editor: propagate SIGINT from editor to git

2012-11-11 Thread Jeff King
We block SIGINT 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 SIGINT 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 p...@peff.net
---
 editor.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/editor.c b/editor.c
index 28aae85..1275527 100644
--- a/editor.c
+++ b/editor.c
@@ -51,6 +51,8 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
sigchain_push(SIGINT, SIG_IGN);
ret = finish_command(p);
sigchain_pop(SIGINT);
+   if (WIFSIGNALED(ret)  WTERMSIG(ret) == SIGINT)
+   raise(SIGINT);
if (ret)
return error(There was a problem with the editor 
'%s'.,
editor);
-- 
1.8.0.207.gdf2154c
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html