On Mon, Oct 25, 2021 at 01:41:18PM -0400, Jeff King wrote:
On Sun, Oct 24, 2021 at 07:03:49PM +0200, SZEDER Gábor wrote:
quoted
On Sat, Oct 23, 2021 at 05:04:42PM -0700, Junio C Hamano wrote:
quoted
It seems under --stress it is fairly easy to break the said test,
especially the one near the end
I couldn't reproduce a failure with --stress, but after a cursory look
into those tests I doubt that either that test or any of the
preceeding SIGPIPE tests added in c24b7f6736 (pager: test for exit
code with and without SIGPIPE, 2021-02-02) actually check what they
are supposed to.
Yeah, I am puzzled that they are using test_terminal in the first place
(as opposed to just "git -p"). And you are right that a raw git-log is
unlikely to be slow enough to get SIGPIPE in most cases.
My usual test for an intentional SIGPIPE is "yes". So something like:
git -p \
-c core.pager='exit 0' \
-c alias.yes='!yes' \
yes
will reliably trigger SIGPIPE from yes, which git.c will then translate
into an exit code of 141.
Oh, that's clever. Alas it's not applicable to our tests, because
'yes' is not portable; 8648732e29 (t/test-lib.sh: provide a shell
implementation of the 'yes' utility, 2009-08-28).
If you really want to see SIGPIPE from a builtin (which arguably is the
more interesting case here, though I think it behaves the same with
respect to the pager), it's a bit trickier. One way to do it is with a
command that doesn't generate output until after it gets EOF on stdin.
So something like "git log --stdin" works, but you have to contort
yourself a bit to make it race-free:
-- >8 --
# The I/O setup here is:
#
# fifo:log-in stdout
# shell -----------> git-log ------> pager
# ^ /
# \-------------------------------/
# fifo:pager-closed
#
# The pager closes its stdin, which will give git-log SIGPIPE. But the
# tricky part is that after doing so, it signals via fifo to the shell,
# which then writes to git-log's stdin, triggering it to actually
# generate output (and get SIGPIPE).
#
# You can verify that it's race-free by inserting a "sleep 3" at the
# front of the pager command (before the exec) and seeing that the
# other processes wait (and we still get SIGPIPE).
mkfifo pager-closed
mkfifo log-in
git config core.pager 'exec 0<&-; echo ready >pager-closed; exit 0'
(git -p log --stdin <log-in; echo $? >exit-code) &
# we have to open a descriptor rather than just "echo HEAD >log-in", because
# that will give git-log an immediate EOF on its input when echo closes it, and
# we must wait until the signal from pager-closed. Likewise we cannot wait
# for that signal before the echo, because the subshell is blocking on opening
# log-in until somebody is hooked up to the write end of the pipe.
exec 9>log-in
read ok <pager-closed
echo HEAD >&9
exec 9>&-
# now we can wait for the subshell to finish and retrieve any output
# it produced
wait
cat exit-code
-- >8 --
Ugh. I think this would work reliably, but... ugh :)
I wonder whether we could do this as a new pair of 'test-tool'
helpers, one to run the pager through the usual pager-invoking
machinery and to generate a lot of output, the other to be used as the
early-exiting pager, with a pipe between the two to ensure that the
SIGPIPE does happen. Well, essentially the same that you outlined
above but in C instead of shell, which I somehow find less "ugh".