On Sat, Nov 1, 2014 at 12:04 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Fri, Oct 31, 2014 at 07:04:04AM -0400, 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 explicitly setting up a SIGPIPE handler and exiting
>> the process.
>
> My first thought was that this should be handled already by 7559a1b
> (unblock and unignore SIGPIPE, 2014-09-18), but after re-reading your
> message, it sounds like you are using diff-highlight with non-git
> programs?
Yes, that's correct. It's useful, so with a few tools that use diffs,
I like to run the output through diff-highlight.
>> +# Some scripts may not realize that SIGPIPE is being ignored when launching the
>> +# pager--for instance scripts written in Python. Setting $SIG{PIPE} = 'DEFAULT'
>> +# doesn't work in these instances, so we install our own signal handler instead.
>
> Why doesn't $SIG{PIPE} = 'DEFAULT' work? I did some limited testing and
> it seemed to work fine for me. Though I simulated the condition with:
>
> (
> trap '' PIPE
> perl -e '$|=1; print "foo\n"; print STDERR "bar\n"'
> ) | true
>
> which should not ever print "bar".
Hehe, now that I see you right it out, I realize my mistake: I didn't
capitalize 'default'. Trying it out again, it does appear that does
the trick.
[snip]
> Can we exit 141 here? If we are part of a pipeline to a pager, it should
> not matter either way, but I'd rather not lose the exit code if we can
> avoid it (in case of running the script standalone).
>
>> +$SIG{PIPE} = \&pipe_handler;
>
> A minor nit, but would:
>
> $SIG{PIPE} = sub { ... };
>
> be nicer to avoid polluting the function namespace?
Sorry, my Perl-fu is kind of low these days. I used to use it all the
time but switched away from it quite a while ago. Given that
'DEFAULT' does the trick, I'll just re-roll my patch to use that.
Does that sound fair?
-John
PS Sorry for the late response, I've been traveling.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html