Junio C Hamano <junkio@xxxxxxx> wrote:
> Jim Meyering <jim@xxxxxxxxxxxx> writes:
>
>> Of course error messages are annoying when your short-pipe-read is
>> _deliberate_ (tho, most real uses of git tools will actually get no
>> message to be annoyed about[*]), but what if there really *is* a mistake?
>> Try this:
>>
>> # You want to force git to ignore the error.
>> $ trap '' PIPE; git-rev-list HEAD | sync
>> $
>
> It is perfectly valid (although it is stupid) for a Porcelain
> script to do this:
>
> latest_by_jim=$(git log --pretty=oneline --author='Jim' | head -n 1)
> case "$latest_by_jim" in
> '') echo "No commit by Jim" ;;
> *) # do something interesting on the commit
> ;;;
> esac
Hi Junio,
The above snippet (prepending a single #!/bin/bash line) doesn't provoke
an EPIPE diagnostic from my patched git. In fact, even if you're using
an old, unpatched version of bash, it provokes *no* diagnostic at all.
To provoke a diagnostic (from bash, not git), using old unpatched bash,
you need a script doing output from a subshell, e.g.:
#!/tmp/bash-3.0/bash
for x in 1; do
git-log
done | head -1
With unpatched bash-3.0, it does this:
commit 42e3a6f676e9ae4e9640bc2ff36b7ab0b061a60e
/tmp/bp-demo: line 2: 24864 Broken pipe git-log
It's only if you try to avoid the above and change your script to
ignore SIGPIPE do you finally get an offending EPIPE diagnostic:
#!/tmp/bash-3.0/bash
trap '' PIPE
for x in 1; do
./git-log; echo $? 1>&2
done | head -1
Here's its output, using my patch:
commit 42e3a6f676e9ae4e9640bc2ff36b7ab0b061a60e
fatal: write failure on standard output
128
That trap has the nasty side effect of making the poorly written script
wait until "git-log" has completed (before, it was interrupted right
away), so it can take a lot longer. With my patch, it also gives a
diagnostic, which might serve to inform someone that they should not
ignore SIGPIPE.
No porcelain (modulo [*]) in git proper or cogito ignores SIGPIPE,
so I don't see how EPIPE error diagnostics can be a problem.
[*] These scripts do ignore SIGPIPE, but either don't need to,
or can/should be fixed not to:
git-archimport.perl
git-cvsimport.perl
git-svnimport.perl
And, yes, I'd be happy to fix them, if anyone is interested.
> In such a case, it is a bit too much for my taste to force the
> script to redirect what comes out of fd 2 of the upstream of the
> pipe, so that it can filter out only the "write error" message
> but still show other kinds of error messages. You could do so
> by elaborate shell magic, perhaps like this:
>
> filter_pipe_error () {
> exec 3>&1
> (eval "$1" 2>&1 1>&3 | grep >&2 -v 'Broken pipe')
> }
>
> latest_by_jim=$(filter_pipe_error \
> 'git log --pretty=oneline --author='\''Jim'\'' | head -n 1'
> )
I agree that would be extreme. But it's not necessary, since the
'Broken pipe' diagnostic appears now only in contrived circumstances.
> but what's the point?
>
> I think something like this instead might be more palatable.
...[patch to make git/EPIPE exit nonzero, but with no diagnostic]
Thank you for taking the time to reply and to come up with a compromise.
At first I thought this would be a step in the right direction, but,
now that I understand how infrequently EPIPE actually comes into play,
I think it'd be better to avoid a half-measure fix, since that would
just perpetuate the idea that EPIPE is worth handling specially.
Jim
-
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
