Subject: [PATCH v2 08/10] submodule: remove validate_submodule_git_dir()

From: Adrian Ratiu <[email-masked]>
Date: Mon, 8 Sep 2025 17:01:15 +0300

The validate_submodule_git_dir test is not very useful anymore, after
submodule names are encoded to resolve gitdir path conflicts.

In other words, the purpouse of gitdir path encoding is precisely to
avoid such conflicts as this function tries to also prevent.

The first test from the function can be kept though, because it just
verifies invariants which should always be true and raise a BUG if:

  - no "/" separator is between dirs/names.
  - len(full_gitdir) < len(name).
  - name does not match the gitdir path suffix.

Thus we move the invariant checks to submodule_name_to_gitdir() and
clean up the rest of validate_submodule_git_dir() and its uses.

Signed-off-by: Adrian Ratiu <[email-masked]>
---
 builtin/submodule--helper.c | 21 -----------
 submodule.c                 | 74 ++++---------------------------------
 submodule.h                 |  5 ---
 3 files changed, 7 insertions(+), 93 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 564f7aadf8..8af7062ff2 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1725,10 +1725,6 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		clone_data_path = to_free = xstrfmt("%s/%s", repo_get_work_tree(the_repository),
 						    clone_data->path);
 
-	if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)
-		die(_("refusing to create/use '%s' in another submodule's "
-		      "git dir"), sm_gitdir);
-
 	if (!file_exists(sm_gitdir)) {
 		if (clone_data->require_init && !stat(clone_data_path, &st) &&
 		    !is_empty_dir(clone_data_path))
@@ -1802,23 +1798,6 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		free(path);
 	}
 
-	/*
-	 * We already performed this check at the beginning of this function,
-	 * before cloning the objects. This tries to detect racy behavior e.g.
-	 * in parallel clones, where another process could easily have made the
-	 * gitdir nested _after_ it was created.
-	 *
-	 * To prevent further harm coming from this unintentionally-nested
-	 * gitdir, let's disable it by deleting the `HEAD` file.
-	 */
-	if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0) {
-		char *head = xstrfmt("%s/HEAD", sm_gitdir);
-		unlink(head);
-		free(head);
-		die(_("refusing to create/use '%s' in another submodule's "
-		      "git dir"), sm_gitdir);
-	}
-
 	connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
 
 	p = repo_submodule_path(the_repository, clone_data_path, "config");
diff --git a/submodule.c b/submodule.c
index 016509806e..dbe1a7b091 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2163,27 +2163,10 @@ int submodule_move_head(const char *path, const char *super_prefix,
 			if (!submodule_uses_gitfile(path))
 				absorb_git_dir_into_superproject(path,
 								 super_prefix);
-			else {
-				char *dotgit = xstrfmt("%s/.git", path);
-				char *git_dir = xstrdup(read_gitfile(dotgit));
-
-				free(dotgit);
-				if (validate_submodule_git_dir(git_dir,
-							       sub->name) < 0)
-					die(_("refusing to create/use '%s' in "
-					      "another submodule's git dir"),
-					    git_dir);
-				free(git_dir);
-			}
 		} else {
 			struct strbuf gitdir = STRBUF_INIT;
 			submodule_name_to_gitdir(&gitdir, the_repository,
 						 sub->name);
-			if (validate_submodule_git_dir(gitdir.buf,
-						       sub->name) < 0)
-				die(_("refusing to create/use '%s' in another "
-				      "submodule's git dir"),
-				    gitdir.buf);
 			connect_work_tree_and_git_dir(path, gitdir.buf, 0);
 			strbuf_release(&gitdir);
 
@@ -2263,52 +2246,6 @@ int submodule_move_head(const char *path, const char *super_prefix,
 	return ret;
 }
 
-int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
-{
-	size_t len = strlen(git_dir), suffix_len = strlen(submodule_name);
-	char *p;
-	int ret = 0;
-
-	if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
-	    strcmp(p, submodule_name))
-		/*
-		 * TODO: revisit and cleanup this test short-circuit, because
-		 * submodules with encoded names are expected to take this path.
-		 * Likely just move the invariants to submodule_name_to_gitdir()
-		 * and delete this entire function in a future commit.
-		 */
-		return 0;
-
-	/*
-	 * We prevent the contents of sibling submodules' git directories to
-	 * clash.
-	 *
-	 * Example: having a submodule named `hippo` and another one named
-	 * `hippo/hooks` would result in the git directories
-	 * `.git/submodules/hippo/` and `.git/submodules/hippo/hooks/`, respectively,
-	 * but the latter directory is already designated to contain the hooks
-	 * of the former.
-	 */
-	for (; *p; p++) {
-		if (is_dir_sep(*p)) {
-			char c = *p;
-
-			*p = '\0';
-			if (is_git_directory(git_dir))
-				ret = -1;
-			*p = c;
-
-			if (ret < 0)
-				return error(_("submodule git dir '%s' is "
-					       "inside git dir '%.*s'"),
-					     git_dir,
-					     (int)(p - git_dir), git_dir);
-		}
-	}
-
-	return 0;
-}
-
 int validate_submodule_path(const char *path)
 {
 	char *p = xstrdup(path);
@@ -2367,9 +2304,6 @@ static void relocate_single_git_dir_into_superproject(const char *path,
 		die(_("could not lookup name for submodule '%s'"), path);
 
 	submodule_name_to_gitdir(&new_gitdir, the_repository, sub->name);
-	if (validate_submodule_git_dir(new_gitdir.buf, sub->name) < 0)
-		die(_("refusing to move '%s' into an existing git dir"),
-		    real_old_git_dir);
 	if (safe_create_leading_directories_const(the_repository, new_gitdir.buf) < 0)
 		die(_("could not create directory '%s'"), new_gitdir.buf);
 	real_new_git_dir = real_pathdup(new_gitdir.buf, 1);
@@ -2611,7 +2545,7 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
 {
 	struct strbuf encoded_sub_name = STRBUF_INIT, tmp = STRBUF_INIT;
 	size_t base_len, encoded_len;
-	char *gitdir_path, *key;
+	char *gitdir_path, *key, *p;
 	long name_max;
 
 	/* Allow config override. */
@@ -2655,5 +2589,11 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
 		die(_("encoded submodule name '%s' is too long (%"PRIuMAX" bytes, limit %"PRIuMAX")"),
 		    encoded_sub_name.buf, (uintmax_t)encoded_len, (uintmax_t)name_max);
 
+	/* Trigger a BUG if these invariants do not hold */
+	p = buf->buf + buf->len - encoded_len;
+	if (buf->len <= encoded_len || p[-1] != '/' || strcmp(p, encoded_sub_name.buf))
+		BUG("encoded submodule name '%s' is not a suffix of git dir '%s'",
+		    encoded_sub_name.buf, buf->buf);
+
 	strbuf_release(&encoded_sub_name);
 }
diff --git a/submodule.h b/submodule.h
index b10e16e6c0..0b7692bc20 100644
--- a/submodule.h
+++ b/submodule.h
@@ -137,11 +137,6 @@ int submodule_to_gitdir(struct repository *repo,
 void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
 			      const char *submodule_name);
 
-/*
- * Make sure that no submodule's git dir is nested in a sibling submodule's.
- */
-int validate_submodule_git_dir(char *git_dir, const char *submodule_name);
-
 /*
  * Make sure that the given submodule path does not follow symlinks.
  */
-- 
2.51.GIT