Subject: RE: [Problem] test_must_fail makes possibly questionable assumptions about exit_code.

From: "Randall S. Becker" <[email-masked]>
Date: Wed, 28 Feb 2018 12:32:30 -0500

On February 28, 2018 12:19 PM, demerphq wrote:
> On 28 February 2018 at 18:10, Randall S. Becker <[email-masked]>
> wrote:
> > On February 28, 2018 11:46 AM, demerphq wrote:
> >> On 28 February 2018 at 08:49, Jeff King <[email-masked]> wrote:
> >> > On Wed, Feb 28, 2018 at 07:42:51AM +0000, Eric Wong wrote:
> >> >
> >> >> > > >  a) We could override the meaning of die() in Git.pm.  This feels
> >> >> > > >     ugly but if it works, it would be a very small patch.
> >> >> > >
> >> >> > > Unlikely to work since I think we use eval {} to trap
> >> >> > > exceptions from die.
> >> >> > >
> >> >> > > >  b) We could forbid use of die() and use some git_die() instead
> (but
> >> >> > > >     with a better name) for our own error handling.
> >> >> > >
> >> >> > > Call sites may be dual-use: "die" can either be caught by an
> >> >> > > eval or used to show an error message to the user.
> >> >>
> >> >> <snip>
> >> >>
> >> >> > > >  d) We could wrap each command in an eval {...} block to convert
> the
> >> >> > > >     result from die() to exit 128.
> >> >> > >
> >> >> > > I prefer option d)
> >> >> >
> >> >> > FWIW, I agree with all of that. You can do (d) without an
> >> >> > enclosing eval block by just hooking the __DIE__ handler, like:
> >> >> >
> >> >> > $SIG{__DIE__} = sub {
> >> >> >   print STDERR "fatal: @_\n";
> >> >> >   exit 128;
> >> >> > };
> >> >>
> >> >> Looks like it has the same problems I pointed out with a) and b).
> >> >
> >> > You're right. I cut down my example too much and dropped the
> >> > necessary eval magic. Try this:
> >> >
> >> > -- >8 --
> >> > SIG{__DIE__} = sub {
> >> >   CORE::die @_ if $^S || !defined($^S);
> >> >   print STDERR "fatal: @_";
> >> >   exit 128;
> >> > };
> >>
> >> FWIW, this doesn't need to use CORE::die like that unless you have
> >> code that overrides die() or CORE::GLOBAL::die, which would be pretty
> unusual.
> >>
> >> die() within $SIG{__DIE__} is special cased not to trigger
> >> $SIG{__DIE__} again.
> >>
> >> Of course it doesn't hurt, but it might make a perl hacker do a
> >> double take why you are doing it. Maybe add a comment like
> >>
> >> # using CORE::die to armor against overridden die()
> >
> > The problem is actually in git code in its test suite that uses perl inline, not in
> my test code itself. The difficulty I'm having is placing this appropriate so that
> the signal handler gets used throughout the test suite including in the perl -e
> invocations. This is more a lack of my own understanding of plumbing of git
> test framework rather than of using or coding perl.
> 
> Did you reply to the wrong mail?
> 
> Create a file like:
> 
> .../Git/DieTrap.pm
> 
> which would look like  this:
> 
> package Git::DieTrap;
> use strict;
> use warnings;
> 
> SIG{__DIE__} = sub {
>    CORE::die @_ if $^S || !defined($^S);
>    print STDERR "fatal: @_";
>    exit 128;
> };
> 
> 1;
> __END__
> 
> and then you would do:
> 
> export PERL5OPT=-MGit::DieTrap
> 
> before executing any tests. ANY use of perl from that point on will behave as
> though it has:
> 
> use Git::DieTrap;
> 
> at the top of the script, be it a -e, or any other way that Perl code is
> executed.

The context of this request, perhaps missing, what that I have been trying to move the platform to the standard git code base. It is not my issue specifically. Rather, if someone else wants to build and test git on the platform, they should not have to have any knowledge of putting in hacks to make it work. I can personally make this work. That's not the point. It is to allow others on platform to make it work without deep knowledge. Otherwise, I am not being productive with my efforts.

Cheers,
Randall