git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: Vincent Lefevre <vincent@vinc17.net>
Subject: [PATCH] pager: exit without error on SIGPIPE
Date: Fri, 29 Jan 2021 15:48:54 -0800	[thread overview]
Message-ID: <bc88492979fee215d5be06ccbc246ae0171a9ced.1611910122.git.liu.denton@gmail.com> (raw)
In-Reply-To: <YAG/vzctP4JwSp5x@zira.vinc17.org>

If the pager closes before the git command feeding the pager finishes,
git is killed by a SIGPIPE and the corresponding exit code is 141.
Since the pipe is just an implementation detail, it does not make sense
for this error code to be user-facing.

Handle SIGPIPEs by simply calling exit(0) in wait_for_pager_signal().

Introduce `test-tool pager` which infinitely prints `y` to the pager in
order to test the new behavior. This cannot be tested with any existing
git command because there are no other commands which produce infinite
output. Without the change to pager.c, the newly introduced test fails.

Reported-by: Vincent Lefevre <vincent@vinc17.net>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Sorry for the resend, it seems like vger has dropped the first patch.

 Makefile              |  1 +
 pager.c               |  2 ++
 t/helper/test-pager.c | 12 ++++++++++++
 t/helper/test-tool.c  |  1 +
 t/helper/test-tool.h  |  1 +
 t/t7006-pager.sh      |  4 ++++
 6 files changed, 21 insertions(+)
 create mode 100644 t/helper/test-pager.c

diff --git a/Makefile b/Makefile
index 4edfda3e00..38a1a20f31 100644
--- a/Makefile
+++ b/Makefile
@@ -719,6 +719,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
 TEST_BUILTINS_OBJS += test-oid-array.o
 TEST_BUILTINS_OBJS += test-oidmap.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
+TEST_BUILTINS_OBJS += test-pager.o
 TEST_BUILTINS_OBJS += test-parse-options.o
 TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
 TEST_BUILTINS_OBJS += test-path-utils.o
diff --git a/pager.c b/pager.c
index ee435de675..5922d99dc8 100644
--- a/pager.c
+++ b/pager.c
@@ -34,6 +34,8 @@ static void wait_for_pager_atexit(void)
 static void wait_for_pager_signal(int signo)
 {
 	wait_for_pager(1);
+	if (signo == SIGPIPE)
+		exit(0);
 	sigchain_pop(signo);
 	raise(signo);
 }
