submodule: introduce extensions.submodulePathConfig

The idea of this extension is to abstract away the submodule gitdir
path implementation: everyone is expected to use the config and not
worry about how the path is computed internally, either in git or
other implementations.

With this extension enabled, the submodule.<name>.gitdir repo config
becomes the single source of truth for all submodule gitdir paths.

The submodule.<name>.gitdir config is added automatically for all new
submodules when this extension is enabled.

Git will throw an error if the extension is enabled and a config is
missing, advising users how to migrate. Migration is manual for now.

E.g. to add a missing config entry for an existing "foo" module:
git config submodule.foo.gitdir .git/modules/foo

Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Documentation/config/extensions.adoc

@@ -73,6 +73,29 @@ relativeWorktrees:::
 	repaired with either the `--relative-paths` option or with the
 	`worktree.useRelativePaths` config set to `true`.
 
+submodulePathConfig:::
+	This extension is for the minority of users who:
++
+--
+* Encounter errors like `refusing to create ... in another submodule's git dir`
+  due to a number of reasons, like case-insensitive filesystem conflicts when
+  creating modules named `foo` and `Foo`.
+* Require more flexible submodule layouts, for example due to nested names like
+  `foo`, `foo/bar` and `foo/baz` not supported by the default gitdir mechanism
+  which uses `.git/modules/<plain-name>` locations, causing further conflicts.
+--
++
+When `extensions.submodulePathConfig` is enabled, the `submodule.<name>.gitdir`
+config becomes the single source of truth for all submodule gitdir paths and is
+automatically set for all new submodules both during clone and init operations.
++
+Git will error out if a module does not have a corresponding
+`submodule.<name>.gitdir` set.
++
+Existing (pre-extension) submodules need to be migrated by adding the missing
+config entries. This is done manually for now, e.g. for each submodule:
+`git config submodule.<name>.gitdir .git/modules/<name>`.
+
 worktreeConfig:::
 	If enabled, then worktrees will load config settings from the
 	`$GIT_DIR/config.worktree` file in addition to the

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.
+	See linkgit:git-config[1] for details.
+
 submodule.active::
 	A repeated field which contains a pathspec used to match against a
 	submodule's path to determine if the submodule is of interest to git

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);
+
+	/* Nothing to do if the config already exists. */
+	if (!repo_config_get_string_tmp(the_repository, key, &value)) {
+		free(key);
+		return 0;
+	}
+
+	if (repo_config_set_gently(the_repository, key, gitdir_path->buf)) {
+		free(key);
+		return -1;
+	}
+
+	free(key);
+	return 0;
+}
+
+static void create_default_gitdir_config(const char *submodule_name)
+{
+	struct strbuf gitdir_path = STRBUF_INIT;
+
+	repo_git_path_append(the_repository, &gitdir_path, "modules/%s", submodule_name);
+	if (!validate_and_set_submodule_gitdir(&gitdir_path, submodule_name)) {
+		strbuf_release(&gitdir_path);
+		return;
+	}
+
+	die(_("failed to set a valid default config for 'submodule.%s.gitdir'. "
+	      "Please ensure it is set, for example by running something like: "
+	      "'git config submodule.%s.gitdir .git/modules/%s'"),
+	    submodule_name, submodule_name, submodule_name);
+}
+
 static void init_submodule(const char *path, const char *prefix,
 			   const char *super_prefix,
 			   unsigned int flags)
@@ -511,6 +553,10 @@ static void init_submodule(const char *path, const char *prefix,
 		if (repo_config_set_gently(the_repository, sb.buf, upd))
 			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
 	}
+
+	if (the_repository->repository_format_submodule_path_cfg)
+		create_default_gitdir_config(sub->name);
+
 	strbuf_release(&sb);
 	free(displaypath);
 	free(url);
@@ -1805,8 +1851,9 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		char *head = xstrfmt("%s/HEAD", sm_gitdir);
 		unlink(head);
 		free(head);
-		die(_("refusing to create/use '%s' in another submodule's "
+		die(_("refusing to create/use '%s' in another submodule's git dir. "
-		      "git dir"), sm_gitdir);
+		      "Enabling extensions.submodulePathConfig should fix this."),
+		    sm_gitdir);
 	}
 
 	connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
