Re: [PATCH] Don't ignore write failure from git-diff, git-log, etc.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jim Meyering <jim@xxxxxxxxxxxx> writes:

> 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

I haven't thought it through, but isn't the above example only
talking about the "Broken pipe" message?  Surely, you would get
that message from older Bash if you have a shell loop on the
upstream side of the pipe no matter what we (the command that is
run by the shell loop) do, and trap is needed to squelch it.

But I do not see how this pipeline, where git-rev-list produces
more than what fits in the in-kernel pipe buffer:

	"git-rev-list a lot of data | head -n 1"

would not catch EPIPE and say "Broken Pipe" with your patch.
Especially if the downstream is sufficiently slow (say, replace
it with "(sleep 10 && head -n 1)", perhaps), wouldn't the
upstream produce enough without being read, gets stuck on write,
and when the downstream exits, it would notice its write(2)
failed with EPIPE, wouldn't it?

Maybe you are talking about your updated patch?

> ...[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.

After having read what Linus said about how the "fixed" one
would behave differently, depending on the amount of data we
produce before the consumer says "I've seen enough" and
depending on the amount of data that would fit in the in-kernel
pipe buffer, I no longer think the compromise patch you mention
above is improvement anymore.

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux