Re: [PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)

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

Johannes Sixt <j6t@kdbg.org> writes:

> There are many instances where the treatment of symbolic links in the
> object model and the algorithms are tested, but where it is not
> necessary to actually have a symbolic link in the worktree. Make
> adjustments to the tests and remove the SYMLINKS prerequisite when
> appropriate in trivial cases, where "trivial" means:
>
> - merely a replacement of 'ln -s a b' to test_ln_s or of
>   'ln -s a b && git add b' to test_ln_s_add is needed;
>
> - a test for symbolic link on the file system can be split off (and
>   remains protected by SYMLINKS);
>
> - existing code is equivalent to test_ln_s[_add].

This is too big to review in one go, so I may have separate messages
later on this same patch.

> diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh
> index ff163cf..bd17ba2 100755
> --- a/t/t2003-checkout-cache-mkdir.sh
> +++ b/t/t2003-checkout-cache-mkdir.sh
> @@ -19,10 +19,10 @@ test_expect_success 'setup' '
>  	git update-index --add path0 path1/file1
>  '
>  
> -test_expect_success SYMLINKS 'have symlink in place where dir is expected.' '
> +test_expect_success 'have symlink in place where dir is expected.' '
>  	rm -fr path0 path1 &&
>  	mkdir path2 &&
> -	ln -s path2 path1 &&
> +	test_ln_s path2 path1 &&
>  	git checkout-index -f -a &&
>  	test ! -h path1 && test -d path1 &&
>  	test -f path1/file1 && test ! -f path2/file1

I do not think this hunk is correct.

We have two regular files in the index: path0, path1/file1, and we
add a symbolic link path1 that happens to point at directory path2/
in the working tree.

The test is about making sure that checkout-index is not confused by
the symbolic link in the working tree, by attempting to checkout
path1/file1.

Under the precondition checkout-index runs in this test, a casual

	echo rezrov >path1/file1

would leave path1 as a symlink without turning it into a real
directory, and we will end up creating path2/file1.  We are making
sure that checkout-index does not behave that way, and it is
essential to have symlink support in the working tree for the "bug"
to trigger.

On a filesystem without symbolic links, the patched test would pass
just fine, but there can be no aliasing between path1 and path2 in
the first place.

> @@ -79,10 +79,10 @@ test_expect_success SYMLINKS 'use --prefix=tmp/orary- where tmp is a symlink' '
>  	test -h tmp
>  '
>  
> -test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' '
> +test_expect_success 'use --prefix=tmp- where tmp-path1 is a symlink' '
>  	rm -fr path0 path1 path2 tmp* &&
>  	mkdir tmp1 &&
> -	ln -s tmp1 tmp-path1 &&
> +	test_ln_s tmp1 tmp-path1 &&
>  	git checkout-index --prefix=tmp- -f -a &&
>  	test -f tmp-path0 &&
>  	test ! -h tmp-path1 &&

This change has the same issue, I think.  We prepare tmp-path1
symbolic link to trap a casual "echo rezrov >tmp-path1/file1" to be
redirected to tmp1/file1 while leaving tmp-path1 as a symlink,
making sure we do the equivalent of "rm tmp-path1; mkdir tmp-path1"
before "echo rezrov >tmp-path1/file1".