Re: [PATCH/WIP 03/11] t5403: avoid doing "git add foo/bar" where foo/.git exists

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> In this case, "foo" is considered a submodule and bar, if added,
> belongs to foo/.git. "git add" should only allow "git add foo" in this
> case, but it passes somehow.

I do not think the above description is correct.

The test:

 - populates the current directory;
 - makes a clone in ./clone2;
 - creates a file clone2/b;
 - runs "git add clone2/b" with GIT_DIR set to clone2/.git, without
   setting GIT_WORK_TREE nor having core.worktree in clone2/.git/config.

The last step should add a path "clone2/b" to $GIT_DIR/index (which is
clone2/.git/index in this case).  The clone2 is not a submodule to the top
level repository in this case, but even if it were, that would not change
the definition of what the command should do when GIT_DIR is set without
GIT_WORK_TREE nor core.worktree in $GIT_DIR/config.

Running (cd clone2 && git add b) is a _more natural_ thing to do, and it
will result in a path "b" added to the clone2 repository, so that the
result is more useful if you are going to chdir to the repository and keep
working on it.  But that does not mean the existing test is incorrect. It
does not just pass somehow but the test passes by design.

I did not check if later tests look at the contents of commit "new2" to
make sure it contains "clone2/b", but if they do this change should break
such tests.

So I am puzzled by this change; what is this trying to achieve?

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  t/t5403-post-checkout-hook.sh |   17 ++++++++++-------
>  1 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 1753ef2..3b3e2c1 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -16,10 +16,13 @@ test_expect_success setup '
>  	git update-ref refs/heads/master $commit0 &&
>  	git clone ./. clone1 &&
>  	git clone ./. clone2 &&
> -	GIT_DIR=clone2/.git git branch new2 &&
> -	echo Data for commit1. >clone2/b &&
> -	GIT_DIR=clone2/.git git add clone2/b &&
> -	GIT_DIR=clone2/.git git commit -m new2
> +	(
> +		cd clone2 &&
> +		git branch new2 &&
> +		echo Data for commit1. >b &&
> +		git add b &&
> +		git commit -m new2
> +	)
>  '
>  
>  for clone in 1 2; do
> @@ -48,7 +51,7 @@ test_expect_success 'post-checkout runs as expected ' '
>  '
>  
>  test_expect_success 'post-checkout args are correct with git checkout -b ' '
> -	GIT_DIR=clone1/.git git checkout -b new1 &&
> +	( cd clone1 && git checkout -b new1 ) &&
>  	old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
>  	new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
>  	flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
> @@ -56,7 +59,7 @@ test_expect_success 'post-checkout args are correct with git checkout -b ' '
>  '
>  
>  test_expect_success 'post-checkout receives the right args with HEAD changed ' '
> -	GIT_DIR=clone2/.git git checkout new2 &&
> +	( cd clone2 && git checkout new2 ) &&
>  	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
>  	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
>  	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
> @@ -64,7 +67,7 @@ test_expect_success 'post-checkout receives the right args with HEAD changed ' '
>  '
>  
>  test_expect_success 'post-checkout receives the right args when not switching branches ' '
> -	GIT_DIR=clone2/.git git checkout master b &&
> +	( cd clone2 && git checkout master b ) &&
>  	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
>  	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
>  	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&