Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases

From: Johannes Sixt <j6t@kdbg.org>
Date: Jan 10, 2013, 21:52 UTC
Message-ID: <50EF380A.9010809@kdbg.org>
In-Reply-To: <7vbocw23fq.fsf@alter.siamese.dyndns.org>

Am 10.01.2013 21:22, schrieb Junio C Hamano:

Show 16 quoted lines

> Jeff King <peff@peff.net> writes:
> 
>> Maybe the right rule is "if we are using the shell to execute, do not
>> mention SIGPIPE"? It seems a little iffy at first, but:
>>
>>   1. It tends to coincide with direct use of internal tools versus
>>      external tools.
>>
>>   2. We do not reliably get SIGPIPE there, anyway, since most shells
>>      will convert it into exit code 141 before we see it.
>>
>> I.e., something like:
> 
> Hmph.  That may be a good heuristics, but I wonder if we also want
> to special case WIFEXITED(status) && WEXITSTATUS(status) == 141 to
> pretend as if nothing went wrong, when ignore_sigpipe is in effect?

The purpose of Peff's patch is to remove the error message, but not to pretend success (the return code remains 141).

I looked at all instances with use_shell=1 or RUN_USING_SHELL:

Most of the time, we do not care where the output of the command goes to, which I regard as the same case as when a shell runs a command: We don't need to report the SIGPIPE death.

The interesting cases are when git reads back the output of the command. Here, a SIGPIPE death of the child would indicate a bug in git, I think, and some diagnostic would be worth it. But we can just as well declare that git doesn't have bugs ;)

These are the interesting cases:
connect.c:640:          conn->use_shell = 1;
    a connection to a local repository
convert.c:372:  child_process.use_shell = 1;
    clean/smudge filter
credential.c:216:       helper.use_shell = 1;
    credential helper
diff.c:4851:    child.use_shell = 1;
    textconv

All in all, I think the heuristics makes sense.

-- Hannes