git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Paul Fox <pgf@foxharp.boston.ma.us>,
	Krzysztof Mazur <krzysiek@podlesie.net>
Subject: [PATCH 0/5] ignore SIG{INT,QUIT} when launching editor
Date: Fri, 30 Nov 2012 17:39:43 -0500	[thread overview]
Message-ID: <20121130223943.GA27120@sigill.intra.peff.net> (raw)

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

             reply	other threads:[~2012-11-30 22:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-30 22:39 Jeff King [this message]
2012-11-30 22:40 ` [PATCH 1/5] run-command: drop silent_exec_failure arg from wait_or_whine Jeff King
2012-11-30 22:41 ` [PATCH 2/5] launch_editor: refactor to use start/finish_command Jeff King
2012-11-30 22:41 ` [PATCH 3/5] launch_editor: ignore terminal signals while editor has control Jeff King
2012-11-30 22:41 ` [PATCH 4/5] run-command: do not warn about child death from terminal Jeff King
2012-11-30 22:41 ` [PATCH 5/5] launch_editor: propagate signals from editor to git Jeff King
2012-12-01 12:34 ` [PATCH 0/5] ignore SIG{INT,QUIT} when launching editor Krzysztof Mazur
2012-12-01 15:48 ` Paul Fox
2012-12-02 10:04 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121130223943.GA27120@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=krzysiek@podlesie.net \
    --cc=pgf@foxharp.boston.ma.us \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).