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

On Mon, Aug 16, 2021 at 05:54:44PM -0400, Randall S. Becker wrote:

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.

OK, at least that absolves git.c. :)

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

Ouch. I mean, sure, if you need a specific code, I get that die is not a
good tool. But getting arbitrary values seems kind of weird and
unfriendly. The perldoc for die does say it gives you $! (errno), or $?
(the last child exit value) if appropriate. So it's not completely
arbitrary, but I think your errno value may just be unlucky.

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 $@;

Yeah, I think we are better to get a consistent exit code from perl.
There are a few options here:

 - wrapping in an eval works, as you showed above. It's a little awkward
   to wrap the whole script, though.

 - there's $SIG{__DIE__}, but the manpage warns against using it. You
   can use it something like this:

     sub catch_top {
       CORE::die @_ if $^S; # in an eval; use regular die
       CORE::die @_ if !defined $^S; in perl's parser
       print STDERR "@_\n";
       exit 255; # or whatever we want
     }
     $SIG{__DIE__} = \&catch_top;

 - you can hook die() like this:

     BEGIN { *CORE::GLOBAL::die = \&my_die; }

   but I expect would still need to check that you're not in an eval, as
   above.

  - The SIG{__DIE__} docs mention using an END{} block, but I'm not sure
    how you determine if we hit a die or not (at that point, $@ won't
    actually be set).

I've used the catch_top() thing before and it does work (it's just ugly
that you have to deal with $^S).

I guess yet another alternative is that we could avoid using perl's
die() in favor of our own custom-named function. It seems like that may
confuse folks who come later, though.

-Peff