All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Plato Kiorpelidis <kioplato@gmail.com>
Cc: git@vger.kernel.org, phillip.wood123@gmail.com, avarab@gmail.com
Subject: Re: [PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS
Date: Wed, 18 May 2022 10:57:54 -0700	[thread overview]
Message-ID: <xmqq7d6iwwnx.fsf@gitster.g> (raw)
In-Reply-To: <20220518141321.uynaxzaoivlthi7b@compass> (Plato Kiorpelidis's message of "Wed, 18 May 2022 17:13:21 +0300")

Plato Kiorpelidis <kioplato@gmail.com> writes:

>> And less portable for the invoking process of "git".  The invoking
>> process used to be able to depend ou getting WEXITSTATUS() from our
>> exit status to obtain "1" when we exited with 1; if we start exiting
>> with EXIT_FAILURE, there is no guarantee what non-zero value is used.
>> 
>> So, I am not sure if this is a good direction to go in.
>
> From what I understand, this is a point about why EXIT_FAILURE and EXIT_SUCCESS
> are a bad idea in Git's case in general; not specifically in test-tool's case.
> The test-tool doesn't use child processes, therefore it doesn't use the macro
> WEXITSTATUS. As a result, supposedly, we could use EXIT_FAILURE and EXIT_SUCCESS
> constants in this case. However, we don't want to use them in order to stay
> consistent throughtout Git's implementation. Is my understanding correct?

Yes, it is a bad idea for any tool (not limited to Git) whose
documentation does not say "on failure, it exits with non-zero exit
status", but signals what kind of failure with different non-zero
exit status values.  Perhaps the calling test scripts of test-tool
may only care about exit status being (or not being) zero, so we
could use EXIT_FAILURE vs EXIT_SUCCESS, as long as EXIT_FAILURE does
not exit with a value that confuses test_must_fail, but by design,
the code that uses EXIT_FAILURE cannot guarantee that---the whole
point of EXIT_FAILURE macro is to hide which exact value is used.
You are correct to assume that we'd want to avoid using EXIT_FAILURE
in test-tool because we do not want to tempt people to
copy-and-paste.

Thanks.

  reply	other threads:[~2022-05-18 17:58 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-09 17:51 [PATCH v2 00/15][GSoC] iterate dirs before or after their contents Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 01/15] t0066: refactor dir-iterator tests Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 02/15] t0066: remove dependency between unrelated tests Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 03/15] t0066: shorter expected and actual output file names Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS Plato Kiorpelidis
2022-05-09 21:03   ` Junio C Hamano
2022-05-18 14:13     ` Plato Kiorpelidis
2022-05-18 17:57       ` Junio C Hamano [this message]
2022-05-09 17:51 ` [PATCH v2 05/15] test-dir-iterator: print EACCES and ELOOP errno set by dir_iterator Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 06/15] test-dir-iterator: print errno name set by dir_iterator_advance Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 07/15] t0066: better test coverage for dir-iterator Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 08/15] t0066: reorder tests from simple to more complex Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 09/15] t0066: rename test directories Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 10/15] dir-iterator: refactor dir_iterator_advance() Plato Kiorpelidis
2022-05-09 21:16   ` Junio C Hamano
2022-05-18 15:39     ` Plato Kiorpelidis
2022-05-10 13:04   ` Phillip Wood
2022-05-09 17:51 ` [PATCH v2 11/15] dir-iterator: open root dir in dir_iterator_begin() Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 12/15] t0066: rename subtest descriptions Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order Plato Kiorpelidis
2022-05-10 13:07   ` Phillip Wood
2022-05-18 17:40     ` Plato Kiorpelidis
2022-05-18 17:47       ` rsbecker
2022-05-18 18:09         ` Junio C Hamano
2022-05-18 18:36           ` rsbecker
2022-05-09 17:51 ` [PATCH v2 14/15] dir-iterator: option to iterate dirs in post-order Plato Kiorpelidis
2022-05-09 17:51 ` [PATCH v2 15/15] entry.c: use dir-iterator to avoid explicit dir traversal Plato Kiorpelidis
2022-05-10 13:10   ` Phillip Wood
2022-05-10 13:13 ` [PATCH v2 00/15][GSoC] iterate dirs before or after their contents Phillip Wood
2022-05-10 16:31 ` Junio C Hamano
2022-05-20 17:43   ` Plato Kiorpelidis

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=xmqq7d6iwwnx.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kioplato@gmail.com \
    --cc=phillip.wood123@gmail.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.