git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Randall S. Becker" <rsbecker@nexbridge.com>
To: "'demerphq'" <demerphq@gmail.com>
Cc: "'Jeff King'" <peff@peff.net>, "'Eric Wong'" <e@80x24.org>,
	"'Jonathan Nieder'" <jrnieder@gmail.com>,
	'Git' <git@vger.kernel.org>,
	"'Joachim Schmitz'" <jojo@schmitz-digital.de>,
	"'Ævar Arnfjörð Bjarmason'" <avarab@gmail.com>
Subject: RE: [Problem] test_must_fail makes possibly questionable assumptions about exit_code.
Date: Wed, 28 Feb 2018 12:32:30 -0500	[thread overview]
Message-ID: <004801d3b0ba$1f2ffa80$5d8fef80$@nexbridge.com> (raw)
In-Reply-To: <CANgJU+WFKqou=ZXcdr8V_ST=opHSBm9ksEVowxRB7o7DDEmuvg@mail.gmail.com>

On February 28, 2018 12:19 PM, demerphq wrote:
> On 28 February 2018 at 18:10, Randall S. Becker <rsbecker@nexbridge.com>
> wrote:
> > On February 28, 2018 11:46 AM, demerphq wrote:
> >> On 28 February 2018 at 08:49, Jeff King <peff@peff.net> 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


  parent reply	other threads:[~2018-02-28 17:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-27 23:50 [Problem] test_must_fail makes possibly questionable assumptions about exit_code Randall S. Becker
2018-02-28  0:16 ` Jonathan Nieder
2018-02-28  4:07   ` Eric Wong
2018-02-28  5:00     ` Jeff King
2018-02-28  7:42       ` Eric Wong
2018-02-28  7:49         ` Jeff King
2018-02-28 14:55           ` Randall S. Becker
2018-02-28 16:51             ` demerphq
2018-03-01  7:36               ` Jeff King
2018-03-01  8:16                 ` demerphq
2018-03-01 14:28                 ` Randall S. Becker
2018-03-01 15:08                   ` Jeff King
2018-03-01 15:30                     ` demerphq
2018-02-28 16:22           ` Junio C Hamano
2018-02-28 16:46           ` demerphq
2018-02-28 17:10             ` Randall S. Becker
2018-02-28 17:19               ` demerphq
2018-02-28 17:20                 ` demerphq
2018-02-28 17:32                 ` Randall S. Becker [this message]
2018-02-28 17:44               ` Jonathan Nieder
2018-02-28 18:21                 ` Randall S. Becker
2018-02-28 18:51                   ` Jonathan Nieder
2018-02-28 20:04                     ` Randall S. Becker
2018-02-28 22:02                       ` Randall S. Becker
2018-02-28 23:16                         ` Jonathan Nieder
2018-03-01  7:34             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='004801d3b0ba$1f2ffa80$5d8fef80$@nexbridge.com' \
    --to=rsbecker@nexbridge.com \
    --cc=avarab@gmail.com \
    --cc=demerphq@gmail.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=jojo@schmitz-digital.de \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).