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

From: Jeff King <peff@peff.net>
Date: Aug 16, 2021, 22:22 UTC
Message-ID: <YRrlQwLENaWs8zWm@coredump.intra.peff.net>
In-Reply-To: <01d001d792e9$55f45470$01dcfd50$@nexbridge.com>

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

Show 6 quoted lines

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

Show 12 quoted lines

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

Show 10 quoted lines

> 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