Commit graph

12 commits

Author SHA1 Message Date
Jeff King
709ca730f8 run-command: encode signal death as a positive integer
When a sub-command dies due to a signal, we encode the
signal number into the numeric exit status as "signal -
128". This is easy to identify (versus a regular positive
error code), and when cast to an unsigned integer (e.g., by
feeding it to exit), matches what a POSIX shell would return
when reporting a signal death in $? or through its own exit
code.

So we have a negative value inside the code, but once it
passes across an exit() barrier, it looks positive (and any
code we receive from a sub-shell will have the positive
form). E.g., death by SIGPIPE (signal 13) will look like
-115 to us in inside git, but will end up as 141 when we
call exit() with it. And a program killed by SIGPIPE but run
via the shell will come to us with an exit code of 141.

Unfortunately, this means that when the "use_shell" option
is set, we need to be on the lookout for _both_ forms. We
might or might not have actually invoked the shell (because
we optimize out some useless shell calls). If we didn't invoke
the shell, we will will see the sub-process's signal death
directly, and run-command converts it into a negative value.
But if we did invoke the shell, we will see the shell's
128+signal exit status. To be thorough, we would need to
check both, or cast the value to an unsigned char (after
checking that it is not -1, which is a magic error value).

Fortunately, most callsites do not care at all whether the
exit was from a code or from a signal; they merely check for
a non-zero status, and sometimes propagate the error via
exit(). But for the callers that do care, we can make life
slightly easier by just using the consistent positive form.

This actually fixes two minor bugs:

  1. In launch_editor, we check whether the editor died from
     SIGINT or SIGQUIT. But we checked only the negative
     form, meaning that we would fail to notice a signal
     death exit code which was propagated through the shell.

  2. In handle_alias, we assume that a negative return value
     from run_command means that errno tells us something
     interesting (like a fork failure, or ENOENT).
     Otherwise, we simply propagate the exit code. Negative
     signal death codes confuse us, and we print a useless
     "unable to run alias 'foo': Success" message. By
     encoding signal deaths using the positive form, the
     existing code just propagates it as it would a normal
     non-zero exit code.

The downside is that callers of run_command can no longer
differentiate between a signal received directly by the
sub-process, and one propagated. However, no caller
currently cares, and since we already optimize out some
calls to the shell under the hood, that distinction is not
something that should be relied upon by callers.

Fix the same logic in t/test-terminal.perl for consistency [jc:
raised by Jonathan in the discussion].

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johannes Sixt <j6t@kdbg.org>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-06 11:09:18 -08:00
Jeff King
1250857c6c 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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-02 02:07:08 -08:00
Paul Fox
913ef36093 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 <pgf@foxharp.boston.ma.us>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-02 02:06:04 -08:00
Jeff King
f42ca31d8d 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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-02 02:05:41 -08:00
Jeff King
bac8037081 editor: use run_command's shell feature
Now that run_command implements the same code in a more
general form, we can make use of it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-05 23:41:51 -08:00
Jonathan Nieder
8f4b576ad1 Provide a build time default-editor setting
Provide a DEFAULT_EDITOR knob to allow setting the fallback
editor to use instead of vi (when VISUAL, EDITOR, and GIT_EDITOR
are unset).  The value can be set at build time according to a
system’s policy.  For example, on Debian systems, the default
editor should be the 'editor' command.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-13 12:20:54 -08:00
Jonathan Nieder
44fcb4977c Teach git var about GIT_EDITOR
Expose the command used by launch_editor() for scripts to use.
This should allow one to avoid searching for a proper editor
separately in each command.

git_editor(void) uses the logic to decide which editor to use
that used to live in launch_editor().  The function returns NULL
if there is no suitable editor; the caller is expected to issue
an error message when appropriate.

launch_editor() uses git_editor() and gives the error message the
same way as before when EDITOR is not set.

"git var GIT_EDITOR" gives the editor name, or an error message
when there is no appropriate one.

"git var -l" gives GIT_EDITOR=name only if there is an
appropriate editor.

Originally-submitted-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-13 12:17:00 -08:00
Jonathan Nieder
d33738d7d3 Do not use VISUAL editor on dumb terminals
Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset
or set to "dumb".  Traditionally, VISUAL is set to a screen
editor and EDITOR to a line-based editor, which should be more
useful in that situation.

vim, for example, is happy to assume a terminal supports ANSI
sequences even if TERM is dumb (e.g., when running from a text
editor like Acme).  git already refuses to fall back to vi on a
dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are
unset, but without this patch, that check is suppressed by
VISUAL=vi.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-13 12:15:28 -08:00
Jonathan Nieder
eab58f1e8e Handle more shell metacharacters in editor names
Pass the editor name to the shell if it contains any susv3 shell
special character (globs, redirections, variable substitutions,
escapes, etc).  This way, the meaning of some characters will not
meaninglessly change when others are added, and git commands
implemented in C and in shell scripts will interpret editor names
in the same way.

This does not make the GIT_EDITOR setting any more expressive,
since one could always use single quotes to force the editor to
be passed to the shell.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-30 19:15:38 -07:00
Brandon Casey
f285a2d7ed Replace calls to strbuf_init(&foo, 0) with STRBUF_INIT initializer
Many call sites use strbuf_init(&foo, 0) to initialize local
strbuf variable "foo" which has not been accessed since its
declaration. These can be replaced with a static initialization
using the STRBUF_INIT macro which is just as readable, saves a
function call, and takes up fewer lines.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2008-10-12 12:36:19 -07:00
Stephan Beyer
7198203ae3 editor.c: Libify launch_editor()
This patch removes exit()/die() calls and builtin-specific messages
from launch_editor(), so that it can be used as a general libgit.a
function to launch an editor.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-25 17:09:38 -07:00
Stephan Beyer
d82f33e20d Move launch_editor() from builtin-tag.c to editor.c
launch_editor() is declared in strbuf.h but defined in builtin-tag.c.
This patch moves launch_editor() into a new source file editor.c,
but keeps the declaration in strbuf.h.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-25 17:08:34 -07:00