Re: [PATCH 03/11] test-lib: introduce 'test_atexit'

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 18, 2019, 01:50 UTC
Message-ID: <xmqqwokx6jdo.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190314174024.GH28939@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

Show 11 quoted lines

>> but it even skips when there is no point doing the
>> test_eval_ of the "accumulated" scriptlet when it is empty.
>
> But this is not, because $test_cleanup is initialized to this special
> value and it can never be empty, and indeed 'test_eval_' uses this
> condition:
>
>   if test -z "$immediate" || test $eval_ret = 0 ||
>      test -n "$expecting_failure" && test "$test_cleanup" != ":"
>
> and it never checks $test_cleanup's emptiness.

Yeah, I used "empty" not in the literal sense; I know why the "empty in spirit" setting is a single colon for $test_cleanup.

I just did not realize that this new variable was using that exact pattern and using ":" as the empty in spirit, and that was where my fuzzy wordibng came from.

So in short, I misread the code, and part of that is because I was misled by the comment:

> +	# This condition and resetting 'test_atexit_cleanup' below makes
> +	# sure that the registered cleanup commands are run only once.
> +	test : != "$test_atexit_cleanup" || return 0

It over-stresses the "run only once", but the true value of this is that it avoids running an "empty in spirit" clean-up sequence.

The avoidance of double execution merely takes advantage of this implementation detail by "resetting" the variable is better explained where the "resetting" happens (i.e. "we reset the variable to the 'no-command' state, as we've run all of them here, but just before the process finally exits, the helper will be called and we do not want to run these commands again when it happens").

Thanks.