On Sat, Nov 1, 2014 at 12:04 AM, Jeff King <p...@peff.net> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html