Re: [PATCH] pager: exit without error on SIGPIPE

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

quoted

Would it be the matter of propagating the exit status of the pager
noticed by wait_or_white() down thru finish_command_in_signal() and
wait_for_pager(1) to here, so

 - If we know pager exited with non-zero status, we would report,
   perhaps with warning(_("..."));

 - If we notice we got a SIGPIPE, we ignore it---it is nothing of
   interest to the end-user;

 - Otherwise we do not do anything differently.

would be sufficient?  Implementors of "git -p" may know that "git"
happens to implement its paging by piping its output to an external
pager, but the end-users do not care.  Implementors may say they are
giving 'q' to their pager "less", but to the end-users, who report
"I ran 'git log' and after reading a pageful, I told it to 'q'uit",
the distinction does not have any importance.

Or are there more to it, in that the exit status we get from the
pager, combined with the kind of signal we are getting, is not
sufficient for us to tell what is going on?

It is, I just wonder if ignoring the exit code is a practical issue as
long as we're not clobbering SIGPIPE, particularly with my trace2
logging patch in this thread.

But yeah, we could patch git to handle this in the general case....

Sorry, but now you lost me.

I was merely wondering if Denton's patch can become a small update
on top of these, if we just made sure that the exit code of the
pager noticed by wait_or_whine() is reported to the code where
Denton makes the decision to say "let's not re-raise but simply exit
with 0 return as what we got is SIGPIPE".  I guess we could even
make git exit with the pager's return code in that case, as the
end-user observable result would be similar to "git log | less"
where 'less' may be segfaulting or exiting cleanly.

IOW, something like this on top of your three-patch series?

 pager.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git c/pager.c w/pager.c
index 3d37dd7ada..73bc5fc0e4 100644
--- c/pager.c
+++ w/pager.c

@@ -28,8 +28,14 @@ static void wait_for_pager_atexit(void)
 
 static void wait_for_pager_signal(int signo)
 {
+	int status;
+
 	close_pager_fds();
-	finish_command_in_signal(&pager_process);
+	status = finish_command_in_signal(&pager_process);
+
+	if (signo == SIGPIPE)
+		exit(status);
+
 	sigchain_pop(signo);
 	raise(signo);
 }