Merge branch 'ar/submodule-gitdir-tweak' into seen

Avoid local submodule repository directory paths overlapping with
each other by encoding submodule names before using them as path
components.

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

Author: gitster

Documentation/config/extensions.adoc

@@ -73,6 +73,14 @@ relativeWorktrees:::
 	repaired with either the `--relative-paths` option or with the
 	`worktree.useRelativePaths` config set to `true`.
 
+submoduleEncoding:::
+	If enabled, submodule gitdir paths are encoded to avoid filesystem
+	conflicts due to nested gitdirs, case insensitivity or other issues.
+	When enabled, the submodule.<name>.gitdir config is always set for
+	all submodules and is the single point of authority for gitdir paths.
+	Can also be enabled via the submodule-encoding build option. The repo
+	config takes precedence over the build-time default.
+
 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,11 @@ submodule.<name>.active::
 	submodule.active config option. See linkgit:gitsubmodules[7] for
 	details.
 
+submodule.<name>.gitdir::
+	This option sets the gitdir path for submodule <name>, allowing users to
+	override the default path. Only works when `extensions.submoduleEncoding`
+	is enabled, otherwise does nothing. 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

Makefile

@@ -2348,6 +2348,11 @@ ifdef INCLUDE_LIBGIT_RS
 	BASIC_CFLAGS += -fvisibility=hidden
 endif
 
+ifdef SUBMODULE_ENCODING_BY_DEFAULT
+	# Set submoduleEncoding extension default specified at build time
+	BASIC_CFLAGS += -DSUBMODULE_ENCODING_BY_DEFAULT=$(SUBMODULE_ENCODING_BY_DEFAULT)
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif

builtin/credential-store.c

@@ -7,6 +7,7 @@
 #include "path.h"
 #include "string-list.h"
 #include "parse-options.h"
+#include "url.h"
 #include "write-or-die.h"
 
 static struct lock_file credential_lock;
@@ -76,12 +77,6 @@ static void rewrite_credential_file(const char *fn, struct credential *c,
 		die_errno("unable to write credential store");
 }
 
-static int is_rfc3986_unreserved(char ch)
-{
-	return isalnum(ch) ||
-		ch == '-' || ch == '_' || ch == '.' || ch == '~';
-}
-
 static int is_rfc3986_reserved_or_unreserved(char ch)
 {
 	if (is_rfc3986_unreserved(ch))

builtin/submodule--helper.c

@@ -1204,6 +1204,22 @@ static int module_summary(int argc, const char **argv, const char *prefix,
 	return ret;
 }
 
+static int module_gitdir(int argc, const char **argv, const char *prefix UNUSED,
+			 struct repository *repo)
+{
+	struct strbuf gitdir = STRBUF_INIT;
+
+	if (argc != 2)
+		usage(_("git submodule--helper gitdir <name>"));
+
+	submodule_name_to_gitdir(&gitdir, repo, argv[1]);
+
+	printf("%s\n", gitdir.buf);
+
+	strbuf_release(&gitdir);
+	return 0;
+}
+
 struct sync_cb {
 	const char *prefix;
 	const char *super_prefix;
@@ -1699,10 +1715,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))
@@ -1776,23 +1788,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");
@@ -3190,13 +3185,13 @@ static void append_fetch_remotes(struct strbuf *msg, const char *git_dir_path)
 
 static int add_submodule(const struct add_data *add_data)
 {
-	char *submod_gitdir_path;
 	struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;
 	struct string_list reference = STRING_LIST_INIT_NODUP;
 	int ret = -1;
 
 	/* perhaps the path already exists and is already a git repo, else clone it */
 	if (is_directory(add_data->sm_path)) {
+		char *submod_gitdir_path;
 		struct strbuf sm_path = STRBUF_INIT;
 		strbuf_addstr(&sm_path, add_data->sm_path);
 		submod_gitdir_path = xstrfmt("%s/.git", add_data->sm_path);
@@ -3210,10 +3205,11 @@ static int add_submodule(const struct add_data *add_data)
 		free(submod_gitdir_path);
 	} else {
 		struct child_process cp = CHILD_PROCESS_INIT;
+		struct strbuf submod_gitdir = STRBUF_INIT;
 
-		submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name);
+		submodule_name_to_gitdir(&submod_gitdir, the_repository, add_data->sm_name);
 
-		if (is_directory(submod_gitdir_path)) {
+		if (is_directory(submod_gitdir.buf)) {
 			if (!add_data->force) {
 				struct strbuf msg = STRBUF_INIT;
 				char *die_msg;
@@ -3222,8 +3218,8 @@ static int add_submodule(const struct add_data *add_data)
 						    "locally with remote(s):\n"),
 					    add_data->sm_name);
 
-				append_fetch_remotes(&msg, submod_gitdir_path);
-				free(submod_gitdir_path);
+				append_fetch_remotes(&msg, submod_gitdir.buf);
+				strbuf_release(&submod_gitdir);
 
 				strbuf_addf(&msg, _("If you want to reuse this local git "
 						    "directory instead of cloning again from\n"
@@ -3241,7 +3237,7 @@ static int add_submodule(const struct add_data *add_data)
 					 "submodule '%s'\n"), add_data->sm_name);
 			}
 		}
