Re: [PATCH 00/41] use the stdlib EXIT_SUCCESS or EXIT_FAILURE exit status

From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Elia Pinto <gitter.spiros@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 00/41] use the stdlib EXIT_SUCCESS or EXIT_FAILURE exit status
Date: Wed, 23 Mar 2022 04:13:05 -0700	[thread overview]
Message-ID: <xmqqsfr8udku.fsf@gitster.g> (raw)
In-Reply-To: 220322.86r16unzer.gmgdl@evledraar.gmail.com

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> We also document for some of these programs that on failure we'll
> return 1 specifically, not whatever EXIT_FAILURE is.

I view this as a real issue.  EXIT_FAILURE could by happenstance be
defined to be the same value on all platforms we care about, but if
it leaves the possibility that the next major thing will break our
assumption, I do not see much point in adopting it.  Whole-sale
rewriting of 0 and 1 to EXIT_SUCCESS and EXIT_FAILURE smells like
adopting a bad standardization without thinking things through, only
for the sake of adopting "standardization".

> ... but we should fix the exit(-1) cases, and that's best done
> with some new GIT_TEST_ASSERT_NO_UNPORTABLE_EXIT testing or
> whatever.

That is probably a good #leftoverbit, even a candidate for future
#microprojects.

> I think we'd be much better off with something like this in
> git-compat-util.h:
>
>     #ifndef BYPASS_EXIT_SANITY
>     #ifdef EXIT_SUCCESS
>     #if EXIT_SUCCESS != 0
>     #error "git assumes EXIT_SUCCESS is 0, not whatever yours is, please report this. Build with -DBYPASS_EXIT_SANITY to continue building at your own risk"

This is not a good idea.  EXIT_SUCCESS does not have to be literally
0.  It only has to be a value that causes the process to exit with 0
when passed to exit().

>     #endif
>     #endif
>     #ifdef EXIT_FAILURE
>     #if EXIT_FAILURE != 0

I think you meant "!= 1".  If we were to take these 41 patches, we
must have this hunk, as we want our plumbing tools to be drivable by
shell scripts, i.e. 

	git foo ||
	case $? in
	1) # generic failure
		...
	esac

and we do not want to be forced to write something like

	. git-stdlib-util.sh ;# for platform-dependent $EXIT_FAILURE

	...
	git foo ||
	case $? in
	$EXIT_FAILURE) # generic failure
		...
	esac

instead.