Re: [PATCH v1 4/4] t1301: do not change $CWD in "shared=all" test case

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

Jiang Xin <worldhello.net@gmail.com> writes:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> In test case "shared=all", the working directory is permanently changed
> to the "sub" directory. This leads to a strange behavior that the
> temporary repositories created by subsequent test cases are all in this
> "sub" directory, such as "sub/new", "sub/child.git". If we bypass this
> test case, all subsequent test cases will have different working
> directory.
>
> Since the test case "shared=all" and all subsequent will work properly
> in the default test repository, we don't need to create and change to
> the "sub" directory in the test case "shared=all".

It is much worse than that.  If existing tests after this step were
running destructive operations in their "..", because we have this
extra "sub" directory and such a destructive test were running
there, the damage would have been contained in $TRASH_DIRECTORY but
with this change, it will touch t/ (or the parent directory of the
$TRASH_DIRECTORY).  So, "will work properly" may not be sufficient;
we need to audit the rest of the script and make sure there is no
such funny "step outside the test enviromnent" happening before we
are sure that this is a "safe" change.

It is the "right" thing to do, though.  I do not see any reason why
we want to have an extra level "sub" directory there.




>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  t/t1301-shared-repo.sh | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
> index 3ca91bf504..c4f2f72f6b 100755
> --- a/t/t1301-shared-repo.sh
> +++ b/t/t1301-shared-repo.sh
> @@ -46,8 +46,6 @@ do
>  done
>  
>  test_expect_success 'shared=all' '
> -	mkdir sub &&
> -	cd sub &&
>  	git init --template= --shared=all &&
>  	test 2 = $(git config core.sharedrepository)
>  '