Note that with Git 2.12 (Q1 2017, 6+ years later), a Ctrl+C in a git pager session should behave better.
See commit 46df690, commit 246f0ed, commit 2b296c9 (07 Jan 2017) by Jeff King (peff
).
(Merged by Junio C Hamano -- gitster
-- in commit 5918bdc, 18 Jan 2017)
execv_dashed_external
: wait for child on signal death
Typing ^C
to pager
, which usually does not kill it, killed Git and took the pager down as a collateral damage in certain process-tree structure.
This has been fixed.
You can run any dashed external with commands like git -p stash list
, where the command finishes running, but the pager is still going.
The short version is that everything should stop normally (Git and pager)
But in detail:
When git
runs a pager
, it's important for the git process to
hang around and wait for the pager
to finish, even though it
has no more data to feed it.
This is because git
spawns the pager
as a child, and thus the git
process is the session leader on the terminal. After it dies, the pager
will finish its current read from the terminal (eating the one
character), and then get EIO
trying to read again.
Note: EIO
(error 5) tands for Error I/O, and is (source) the:
catchall for all manner of unexpected hardware errors. It could be from a physical error, but additionally, an orphaned process (a process whose parent has died) that attempts to read from standard input will get this. BSD systems return this if you try to open a pty device that is already in use.
An attempt to read from a stream that is closed will return EIO, as will a disk read or write that is outside of the physical bounds of the device.
An open of /dev/tty
when the process has no controlling tty will spit back EIO
also.
So (back to ^C
in a pager
):
When you hit ^C
, that sends SIGINT
to git
and to the pager
,
and it's a similar situation.
The pager
ignores it, but the git
process needs to hang around until the pager is done. We addressed that long ago in a3da882 (pager: do
wait_for_pager on signal death, 2009-01-22).
But when you have a dashed external (or an alias pointing to a builtin, which will re-exec git for the builtin), there's an extra process in the mix.
For instance, running:
$ git -c alias.l=log l
will end up with a process tree like:
git (parent)
\
git-log (child)
\
less (pager)
If you hit ^C
, SIGINT
goes to all of them. The pager ignores it, and the child git process will end up in wait_for_pager().
But the parent git process will die, and the usual EIO trouble happens.
With Git 2.28 (Q3 2020), when an aliased command, whose output is piped to a pager by git, gets killed by a signal, the pager got into a funny state, which has been corrected (again).
See commit c0d73a5, commit e662df7 (07 Jul 2020) by Trygve Aaberge (trygveaa
).
(Merged by Junio C Hamano -- gitster
-- in commit 05920f0, 15 Jul 2020)
Ctrl+C
:Wait for child on signal death for aliases to builtins
Signed-off-by: Trygve Aaberge
When you hit ^C all the processes in the tree receives it.
When a git
command uses a pager, git ignores this and waits until the pager quits.
However, when using an alias there is an additional process in the tree which didn't ignore the signal. That caused it to exit which in turn caused the pager to exit. This fixes that for aliases to builtins.
This was originally fixed in 46df6906 (execv_dashed_external
: wait for child on signal death, 2017-01-06), but was broken by ee4512ed ("trace2
: create new combined trace facility", 2019-02-22, Git v2.22.0-rc0 -- merge listed in batch #2) and then b9140840 ("git
: avoid calling aliased builtins via their dashed form", 2019-07-29, Git v2.23.0-rc1 -- merge).
And:
When we are running an alias to an external command, we want to wait for that process to exit even after receiving ^C which normally kills the git process. This is useful when the process is ignoring SIGINT (which e.g. pagers often do), and then we don't want it to be killed.
Having an alias which invokes a pager is probably not common, but it can be useful e.g. if you have an alias to a git command which uses a subshell as one of the arguments (in which case you have to use an external command, not an alias to a builtin).
This patch is similar to the previous commit, but the previous commit fixed this only for aliases to builtins, while this commit does the same for aliases to external commands. In addition to waiting after clean like the previous commit, this also enables cleaning the child (that was already enabled for aliases to builtins before the previous commit), because wait_after_clean
relies on it. Lastly, while the previous commit fixed a regression, I don't think this has ever worked properly.
Note that if you are using trace2 to debug the situation, the pager is now properly traced.
When a pager spawned by the user exited, the trace log did not record its exit status correctly, which has been corrected with Git 2.31 (Q1 2021).
See commit be8fc53, commit 85db79a, commit c24b7f6, commit 61ff12f (02 Feb 2021) by Ævar Arnfjörð Bjarmason (avar
).
(Merged by Junio C Hamano -- gitster
-- in commit dcb11fc, 22 Feb 2021)
pager
: properly log pager exit code when signalled
Signed-off-by: Ævar Arnfjörð Bjarmason
When git invokes a pager that exits with non-zero the common case is that well already return the correct SIGPIPE
failure from it itself, but the exit code logged in trace2 has always been incorrectly reported1.
Fix that and log the correct exit code in the logs.
Since this gives us something to test outside of our recently-added tests needing a !MINGW
prerequisite, let's refactor the test to run on MINGW
and actually check for SIGPIPE
outside of MINGW
.
The wait_or_whine()
is only called with a true "in_signal"
from from finish_command_in_signal()
, which in turn is only used in pager.c
.
The "in_signal && !WIFEXITED(status)
" case is not covered by tests.
Let's log the default -1 in that case for good measure.
- The incorrect logging of the exit code in was seemingly copy/pasted into
finish_command_in_signal()
in ee4512e ("trace2
: create new combined trace facility", 2019-02-22, Git v2.22.0-rc0 -- merge listed in batch #2)