Re: [PATCH] run-command: simplify wait_or_whine
- From
Jeff King <peff@peff.net>
- Date
- Jun 1, 2013, 17:01 UTC
- Message-ID
- <20130601170147.GA19234@sigill.intra.peff.net>
- In-Reply-To
- <CAMP44s0L1M+_s2eDM=Ogy=rxLhpZYwSb8qWTuEe30pB4KGDVtA@mail.gmail.com>
On Sat, Jun 01, 2013 at 09:30:50AM -0500, Felipe Contreras wrote:
Show 21 quoted lines
> > The original commit that introduces this says
> >
> > run_command: encode deadly signal number in the return value
> >
> > We now write the signal number in the error message if the program
> > terminated by a signal. The negative return value is constructed such that
> > after truncation to 8 bits it looks like a POSIX shell's $?:
> >
> > $ echo 0000 | { git upload-pack .; echo $? >&2; } | :
> > error: git-upload-pack died of signal 13
> > 141
> >
> > Previously, the exit code was 255 instead of 141.
> >
> > So this is part of the interface to the user. With your changes, the
> > exit code is now different. I tested by force segfaulting upload-pack.
> > $? returned 11. So NAK.
>
> Yeah, and last year we returned a different code. The world didn't
> end, because nobody is checking for the specific code. But if you want
> to retain complexity forever, suit yourselves.Last year we returned a different code from the function that other C code saw. But what got returned via exit() to exterior programs was always 141 in the SIGPIPE case, both before and after my 709ca730. That is explained in the first two paragraphs here:
Show 21 quoted lines
> commit 709ca730f8e093005cc882bfb86c0ca9c83d345b > Author: Jeff King <peff@peff.net> > Date: Sat Jan 5 09:49:49 2013 -0500 > > run-command: encode signal death as a positive integer > > When a sub-command dies due to a signal, we encode the > signal number into the numeric exit status as "signal - > 128". This is easy to identify (versus a regular positive > error code), and when cast to an unsigned integer (e.g., by > feeding it to exit), matches what a POSIX shell would return > when reporting a signal death in $? or through its own exit > code. > > So we have a negative value inside the code, but once it > passes across an exit() barrier, it looks positive (and any > code we receive from a sub-shell will have the positive > form). E.g., death by SIGPIPE (signal 13) will look like > -115 to us in inside git, but will end up as 141 when we > call exit() with it. And a program killed by SIGPIPE but run > via the shell will come to us with an exit code of 141.
Your patch changes the error code that is propagated via exit() in this case. We cannot know "nobody is checking for the specific code", because the list of callers is every shell script or program which execs git. Some of them do care about the exit code. I can give an example of a case I have that cares, but I do not think it is even important. The point is that we would be regressing an existing interface, and cannot know who is broken by it.
-Peff