From: Adrian Ratiu <adrian.ratiu@collabora.com>
Date: Wed, 19 Nov 2025 21:10:23 GMT
Subject: [PATCH v5 0/7] Encode submodule gitdir names to avoid conflicts
Message-ID: <20251119211030.2008441-1-adrian.ratiu@collabora.com>
In-Reply-To: <20250816213642.3517822-1-adrian.ratiu@collabora.com>

Hello everyone,

For those new to the series, we are adding an extension to fix submodule
gitdir path conflicts due to nested gitdirs or case-folding filesystems.

v5 contains code cleanups, more corner-case fixes and a few new commits
briefly described below, after which a range-diff follows.

As always this is based on the latest master, I've checked for conflicts
with the next/seen branches, pushed to GitHub [1] and CI passes [2].

Many thanks to all reviewers!

1: https://github.com/10ne1/git/tree/dev/aratiu/encoding-v5
2: https://github.com/10ne1/git/actions/runs/19515373994

Changes between v4 -> v5:
* Derive some new gitdir names in case the uppercase-extended encoding fails. (Aaron)
* Add more case-folding and extension disabling / cloning test corner-cases. (Aaron, Junio & Josh)
* Run the gitdir validation on the submodules.<name>.gitdir config after reading it. (Adrian)
* New commit: add a build-time option to enable the extension. (Josh)
* New commit: fallback to hashing if all encoding attempts fail. (Adrian)
* New commit: always validate the gitdir in submodule_name_to_git_dir. (Adrian)
* Split encoding validation in a separate function and leave non-enc validation intact. (Adrian)
* Add a CASE_INSENSITIVE_FS prereq for the case-folding tests. This will also significantly
  simplify case-folding validation, by not attempting to run on case-sensitive FS. (Adrian)
* Improve case-folding conflict detection by making validation more robust. (Adrian)
* Minor code fixes and improvements (spelling, error checking, indentation). (Adrian)

