threads / patch / 37877

v2diff-highlight: exit when a pipe is broken

Subject: [PATCH v2] diff-highlight: exit when a pipe is broken

## tl;dr

3 messages between Nov 4, 2014 and Nov 4, 2014. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

John Szakmeister· Nov 4, 2014, 20:01 UTC · lore

While using diff-highlight with other tools, I have discovered that Python ignores SIGPIPE by default. Unfortunately, this also means that tools attempting to launch a pager under Python--and don't realize this is happening--means that the subprocess inherits this setting. In this case, it means diff-highlight will be launched with SIGPIPE being ignored. Let's work with those broken scripts by restoring the default SIGPIPE handler.

Signed-off-by: John Szakmeister <john@szakmeister.net>
---
Incorporates feedback from Jeff King and now we just restore the default signal
handler using the correct case of 'DEFAULT'.

 contrib/diff-highlight/diff-highlight | 4 ++++
 1 file changed, 4 insertions(+)

Show changes to contrib/diff-highlight/diff-highlight +4 −0

diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight
index c4404d4..69a652e 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -14,6 +14,10 @@ my @removed;
 my @added;
 my $in_hunk;
 
+# Some scripts may not realize that SIGPIPE is being ignored when launching the
+# pager--for instance scripts written in Python.
+$SIG{PIPE} = 'DEFAULT';
+
 while (<>) {
 	if (!$in_hunk) {
 		print;

-- 
2.0.1

Jeff King· Nov 4, 2014, 20:09 UTC · re: John Szakmeister · lore

Re: [PATCH v2] diff-highlight: exit when a pipe is broken

On Tue, Nov 04, 2014 at 03:01:12PM -0500, John Szakmeister wrote:

Show 11 quoted lines

> While using diff-highlight with other tools, I have discovered that Python
> ignores SIGPIPE by default.  Unfortunately, this also means that tools
> attempting to launch a pager under Python--and don't realize this is
> happening--means that the subprocess inherits this setting.  In this case, it
> means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
> with those broken scripts by restoring the default SIGPIPE handler.
> 
> Signed-off-by: John Szakmeister <john@szakmeister.net>
> ---
> Incorporates feedback from Jeff King and now we just restore the default signal
> handler using the correct case of 'DEFAULT'.

Thanks, this version looks much simpler. :)

Acked-by: Jeff King <peff@peff.net>

-Peff

Junio C Hamano· Nov 4, 2014, 20:52 UTC · re: Jeff King · lore

Re: [PATCH v2] diff-highlight: exit when a pipe is broken

Jeff King <peff@peff.net> writes:

Show 17 quoted lines

> On Tue, Nov 04, 2014 at 03:01:12PM -0500, John Szakmeister wrote:
>
>> While using diff-highlight with other tools, I have discovered that Python
>> ignores SIGPIPE by default.  Unfortunately, this also means that tools
>> attempting to launch a pager under Python--and don't realize this is
>> happening--means that the subprocess inherits this setting.  In this case, it
>> means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
>> with those broken scripts by restoring the default SIGPIPE handler.
>> 
>> Signed-off-by: John Szakmeister <john@szakmeister.net>
>> ---
>> Incorporates feedback from Jeff King and now we just restore the default signal
>> handler using the correct case of 'DEFAULT'.
>
> Thanks, this version looks much simpler. :)
>
> Acked-by: Jeff King <peff@peff.net>

Thanks, both.  The patch looks good.  Will queue.