@@ -3578,6 +3625,9 @@ static int module_add(int argc, const char **argv, const char *prefix,
 	add_data.progress = !!progress;
 	add_data.dissociate = !!dissociate;
 
+	if (the_repository->repository_format_submodule_path_cfg)
+		create_default_gitdir_config(add_data.sm_name);
+
 	if (add_submodule(&add_data))
 		goto cleanup;
 	configure_added_submodule(&add_data);

repository.c

@@ -288,6 +288,7 @@ int repo_init(struct repository *repo,
 	repo->repository_format_worktree_config = format.worktree_config;
 	repo->repository_format_relative_worktrees = format.relative_worktrees;
 	repo->repository_format_precious_objects = format.precious_objects;
+	repo->repository_format_submodule_path_cfg = format.submodule_path_cfg;
 
 	/* take ownership of format.partial_clone */
 	repo->repository_format_partial_clone = format.partial_clone;

repository.h

@@ -158,6 +158,7 @@ struct repository {
 	int repository_format_worktree_config;
 	int repository_format_relative_worktrees;
 	int repository_format_precious_objects;
+	int repository_format_submodule_path_cfg;
 
 	/* Indicate if a repository has a different 'commondir' from 'gitdir' */
 	unsigned different_commondir:1;

setup.c

@@ -687,6 +687,9 @@ static enum extension_result handle_extension(const char *var,
 	} else if (!strcmp(ext, "relativeworktrees")) {
 		data->relative_worktrees = git_config_bool(var, value);
 		return EXTENSION_OK;
+	} else if (!strcmp(ext, "submodulepathconfig")) {
+		data->submodule_path_cfg = git_config_bool(var, value);
+		return EXTENSION_OK;
 	}
 	return EXTENSION_UNKNOWN;
 }
@@ -1865,6 +1868,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				repo_fmt.worktree_config;
 			the_repository->repository_format_relative_worktrees =
 				repo_fmt.relative_worktrees;
+			the_repository->repository_format_submodule_path_cfg =
+				repo_fmt.submodule_path_cfg;
 			/* take ownership of repo_fmt.partial_clone */
 			the_repository->repository_format_partial_clone =
 				repo_fmt.partial_clone;
@@ -1963,6 +1968,8 @@ void check_repository_format(struct repository_format *fmt)
 				    fmt->ref_storage_format);
 	the_repository->repository_format_worktree_config =
 		fmt->worktree_config;
+	the_repository->repository_format_submodule_path_cfg =
+		fmt->submodule_path_cfg;
 	the_repository->repository_format_relative_worktrees =
 		fmt->relative_worktrees;
 	the_repository->repository_format_partial_clone =

setup.h

@@ -130,6 +130,7 @@ struct repository_format {
 	char *partial_clone; /* value of extensions.partialclone */
 	int worktree_config;
 	int relative_worktrees;
+	int submodule_path_cfg;
 	int is_bare;
 	int hash_algo;
 	int compat_hash_algo;

submodule.c

@@ -31,6 +31,7 @@
 #include "commit-reach.h"
 #include "read-cache-ll.h"
 #include "setup.h"
+#include "advice.h"
 
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
 static int initialized_fetch_ref_tips;
@@ -2164,8 +2165,9 @@ int submodule_move_head(const char *path, const char *super_prefix,
 				if (validate_submodule_git_dir(git_dir,
 							       sub->name) < 0)
 					die(_("refusing to create/use '%s' in "
-					      "another submodule's git dir"),
+					      "another submodule's git dir. "
-					    git_dir);
+					      "Enabling extensions.submodulePathConfig "
+					      "should fix this."), git_dir);
 				free(git_dir);
 			}
 		} else {
@@ -2576,30 +2578,37 @@ cleanup:
 void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
 			      const char *submodule_name)
 {
-	/*
+	if (!r->repository_format_submodule_path_cfg) {
-	 * NEEDSWORK: The current way of mapping a submodule's name to
+		/*
-	 * its location in .git/modules/ has problems with some naming
+		 * If extensions.submodulePathConfig is disabled,
-	 * schemes. For example, if a submodule is named "foo" and
+		 * continue to use the plain path.
-	 * another is named "foo/bar" (whether present in the same
+		 */
-	 * superproject commit or not - the problem will arise if both
+		repo_git_path_append(r, buf, "modules/%s", submodule_name);
-	 * superproject commits have been checked out at any point in
+	} else {
-	 * time), or if two submodule names only have different cases in
+		const char *gitdir;
-	 * a case-insensitive filesystem.
+		char *key;
-	 *
+		int ret;
-	 * 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 (validate_submodule_git_dir(buf->buf, submodule_name) < 0)
+		/* 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);
+	}
+
+	/* validate because users might have modified the config */
+	if (validate_submodule_git_dir(buf->buf, submodule_name)) {
+		advise(_("enabling extensions.submodulePathConfig might fix the "
+			 "following error, if it's not already enabled."));
 		die(_("refusing to create/use '%s' in another submodule's "
-		      "git dir"), buf->buf);
+		      " git dir."), buf->buf);
+	}
 }

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 () {
+	repo="$1" &&
+	name="$2" &&
+	path="$3" &&
+	(
+		cd "$repo" &&
+		# Compute expected absolute path
+		expected="$(git rev-parse --git-common-dir)/$path" &&
+		expected="$(test-tool path-utils real_path "$expected")" &&
+		# Compute actual absolute path
+		actual="$(git submodule--helper gitdir "$name")" &&
+		actual="$(test-tool path-utils real_path "$actual")" &&
+		echo "$expected" >expect &&
+		echo "$actual" >actual &&
+		test_cmp expect actual
+	)
+}

t/meson.build

