Re: [PATCH v7 04/11] submodule: introduce extensions.submodulePathConfig

> diff --git a/Documentation/config/submodule.adoc b/Documentation/config/submodule.adoc
> index 0672d99117..9c260a69f6 100644
> --- a/Documentation/config/submodule.adoc
> +++ b/Documentation/config/submodule.adoc
> @@ -52,6 +52,13 @@ submodule.<name>.active::
>  	submodule.active config option. See linkgit:gitsubmodules[7] for
>  	details.
>  
> +submodule.<name>.gitdir::
> +	This sets the gitdir path for submodule <name>. This configuration is
> +	respected when `extensions.submodulePathConfig` is enabled, otherwise it
> +	has no effect. When enabled, this config becomes the single source of
> +	truth for submodule gitdir paths and git will error if it is missing.

> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 3bc139ff9c..f8cae345a5 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -435,6 +435,48 @@ struct init_cb {
>  };
>  #define INIT_CB_INIT { 0 }
>  
> +static int validate_and_set_submodule_gitdir(struct strbuf *gitdir_path,
> +					     const char *submodule_name)
> +{
> +	const char *value;
> +	char *key;
> +
> +	if (validate_submodule_git_dir(gitdir_path->buf, submodule_name))
> +		return -1;
> +
> +	 key = xstrfmt("submodule.%s.gitdir", submodule_name);

> diff --git a/submodule.c b/submodule.c
> index f645372a18..e3692009cd 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -2570,30 +2571,35 @@ int submodule_to_gitdir(struct repository *repo,
>  void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
>  			      const char *submodule_name)
>  {
> -	/*
> -	 * NEEDSWORK: The current way of mapping a submodule's name to
> -	 * its location in .git/modules/ has problems with some naming
> -	 * schemes. For example, if a submodule is named "foo" and
> -	 * another is named "foo/bar" (whether present in the same
> -	 * superproject commit or not - the problem will arise if both
> -	 * superproject commits have been checked out at any point in
> -	 * time), or if two submodule names only have different cases in
> -	 * a case-insensitive filesystem.
> -	 *
> -	 * There are several solutions, including encoding the path in
> -	 * some way, introducing a submodule.<name>.gitdir config in
> -	 * .git/config (not .gitmodules) that allows overriding what the
> -	 * gitdir of a submodule would be (and teach Git, upon noticing
> -	 * a clash, to automatically determine a non-clashing name and
> -	 * to write such a config), or introducing a
> -	 * submodule.<name>.gitdir config in .gitmodules that repo
> -	 * administrators can explicitly set. Nothing has been decided,
> -	 * so for now, just append the name at the end of the path.
> -	 */
> -	repo_git_path_append(r, buf, "modules/");
> -	strbuf_addstr(buf, submodule_name);
> +	if (!r->repository_format_submodule_path_cfg) {
> +		/*
> +		 * If extensions.submodulePathConfig is disabled,
> +		 * continue to use the plain path.
> +		 */
> +		repo_git_path_append(r, buf, "modules/%s", submodule_name);
> +	} else {
> +		const char *gitdir;
> +		char *key;
> +		int ret;
> +
> +		/* Otherwise the extension is enabled, so use the gitdir config. */
> +		key = xstrfmt("submodule.%s.gitdir", submodule_name);
> +		ret = repo_config_get_string_tmp(r, key, &gitdir);
> +		FREE_AND_NULL(key);
> +
> +		if (ret)
> +			die(_("the 'submodule.%s.gitdir' config does not exist for module '%s'. "
> +			      "Please ensure it is set, for example by running something like: "
> +			      "'git config submodule.%s.gitdir .git/modules/%s'. For details "
> +			      "see the extensions.submodulePathConfig documentation."),
> +			    submodule_name, submodule_name, submodule_name, submodule_name);
> +
> +		strbuf_addstr(buf, gitdir);
> +	}
>  
> -	if (validate_submodule_git_dir(buf->buf, submodule_name) < 0)
> -		die(_("refusing to create/use '%s' in another submodule's "
> -		      "git dir"), buf->buf);
> +	/* validate because users might have modified the config */
> +	if (validate_submodule_git_dir(buf->buf, submodule_name))
> +		die(_("invalid 'submodule.%s.gitdir' config: '%s' please check "
> +		      "if it is unique or conflicts with another module"),
> +		    submodule_name, buf->buf);
>  }

> diff --git a/t/lib-verify-submodule-gitdir-path.sh b/t/lib-verify-submodule-gitdir-path.sh
> new file mode 100644
> index 0000000000..62794df976
> --- /dev/null
> +++ b/t/lib-verify-submodule-gitdir-path.sh
> @@ -0,0 +1,24 @@
> +# Helper to verify if repo $1 contains a submodule named $2 with gitdir path $3
> +
> +# This does not check filesystem existence. That is done in submodule.c via the
> +# submodule_name_to_gitdir() API which this helper ends up calling. The gitdirs
> +# might or might not exist (e.g. when adding a new submodule), so this only
> +# checks the expected configuration path, which might be overridden by the user.
> +
> +verify_submodule_gitdir_path() {

> diff --git a/t/t7425-submodule-gitdir-path-extension.sh b/t/t7425-submodule-gitdir-path-extension.sh
> new file mode 100755
> index 0000000000..5d52a289f8
> --- /dev/null
> +++ b/t/t7425-submodule-gitdir-path-extension.sh
> @@ -0,0 +1,138 @@
> +#!/bin/sh
> +
> +test_description='submodulePathConfig extension works as expected'

> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-verify-submodule-gitdir-path.sh
> +
> +test_expect_success 'setup: allow file protocol' '
> +       git config --global protocol.file.allow always
> +'
> +
> +test_expect_success 'create repo with mixed extension submodules' '
> +	git init -b main legacy-sub &&
> +	test_commit -C legacy-sub legacy-initial &&
> +	legacy_rev=$(git -C legacy-sub rev-parse HEAD) &&
> +
> +	git init -b main new-sub &&
> +	test_commit -C new-sub new-initial &&
> +	new_rev=$(git -C new-sub rev-parse HEAD) &&
> +
> +	git init -b main main &&
> +	(
> +		cd main &&
> +		git submodule add ../legacy-sub legacy &&
> +		test_commit legacy-sub &&
> +
> +		# trigger the "die_path_inside_submodule" check
> +		test_must_fail git submodule add ../new-sub "legacy/nested" &&
> +
> +		git config core.repositoryformatversion 1 &&
> +		git config extensions.submodulePathConfig true &&
> +
> +		git submodule add ../new-sub "New Sub" &&
> +		test_commit new &&
> +
> +		# retrigger the "die_path_inside_submodule" check with encoding
> +		test_must_fail git submodule add ../new-sub "New Sub/nested2"
> +       )
> +'
> +
> +test_expect_success 'verify new submodule gitdir config' '
> +	git -C main config submodule."New Sub".gitdir > actual &&
> +	echo ".git/modules/New Sub" > expect &&