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

From: Junio C Hamano <[email-masked]>
Date: Mon, 28 Feb 2022 23:00:57 -0800

Emily Shaffer <[email-masked]> writes:

> +	/*
> +	 * Note location of superproject's gitdir. Because the submodule already
> +	 * has a gitdir and local config, we can store this pointer from
> +	 * worktree config to worktree config, if the submodule has
> +	 * extensions.worktreeConfig set.
> +	 */

Probably the comment is a bit stale.  There is no longer a pointer
or location of superproject's gitdir recorded anywhere.

> +	strbuf_addf(&config_path, "%s/config", real_new_git_dir);
> +	git_configset_init(&sub_cs);
> +	git_configset_add_file(&sub_cs, config_path.buf);
> +
> +	git_config_set_in_file(config_path.buf, "submodule.hasSuperproject",
> +			       "true");
> +
> +	git_configset_clear(&sub_cs);
> +	strbuf_release(&config_path);
> +	strbuf_release(&sb);
>  	free(old_git_dir);
>  	free(real_old_git_dir);
>  	free(real_new_git_dir);
> 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"

Are we sufficiently happy to see the variable is set to anything, or
do we require it to be set to boolean true?

If the former, the above is fine, with trailing && added.

If the latter, then something like

	val=$(git config --type=bool "submodule.hasSuperproject") &&
	test "$val" = true &&

would be more appropriate, but I wonder something like

test_config_is () {
	local var expect val
	var="$1" expect="$2"
	shift 2
        val=$(git "$@" config --type=bool "$var") &&
	test "$val" = "$expect"
}

would be in order.  That would allow us to write

	test_config_is submodule.hasSuperproject true -C "$sub_dir" &&

>  	git -C "$sub_dir" clean -n -d -x >untracked
>  }
>  
> 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 &&

Are we testing that submodule.hasSuperproject is set, and that
it can successfully be unset?  "config --unset no.such.var" will
exit with non-zero status.

> +	 git submodule update &&
> +	 git -C submodule config submodule.hasSuperproject

Ditto.