Re: [PATCH v8 2/3] introduce submodule.hasSuperproject record

From: Glen Choo <chooglen@google.com>
Date: Mar 8, 2022, 22:13 UTC
Message-ID: <kl6lee3c5bzl.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <20220301002613.1459916-3-emilyshaffer@google.com>

Reviewing this series lightly because I will need to base 'ar/submodule-update reroll pt 2' on this (pt.1 is at https://lore.kernel.org/git/20220305001401.20888-1-chooglen@google.com).

Emily Shaffer <emilyshaffer@google.com> writes:

Show 14 quoted lines

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 652861aa66..59dffda775 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -449,6 +449,9 @@ cmd_update()
>  			;;
>  		esac
>  
> +		# Note that the submodule is a submodule.
> +		git -C "$sm_path" config submodule.hasSuperproject "true"
> +
>  		if test -n "$recursive"
>  		then
>  			(

This hunk has a textual conflict with 'ar/submodule-update reroll pt 2', but the fix is easy - just teach "git submodule--helper update" to set the config in C.

Show 17 quoted lines

> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index 11cccbb333..422c3cc343 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -1061,4 +1061,12 @@ test_expect_success 'submodule update --quiet passes quietness to fetch with a s
>  	)
>  '
>  
> +test_expect_success 'submodule update adds submodule.hasSuperproject to older repos' '
> +	(cd super &&
> +	 git -C submodule config --unset submodule.hasSuperproject &&
> +	 git submodule update &&
> +	 git -C submodule config submodule.hasSuperproject
> +	)
> +'
> +
>  test_done

I think there is a gap in the test coverage. I notice that this doesn't test that we set submodule.hasSuperproject when the submodule is cloned for the first time with 'git submodule update'. I thought that maybe the test for this was here...

Show 15 quoted lines

> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 40cf8d89aa..833fa01961 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -115,6 +115,10 @@ inspect() {
>  	git -C "$sub_dir" rev-parse HEAD >head-sha1 &&
>  	git -C "$sub_dir" update-index --refresh &&
>  	git -C "$sub_dir" diff-files --exit-code &&
> +
> +	# Ensure that submodule.hasSuperproject is set.
> +	git -C "$sub_dir" config "submodule.hasSuperproject"
> +
>  	git -C "$sub_dir" clean -n -d -x >untracked
>  }
>

But when I removed the "set submodule.hasSuperproject in submodule" line, i.e.

 		git -C "$sm_path" config submodule.hasSuperproject "true"

t7400 still passes.