git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Randall S. Becker" <rsbecker@nexbridge.com>
To: "'Jeff King'" <peff@peff.net>
Cc: <git@vger.kernel.org>
Subject: RE: [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh
Date: Fri, 19 Jan 2018 18:29:57 -0500	[thread overview]
Message-ID: <001d01d3917d$6de77060$49b65120$@nexbridge.com> (raw)
In-Reply-To: <000c01d39174$eacfff10$c06ffd30$@nexbridge.com>

On January 19, 2018 5:29 PM, I wrote:
> On January 19, 2018 4:27 PM, Jeff King wrote:
> > On Fri, Jan 19, 2018 at 12:34:04PM -0500, randall.s.becker@rogers.com
> wrote:
> >
> > > From: "Randall S. Becker" <rsbecker@nexbridge.com>
> > >
> > > * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being
> > >   cleared automatically on platform. This caused tests to seem to fail
> > >   while actually succeeding.
> > >
> > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> > > ---
> > >  t/lib-git-daemon.sh | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index
> > > 987d40680..955beecd9 100644
> > > --- a/t/lib-git-daemon.sh
> > > +++ b/t/lib-git-daemon.sh
> > > @@ -68,6 +68,7 @@ start_git_daemon() {
> > >  		test_skip_or_die $GIT_TEST_GIT_DAEMON \
> > >  			"git daemon failed to start"
> > >  	fi
> > > +	trap '' EXIT
> > >  }
> >
> > I don't think this can be right. The way these traps are supposed to work is:
> >
> >   - as soon as test-lib.sh is loaded, we "trap die EXIT" to catch an
> >     accidental death/exit from one of the scripts
> >
> >   - when test_done is called, we clear the trap (i.e., it is OK to exit
> >     now because the script has given us a positive indication that all
> >     tests have been run)
> >
> >   - while the child git-daemon is running, we'd want to clean up after
> >     ourselves. So during start_git_daemon() we add our cleanup (followed
> >     by the original "die", because shell traps don't push onto a stack).
> >     And then at stop_git_daemon(), we restore the original die trap.
> >
> > But this patch means that while git-daemon is running, we have no trap at
> all!
> > That means that a failed test which causes us to exit would leave a
> > child daemon running.
> >
> > Furthermore, both of these functions now drop the 'die' trap entirely,
> > meaning the script would fail to notice premature exit from one of the
> > test snippets.
> >
> > So while this may be papering over a problem on NonStop, I think it's
> > breaking the intent of the traps.
> >
> > I'm not sure what the root of the problem is, but it sounds like ksh
> > may be triggering EXIT traps when we don't expect (during function
> returns?
> > Subshell exits? Other?)
> 
> The "unexpected" EXIT traps are exactly the issue we found when working
> with the platform support team. There was some discussion about what the
> right expectation was, and I was not able to have a change made to ksh on
> the platform. The problem may have a similar (identical?) root cause to the
> Perl exit issues we are also experiencing that is in their hands. I'm currently
> retesting without this change (results in 36 hours ☹ ). Is there a preferred
> way you would like me to bypass this except on NonStop? I can add a check
> based on uname.

The option that may work, if the tests that are currenting running until Sunday (sadly) fail miserably, is to use:

if [ `uname` = "NONSTOP_KERNEL" ]; then trap '' EXIT; fi

or perhaps to add a descriptive function along those lines. We have had two major operating system upgrades since the original case relating to ksh traps, so perhaps things might improve. Our baseline is that there are currently 6 breaks (t1308#23, t1405#52, t9001#31/106/107/134), most of which have been traced back to perl completion codes.

Cheers,
Randall
P.S. I am happy to explain why the tests perform the at the rate they do on the development machines I have, if anyone is interested, although dissertations might be involved 😉

-- Brief whoami:
 NonStop developer since approximately 211288444200000000
 UNIX developer since approximately 421664400
-- In my real life, I talk too much.




  reply	other threads:[~2018-01-19 23:30 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-19 17:33 [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform randall.s.becker
2018-01-19 17:34 ` [PATCH v2 0/6] Cover Letter for NonStop Port Patches randall.s.becker
2018-01-19 17:34 ` [PATCH v2 1/6] Bypass GCC attributes on NonStop platform where used randall.s.becker
2018-01-19 19:25   ` Stefan Beller
2018-01-19 20:28   ` Ramsay Jones
2018-01-19 21:20     ` Jeff King
2018-01-19 22:43       ` Ramsay Jones
2018-01-19 22:53         ` Randall S. Becker
2018-01-19 17:34 ` [PATCH v2 2/6] Add tar extract install options override in installation processing randall.s.becker
2018-01-20 12:30   ` René Scharfe
2018-01-20 13:44     ` Randall S. Becker
2018-01-20 14:24       ` René Scharfe
2018-01-20 15:35         ` Randall S. Becker
2018-01-20 20:34           ` Ævar Arnfjörð Bjarmason
2018-01-20 20:52             ` Randall S. Becker
2018-01-19 17:34 ` [PATCH v2 3/6] Define config options required for the HPE NonStop NSX and NSE platforms randall.s.becker
2018-01-19 17:34 ` [PATCH v2 4/6] Force test suite traps to be cleared for NonStop ksh randall.s.becker
2018-01-19 19:55   ` Stefan Beller
2018-01-19 21:27   ` Jeff King
2018-01-19 22:29     ` Randall S. Becker
2018-01-19 23:29       ` Randall S. Becker [this message]
2018-01-21  4:07       ` Randall S. Becker
2018-01-19 17:34 ` [PATCH v2 5/6] Bring NonStop platform definitions up to date in git-compat-util.h randall.s.becker
2018-01-19 17:34 ` [PATCH v2 6/6] Add intptr_t and uintptr_t to regcomp.c for NonStop platform randall.s.becker
2018-01-19 19:34 ` [PATCH v2 0/6] Force pipes to flush immediately on " Stefan Beller
2018-01-20 11:10 ` Torsten Bögershausen
2018-01-20 13:23   ` Randall S. Becker
2018-01-22 22:36   ` Junio C Hamano
2018-01-22 22:43     ` Randall S. Becker
2018-01-23 18:13       ` Junio C Hamano
2018-01-23 20:46         ` Randall S. Becker
2018-01-25  3:42         ` Randall S. Becker

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='001d01d3917d$6de77060$49b65120$@nexbridge.com' \
    --to=rsbecker@nexbridge.com \
    --cc=git@vger.kernel.org \
    --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).