diff --git a/t/helper/test-pager.c b/t/helper/test-pager.c
new file mode 100644
index 0000000000..feb68b8643
--- /dev/null
+++ b/t/helper/test-pager.c
@@ -0,0 +1,12 @@
+#include "test-tool.h"
+#include "cache.h"
+
+int cmd__pager(int argc, const char **argv)
+{
+	if (argc > 1)
+		usage("\ttest-tool pager");
+
+	setup_pager();
+	for (;;)
+		puts("y");
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 9d6d14d929..88269a7156 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -43,6 +43,7 @@ static struct test_cmd cmds[] = {
 	{ "oid-array", cmd__oid_array },
 	{ "oidmap", cmd__oidmap },
 	{ "online-cpus", cmd__online_cpus },
+	{ "pager", cmd__pager },
 	{ "parse-options", cmd__parse_options },
 	{ "parse-pathspec-file", cmd__parse_pathspec_file },
 	{ "path-utils", cmd__path_utils },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index a6470ff62c..78900f7938 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -32,6 +32,7 @@ int cmd__mergesort(int argc, const char **argv);
 int cmd__mktemp(int argc, const char **argv);
 int cmd__oidmap(int argc, const char **argv);
 int cmd__online_cpus(int argc, const char **argv);
+int cmd__pager(int argc, const char **argv);
 int cmd__parse_options(int argc, const char **argv);
 int cmd__parse_pathspec_file(int argc, const char** argv);
 int cmd__path_utils(int argc, const char **argv);
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index fdb450e446..2eb89e8f75 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -656,4 +656,8 @@ test_expect_success TTY 'git tag with auto-columns ' '
 	test_cmp expect actual
 '
 
+test_expect_success TTY 'SIGPIPE from pager returns success' '
+	test_terminal env PAGER=true test-tool pager
+'
+
 test_done
-- 
2.30.0.478.g8a0d178c01


  reply	other threads:[~2021-01-29 23:50 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15 16:15 git fails with a broken pipe when one quits the pager Vincent Lefevre
2021-01-29 23:48 ` Denton Liu [this message]
2021-01-30  8:29   ` [PATCH] pager: exit without error on SIGPIPE Johannes Sixt
2021-01-30 12:52     ` Johannes Sixt
2021-02-01 15:03   ` Ævar Arnfjörð Bjarmason
2021-02-01 17:47     ` Junio C Hamano
2021-02-01 19:52       ` Ævar Arnfjörð Bjarmason
2021-02-01 20:55         ` Junio C Hamano
2021-02-02  2:05           ` Ævar Arnfjörð Bjarmason
2021-02-02  4:45             ` Junio C Hamano
2021-02-02  5:25               ` Junio C Hamano
2021-02-02  7:45                 ` Johannes Sixt
2021-02-02 20:13                   ` Junio C Hamano
2021-02-02 22:15                     ` Johannes Sixt
2021-02-02 22:21                       ` Junio C Hamano
2021-02-03 17:07                         ` Johannes Sixt
2021-02-03 18:12                           ` Junio C Hamano
2021-02-04 15:10                           ` Vincent Lefevre
2021-02-03  2:45                 ` Ævar Arnfjörð Bjarmason
2021-02-03  2:54                   ` Junio C Hamano
2021-02-03  3:36                     ` Ævar Arnfjörð Bjarmason
2021-02-03 17:19                     ` Johannes Sixt
2021-01-31  1:47 ` git fails with a broken pipe when one quits the pager Ævar Arnfjörð Bjarmason
2021-01-31  3:36   ` Vincent Lefevre
2021-01-31  3:47     ` Vincent Lefevre
2021-01-31 20:49     ` Ævar Arnfjörð Bjarmason
2021-02-01 10:34       ` Vincent Lefevre
2021-02-01 11:33         ` Chris Torek
2021-02-01 12:36           ` Vincent Lefevre
2021-02-01 12:53             ` Chris Torek
2021-02-01 15:17               ` Vincent Lefevre
2021-02-01 15:00           ` Ævar Arnfjörð Bjarmason
2021-02-01 12:10         ` Ævar Arnfjörð Bjarmason
2021-02-01 14:48           ` Vincent Lefevre
2021-02-01 15:44             ` Ævar Arnfjörð Bjarmason
2021-02-01 22:16               ` Johannes Sixt
2021-02-03  2:48                 ` Ævar Arnfjörð Bjarmason
2021-02-03 17:11                   ` Johannes Sixt
2021-02-03 15:26               ` Vincent Lefevre
2021-02-04  0:14                 ` Ævar Arnfjörð Bjarmason
2021-02-04 15:38                   ` Vincent Lefevre
2021-02-01 14:49           ` [PATCH 0/3] pager: test for exit behavior & trace2 bug fix Ævar Arnfjörð Bjarmason
2021-02-02  1:59             ` [PATCH v2 0/5] " Ævar Arnfjörð Bjarmason
2021-02-02  1:59             ` [PATCH v2 1/5] pager: refactor wait_for_pager() function Ævar Arnfjörð Bjarmason
2021-02-02  1:59             ` [PATCH v2 2/5] pager: test for exit code with and without SIGPIPE Ævar Arnfjörð Bjarmason
2021-02-02  8:50               ` Denton Liu
2021-02-05  7:47               ` Johannes Sixt
2021-02-02  1:59             ` [PATCH v2 3/5] run-command: add braces for "if" block in wait_or_whine() Ævar Arnfjörð Bjarmason
2021-02-02  2:00             ` [PATCH v2 4/5] pager: properly log pager exit code when signalled Ævar Arnfjörð Bjarmason
2021-02-05  7:58               ` Johannes Sixt
2021-02-05 11:37                 ` Junio C Hamano
2021-02-02  2:00             ` [WIP/PATCH v2 5/5] WIP pager: respect exit code of pager over SIGPIPE Ævar Arnfjörð Bjarmason
2021-02-01 14:49           ` [PATCH 1/3] pager: test for exit code Ævar Arnfjörð Bjarmason
2021-02-01 14:49           ` [PATCH 2/3] pager: refactor wait_for_pager() function Ævar Arnfjörð Bjarmason
2021-02-01 14:49           ` [PATCH 3/3] pager: properly log pager exit code when signalled Ævar Arnfjörð Bjarmason
2021-02-01 18:07             ` Junio C Hamano
2021-02-01 19:21               ` Ævar Arnfjörð Bjarmason
2021-02-01 18:15             ` Junio C Hamano
2021-02-01 19:23               ` Ævar Arnfjörð Bjarmason
2021-02-01 22:04       ` git fails with a broken pipe when one quits the pager Johannes Sixt

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=bc88492979fee215d5be06ccbc246ae0171a9ced.1611910122.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=vincent@vinc17.net \
    /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).