Range-diff between v4 -> v5:
1:  7d34507692 = 1:  9d5855f3bf submodule--helper: use submodule_name_to_gitdir in add_submodule
2:  1e609bdd1a = 2:  8cfa970a9d builtin/credential-store: move is_rfc3986_unreserved to url.[ch]
-:  ---------- > 3:  7bcadf1116 submodule: always validate gitdirs inside submodule_name_to_gitdir
3:  c51fd58669 ! 4:  1b5d0b50ef submodule: add extension to encode gitdir paths
    @@ Documentation/config/extensions.adoc: relativeWorktrees:::
      
     +submoduleEncoding:::
     +	If enabled, submodule gitdir paths are encoded to avoid filesystem
    -+	conflicts due to nested gitdirs, case insensitivity or other issues
    ++	conflicts due to nested gitdirs, case insensitivity or other issues.
     +	When enabled, the submodule.<name>.gitdir config is always set for
    -+	all submodulesand is the single point of authority for gitdir paths.
    ++	all submodules and is the single point of authority for gitdir paths.
     +
      worktreeConfig:::
      	If enabled, then worktrees will load config settings from the
    @@ submodule.c: int submodule_move_head(const char *path, const char *super_prefix,
      }
      
     +/*
    -+ * Find the last submodule name in the gitdir path (modules can be nested).
    -+ * Returns a pointer into `path` to the beginning of the name or NULL if not found.
    ++ * Encoded gitdir validation function used when extensions.submoduleEncoding is enabled.
    ++ * This does not print errors like the non-encoded version, because encoding is supposed
    ++ * to mitigate / fix all these.
     + */
    -+static char *find_last_submodule_name(char *git_dir_path)
    ++static int validate_submodule_encoded_git_dir(char *git_dir, const char *submodule_name UNUSED)
     +{
     +	const char *modules_marker = "/modules/";
    -+	char *p = git_dir_path;
    -+	char *last = NULL;
    ++	char *p = git_dir, *last_submodule_name = NULL;
     +
    ++	if (!the_repository->repository_format_submodule_encoding)
    ++		BUG("validate_submodule_encoded_git_dir() must be called with "
    ++		    "extensions.submoduleEncoding enabled.");
    ++
    ++	/* Find the last submodule name in the gitdir path (modules can be nested). */
     +	while ((p = strstr(p, modules_marker))) {
    -+		last = p + strlen(modules_marker);
    ++		last_submodule_name = p + strlen(modules_marker);
     +		p++;
     +	}
     +
    -+	return last;
    ++	/* Prevent the use of '/' in encoded names */
    ++	if (!last_submodule_name || strchr(last_submodule_name, '/'))
    ++		return -1;
    ++
    ++	return 0;
     +}
     +
    - int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
    + static 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;
    -+	char *p = git_dir + len - suffix_len;
    -+	bool suffixes_match = !strcmp(p, submodule_name);
    + 	char *p;
      	int ret = 0;
      
    --	if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
    --	    strcmp(p, submodule_name))
    --		BUG("submodule name '%s' not a suffix of git dir '%s'",
    --		    submodule_name, git_dir);
    --
    - 	/*
    - 	 * We prevent the contents of sibling submodules' git directories to
    - 	 * clash.
    -@@ submodule.c: int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
    - 	 * but the latter directory is already designated to contain the hooks
    - 	 * of the former.
    - 	 */
    --	for (; *p; p++) {
    -+	for (; *p && suffixes_match; p++) {
    - 		if (is_dir_sep(*p)) {
    - 			char c = *p;
    - 
    -@@ submodule.c: int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
    - 		}
    - 	}
    - 
    -+	/* tests after this check are only for encoded names, when the extension is enabled */
    -+	if (!the_repository->repository_format_submodule_encoding)
    -+		return 0;
    -+
    -+	/* Prevent the use of '/' in names */
    -+	p = find_last_submodule_name(git_dir);
    -+	if (p && strchr(p, '/') != NULL)
    -+		return error("submodule gitdir name '%s' contains unexpected '/'", p);
    ++	if (the_repository->repository_format_submodule_encoding)
    ++		BUG("validate_submodule_git_dir() must be called with "
    ++		    "extensions.submoduleEncoding disabled.");
     +
    - 	return 0;
    - }
    - 
    + 	if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
    + 	    strcmp(p, submodule_name))
    + 		BUG("submodule name '%s' not a suffix of git dir '%s'",
     @@ submodule.c: int submodule_to_gitdir(struct repository *repo,
      	return ret;
      }
    @@ submodule.c: int submodule_to_gitdir(struct repository *repo,
     +{
     +	char *key;
     +
    -+	if (validate_submodule_git_dir(gitdir_path->buf, submodule_name))
    ++	if (validate_submodule_encoded_git_dir(gitdir_path->buf, submodule_name))
     +		return -1;
     +
     +	key = xstrfmt("submodule.%s.gitdir", submodule_name);
    @@ submodule.c: int submodule_to_gitdir(struct repository *repo,
     +	FREE_AND_NULL(key);
     +
     +	return 0;
    -+
     +}
     +
      void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
    @@ submodule.c: int submodule_to_gitdir(struct repository *repo,
     +	strbuf_addstr(buf, submodule_name);
     +
     +	/* If extensions.submoduleEncoding is disabled, use the plain path set above */
    -+	if (!r->repository_format_submodule_encoding)
    -+		return;
    ++	if (!r->repository_format_submodule_encoding) {
    ++		if (validate_submodule_git_dir(buf->buf, submodule_name) < 0)
    ++			die(_("refusing to create/use '%s' in another submodule's "
    ++			      "git dir"), buf->buf);
    ++
    ++		return; /* plain gitdir is valid for use */
    ++	}
     +
     +	/* Extension is enabled: use the gitdir config if it exists */
     +	key = xstrfmt("submodule.%s.gitdir", submodule_name);
    @@ submodule.c: int submodule_to_gitdir(struct repository *repo,
     +		strbuf_reset(buf);
     +		strbuf_addstr(buf, gitdir);
     +		FREE_AND_NULL(key);
    ++
    ++		/* validate because users might have modified the config */
    ++		if (validate_submodule_encoded_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, gitdir);
    ++
     +		return;
     +	}
     +	FREE_AND_NULL(key);
    @@ submodule.c: int submodule_to_gitdir(struct repository *repo,
     +	strbuf_addstr_urlencode(buf, submodule_name, is_rfc3986_unreserved);
     +	if (!validate_and_set_submodule_gitdir(buf, submodule_name))
     +		return;
    -+
    -+	/* Case 3: error out */
    + 
    +-	if (validate_submodule_git_dir(buf->buf, submodule_name) < 0)
    +-		die(_("refusing to create/use '%s' in another submodule's "
    +-		      "git dir"), buf->buf);
    ++	/* Case 3: Nothing worked: error out */
     +	die(_("Cannot construct a valid gitdir path for submodule '%s': "
     +	      "please set a unique git config for 'submodule.%s.gitdir'."),
     +	    submodule_name, submodule_name);
    @@ t/t7425-submodule-encoding.sh (new)
     +		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.submoduleEncoding true &&
     +
     +		git submodule add ../new-sub "New Sub" &&
    -+		test_commit new
    ++		test_commit new &&
    ++
    ++		# retrigger the "die_path_inside_submodule" check with encoding
    ++		test_must_fail git submodule add ../new-sub "New Sub/nested2"
     +	)
     +'
     +
    @@ t/t7425-submodule-encoding.sh (new)
     +	verify_submodule_gitdir_path clone_parallel hippo/hooks modules/hippo%2fhooks
     +'
     +
    ++test_expect_success 'disabling extensions.submoduleEncoding prevents nested submodules' '
    ++	(
    ++		cd clone_nested &&
    ++		# disable extension and verify failure
    ++		git config extensions.submoduleEncoding false &&
    ++		test_must_fail git submodule add ./thing2 hippo/foobar &&
    ++		# re-enable extension and verify it works
    ++		git config extensions.submoduleEncoding true &&
    ++		git submodule add ./thing2 hippo/foobar
    ++	)
    ++'
    ++
     +test_done
     
      ## t/t9902-completion.sh ##
4:  01a5b10d5a < -:  ---------- submodule: fix case-folding gitdir filesystem colisions
-:  ---------- > 5:  2bf1c116a2 submodule: fix case-folding gitdir filesystem colisions
-:  ---------- > 6:  b607d7ca39 submodule: use hashed name for gitdir
-:  ---------- > 7:  9b4890cfd2 meson/Makefile: allow setting submodule encoding at build time

Adrian Ratiu (7):
  submodule--helper: use submodule_name_to_gitdir in add_submodule
  builtin/credential-store: move is_rfc3986_unreserved to url.[ch]
  submodule: always validate gitdirs inside submodule_name_to_gitdir
  submodule: add extension to encode gitdir paths
  submodule: fix case-folding gitdir filesystem colisions
  submodule: use hashed name for gitdir
  meson/Makefile: allow setting submodule encoding at build time

 Documentation/config/extensions.adoc  |   8 +
 Documentation/config/submodule.adoc   |   5 +
 Makefile                              |   5 +
 builtin/credential-store.c            |   7 +-
 builtin/submodule--helper.c           |  51 +++--
 configure.ac                          |  23 +++
 meson.build                           |   4 +
 meson_options.txt                     |   2 +
 repository.c                          |   1 +
 repository.h                          |   1 +
 setup.c                               |  15 ++
 setup.h                               |   1 +
 submodule.c                           | 234 ++++++++++++++++++-----
 submodule.h                           |   5 -
 t/lib-verify-submodule-gitdir-path.sh |  24 +++
 t/meson.build                         |   1 +
 t/t7425-submodule-encoding.sh         | 258 ++++++++++++++++++++++++++
 t/t9902-completion.sh                 |   1 +
 url.c                                 |  23 +++
 url.h                                 |   3 +
 20 files changed, 591 insertions(+), 81 deletions(-)
 create mode 100644 t/lib-verify-submodule-gitdir-path.sh
 create mode 100755 t/t7425-submodule-encoding.sh

-- 
2.51.0