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]

 



Junio C Hamano <junkio@xxxxxxx> wrote:
> 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

I'm not sure which "Broken pipe" message you mean.

There are two types of "Broken pipe" messages.
There's the old, verbose one from bash that includes script line-number,
pid, and killed command name.  Old, unpatched versions of bash
print that message whenever bash kills a process with SIGPIPE.

Then there's the application (EPIPE) one, that can be printed
by a writing application like git,cat,seq,etc. only when SIGPIPE
stops a write but doesn't kill the writer.
In that case, the write syscall fails with errno==EPIPE,
and if it's diagnosed by the application, you get e.g.,

  seq: write error: Broken pipe

Since the script above is not ignoring SIGPIPE, the git-log process
is killed by bash-3.0, and that old version of bash announces the killing
with the verbose message:

  /t/bp-demo: line 2: 14474 Broken pipe             git-log

Any other patched or newer version of bash will print the
single requested line on stdout and nothing on stderr.

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

Right.
That's why no one is using such broken shells anymore.
And why no porcelain tools incur the penalty of ignoring
SIGQUIT anymore either.

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

I haven't debugged the old bash to see why that first one
fails to provoke a broken pipe message (from bash).
Unless you add a line like "trap '' PIPE" before it, bash kills
the writer with SIGPIPE, and so my patch is irrelevant,
because the failing write syscall never returns.

> 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?

Are you presuming SIGPIPE is ignored?

> Maybe you are talking about your updated patch?

I was talking about the initial patch, or the one that
also removed the errno == EPIPE tests.
-
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