git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: "Randall S. Becker" <rsbecker@nexbridge.com>
Cc: git@vger.kernel.org,
	"'Joachim Schmitz'" <jojo@schmitz-digital.de>,
	"Eric Wong" <e@80x24.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [Problem] test_must_fail makes possibly questionable assumptions about exit_code.
Date: Tue, 27 Feb 2018 16:16:16 -0800	[thread overview]
Message-ID: <20180228001616.GJ174036@aiede.svl.corp.google.com> (raw)
In-Reply-To: <005501d3b025$c0057ce0$401076a0$@nexbridge.com>

Hi,

Randall S. Becker wrote:

> After months of arguing with some platform developers on this subject, the
> perl spec was held over my head repeatedly about a few lines that are
> causing issues. The basic problem is this line (test-lib-functions.sh, line
> 633, still in ffa952497)
>
>>        elif test $exit_code -gt 129 && test $exit_code -le 192
>>       then
>>               echo >&2 "test_must_fail: died by signal $(($exit_code - 128)):
>
> According to the perl spec http://perldoc.perl.org/functions/die.html, die
> basically takes whatever errno is, mods it with 256 and there you go. EBADF
> is what is used when perl reads from stdin and calls die - that's standard
> perl. In most systems, you end up with something useful, when EBADF is 9.
> But when it is 4009, you get a garbage answer (4009 mod 256 a.k.a. 169).
> However, only 128-165 are technically reserved for signals, rather than all
> the way up to 192, which may be true in some places but not everywhere.
>
> The advice (I'm putting that nicely) I received was to use exit so that the
> result is predictable - unlikely to be useful in the 15K test suites in git.

The fundamental thing is the actual Git commands, not the tests in the
testsuite, no?

In the rest of git, die() makes a command exit with status 128.  The
trouble here is that our code in Perl is assuming the same meaning for
die() but using perl's die builtin instead.  That suggests a few
options:

 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.

 b) We could forbid use of die() and use some git_die() instead (but
    with a better name) for our own error handling.

 c) We could have a special different exit code convention for
    commands written in Perl.  And then change expectations whenever a
    command is rewritten in C.  As you might expect, I don't like this
    option.

 d) We could wrap each command in an eval {...} block to convert the
    result from die() to exit 128.

Option (b) feels simplest to me.

Thoughts?

Thanks,
Jonathan

  reply	other threads:[~2018-02-28  0:16 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 [this message]
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
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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20180228001616.GJ174036@aiede.svl.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=avarab@gmail.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=jojo@schmitz-digital.de \
    --cc=rsbecker@nexbridge.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).