Re: [PATCH 1/2] tests: make sure nested lazy prereqs work reliably

[Junio C Hamano <gitster@pobox.com>]

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

> This can be problematic, because lazy prereqs are evaluated in the
> '$TRASH_DIRECTORY/prereq-test-dir' directory, which is the same for
> every prereq, and which is automatically removed after the prereq has
> been evaluated.  So if the inner prereq (BAR above) is a lazy prereq
> that hasn't been evaluated yet, then after its evaluation the
> 'prereq-test-dir' shared with the outer prereq will be removed.

OK.

> Consequently, 'check-foo' will find itself in a non-existing
> directory, and won't be able to create/access any files in its cwd,
> which could result in an unfulfilled outer prereq.

I have a feeling that this would be filesystem dependent what
happens in an unlinked directory.  But in any case, if an outer lazy
prereq creates a file in the test directory, tries to evaluate
another lazy prereq (which would use the same directory and then
would remove everything in it and the directory itself when done),
and then expects that the file it created before evaluating the
inner lazy prereq is still there, it would not work, so I think this
is a good change.  I just think "won't be able to" is a bit too
strong here ("may not be able to (depending on the filesystem)", I
would buy).

> +test_lazy_prereq NESTED_INNER '
> +	>inner &&
> +	rm -f outer
> +'
> +test_lazy_prereq NESTED_PREREQ '
> +	>outer &&
> +	test_have_prereq NESTED_INNER &&
> +	echo "can create new file in cwd" >file &&
> +	test -f outer &&
> +	test ! -f inner
> +'

And the existence check for outer is exactly what I wrote above.
That would portably break without separte directory.  I do not know
if "can create new file" step would fail portably.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 8d59b90348..94395b9807 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -474,15 +474,15 @@ test_lazy_prereq () {
>  
>  test_run_lazy_prereq_ () {
>  	script='
> -mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" &&
> +mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-'"$1"'" &&
>  (
> -	cd "$TRASH_DIRECTORY/prereq-test-dir" &&'"$2"'
> +	cd "$TRASH_DIRECTORY/prereq-test-dir-'"$1"'" &&'"$2"'
>  )'

Qoting rules are a bit tricky here, but I think it does not matter
too much, as $1 (name or prereq) won't have $IFS or "'" or anything
funny in it.

>  	say >&3 "checking prerequisite: $1"
>  	say >&3 "$script"
>  	test_eval_ "$script"
>  	eval_ret=$?
> -	rm -rf "$TRASH_DIRECTORY/prereq-test-dir"
> +	rm -rf "$TRASH_DIRECTORY/prereq-test-dir-$1"

And this is obviously safe no matter what is in $1 ;-)

>  	if test "$eval_ret" = 0; then
>  		say >&3 "prerequisite $1 ok"
>  	else

Looks good.