-		free(submod_gitdir_path);
+		strbuf_release(&submod_gitdir);
 
 		clone_data.prefix = add_data->prefix;
 		clone_data.path = add_data->sm_path;
@@ -3594,6 +3590,7 @@ int cmd_submodule__helper(int argc,
 		NULL
 	};
 	struct option options[] = {
+		OPT_SUBCOMMAND("gitdir", &fn, module_gitdir),
 		OPT_SUBCOMMAND("clone", &fn, module_clone),
 		OPT_SUBCOMMAND("add", &fn, module_add),
 		OPT_SUBCOMMAND("update", &fn, module_update),

configure.ac

@@ -229,6 +229,29 @@ AC_ARG_ENABLE([cssmin],
   GIT_CONF_SUBST([CSSMIN])
 ])
 
+# Define option to enable the submodule encoding extension by default
+AC_ARG_ENABLE([submodule-encoding],
+ [AS_HELP_STRING([--enable-submodule-encoding],
+  [Enable the submoduleEncoding extension by default at build time.]
+  [--disable-submodule-encoding will keep the current default (disabled).])],
+[
+case "$enableval" in
+  yes) SUBMODULE_ENCODING_BY_DEFAULT=1
+       AC_MSG_NOTICE([Submodule encoding will be enabled by default.])
+       ;;
+  no)  SUBMODULE_ENCODING_BY_DEFAULT=0
+       AC_MSG_NOTICE([Submodule encoding will not be enabled by default.])
+       ;;
+  *)   AC_MSG_ERROR([--enable-submodule-encoding takes yes or no.])
+       ;;
+esac
+],
+[
+  SUBMODULE_ENCODING_BY_DEFAULT=0
+])
+
+GIT_CONF_SUBST([SUBMODULE_ENCODING_BY_DEFAULT])
+
 ## Site configuration (override autodetection)
 ## --with-PACKAGE[=ARG] and --without-PACKAGE
 AC_MSG_NOTICE([CHECKS for site configuration])

meson.build

@@ -947,6 +947,10 @@ else
   build_options_config.set('NO_PERL_CPAN_FALLBACKS', '')
 endif
 
+submodule_encoding_by_default = get_option('submodule-encoding').to_int()
+libgit_c_args += '-DSUBMODULE_ENCODING_BY_DEFAULT=' + submodule_encoding_by_default.to_string()
+build_options_config.set('SUBMODULE_ENCODING_BY_DEFAULT', submodule_encoding_by_default)
+
 zlib_backend = get_option('zlib_backend')
 if zlib_backend in ['auto', 'zlib-ng']
   zlib_ng = dependency('zlib-ng', required: zlib_backend == 'zlib-ng')

meson_options.txt

@@ -21,6 +21,8 @@ option('runtime_prefix', type: 'boolean', value: false,
   description: 'Resolve ancillary tooling and support files relative to the location of the runtime binary instead of hard-coding them into the binary.')
 option('sane_tool_path', type: 'array', value: [],
   description: 'An array of paths to pick up tools from in case the normal tools are broken or lacking.')
+option('submodule-encoding', type: 'boolean', value: false,
+  description: 'Enable submoduleEncoding extension by default at build time.')
 
 # Build information compiled into Git and other parts like documentation.
 option('build_date', type: 'string', value: '',

repository.c

@@ -292,6 +292,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_encoding = format.submodule_encoding;
 
 	/* 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_encoding;
 
 	/* Indicate if a repository has a different 'commondir' from 'gitdir' */
 	unsigned different_commondir:1;