@@ -884,6 +884,7 @@ integration_tests = [
   't7422-submodule-output.sh',
   't7423-submodule-symlinks.sh',
   't7424-submodule-mixed-ref-formats.sh',
+  't7425-submodule-gitdir-path-extension.sh',
   't7450-bad-git-dotfiles.sh',
   't7500-commit-template-squash-signoff.sh',
   't7501-commit-basic-functionality.sh',

t/t7425-submodule-gitdir-path-extension.sh

@@ -0,0 +1,160 @@
+#!/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 &&
+	test_cmp expect actual &&
+	verify_submodule_gitdir_path main "New Sub" "modules/New Sub"
+'
+
+test_expect_success 'manual add and verify legacy submodule gitdir config' '
+	# the legacy module should not contain a gitdir config, because it
+	# was added before the extension was enabled. Add and test it.
+	test_must_fail git -C main config submodule.legacy.gitdir &&
+	git -C main config submodule.legacy.gitdir .git/modules/legacy &&
+	git -C main config submodule.legacy.gitdir >actual &&
+	echo ".git/modules/legacy" >expect &&
+	test_cmp expect actual &&
+	verify_submodule_gitdir_path main "legacy" "modules/legacy"
+'
+
+test_expect_success 'gitdir config path is relative for both absolute and relative urls' '
+	test_when_finished "rm -rf relative-cfg-path-test" &&
+	git init -b main relative-cfg-path-test &&
+	(
+		cd relative-cfg-path-test &&
+		git config core.repositoryformatversion 1 &&
+		git config extensions.submodulePathConfig true &&
+
+		# Test with absolute URL
+		git submodule add "$TRASH_DIRECTORY/new-sub" sub-abs &&
+		git config submodule.sub-abs.gitdir >actual &&
+		echo ".git/modules/sub-abs" >expect &&
+		test_cmp expect actual &&
+
+		# Test with relative URL
+		git submodule add ../new-sub sub-rel &&
+		git config submodule.sub-rel.gitdir >actual &&
+		echo ".git/modules/sub-rel" >expect &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'clone from repo with both legacy and new-style submodules' '
+	git clone --recurse-submodules main cloned-non-extension &&
+	(
+		cd cloned-non-extension &&
+
+		test_path_is_dir .git/modules/legacy &&
+		test_path_is_dir .git/modules/"New Sub" &&
+
+		test_must_fail git config submodule.legacy.gitdir &&
+		test_must_fail git config submodule."New Sub".gitdir &&
+
+		git submodule status >list &&
+		test_grep "$legacy_rev legacy" list &&
+		test_grep "$new_rev New Sub" list
+	) &&
+
+	git clone -c extensions.submodulePathConfig=true --recurse-submodules main cloned-extension &&
+	(
+		cd cloned-extension &&
+
+		test_path_is_dir .git/modules/legacy &&
+		test_path_is_dir ".git/modules/New Sub" &&
+
+		git config submodule.legacy.gitdir &&
+		git config submodule."New Sub".gitdir &&
+
+		git submodule status >list &&
+		test_grep "$legacy_rev legacy" list &&
+		test_grep "$new_rev New Sub" list
+	)
+'
+
+test_expect_success 'commit and push changes to encoded submodules' '
+	git -C legacy-sub config receive.denyCurrentBranch updateInstead &&
+	git -C new-sub config receive.denyCurrentBranch updateInstead &&
+	git -C main config receive.denyCurrentBranch updateInstead &&
+	(
+		cd cloned-extension &&
+
+		git -C legacy switch --track -C main origin/main  &&
+		test_commit -C legacy second-commit &&
+		git -C legacy push &&
+
+		git -C "New Sub" switch --track -C main origin/main &&
+		test_commit -C "New Sub" second-commit &&
+		git -C "New Sub" push &&
+
+		# Stage and commit submodule changes in superproject
+		git switch --track -C main origin/main  &&
+		git add legacy "New Sub" &&
+		git commit -m "update submodules" &&
+
+		# push superproject commit to main repo
+		git push
+	) &&
+
+	# update expected legacy & new submodule checksums
+	legacy_rev=$(git -C legacy-sub rev-parse HEAD) &&
+	new_rev=$(git -C new-sub rev-parse HEAD)
+'
+
+test_expect_success 'fetch mixed submodule changes and verify updates' '
+	(
+		cd main &&
+
+		# only update submodules because superproject was
+		# pushed into at the end of last test
+		git submodule update --init --recursive &&
+
+		test_path_is_dir .git/modules/legacy &&
+		test_path_is_dir ".git/modules/New Sub" &&
+
+		# Verify both submodules are at the expected commits
+		git submodule status >list &&
+		test_grep "$legacy_rev legacy" list &&
+		test_grep "$new_rev New Sub" list
+	)
+'
+
+test_done

t/t9902-completion.sh

@@ -3053,6 +3053,7 @@ test_expect_success 'git config set - variable name - __git_compute_second_level
 	submodule.sub.fetchRecurseSubmodules Z
 	submodule.sub.ignore Z
 	submodule.sub.active Z
+	submodule.sub.gitdir Z
 	EOF
 '