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

* ar/submodule-gitdir-tweak:
  fixup! t: submodules: add basic mixed gitdir path tests
  fixup! t: add gitdir encoding tests
  t: add gitdir encoding tests
  t: move nested gitdir tests to proper location
  submodule: remove validate_submodule_git_dir()
  submodule: encode gitdir paths to avoid conflicts
  strbuf: bring back is_rfc3986_unreserved
  t: submodules: add basic mixed gitdir path tests
  submodule: add gitdir path config override
  submodule: create new gitdirs under submodules path
  submodule--helper: use submodule_name_to_gitdir in add_submodule

Author: gitster

Documentation/fetch-options.adoc

@@ -215,7 +215,7 @@ ifndef::git-pull[]
 	submodule that has commits that are referenced by a newly fetched
 	superproject commit but are missing in the local submodule clone. A
 	changed submodule can be fetched as long as it is present locally e.g.
-	in `$GIT_DIR/modules/` (see linkgit:gitsubmodules[7]); if the upstream
+	in `$GIT_DIR/submodules/` (see linkgit:gitsubmodules[7]); if the upstream
 	adds a new submodule, that submodule cannot be fetched until it is
 	cloned e.g. by `git submodule update`.
 +

Documentation/git-fetch.adoc

@@ -304,7 +304,7 @@ include::config/fetch.adoc[]
 BUGS
 ----
 Using --recurse-submodules can only fetch new commits in submodules that are
-present locally e.g. in `$GIT_DIR/modules/`. If the upstream adds a new
+present locally e.g. in `$GIT_DIR/submodules/`. If the upstream adds a new
 submodule, that submodule cannot be fetched until it is cloned e.g. by `git
 submodule update`. This is expected to be fixed in a future Git version.
 

Documentation/git-submodule.adoc

@@ -266,7 +266,7 @@ registered submodules, and sync any nested submodules within.
 absorbgitdirs::
 	If a git directory of a submodule is inside the submodule,
 	move the git directory of the submodule into its superproject's
-	`$GIT_DIR/modules` path and then connect the git directory and
+	`$GIT_DIR/submodules` path and then connect the git directory and
 	its working directory by setting the `core.worktree` and adding
 	a .git file pointing to the git directory embedded in the
 	superprojects git directory.

Documentation/gitsubmodules.adoc

@@ -22,12 +22,12 @@ The submodule has its own history; the repository it is embedded
 in is called a superproject.
 
 On the filesystem, a submodule usually (but not always - see FORMS below)
-consists of (i) a Git directory located under the `$GIT_DIR/modules/`
+consists of (i) a Git directory located under the `$GIT_DIR/submodules/`
 directory of its superproject, (ii) a working directory inside the
 superproject's working directory, and a `.git` file at the root of
 the submodule's working directory pointing to (i).
 
-Assuming the submodule has a Git directory at `$GIT_DIR/modules/foo/`
+Assuming the submodule has a Git directory at `$GIT_DIR/submodules/foo/`
 and a working directory at `path/to/bar/`, the superproject tracks the
 submodule via a `gitlink` entry in the tree at `path/to/bar` and an entry
 in its `.gitmodules` file (see linkgit:gitmodules[5]) of the form
@@ -138,7 +138,7 @@ using older versions of Git.
 It is possible to construct these old form repositories manually.
 +
 When deinitialized or deleted (see below), the submodule's Git
-directory is automatically moved to `$GIT_DIR/modules/<name>/`
+directory is automatically moved to `$GIT_DIR/submodules/<name>/`
 of the superproject.
 
  * Deinitialized submodule: A `gitlink`, and a `.gitmodules` entry,
@@ -163,7 +163,7 @@ possible to checkout past commits without requiring fetching
 from another repository.
 +
 To completely remove a submodule, manually delete
-`$GIT_DIR/modules/<name>/`.
+`$GIT_DIR/submodules/<name>/`.
 
 ACTIVE SUBMODULES
 -----------------

builtin/credential-store.c

@@ -76,12 +76,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

@@ -1214,6 +1214,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 UNUSED)
+{
+	struct strbuf gitdir = STRBUF_INIT;
+
+	if (argc != 2)
+		usage(_("git submodule--helper gitdir <name>"));
+
+	submodule_name_to_gitdir(&gitdir, the_repository, argv[1]);
+
+	printf("%s\n", gitdir.buf);
+
+	strbuf_release(&gitdir);
+	return 0;
+}
+
 struct sync_cb {
 	const char *prefix;
 	const char *super_prefix;
@@ -1709,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))
@@ -1786,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");
@@ -3213,10 +3208,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;
@@ -3225,8 +3221,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"
@@ -3244,7 +3240,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;
@@ -3596,6 +3592,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),

setup.c

@@ -1416,7 +1416,7 @@ static int is_implicit_bare_repo(const char *path)
 	 * we are inside $GIT_DIR of a worktree of a non-embedded
 	 * submodule, whose superproject is not a bare repository.
 	 */
-	if (strstr(path, "/.git/modules/"))
+	if (strstr(path, "/.git/modules/") || strstr(path, "/.git/submodules/"))
 		return 1;
 
 	return 0;

strbuf.c

@@ -817,6 +817,12 @@ void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)
 	}
 }
 
+int is_rfc3986_unreserved(char ch)
+{
+	return isalnum(ch) ||
+		ch == '-' || ch == '_' || ch == '.' || ch == '~';
+}
+
 static void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,
 				 char_predicate allow_unencoded_fn)
 {

strbuf.h

@@ -640,6 +640,8 @@ static inline void strbuf_complete_line(struct strbuf *sb)
 
 typedef int (*char_predicate)(char ch);
 
+int is_rfc3986_unreserved(char ch);
+
 void strbuf_addstr_urlencode(struct strbuf *sb, const char *name,
 			     char_predicate allow_unencoded_fn);