[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    [PATCH 0/5] ignore SIG{INT,QUIT} when launching editor
From:       Jeff King <peff () peff ! net>
Date:       2012-11-30 22:39:43
Message-ID: 20121130223943.GA27120 () sigill ! intra ! peff ! net
[Download RAW message or body]

This is a re-roll of the pf/editor-ignore-sigint series.

There are two changes from the original:

  1. We ignore both SIGINT and SIGQUIT for "least surprise" compared to
     system(3).

  2. We now use "code + 128" to look for signal death (instead of
     WTERMSIG), as per run-command's documentation on how it munges the
     code.

People mentioned some buggy editors which go into an infinite EIO loop
when their parent dies due to SIGQUIT. That should be a non-issue now,
as we will be ignoring SIGQUIT. And even if you could replicate it
(e.g., with another signal) those programs should be (and reportedly
have been) fixed. It is not git's job to babysit its child processes.

The patches are:

  [1/5]: run-command: drop silent_exec_failure arg from wait_or_whine
  [2/5]: launch_editor: refactor to use start/finish_command
  [3/5]: launch_editor: ignore terminal signals while editor has control
  [4/5]: run-command: do not warn about child death from terminal
  [5/5]: launch_editor: propagate signals from editor to git

Since this can be thought of as "act more like system(3)", I wondered
whether the signal-ignore logic should be moved into run-command, or
even used by default for blocking calls to run_command (which are
basically our version of system(3)). But it is detrimental in the common
case that the child is not taking control of the terminal, and is just
an implementation detail (e.g., we call "git update-ref" behind the
scenes, but the user does not know or care). If they hit ^C during such
a run and we are ignoring SIGINT, then either:

  1. we will notice the child died by signal and report an
     error in the subprocess rather than just dying; the end result is
     similar, but the error is unnecessarily confusing

  2. we do not bother to check the child's return code (because we do
     not care whether the child succeeded or not, like a "gc --auto");
     we end up totally ignoring the user's request to abort the
     operation

So I do not think we care about this behavior except for launching the
editor. And the signal-propagation behavior of 5/5 is really so weirdly
editor-specific (because it is about behaving well whether the child
blocks signals or not).

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