RE: [ANNOUNCE] Git v2.33.0-rc2 (Build/Test Report)

From: Randall S. Becker <rsbecker@nexbridge.com>
Date: Aug 16, 2021, 21:54 UTC
Message-ID: <01d001d792e9$55f45470$01dcfd50$@nexbridge.com>
In-Reply-To: <YRrSXhbaZMLMq+1/@coredump.intra.peff.net>

On August 16, 2021 5:02 PM, Jeff King wrote:

Show 42 quoted lines

>To: Randall S. Becker <rsbecker@nexbridge.com>
>Cc: 'Junio C Hamano' <gitster@pobox.com>; git@vger.kernel.org
>Subject: Re: [ANNOUNCE] Git v2.33.0-rc2 (Build/Test Report)
>
>On Mon, Aug 16, 2021 at 04:43:25PM -0400, Randall S. Becker wrote:
>
>> >Oh. Then the notion from my other mail of "if it's die(), then other
>> >tests would presumably see similar failures" might be true. ;)
>>
>> When running
>>
>> /home/git/git/t/trash directory.t9001-send-email: git send-email
>> --from="Example <nobody@example.com>" --to=nobody@example.com
>> --smtp-server="/home/git/git/t/trash
>> directory.t9001-send-email/fake.sendmail" --transfer-encoding=8bit
>> 0001-Second.patch longline.patch
>> fatal: longline.patch:35 is longer than 998 characters
>> warning: no patches were sent
>> /home/git/git/t/trash directory.t9001-send-email: echo $?
>> 162
>
>Well, that's a promising start to finding the source. :)
>
>> So this is strange. Where is perl run? I'd like to catch the completion inside git.
>
>This will all go through execv_dashed_external() in git.c. So we should just be exiting with the status code we got from the child via wait().
>
>You could try:
>
>  - running it as git-send-email (with a dash), which will exec the perl
>    script directly, rather than going through the main git binary
>
>  - instrumenting run-command.c:wait_or_whine() to see how it interprets
>    the value. If perl really is returning 255, then perhaps your
>    platform's WIFSIGNALED() is confused by that.
>
>If the problem does only show when going through the git binary, then I
>suspect:
>
>  git -c alias.foo='!perl -e die' foo
>
>may be an easier reproduction.

Running git-send-email reports completion 162. The code variable is optimized out but looks like it also is 162 when returning. The WIFEXITED(status) code did not appear to execute, although I think that also was optimized out. finish_command ret is 162. So perl looks like it is completing with a bad completion code. This percolates up to git, which also reports the same value.

I went to the perl maintainer on this subject. What I got back was that die is not guaranteed to return a specific value other than 0 for success and non-zero for failure. There are platforms where the return might known and has meaning but that is not portable. According to the current official perl documentation:

"die raises an exception. Inside an eval the exception is stuffed into $@ and the eval is terminated with the undefined value. If the exception is outside of all enclosing evals, then the uncaught exception is printed to STDERR and perl exits with an exit code indicating failure. If you need to exit the process with a specific exit code, see exit."

So assuming that a signal occurred because the value is between 129 and 192 is not correct in the case of perl. Could we do something like test_expect_perl_die that does not perform the signal check at line 980 in test-lib-functions.sh so just checks 0 vs. non-zero, which would be semantically correct no matter what the platform? Alternatively, and possibly better, the die could be caught and then exit() called in git-send-email, as in:

eval { die "Something bad happened" }; exit(255) if $@;