Subject: Re: [PATCH v2 2/6] tests: Use `git submodule add` instead of `git add`

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Feb 2023 15:30:29 -0800

Calvin Wan <[email-masked]> writes:

> diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
> index b2bdd1fcb4..dd2858648b 100755
> --- a/t/t2013-checkout-submodule.sh
> +++ b/t/t2013-checkout-submodule.sh
> @@ -10,7 +10,7 @@ test_expect_success 'setup' '
>  	(cd submodule &&
>  	 git init &&
>  	 test_commit first) &&
> -	git add submodule &&
> +	git submodule add ./submodule &&

The change from "submodule" to "./submodule" was not explained in
the proposed log message.  I think this is necessary for "git
submodule add" to function as expected, but if that is why we are
making this change, perhaps we should mention it?

> @@ -51,6 +51,7 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .gitm
>  	git config diff.ignoreSubmodules none &&
>  	git config -f .gitmodules submodule.submodule.path submodule &&
>  	git config -f .gitmodules submodule.submodule.ignore untracked &&
> +	git commit -m "Update patterns in .gitmodules" .gitmodules &&

What does "patterns" refer here (another one in the next hunk)?

> diff --git a/t/t2103-update-index-ignore-missing.sh b/t/t2103-update-index-ignore-missing.sh
> index e9451cd567..11bc136f6e 100755
> --- a/t/t2103-update-index-ignore-missing.sh
> +++ b/t/t2103-update-index-ignore-missing.sh
> @@ -36,7 +36,7 @@ test_expect_success basics '
>  		git add file &&
>  		git commit -m "sub initial"
>  	) &&
> -	git add xyzzy &&
> +	git add ./xyzzy &&

Is this supposed to have become "git submodule add ./xyzzy"?  Or
"git add xyzzy" will trigger "don't add gitlink" warning but you can
write "git add ./xyzzy" as a way to work around the warning?

Or is this an incomplete change that wasn't spotted during
proofreading?

> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> index 302e4cbdba..f8ef70b5a2 100755
> --- a/t/t5531-deep-submodule-push.sh
> +++ b/t/t5531-deep-submodule-push.sh
> @@ -28,7 +28,7 @@ test_expect_success setup '
>  			git add junk &&
>  			git commit -m "Initial junk"
>  		) &&
> -		git add gar/bage &&
> +		git submodule add ./gar/bage ./gar/bage &&

Why does this one (and only this one) look different?  Everybody
else changed "git add A" to "git submodule add ./A", it seems?

> diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
> ...

I think I saw a code section that was touched in the previous patch
that hand-crafted .gitmodules file to make the gitlink it adds into
a submodule.  It is unexpected and puzzling that there is no removal
of that "cat >.gitmodules" from t4060.