[RFC 00/10] Make .the gitmodules file path configurable

[RFC 00/10] Make .the gitmodules file path configurable

[Antonio Ospite]

Hi,

vcsh[1] uses bare git repositories and detached work-trees to manage
*distinct* sets of configuration files directly into $HOME.

In general, submodules have worked perfectly fine with detached
work-trees for some time[2,3,4].

However when multiple repositories take turns using the same directory
as their work-tree, and more than one of them want to use submodules,
there could still be conflicts about the '.gitmodules' file because git
hardcodes this path.

For comparison, in case of '.gitignore' a similar conflict might arise,
but git has alternative ways to specify exclude files, so vcsh solves
this by setting core.excludesFile for each repository and track ignored
files somewhere else (in ~/.gitignore.d/$VCSH_REPO_NAME).

This is currently not possible with submodules configuration.

So this series proposes a mechanism to set an alternative path for the
submodules configuration file (from now on "gitmodules file").

Patches are based on fe0a9eaf31dd0c349ae4308498c33a5c3794b293.

In commit 4c0eeafe4755 (cache.h: add GITMODULES_FILE macro)[5] the
gitmodules file path definition was centralized, AFAIU this was done
mainly to prevent typos, as checking a symbolic constant is something
the compiler will do for us.

Expanding on that change the first patch in the series makes the path
customizable exposing a 'core.submodulesFile' configuration setting.

The new configuration setting can be used to set an *alternative*
location for the gitmodules file; IMHO there is no need to provide
*additional* locations like in the case of exclude files.

For instance vcsh could set the location to
'~/.gitmodules.d/$VCSH_REPO_NAME' to avoid conflicts.

Since the gitmodules file is meant to be checked in into the repository,
the overridden file path should be relative to the work-tree; is there
a way to enforce this constraint at run time (i.e. validate the config
value), or is it enough to have it documented?

The last patch adds a hacky 'test-custom-gitmodules-file.sh' script
which patches the test suite to fix all the hardcoded occurrences of
'.gitmodules' and runs it twice: once with the default value and once
with a custom path, passed via an environmental variable.

I guess in the final version just testing for a custom path (e.g.
'.gitmodules_custom') could be enough, as the default value can be seen
as a particular case.

The last thing I noticed is that git does not create config files
automatically if they are under a subdirectory which does not exist
already; for instance the following command would fail:

  git config -f newsubdir/test-config user.name Antonio

This means that if the user wanted to use something like:

  git -c core.submodulesFile=.gitmodules.d/repo_submodules ...

the subdirectory would have to be created beforehand. This is not a big
deal of course, but I was wondering if this is mentioned anywhere.
Fixing the current behavior to create the leading directories does not
seem hard, but I am not sure it is worth it.

Thanks,
   Antonio

[1] https://github.com/RichiH/vcsh
[2] http://git.661346.n2.nabble.com/git-submodule-vs-GIT-WORK-TREE-td7562165.html
[3] http://git.661346.n2.nabble.com/PATCH-Solve-git-submodule-issues-with-detached-work-trees-td7563377.html
[4] https://github.com/git/git/commit/be8779f7ac9a3be9aa783df008d59082f4054f67
[5] https://github.com/git/git/commit/4c0eeafe4755345b0f4636bf09904cf689703e11

Antonio Ospite (10):
  submodule: add 'core.submodulesFile' to override the '.gitmodules'
    path
  submodule: fix getting custom gitmodule file in fetch command
  submodule: use the 'submodules_file' variable in output messages
  submodule: document 'core.submodulesFile' and fix references to
    '.gitmodules'
  submodule: adjust references to '.gitmodules' in comments
  completion: add 'core.submodulesfile' to the git-completion.bash file
  XXX: wrap-for-bin.sh: set 'core.submodulesFile' for each git
    invocation
  XXX: submodule: fix t1300-repo-config.sh to take into account the new
    config
  XXX: submodule: pass custom gitmodules file to 'test-tool
    submodule-config'
  XXX: add a hacky script to test the changes with a patched test suite

 Documentation/config.txt                      | 18 +++--
 Documentation/git-add.txt                     |  4 +-
 Documentation/git-submodule.txt               | 45 +++++------
 Documentation/gitmodules.txt                  | 15 ++--
 Documentation/gitsubmodules.txt               | 18 ++---
 .../technical/api-submodule-config.txt        |  6 +-
 Makefile                                      |  3 +-
 builtin/fetch.c                               |  2 +-
 builtin/mv.c                                  |  3 +-
 builtin/rm.c                                  |  3 +-
 builtin/submodule--helper.c                   | 20 ++---
 cache.h                                       |  1 +
 config.c                                      | 13 ++--
 config.h                                      |  7 +-
 contrib/completion/git-completion.bash        |  1 +
 contrib/subtree/git-subtree.txt               |  2 +-
 environment.c                                 |  1 +
 git-submodule.sh                              | 24 +++---
 repository.h                                  |  2 +-
 submodule-config.c                            | 16 ++--
 submodule-config.h                            |  2 +-
 submodule.c                                   | 54 ++++++-------
 t/helper/test-submodule-config.c              |  7 ++
 t/t0001-init.sh                               |  1 +
 t/t1300-repo-config.sh                        | 26 ++++++-
 test-custom-gitmodules-file.sh                | 75 +++++++++++++++++++
 unpack-trees.c                                |  2 +-
 wrap-for-bin.sh                               |  2 +
 28 files changed, 250 insertions(+), 123 deletions(-)
 create mode 100755 test-custom-gitmodules-file.sh
 mode change 100644 => 100755 wrap-for-bin.sh

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Antonio Ospite]

When multiple repositories with detached work-trees take turns using the
same directory as their work-tree, and more than one of them want to use
submodules, there will be conflicts about the '.gitmodules' file.

git hardcodes this path so it's not possible to override its location on
a per-repository basis to allow such repositories to coexists
peacefully.

Make the path of the "gitmodules file" customizable exposing
a 'core.submodulesFile' configuration setting.

The default value will still be '.gitmodules' when 'core.submodulesFile'
is not set.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 cache.h            |  1 +
 config.c           |  6 +++++-
 environment.c      |  1 +
 git-submodule.sh   | 14 +++++++++-----
 submodule-config.c |  4 ++--
 submodule.c        | 20 ++++++++++----------
 unpack-trees.c     |  2 +-
 7 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/cache.h b/cache.h
index bbaf5c349..f4dd8bc00 100644
--- a/cache.h
+++ b/cache.h
@@ -1774,6 +1774,7 @@ extern void prepare_pager_args(struct child_process *, const char *pager);
 extern const char *editor_program;
 extern const char *askpass_program;
 extern const char *excludes_file;
+extern const char *submodules_file;
 
 /* base85 */
 int decode_85(char *dst, const char *line, int linelen);
diff --git a/config.c b/config.c
index c698988f5..6ffb1d501 100644
--- a/config.c
+++ b/config.c
@@ -1199,6 +1199,10 @@ static int git_default_core_config(const char *var, const char *value)
 	if (!strcmp(var, "core.excludesfile"))
 		return git_config_pathname(&excludes_file, var, value);
 
+	if (!strcmp(var, "core.submodulesfile")) {
+		return git_config_pathname(&submodules_file, var, value);
+	}
+
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
 			return config_error_nonbool(var);
@@ -2084,7 +2088,7 @@ int git_config_get_pathname(const char *key, const char **dest)
 void config_from_gitmodules(config_fn_t fn, void *data)
 {
 	if (the_repository->worktree) {
-		char *file = repo_worktree_path(the_repository, GITMODULES_FILE);
+		char *file = repo_worktree_path(the_repository, submodules_file);
 		git_config_from_file(fn, file, data);
 		free(file);
 	}
diff --git a/environment.c b/environment.c
index 39b3d906c..69cae6d68 100644
--- a/environment.c
+++ b/environment.c
@@ -49,6 +49,7 @@ int pager_use_color = 1;
 const char *editor_program;
 const char *askpass_program;
 const char *excludes_file;
+const char *submodules_file = GITMODULES_FILE;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 int check_replace_refs = 1;
 char *git_replace_ref_base;
diff --git a/git-submodule.sh b/git-submodule.sh
index 24914963c..610fd0dc5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -71,7 +71,9 @@ get_submodule_config () {
 	value=$(git config submodule."$name"."$option")
 	if test -z "$value"
 	then
-		value=$(git config -f .gitmodules submodule."$name"."$option")
+		gitmodules_file=$(git config core.submodulesfile)
+		: ${gitmodules_file:=.gitmodules}
+		value=$(git config -f "$gitmodules_file" submodule."$name"."$option")
 	fi
 	printf '%s' "${value:-$default}"
 }
@@ -271,13 +273,15 @@ or you are unsure what this means choose another name with the '--name' option."
 	git add --no-warn-embedded-repo $force "$sm_path" ||
 	die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
 
-	git config -f .gitmodules submodule."$sm_name".path "$sm_path" &&
-	git config -f .gitmodules submodule."$sm_name".url "$repo" &&
+	gitmodules_file=$(git config core.submodulesfile)
+	: ${gitmodules_file:=.gitmodules}
+	git config -f "$gitmodules_file" submodule."$sm_name".path "$sm_path" &&
+	git config -f "$gitmodules_file" submodule."$sm_name".url "$repo" &&
 	if test -n "$branch"
 	then
-		git config -f .gitmodules submodule."$sm_name".branch "$branch"
+		git config -f "$gitmodules_file" submodule."$sm_name".branch "$branch"
 	fi &&
-	git add --force .gitmodules ||
+	git add --force "$gitmodules_file" ||
 	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
 
 	# NEEDSWORK: In a multi-working-tree world, this needs to be
diff --git a/submodule-config.c b/submodule-config.c
index 3f2075764..8a3396ade 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -468,7 +468,7 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
 		return 1;
 	}
 
-	strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
+	strbuf_addf(rev, "%s:%s", oid_to_hex(treeish_name), submodules_file);
 	if (get_oid(rev->buf, gitmodules_oid) >= 0)
 		ret = 1;
 
@@ -583,7 +583,7 @@ void repo_read_gitmodules(struct repository *repo)
 		if (repo_read_index(repo) < 0)
 			return;
 
-		gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
+		gitmodules = repo_worktree_path(repo, submodules_file);
 
 		if (!is_gitmodules_unmerged(repo->index))
 			git_config_from_file(gitmodules_cb, gitmodules, repo);
diff --git a/submodule.c b/submodule.c
index 9a50168b2..2afbdb644 100644
--- a/submodule.c
+++ b/submodule.c
@@ -36,13 +36,13 @@ static struct oid_array ref_tips_after_fetch;
  */
 int is_gitmodules_unmerged(const struct index_state *istate)
 {
-	int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
+	int pos = index_name_pos(istate, submodules_file, strlen(submodules_file));
 	if (pos < 0) { /* .gitmodules not found or isn't merged */
 		pos = -1 - pos;
 		if (istate->cache_nr > pos) {  /* there is a .gitmodules */
 			const struct cache_entry *ce = istate->cache[pos];
-			if (ce_namelen(ce) == strlen(GITMODULES_FILE) &&
-			    !strcmp(ce->name, GITMODULES_FILE))
+			if (ce_namelen(ce) == strlen(submodules_file) &&
+			    !strcmp(ce->name, submodules_file))
 				return 1;
 		}
 	}
@@ -60,11 +60,11 @@ int is_gitmodules_unmerged(const struct index_state *istate)
  */
 int is_staging_gitmodules_ok(struct index_state *istate)
 {
-	int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
+	int pos = index_name_pos(istate, submodules_file, strlen(submodules_file));
 
 	if ((pos >= 0) && (pos < istate->cache_nr)) {
 		struct stat st;
-		if (lstat(GITMODULES_FILE, &st) == 0 &&
+		if (lstat(submodules_file, &st) == 0 &&
 		    ie_match_stat(istate, istate->cache[pos], &st,
 				  CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED)
 			return 0;
@@ -90,7 +90,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 	struct strbuf entry = STRBUF_INIT;
 	const struct submodule *submodule;
 
-	if (!file_exists(GITMODULES_FILE)) /* Do nothing without .gitmodules */
+	if (!file_exists(submodules_file)) /* Do nothing without .gitmodules */
 		return -1;
 
 	if (is_gitmodules_unmerged(&the_index))
@@ -104,7 +104,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 	strbuf_addstr(&entry, "submodule.");
 	strbuf_addstr(&entry, submodule->name);
 	strbuf_addstr(&entry, ".path");
-	if (git_config_set_in_file_gently(GITMODULES_FILE, entry.buf, newpath) < 0) {
+	if (git_config_set_in_file_gently(submodules_file, entry.buf, newpath) < 0) {
 		/* Maybe the user already did that, don't error out here */
 		warning(_("Could not update .gitmodules entry %s"), entry.buf);
 		strbuf_release(&entry);
@@ -124,7 +124,7 @@ int remove_path_from_gitmodules(const char *path)
 	struct strbuf sect = STRBUF_INIT;
 	const struct submodule *submodule;
 
-	if (!file_exists(GITMODULES_FILE)) /* Do nothing without .gitmodules */
+	if (!file_exists(submodules_file)) /* Do nothing without .gitmodules */
 		return -1;
 
 	if (is_gitmodules_unmerged(&the_index))
@@ -137,7 +137,7 @@ int remove_path_from_gitmodules(const char *path)
 	}
 	strbuf_addstr(&sect, "submodule.");
 	strbuf_addstr(&sect, submodule->name);
-	if (git_config_rename_section_in_file(GITMODULES_FILE, sect.buf, NULL) < 0) {
+	if (git_config_rename_section_in_file(submodules_file, sect.buf, NULL) < 0) {
 		/* Maybe the user already did that, don't error out here */
 		warning(_("Could not remove .gitmodules entry for %s"), path);
 		strbuf_release(&sect);
@@ -149,7 +149,7 @@ int remove_path_from_gitmodules(const char *path)
 
 void stage_updated_gitmodules(struct index_state *istate)
 {
-	if (add_file_to_index(istate, GITMODULES_FILE, 0))
+	if (add_file_to_index(istate, submodules_file, 0))
 		die(_("staging updated .gitmodules failed"));
 }
 
diff --git a/unpack-trees.c b/unpack-trees.c
index e73745051..b4d834ef4 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -283,7 +283,7 @@ static int check_submodule_move_head(const struct cache_entry *ce,
 static void load_gitmodules_file(struct index_state *index,
 				 struct checkout *state)
 {
-	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
+	int pos = index_name_pos(index, submodules_file, strlen(submodules_file));
 
 	if (pos >= 0) {
 		struct cache_entry *ce = index->cache[pos];
-- 
2.17.0

[RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Antonio Ospite]

Import the default git configuration before accessing the gitmodules
file.

This is important when a value different from the default one is set via
the 'core.submodulesfile' config.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 builtin/fetch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index dcdfc66f0..d56636f74 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1428,8 +1428,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	for (i = 1; i < argc; i++)
 		strbuf_addf(&default_rla, " %s", argv[i]);
 
-	config_from_gitmodules(gitmodules_fetch_config, NULL);
 	git_config(git_fetch_config, NULL);
+	config_from_gitmodules(gitmodules_fetch_config, NULL);
 
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);
-- 
2.17.0

[RFC 03/10] submodule: use the 'submodules_file' variable in output messages

[Antonio Ospite]

The gitmodules file path can be customized by setting the
'core.submodulesFile' config option.

Use the 'submodules_file' variable instead of the hardcoded
'.gitmodules' in output messages, to reflect the actual path of the
gitmodules file.

NOTE: the default git configuration has to be initialized in
'builtin/submodule--helper.c' to make the actual value of
'submodules_file', overridden by 'core.submodulesFile', available to the
helper command.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 builtin/mv.c                |  3 ++-
 builtin/rm.c                |  3 ++-
 builtin/submodule--helper.c | 18 ++++++++++--------
 submodule-config.c          |  4 ++--
 submodule.c                 | 14 +++++++-------
 5 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 6d141f7a5..ec8f139c4 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -82,7 +82,8 @@ static void prepare_move_submodule(const char *src, int first,
 	if (!S_ISGITLINK(active_cache[first]->ce_mode))
 		die(_("Directory %s is in index and no submodule?"), src);
 	if (!is_staging_gitmodules_ok(&the_index))
-		die(_("Please stage your changes to .gitmodules or stash them to proceed"));
+		die(_("Please stage your changes to %s or stash them to proceed"),
+		    submodules_file);
 	strbuf_addf(&submodule_dotgit, "%s/.git", src);
 	*submodule_gitfile = read_gitfile(submodule_dotgit.buf);
 	if (*submodule_gitfile)
diff --git a/builtin/rm.c b/builtin/rm.c
index 5b6fc7ee8..6fd015d86 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -286,7 +286,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
 		if (list.entry[list.nr++].is_submodule &&
 		    !is_staging_gitmodules_ok(&the_index))
-			die (_("Please stage your changes to .gitmodules or stash them to proceed"));
+			die (_("Please stage your changes to %s or stash them to proceed"),
+			     submodules_file);
 	}
 
 	if (pathspec.nr) {
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a404df3ea..72b95d27b 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -458,8 +458,8 @@ static void init_submodule(const char *path, const char *prefix,
 	sub = submodule_from_path(&null_oid, path);
 
 	if (!sub)
-		die(_("No url found for submodule path '%s' in .gitmodules"),
-			displaypath);
+		die(_("No url found for submodule path '%s' in %s"),
+			displaypath, submodules_file);
 
 	/*
 	 * NEEDSWORK: In a multi-working-tree world, this needs to be
@@ -481,8 +481,8 @@ static void init_submodule(const char *path, const char *prefix,
 	strbuf_addf(&sb, "submodule.%s.url", sub->name);
 	if (git_config_get_string(sb.buf, &url)) {
 		if (!sub->url)
-			die(_("No url found for submodule path '%s' in .gitmodules"),
-				displaypath);
+			die(_("No url found for submodule path '%s' in %s"),
+				displaypath, submodules_file);
 
 		url = xstrdup(sub->url);
 
@@ -623,8 +623,8 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 	int diff_files_result;
 
 	if (!submodule_from_path(&null_oid, path))
-		die(_("no submodule mapping found in .gitmodules for path '%s'"),
-		      path);
+		die(_("no submodule mapping found in %s for path '%s'"),
+		      submodules_file, path);
 
 	displaypath = get_submodule_displaypath(path, prefix);
 
@@ -749,8 +749,8 @@ static int module_name(int argc, const char **argv, const char *prefix)
 	sub = submodule_from_path(&null_oid, argv[1]);
 
 	if (!sub)
-		die(_("no submodule mapping found in .gitmodules for path '%s'"),
-		    argv[1]);
+		die(_("no submodule mapping found in %s for path '%s'"),
+		    submodules_file, argv[1]);
 
 	printf("%s\n", sub->name);
 
@@ -1855,6 +1855,8 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
 	if (argc < 2 || !strcmp(argv[1], "-h"))
 		usage("git submodule--helper <command>");
 
+	git_config(git_default_config, NULL);
+
 	for (i = 0; i < ARRAY_SIZE(commands); i++) {
 		if (!strcmp(argv[1], commands[i].cmd)) {
 			if (get_super_prefix() &&
diff --git a/submodule-config.c b/submodule-config.c
index 8a3396ade..620d522ee 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -347,9 +347,9 @@ static void warn_multiple_config(const unsigned char *treeish_name,
 	const char *commit_string = "WORKTREE";
 	if (treeish_name)
 		commit_string = sha1_to_hex(treeish_name);
-	warning("%s:.gitmodules, multiple configurations found for "
+	warning("%s:%s, multiple configurations found for "
 			"'submodule.%s.%s'. Skipping second one!",
-			commit_string, name, option);
+			commit_string, submodules_file, name, option);
 }
 
 struct parse_config_parameter {
diff --git a/submodule.c b/submodule.c
index 2afbdb644..97213b549 100644
--- a/submodule.c
+++ b/submodule.c
@@ -94,11 +94,11 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 		return -1;
 
 	if (is_gitmodules_unmerged(&the_index))
-		die(_("Cannot change unmerged .gitmodules, resolve merge conflicts first"));
+		die(_("Cannot change unmerged %s, resolve merge conflicts first"), submodules_file);
 
 	submodule = submodule_from_path(&null_oid, oldpath);
 	if (!submodule || !submodule->name) {
-		warning(_("Could not find section in .gitmodules where path=%s"), oldpath);
+		warning(_("Could not find section in %s where path=%s"), submodules_file, oldpath);
 		return -1;
 	}
 	strbuf_addstr(&entry, "submodule.");
@@ -106,7 +106,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 	strbuf_addstr(&entry, ".path");
 	if (git_config_set_in_file_gently(submodules_file, entry.buf, newpath) < 0) {
 		/* Maybe the user already did that, don't error out here */
-		warning(_("Could not update .gitmodules entry %s"), entry.buf);
+		warning(_("Could not update %s entry %s"), submodules_file, entry.buf);
 		strbuf_release(&entry);
 		return -1;
 	}
@@ -128,18 +128,18 @@ int remove_path_from_gitmodules(const char *path)
 		return -1;
 
 	if (is_gitmodules_unmerged(&the_index))
-		die(_("Cannot change unmerged .gitmodules, resolve merge conflicts first"));
+		die(_("Cannot change unmerged %s, resolve merge conflicts first"), submodules_file);
 
 	submodule = submodule_from_path(&null_oid, path);
 	if (!submodule || !submodule->name) {
-		warning(_("Could not find section in .gitmodules where path=%s"), path);
+		warning(_("Could not find section in %s where path=%s"), submodules_file, path);
 		return -1;
 	}
 	strbuf_addstr(&sect, "submodule.");
 	strbuf_addstr(&sect, submodule->name);
 	if (git_config_rename_section_in_file(submodules_file, sect.buf, NULL) < 0) {
 		/* Maybe the user already did that, don't error out here */
-		warning(_("Could not remove .gitmodules entry for %s"), path);
+		warning(_("Could not remove %s entry for %s"), submodules_file, path);
 		strbuf_release(&sect);
 		return -1;
 	}
@@ -150,7 +150,7 @@ int remove_path_from_gitmodules(const char *path)
 void stage_updated_gitmodules(struct index_state *istate)
 {
 	if (add_file_to_index(istate, submodules_file, 0))
-		die(_("staging updated .gitmodules failed"));
+		die(_("staging updated %s failed"), submodules_file);
 }
 
 static int add_submodule_odb(const char *path)
-- 
2.17.0

[RFC 04/10] submodule: document 'core.submodulesFile' and fix references to '.gitmodules'

[Antonio Ospite]

The gitmodules file location can be overridden by setting the
'core.submodulesFile' config option.

Document this new option and reflect the new behavior in the
documentation by using the more generic formula "gitmodules file"
instead of the hardcoded '.gitmodules' file path.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 Documentation/config.txt                      | 18 +++++---
 Documentation/git-add.txt                     |  4 +-
 Documentation/git-submodule.txt               | 45 ++++++++++---------
 Documentation/gitmodules.txt                  | 15 ++++---
 Documentation/gitsubmodules.txt               | 18 ++++----
 .../technical/api-submodule-config.txt        |  6 +--
 contrib/subtree/git-subtree.txt               |  2 +-
 7 files changed, 59 insertions(+), 49 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2659153cb..647e33646 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -742,6 +742,12 @@ core.excludesFile::
 	If `$XDG_CONFIG_HOME` is either not set or empty, `$HOME/.config/git/ignore`
 	is used instead. See linkgit:gitignore[5].
 
+core.submodulesFile::
+	Specifies the pathname to the file that contains the submodules
+	configuration. The path must be relative to the repository work tree.
+	The specified path replaces the default per-repository '.gitmodules'
+	file. See linkgit:gitmodules[5].
+
 core.askPass::
 	Some commands (e.g. svn and http interfaces) that interactively
 	ask for a password can be told to use an external program given
@@ -3170,7 +3176,7 @@ stash.showStat::
 	See description of 'show' command in linkgit:git-stash[1].
 
 submodule.<name>.url::
-	The URL for a submodule. This variable is copied from the .gitmodules
+	The URL for a submodule. This variable is copied from the gitmodules
 	file to the git config via 'git submodule init'. The user can change
 	the configured URL before obtaining the submodule via 'git submodule
 	update'. If neither submodule.<name>.active or submodule.active are
@@ -3191,7 +3197,7 @@ submodule.<name>.update::
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
 	update --remote`.  Set this option to override the value found in
-	the `.gitmodules` file.  See linkgit:git-submodule[1] and
+	the gitmodules file.  See linkgit:git-submodule[1] and
 	linkgit:gitmodules[5] for details.
 
 submodule.<name>.fetchRecurseSubmodules::
@@ -3212,10 +3218,10 @@ submodule.<name>.ignore::
 	let submodules with modified tracked files in their work tree show up.
 	Using "none" (the default when this option is not set) also shows
 	submodules that have untracked files in their work tree as changed.
-	This setting overrides any setting made in .gitmodules for this submodule,
-	both settings can be overridden on the command line by using the
-	"--ignore-submodules" option. The 'git submodule' commands are not
-	affected by this setting.
+	This setting overrides any setting made in the gitmodules file for
+	this submodule, both settings can be overridden on the command line by
+	using the "--ignore-submodules" option. The 'git submodule' commands
+	are not affected by this setting.
 
 submodule.<name>.active::
 	Boolean value indicating if the submodule is of interest to git
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index d50fa339d..8add7e7c0 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -171,8 +171,8 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
 --no-warn-embedded-repo::
 	By default, `git add` will warn when adding an embedded
 	repository to the index without using `git submodule add` to
-	create an entry in `.gitmodules`. This option will suppress the
-	warning (e.g., if you are manually performing operations on
+	create an entry in the gitmodules file. This option will suppress
+	the warning (e.g., if you are manually performing operations on
 	submodules).
 
 --renormalize::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 71c5618e8..6bd8e3d44 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -58,13 +58,13 @@ for commit without cloning. The <path> is also used as the submodule's
 logical name in its configuration entries unless `--name` is used
 to specify a logical name.
 +
-The given URL is recorded into `.gitmodules` for use by subsequent users
-cloning the superproject. If the URL is given relative to the
+The given URL is recorded in the gitmodules file for use by subsequent
+users cloning the superproject. If the URL is given relative to the
 superproject's repository, the presumption is the superproject and
 submodule repositories will be kept together in the same relative
 location, and only the superproject's URL needs to be provided.
 git-submodule will correctly locate the submodule using the relative
-URL in `.gitmodules`.
+URL in the gitmodules file.
 
 status [--cached] [--recursive] [--] [<path>...]::
 	Show the status of the submodules. This will print the SHA-1 of the
@@ -86,8 +86,8 @@ too (and can also report changes to a submodule's work tree).
 init [--] [<path>...]::
 	Initialize the submodules recorded in the index (which were
 	added and committed elsewhere) by setting `submodule.$name.url`
-	in .git/config. It uses the same setting from `.gitmodules` as
-	a template. If the URL is relative, it will be resolved using
+	in .git/config. It uses the same setting from the gitmodules file
+	as a template. If the URL is relative, it will be resolved using
 	the default remote. If there is no default remote, the current
 	repository will be assumed to be upstream.
 +
@@ -162,8 +162,8 @@ The following 'update' procedures are only available via the
 	none;; the submodule is not updated.
 
 If the submodule is not yet initialized, and you just want to use the
-setting as stored in `.gitmodules`, you can automatically initialize the
-submodule with the `--init` option.
+setting as stored in the gitmodules file, you can automatically initialize
+the submodule with the `--init` option.
 
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
@@ -185,8 +185,8 @@ foreach [--recursive] <command>::
 	Evaluates an arbitrary shell command in each checked out submodule.
 	The command has access to the variables $name, $path, $sha1 and
 	$toplevel:
-	$name is the name of the relevant submodule section in `.gitmodules`,
-	$path is the name of the submodule directory relative to the
+	$name is the name of the relevant submodule section in the gitmodules
+	file, $path is the name of the submodule directory relative to the
 	superproject, $sha1 is the commit as recorded in the superproject,
 	and $toplevel is the absolute path to the top-level of the superproject.
 	Any submodules defined in the superproject but not checked out are
@@ -207,11 +207,11 @@ git submodule foreach 'echo $path `git rev-parse HEAD`'
 
 sync [--recursive] [--] [<path>...]::
 	Synchronizes submodules' remote URL configuration setting
-	to the value specified in `.gitmodules`. It will only affect those
-	submodules which already have a URL entry in .git/config (that is the
-	case when they are initialized or freshly added). This is useful when
-	submodule URLs change upstream and you need to update your local
-	repositories accordingly.
+	to the value specified in the gitmodules file. It will only affect
+	those submodules which already have a URL entry in .git/config (that
+	is the case when they are initialized or freshly added). This is
+	useful when submodule URLs change upstream and you need to update your
+	local repositories accordingly.
 +
 "git submodule sync" synchronizes all submodules while
 "git submodule sync \-- A" synchronizes submodule "A" only.
@@ -247,9 +247,9 @@ OPTIONS
 --branch::
 	Branch of repository to add as submodule.
 	The name of the branch is recorded as `submodule.<name>.branch` in
-	`.gitmodules` for `update --remote`.  A special value of `.` is used to
-	indicate that the name of the branch in the submodule should be the
-	same name as the current branch in the current repository.
+	the gitmodules file for `update --remote`.  A special value of `.` is
+	used to indicate that the name of the branch in the submodule should
+	be the same name as the current branch in the current repository.
 
 -f::
 --force::
@@ -289,8 +289,8 @@ OPTIONS
 	is branch's remote (`branch.<name>.remote`), defaulting to `origin`.
 	The remote branch used defaults to `master`, but the branch name may
 	be overridden by setting the `submodule.<name>.branch` option in
-	either `.gitmodules` or `.git/config` (with `.git/config` taking
-	precedence).
+	either the gitmodules file or `.git/config` (with `.git/config`
+	taking precedence).
 +
 This works for any of the supported update procedures (`--checkout`,
 `--rebase`, etc.).  The only change is the source of the target SHA-1.
@@ -378,7 +378,7 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
 --[no-]recommend-shallow::
 	This option is only valid for the update command.
 	The initial clone of a submodule will use the recommended
-	`submodule.<name>.shallow` as provided by the `.gitmodules` file
+	`submodule.<name>.shallow` as provided by the gitmodules file
 	by default. To ignore the suggestions use `--no-recommend-shallow`.
 
 -j <n>::
@@ -394,8 +394,9 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
 
 FILES
 -----
-When initializing submodules, a `.gitmodules` file in the top-level directory
-of the containing repository is used to find the url of each submodule.
+When initializing submodules, a gitmodules file is used to find the url of
+each submodule. By default this file is named `.gitmodules` and is located in
+the top-level directory of the containing repository.
 This file should be formatted in the same way as `$GIT_DIR/config`. The key
 to each submodule url is "submodule.$name.url".  See linkgit:gitmodules[5]
 for details.
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index db5d47eb1..833c0a172 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -13,9 +13,12 @@ $GIT_WORK_DIR/.gitmodules
 DESCRIPTION
 -----------
 
-The `.gitmodules` file, located in the top-level directory of a Git
-working tree, is a text file with a syntax matching the requirements
-of linkgit:git-config[1].
+A gitmodules file is a text file with a syntax matching the requirements of
+linkgit:git-config[1].
+
+By default the file is named `.gitmodules` and is located in the top-level
+directory of a Git working tree, The location can be changed by setting the
+`core.submodulesFile` config option.
 
 The file contains one subsection per submodule, and the subsection value
 is the name of the submodule. The name is set to the path where the
@@ -27,7 +30,7 @@ submodule.<name>.path::
 	Defines the path, relative to the top-level directory of the Git
 	working tree, where the submodule is expected to be checked out.
 	The path name must not end with a `/`. All submodule paths must
-	be unique within the .gitmodules file.
+	be unique within the gitmodules file.
 
 submodule.<name>.url::
 	Defines a URL from which the submodule repository can be cloned.
@@ -60,7 +63,7 @@ submodule.<name>.fetchRecurseSubmodules::
 	This option can be used to control recursive fetching of this
 	submodule. If this option is also present in the submodules entry in
 	.git/config of the superproject, the setting there will override the
-	one found in .gitmodules.
+	one found in the gitmodules file.
 	Both settings can be overridden on the command line by using the
 	"--[no-]recurse-submodules" option to "git fetch" and "git pull".
 
@@ -86,7 +89,7 @@ submodule.<name>.ignore::
 
 	If this option is also present in the submodules entry in .git/config
 	of the superproject, the setting there will override the one found in
-	.gitmodules.
+	the gitmodules file.
 	Both settings can be overridden on the command line by using the
 	"--ignore-submodule" option. The 'git submodule' commands are not
 	affected by this setting.
diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
index 3b9faabdb..abfb70227 100644
--- a/Documentation/gitsubmodules.txt
+++ b/Documentation/gitsubmodules.txt
@@ -29,13 +29,13 @@ the submodule's working directory pointing to (i).
 Assuming the submodule has a Git directory at `$GIT_DIR/modules/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
+in its gitmodules file (see linkgit:gitmodules[5]) of the form
 `submodule.foo.path = path/to/bar`.
 
 The `gitlink` entry contains the object name of the commit that the
 superproject expects the submodule's working directory to be at.
 
-The section `submodule.foo.*` in the `.gitmodules` file gives additional
+The section `submodule.foo.*` in the gitmodules file gives additional
 hints to Git's porcelain layer. For example, the `submodule.foo.url`
 setting specifies where to obtain the submodule.
 
@@ -108,7 +108,7 @@ If the submodule is not yet initialized, then the configuration
 inside the submodule does not exist yet, so where to
 obtain the submodule from is configured here for example.
 
- * The `.gitmodules` file inside the superproject. A project usually
+ * The gitmodules file inside the superproject. A project usually
    uses this file to suggest defaults for the upstream collection
    of repositories for the mapping that is required between a
    submodule's name and its path.
@@ -127,12 +127,12 @@ FORMS
 Submodules can take the following forms:
 
  * The basic form described in DESCRIPTION with a Git directory,
-a working directory, a `gitlink`, and a `.gitmodules` entry.
+a working directory, a `gitlink`, and a linkgit:gitmodules[5] entry.
 
  * "Old-form" submodule: A working directory with an embedded
-`.git` directory, and the tracking `gitlink` and `.gitmodules` entry in
-the superproject. This is typically found in repositories generated
-using older versions of Git.
+`.git` directory, and the tracking `gitlink` and linkgit:gitmodules[5] entry
+in the superproject. This is typically found in repositories generated using
+older versions of Git.
 +
 It is possible to construct these old form repositories manually.
 +
@@ -140,7 +140,7 @@ When deinitialized or deleted (see below), the submodule's Git
 directory is automatically moved to `$GIT_DIR/modules/<name>/`
 of the superproject.
 
- * Deinitialized submodule: A `gitlink`, and a `.gitmodules` entry,
+ * Deinitialized submodule: A `gitlink`, and a linkgit:gitmodules[5] entry,
 but no submodule working directory. The submodule's Git directory
 may be there as after deinitializing the Git directory is kept around.
 The directory which is supposed to be the working directory is empty instead.
@@ -155,7 +155,7 @@ is not affected. This can be undone using `git submodule init`.
 using `git revert`.
 +
 The deletion removes the superproject's tracking data, which are
-both the `gitlink` entry and the section in the `.gitmodules` file.
+both the `gitlink` entry and the section in the gitmodules file.
 The submodule's working directory is removed from the file
 system, but the Git directory is kept around as it to make it
 possible to checkout past commits without requiring fetching
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index ee907c4a8..1b9f86493 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -4,7 +4,7 @@ submodule config cache API
 The submodule config cache API allows to read submodule
 configurations/information from specified revisions. Internally
 information is lazily read into a cache that is used to avoid
-unnecessary parsing of the same .gitmodules files. Lookups can be done by
+unnecessary parsing of the same gitmodules files. Lookups can be done by
 submodule path or name.
 
 Usage
@@ -12,7 +12,7 @@ Usage
 
 To initialize the cache with configurations from the worktree the caller
 typically first calls `gitmodules_config()` to read values from the
-worktree .gitmodules and then to overlay the local git config values
+worktree gitmodules file and then to overlay the local git config values
 `parse_submodule_config_option()` from the config parsing
 infrastructure.
 
@@ -61,6 +61,6 @@ via e.g. `gitmodules_config()`, it will overwrite the null_sha1 entry.
 So in the normal case, when HEAD:.gitmodules is parsed first and then overlayed
 with the repository configuration, the null_sha1 entry contains the local
 configuration of a submodule (e.g. consolidated values from local git
-configuration and the .gitmodules file in the worktree).
+configuration and the gitmodules file).
 
 For an example usage see test-submodule-config.c.
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 352deda69..a86f8b6f5 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -28,7 +28,7 @@ as a subdirectory of your application.
 
 Subtrees are not to be confused with submodules, which are meant for
 the same task. Unlike submodules, subtrees do not need any special
-constructions (like .gitmodules files or gitlinks) be present in
+constructions (like gitmodules files or gitlinks) be present in
 your repository, and do not force end-users of your
 repository to do anything special or to understand how subtrees
 work. A subtree is just a subdirectory that can be
-- 
2.17.0

[RFC 05/10] submodule: adjust references to '.gitmodules' in comments

[Antonio Ospite]

In the comments refer to a more generic "gitmodules file" instead of
using the '.gitmodules' file name directly.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 builtin/submodule--helper.c |  2 +-
 config.c                    |  7 +++----
 config.h                    |  7 +++----
 git-submodule.sh            | 10 +++++-----
 repository.h                |  2 +-
 submodule-config.c          |  8 ++++----
 submodule-config.h          |  2 +-
 submodule.c                 | 24 ++++++++++++------------
 8 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 72b95d27b..7e8fa26c9 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -476,7 +476,7 @@ static void init_submodule(const char *path, const char *prefix,
 	/*
 	 * Copy url setting when it is not set yet.
 	 * To look up the url in .git/config, we must not fall back to
-	 * .gitmodules, so look it up directly.
+	 * the gitmodules file, so look it up directly.
 	 */
 	strbuf_addf(&sb, "submodule.%s.url", sub->name);
 	if (git_config_get_string(sb.buf, &url)) {
diff --git a/config.c b/config.c
index 6ffb1d501..9921b6c2c 100644
--- a/config.c
+++ b/config.c
@@ -2079,11 +2079,10 @@ int git_config_get_pathname(const char *key, const char **dest)
 
 /*
  * Note: This function exists solely to maintain backward compatibility with
- * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
- * NOT be used anywhere else.
+ * 'fetch' and 'update_clone' storing configuration in the gitmodules file and
+ * should NOT be used anywhere else.
  *
- * Runs the provided config function on the '.gitmodules' file found in the
- * working directory.
+ * Runs the provided config function on the gitmodules file.
  */
 void config_from_gitmodules(config_fn_t fn, void *data)
 {
diff --git a/config.h b/config.h
index ef70a9cac..37aef35d5 100644
--- a/config.h
+++ b/config.h
@@ -191,11 +191,10 @@ extern int repo_config_get_pathname(struct repository *repo,
 
 /*
  * Note: This function exists solely to maintain backward compatibility with
- * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
- * NOT be used anywhere else.
+ * 'fetch' and 'update_clone' storing configuration in the gitmodules file and
+ * should NOT be used anywhere else.
  *
- * Runs the provided config function on the '.gitmodules' file found in the
- * working directory.
+ * Runs the provided config function on the gitmodules file.
  */
 extern void config_from_gitmodules(config_fn_t fn, void *data);
 
diff --git a/git-submodule.sh b/git-submodule.sh
index 610fd0dc5..615a65be9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -59,10 +59,10 @@ die_if_unmatched ()
 # $3 = default value
 #
 # Checks in the usual git-config places first (for overrides),
-# otherwise it falls back on .gitmodules.  This allows you to
-# distribute project-wide defaults in .gitmodules, while still
+# otherwise it falls back to the gitmodules file.  This allows you to
+# distribute project-wide defaults in the gitmodules file, while still
 # customizing individual repositories if necessary.  If the option is
-# not in .gitmodules either, print a default value.
+# not in the gitmodules file either, print a default value.
 #
 get_submodule_config () {
 	name="$1"
@@ -95,7 +95,7 @@ sanitize_submodule_env()
 }
 
 #
-# Add a new submodule to the working tree, .gitmodules and the index
+# Add a new submodule to the working tree, the gitmodules file and the index
 #
 # $@ = repo path
 #
@@ -960,7 +960,7 @@ cmd_status()
 #
 # Sync remote urls for submodules
 # This makes the value for remote.$remote.url match the value
-# specified in .gitmodules.
+# specified in the gitmodules file.
 #
 cmd_sync()
 {
diff --git a/repository.h b/repository.h
index 09df94a47..2db62af0e 100644
--- a/repository.h
+++ b/repository.h
@@ -59,7 +59,7 @@ struct repository {
 	 */
 	struct config_set *config;
 
-	/* Repository's submodule config as defined by '.gitmodules' */
+	/* Repository's submodule config as defined by the gitmodules file */
 	struct submodule_cache *submodule_cache;
 
 	/*
diff --git a/submodule-config.c b/submodule-config.c
index 620d522ee..18984473b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -9,7 +9,7 @@
 /*
  * submodule cache lookup structure
  * There is one shared set of 'struct submodule' entries which can be
- * looked up by their sha1 blob id of the .gitmodules file and either
+ * looked up by their sha1 blob id of the gitmodules file and either
  * using path or name as key.
  * for_path stores submodule entries with path as key
  * for_name stores submodule entries with name as key
@@ -91,7 +91,7 @@ static void submodule_cache_clear(struct submodule_cache *cache)
 	/*
 	 * We iterate over the name hash here to be symmetric with the
 	 * allocation of struct submodule entries. Each is allocated by
-	 * their .gitmodules blob sha1 and submodule name.
+	 * their gitmodules file blob sha1 and submodule name.
 	 */
 	hashmap_iter_init(&cache->for_name, &iter);
 	while ((entry = hashmap_iter_next(&iter)))
@@ -476,7 +476,7 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
 }
 
 /* This does a lookup of a submodule configuration by name or by path
- * (key) with on-demand reading of the appropriate .gitmodules from
+ * (key) with on-demand reading of the appropriate gitmodules file from
  * revisions.
  */
 static const struct submodule *config_from(struct submodule_cache *cache,
@@ -614,7 +614,7 @@ static void gitmodules_read_check(struct repository *repo)
 {
 	submodule_cache_check_init(repo);
 
-	/* read the repo's .gitmodules file if it hasn't been already */
+	/* read the repo's gitmodules file if it hasn't been already */
 	if (!repo->submodule_cache->gitmodules_read)
 		repo_read_gitmodules(repo);
 }
diff --git a/submodule-config.h b/submodule-config.h
index a5503a5d1..75fa54cf2 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -17,7 +17,7 @@ struct submodule {
 	const char *ignore;
 	const char *branch;
 	struct submodule_update_strategy update_strategy;
-	/* the sha1 blob id of the responsible .gitmodules file */
+	/* the sha1 blob id of the responsible gitmodules file */
 	unsigned char gitmodules_sha1[20];
 	int recommend_shallow;
 };
diff --git a/submodule.c b/submodule.c
index 97213b549..d9b84760f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -30,16 +30,16 @@ static struct oid_array ref_tips_before_fetch;
 static struct oid_array ref_tips_after_fetch;
 
 /*
- * Check if the .gitmodules file is unmerged. Parsing of the .gitmodules file
- * will be disabled because we can't guess what might be configured in
- * .gitmodules unless the user resolves the conflict.
+ * Check if the gitmodules file is unmerged. Parsing of the gitmodules
+ * file will be disabled because we can't guess what might be configured in
+ * the gitmodules file unless the user resolves the conflict.
  */
 int is_gitmodules_unmerged(const struct index_state *istate)
 {
 	int pos = index_name_pos(istate, submodules_file, strlen(submodules_file));
-	if (pos < 0) { /* .gitmodules not found or isn't merged */
+	if (pos < 0) { /* gitmodules file not found or isn't merged */
 		pos = -1 - pos;
-		if (istate->cache_nr > pos) {  /* there is a .gitmodules */
+		if (istate->cache_nr > pos) {  /* there is a gitmodules file */
 			const struct cache_entry *ce = istate->cache[pos];
 			if (ce_namelen(ce) == strlen(submodules_file) &&
 			    !strcmp(ce->name, submodules_file))
@@ -51,8 +51,8 @@ int is_gitmodules_unmerged(const struct index_state *istate)
 }
 
 /*
- * Check if the .gitmodules file has unstaged modifications.  This must be
- * checked before allowing modifications to the .gitmodules file with the
+ * Check if the gitmodules file has unstaged modifications.  This must be
+ * checked before allowing modifications to the gitmodules file with the
  * intention to stage them later, because when continuing we would stage the
  * modifications the user didn't stage herself too. That might change in a
  * future version when we learn to stage the changes we do ourselves without
@@ -82,7 +82,7 @@ static int for_each_remote_ref_submodule(const char *submodule,
 
 /*
  * Try to update the "path" entry in the "submodule.<name>" section of the
- * .gitmodules file. Return 0 only if a .gitmodules file was found, a section
+ * gitmodules file. Return 0 only if a gitmodules file was found, a section
  * with the correct path=<oldpath> setting was found and we could update it.
  */
 int update_path_in_gitmodules(const char *oldpath, const char *newpath)
@@ -90,7 +90,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 	struct strbuf entry = STRBUF_INIT;
 	const struct submodule *submodule;
 
-	if (!file_exists(submodules_file)) /* Do nothing without .gitmodules */
+	if (!file_exists(submodules_file)) /* Do nothing without a gitmodules file */
 		return -1;
 
 	if (is_gitmodules_unmerged(&the_index))
@@ -115,8 +115,8 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 }
 
 /*
- * Try to remove the "submodule.<name>" section from .gitmodules where the given
- * path is configured. Return 0 only if a .gitmodules file was found, a section
+ * Try to remove the "submodule.<name>" section from the gitmodules file where the given
+ * path is configured. Return 0 only if a gitmodules file was found, a section
  * with the correct path=<path> setting was found and we could remove it.
  */
 int remove_path_from_gitmodules(const char *path)
@@ -124,7 +124,7 @@ int remove_path_from_gitmodules(const char *path)
 	struct strbuf sect = STRBUF_INIT;
 	const struct submodule *submodule;
 
-	if (!file_exists(submodules_file)) /* Do nothing without .gitmodules */
+	if (!file_exists(submodules_file)) /* Do nothing without a gitmodules file */
 		return -1;
 
 	if (is_gitmodules_unmerged(&the_index))
-- 
2.17.0

[RFC 06/10] completion: add 'core.submodulesfile' to the git-completion.bash file

[Antonio Ospite]

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a75707394..19beb6735 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2382,6 +2382,7 @@ _git_config ()
 		core.sparseCheckout
 		core.splitIndex
 		core.sshCommand
+		core.submodulesfile
 		core.symlinks
 		core.trustctime
 		core.untrackedCache
-- 
2.17.0

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Stefan Beller]

Hi Antonio,

On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> Hi,
>
> vcsh[1] uses bare git repositories and detached work-trees to manage
> *distinct* sets of configuration files directly into $HOME.
>
> In general, submodules have worked perfectly fine with detached
> work-trees for some time[2,3,4].
>
> However when multiple repositories take turns using the same directory
> as their work-tree, and more than one of them want to use submodules,
> there could still be conflicts about the '.gitmodules' file because git
> hardcodes this path.
>
> For comparison, in case of '.gitignore' a similar conflict might arise,
> but git has alternative ways to specify exclude files, so vcsh solves
> this by setting core.excludesFile for each repository and track ignored
> files somewhere else (in ~/.gitignore.d/$VCSH_REPO_NAME).
>
> This is currently not possible with submodules configuration.

So far I agree w.r.t. Gits capabilities.

> So this series proposes a mechanism to set an alternative path for the
> submodules configuration file (from now on "gitmodules file").

That sounds interesting, so let's read on.

> Patches are based on fe0a9eaf31dd0c349ae4308498c33a5c3794b293.

... which is the current master branch by Junio.

> In commit 4c0eeafe4755 (cache.h: add GITMODULES_FILE macro)[5] the
> gitmodules file path definition was centralized, AFAIU this was done
> mainly to prevent typos, as checking a symbolic constant is something
> the compiler will do for us.

+cc Brandon author of said patch.

Digging up the discussion, this was indeed only done to prevent typos.
https://public-inbox.org/git/20170802172633.GA36159@google.com/

> Expanding on that change the first patch in the series makes the path
> customizable exposing a 'core.submodulesFile' configuration setting.

I guess the similarity to core.ignoreFile is desired here. Although these
mechanisms are very different.

> The new configuration setting can be used to set an *alternative*
> location for the gitmodules file; IMHO there is no need to provide
> *additional* locations like in the case of exclude files.

I think there *may* be a need for additional files.
Currently there is only the .gitmodules file and the configuration
in .git/config overriding settings from .gitmodules.

There was some discussion on the mailing list in the past, which
presented a intermediate layer in between these two places, in
a special ref, such that:
    base is in .gitmodules
    overwritten via refs/meta/submodules:.gitmodules
    overwritten via the .git/config

The intermediate would be a config file that is tracked on another
ref. This (a) decouples main project history from submodule history
and (b) makes it easier to distribute as it is part of the repository.

For example (a) is desired if you dig up an old project and the
submodules have all moved from one git hosting provider to another.
Another example would be when you fork a project with submodules
and don't want to mess with the main history but you just want to
adjust the submodule URLs. That is possible with such an intermediate
additional place.

For (b) you can imagine the fork that you want to distribute in your
community and you don't want to tell everyone to change the
submodule URLs, but instead you can provide them with a prepared
.gitmodules file, that they have to place into that special ref (which
can be done via fetching).

I digress as these ideas seem to be orthogonal to your patch series,
just FYI. prior discussion starting at:
https://public-inbox.org/git/1462317985-640-1-git-send-email-sbeller@google.com/
I recall there was a better discussion even prior to that, but have no
link handy.


> For instance vcsh could set the location to
> '~/.gitmodules.d/$VCSH_REPO_NAME' to avoid conflicts.
>
> Since the gitmodules file is meant to be checked in into the repository,
> the overridden file path should be relative to the work-tree; is there
> a way to enforce this constraint at run time (i.e. validate the config
> value), or is it enough to have it documented?

I think we'd want to check at run time, if we need this constraint.

Thanks,
Stefan

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Stefan Beller]

Hi Antonio,

On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> When multiple repositories with detached work-trees take turns using the
> same directory as their work-tree, and more than one of them want to use
> submodules, there will be conflicts about the '.gitmodules' file.

unlike other files which would not conflict?
There might be file names such as LICENSE, Readme.md etc,
which are common enough that they would produce conflicts as well?
I find this argument on its own rather weak. ("Just delete everything in
the working dir before using it with another repository"). I might be
missing a crucial bit here?

> git hardcodes this path so it's not possible to override its location on
> a per-repository basis to allow such repositories to coexists
> peacefully.
>
> Make the path of the "gitmodules file" customizable exposing
> a 'core.submodulesFile' configuration setting.
>
> The default value will still be '.gitmodules' when 'core.submodulesFile'
> is not set.

ok.


> --- a/cache.h
> +++ b/cache.h
> @@ -1774,6 +1774,7 @@ extern void prepare_pager_args(struct child_process *, const char *pager);
>  extern const char *editor_program;
>  extern const char *askpass_program;
>  extern const char *excludes_file;
> +extern const char *submodules_file;

Could you place this variable in repository.h in struct repository?
(Some developers currently try to move any global state to that place,
as that makes working with e.g. nested submodules easier in-process
and you would not need to spawn processes for submodules)

Once migrated to the repository struct mentioned above, you'd access
it via the_repository->submodules_file for the main repository.


> diff --git a/git-submodule.sh b/git-submodule.sh
> index 24914963c..610fd0dc5 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -71,7 +71,9 @@ get_submodule_config () {
>         value=$(git config submodule."$name"."$option")
>         if test -z "$value"
>         then
> -               value=$(git config -f .gitmodules submodule."$name"."$option")
> +               gitmodules_file=$(git config core.submodulesfile)
> +               : ${gitmodules_file:=.gitmodules}
> +               value=$(git config -f "$gitmodules_file" submodule."$name"."$option")

I wonder if it would be cheaper to write a special config lookup now, e.g.
in builtin/submodule--helper.c we could have a "config-from-gitmodules"
subcommand that is looking up the modules file and then running the config
on that file.

I am surprised how little access of the .gitmodules is left in git-submodule.sh
(which is partially ported to the builtin/submodule--helper.c)

> diff --git a/submodule-config.c b/submodule-config.c
> index 3f2075764..8a3396ade 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -468,7 +468,7 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
>                 return 1;
>         }
>
> -       strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
> +       strbuf_addf(rev, "%s:%s", oid_to_hex(treeish_name), submodules_file);
>         if (get_oid(rev->buf, gitmodules_oid) >= 0)
>                 ret = 1;
>
> @@ -583,7 +583,7 @@ void repo_read_gitmodules(struct repository *repo)
>                 if (repo_read_index(repo) < 0)
>                         return;
>
> -               gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
> +               gitmodules = repo_worktree_path(repo, submodules_file);
>
>                 if (!is_gitmodules_unmerged(repo->index))
>                         git_config_from_file(gitmodules_cb, gitmodules, repo);
> diff --git a/submodule.c b/submodule.c
> index 9a50168b2..2afbdb644 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -36,13 +36,13 @@ static struct oid_array ref_tips_after_fetch;
>   */
>  int is_gitmodules_unmerged(const struct index_state *istate)
>  {
> -       int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
> +       int pos = index_name_pos(istate, submodules_file, strlen(submodules_file));

Ah, regarding the coverletter: This clearly assumes the modules
file is in the tree. So at least here we would make an exception
for files outside the tree to either not check for un-merged-ness or
disallow that case entirely.



There are quite a few functions in submodule.c which access the new global. :/
So moving them to the_repository should be fine, but eventually (not
in this series)
all these functions would would want to take a repository argument as well
such that they work on more than the_repository.

Thanks,
Stefan

Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Stefan Beller]

Hi Antonio,

the subject line could also be
  fetch: fix custom submodule location
as I was not expecting this patch from the subject line.

On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> Import the default git configuration before accessing the gitmodules
> file.
>
> This is important when a value different from the default one is set via
> the 'core.submodulesfile' config.
>
> Signed-off-by: Antonio Ospite <ao2@ao2.it>
> ---
>  builtin/fetch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index dcdfc66f0..d56636f74 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1428,8 +1428,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>         for (i = 1; i < argc; i++)
>                 strbuf_addf(&default_rla, " %s", argv[i]);
>
> -       config_from_gitmodules(gitmodules_fetch_config, NULL);
>         git_config(git_fetch_config, NULL);
> +       config_from_gitmodules(gitmodules_fetch_config, NULL);

Wouldn't this break the overwriting behavior?
e.g. you configure a submodule URL in .gitmodules and then override it
in .git/config,
then we'd currently first load config from the modules file and then override it
in git_config(git_fetch_config,..), whereas swapping them might have
unintended consequences? Do the tests pass with this patch?

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Antonio Ospite]

On Fri, 13 Apr 2018 00:20:37 +0200
Antonio Ospite <ao2@ao2.it> wrote:

[...]
> Antonio Ospite (10):
>   submodule: add 'core.submodulesFile' to override the '.gitmodules'
>     path
>   submodule: fix getting custom gitmodule file in fetch command
>   submodule: use the 'submodules_file' variable in output messages
>   submodule: document 'core.submodulesFile' and fix references to
>     '.gitmodules'
>   submodule: adjust references to '.gitmodules' in comments
>   completion: add 'core.submodulesfile' to the git-completion.bash file
>   XXX: wrap-for-bin.sh: set 'core.submodulesFile' for each git
>     invocation
>   XXX: submodule: fix t1300-repo-config.sh to take into account the new
>     config
>   XXX: submodule: pass custom gitmodules file to 'test-tool
>     submodule-config'
>   XXX: add a hacky script to test the changes with a patched test suite
> 

I am re-sending the last four highly experimental patches changing the
"XXX" into "FIXME" in the subject lines because vger.kernel.org refused
the original ones with the following message:

  The capital Triple-X in subject is way too often associated with junk
  email, please rephrase.

Sorry for the inconvenience.

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[RFC 07/10] FIXME: wrap-for-bin.sh: set 'core.submodulesFile' for each git invocation

[Antonio Ospite]

This is to test custom gitmodules file paths. The default path can be
overridden using the 'GIT_MODULES_FILE' environmental variable.

Maybe In the final patch the option should be set only when running
tests and not unconditionally in the wrapper script, but as a proof of
concept the wrapper script was a convenient location.

Also, in the final patch a fixed custom file path could be used instead
of the environmental variable: to exercise the code it should be enough
to have a value different from the default one.

The change to 't0001-init.sh' is needed to make the test pass, since now
a config is set on the command line.
---
 Makefile        | 3 ++-
 t/t0001-init.sh | 1 +
 wrap-for-bin.sh | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)
 mode change 100644 => 100755 wrap-for-bin.sh

diff --git a/Makefile b/Makefile
index f18168725..38ee1f6a2 100644
--- a/Makefile
+++ b/Makefile
@@ -2480,7 +2480,8 @@ bin-wrappers/%: wrap-for-bin.sh
 	@mkdir -p bin-wrappers
 	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
-	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))|' < $< > $@ && \
+	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))|' \
+	     -e 's|git\"|git\" $$GIT_OPTIONS|' < $< > $@ && \
 	chmod +x $@
 
 # GNU make supports exporting all variables by "export" without parameters.
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index c413bff9c..6fa3fd24e 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -93,6 +93,7 @@ test_expect_success 'No extra GIT_* on alias scripts' '
 		sed -n \
 			-e "/^GIT_PREFIX=/d" \
 			-e "/^GIT_TEXTDOMAINDIR=/d" \
+			-e "/^GIT_CONFIG_PARAMETERS=/d" \
 			-e "/^GIT_/s/=.*//p" |
 		sort
 	EOF
diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
old mode 100644
new mode 100755
index 584240881..02bf41cbd
--- a/wrap-for-bin.sh
+++ b/wrap-for-bin.sh
@@ -20,6 +20,8 @@ PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
 
 export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
 
+GIT_OPTIONS="-c core.submodulesfile=${GITMODULES_FILE:-.gitmodules}"
+
 if test -n "$GIT_TEST_GDB"
 then
 	unset GIT_TEST_GDB
-- 
2.17.0

[RFC 08/10] FIXME: submodule: fix t1300-repo-config.sh to take into account the new config

[Antonio Ospite]

Tests are run with the 'core.submodulesfile' config set, so
't/t1300-repo-config.sh' needs to be fixed to account for that.

The changes to the HEREDOC lines are temporary and only needed to
support the environmental variable expansion, they could go away
eventually is using a fixed value is good enough.
---
 t/t1300-repo-config.sh | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e95b1e67d..f672a6c37 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -338,6 +338,7 @@ beta.noindent=sillyValue
 nextsection.nonewline=wow2 for me
 123456.a123=987
 version.1.2.3eX.alpha=beta
+core.submodulesfile=${GITMODULES_FILE:-.gitmodules}
 EOF
 
 test_expect_success 'working --list' '
@@ -345,6 +346,7 @@ test_expect_success 'working --list' '
 	test_cmp expect output
 '
 cat > expect << EOF
+core.submodulesfile=${GITMODULES_FILE:-.gitmodules}
 EOF
 
 test_expect_success '--list without repo produces empty output' '
@@ -357,6 +359,7 @@ beta.noindent
 nextsection.nonewline
 123456.a123
 version.1.2.3eX.alpha
+core.submodulesfile
 EOF
 
 test_expect_success '--name-only --list' '
@@ -964,10 +967,11 @@ inued
 inued"
 EOF
 
-cat > expect <<\EOF
+cat > expect << EOF
 section.continued=continued
 section.noncont=not continued
 section.quotecont=cont;inued
+core.submodulesfile=${GITMODULES_FILE:-.gitmodules}
 EOF
 
 test_expect_success 'value continued on next line' '
@@ -984,7 +988,7 @@ cat > .git/config <<\EOF
 	val5
 EOF
 
-cat > expect <<\EOF
+cat > expect << EOF
 section.sub=section.val1
 foo=barQsection.sub=section.val2
 foo
@@ -992,7 +996,8 @@ barQsection.sub=section.val3
 
 
 Qsection.sub=section.val4
-Qsection.sub=section.val5Q
+Qsection.sub=section.val5Qcore.submodulesfile
+${GITMODULES_FILE:-.gitmodules}Q
 EOF
 test_expect_success '--null --list' '
 	git config --null --list >result.raw &&
@@ -1001,6 +1006,17 @@ test_expect_success '--null --list' '
 	test_cmp expect result
 '
 
+cat > expect << EOF
+section.sub=section.val1
+foo=barQsection.sub=section.val2
+foo
+barQsection.sub=section.val3
+
+
+Qsection.sub=section.val4
+Qsection.sub=section.val5Q
+EOF
+
 test_expect_success '--null --get-regexp' '
 	git config --null --get-regexp "val[0-9]" >result.raw &&
 	nul_to_q <result.raw >result &&
@@ -1495,6 +1511,7 @@ test_expect_success '--show-origin with --list' '
 		file:.git/config	user.override=local
 		file:.git/config	include.path=../include/relative.include
 		file:.git/../include/relative.include	user.relative=include
+		command line:	core.submodulesfile=${GITMODULES_FILE:-.gitmodules}
 		command line:	user.cmdline=true
 	EOF
 	git -c user.cmdline=true config --list --show-origin >output &&
@@ -1511,7 +1528,8 @@ test_expect_success '--show-origin with --list --null' '
 		trueQfile:.git/configQuser.override
 		localQfile:.git/configQinclude.path
 		../include/relative.includeQfile:.git/../include/relative.includeQuser.relative
-		includeQcommand line:Quser.cmdline
+		includeQcommand line:Qcore.submodulesfile
+		${GITMODULES_FILE:-.gitmodules}Qcommand line:Quser.cmdline
 		trueQ
 	EOF
 	git -c user.cmdline=true config --null --list --show-origin >output.raw &&
-- 
2.17.0

[RFC 09/10] FIXME: submodule: pass custom gitmodules file to 'test-tool submodule-config'

[Antonio Ospite]

Add a new option to 't/helper/test-submodule-config.c' to set a custom
path for the gitmodules file.

In particular this is needed to make 't/t7411-submodule-config.sh' pass.

The option is actually put in use by the script that patches the test
suite.
---
 t/helper/test-submodule-config.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c
index 5c6e4b010..30b4ea3da 100644
--- a/t/helper/test-submodule-config.c
+++ b/t/helper/test-submodule-config.c
@@ -25,6 +25,13 @@ int cmd__submodule_config(int argc, const char **argv)
 			output_url = 1;
 		if (!strcmp(arg[0], "--name"))
 			lookup_name = 1;
+		if (!strcmp(arg[0], "--submodules_file")) {
+			arg++;
+			my_argc--;
+			submodules_file = expand_user_path(arg[0], 0);
+			if (!submodules_file)
+				die(_("failed to expand user dir in: '%s'"), arg[0]);
+		}
 		arg++;
 		my_argc--;
 	}
-- 
2.17.0

[RFC 10/10] FIXME: add a hacky script to test the changes with a patched test suite

[Antonio Ospite]

Patch all the hardcoded occurrences of '.gitmodules' and make the file
configurable via an environment variable.

This is only for demonstration purposes, the final patch to the test
suite could just use a fixed path for the custom gitmodules file,
matching the path passed in the wrapper script via the
'core.submodulesfile' option.

The rationale would be that testing with a custom gitmodules file covers
also the case of the default gitmodules file path.

Execute 'test-custom-gitmodules-file.sh' to check that the test pass
with either the default anda custom gitmodules file.
---
 test-custom-gitmodules-file.sh | 75 ++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100755 test-custom-gitmodules-file.sh

diff --git a/test-custom-gitmodules-file.sh b/test-custom-gitmodules-file.sh
new file mode 100755
index 000000000..d59b1e40d
--- /dev/null
+++ b/test-custom-gitmodules-file.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+#
+# This is a test script to demonstrate that the changes about
+# 'core.submodulesfile' do not cause regressions and even work as intended
+# when a custom gitmodules file is specified O_O.
+#
+# The script patches the hardcoded '.gitmodules' occurrences to be overridden
+# by an environment variable.
+#
+# Note that 'core.submodulesfile' affects no only the 'submodule' git command
+# but many other git commands that directly or or *indirecly* use the
+# submodules_file variable.
+#
+# In particular all the commands that use unpack_trees(), like 'am', 'clone',
+# etc. are affected.
+#
+# Anyways this is not a problem because the option is passed to all git
+# commands in the "./bin-wrappers/git" wrapper script.
+
+set -e
+
+if [ ! -e .patched_test_suite ];
+then
+
+  echo "Patching the test suite..."
+
+  find t/ -type f -name t7506-status-submodule.sh -exec sed -i \
+    -e "s/^cat \(.*\)\\\EOF/cat \1EOF/g" \
+    -e "s/\(.*\)\.gitmodules$/\1\${GITMODULES_FILE:-.gitmodules}/g" \
+    -e "s/^AA \.gitmodules$/AA \${GITMODULES_FILE:-.gitmodules}/g" \
+    {} \;
+
+  find t/ -type f -name t7508-status.sh -exec sed -i \
+    -e "s/touch \(.*\)\.gitmodules/touch \1\\\"\${GITMODULES_FILE:-.gitmodules}\\\"/g" \
+    -e "s/\t\.gitmodules$/\t\${GITMODULES_FILE:-.gitmodules}/g" \
+    {} \;
+
+  find t/ -type f -exec sed -i \
+    -e "s/git \(.*\)config \(.*\)-f ..\/\.gitmodules/git \1config \2-f ..\/\'\"\${GITMODULES_FILE:-.gitmodules}\"\'/g" \
+    -e "s/git \(.*\)config \(.*\)-f \.gitmodules/git \1config \2-f \"\${GITMODULES_FILE:-.gitmodules}\"/g" \
+    -e "s/git \(.*\)config \(.*\)--file=\.gitmodules/git \1config \2--file=\"\${GITMODULES_FILE:-.gitmodules}\"/g" \
+    -e "s/git add \(.*\)\.gitmodules\(.*\)/git add \1\"\${GITMODULES_FILE:-.gitmodules}\"\2/g" \
+    -e "s/test_cmp expect \.gitmodules/test_cmp expect \"\${GITMODULES_FILE:-.gitmodules}\"/g" \
+    -e "s/cp .gitmodules pristine-\.gitmodules/cp \"\${GITMODULES_FILE:-.gitmodules}\" pristine-gitmodules/g" \
+    -e "s/cp pristine-\.gitmodules .gitmodules/cp pristine-gitmodules \"\${GITMODULES_FILE:-.gitmodules}\"/g" \
+    -e "s/cp \.gitmodules \.gitmodules.bak/cp \'\"\${GITMODULES_FILE:-.gitmodules}\"\' .gitmodules.bak/g" \
+    -e "s/test-tool submodule-config/test-tool submodule-config --submodules_file \'\"\${GITMODULES_FILE:-.gitmodules}\"\'/g" \
+    -e "s/\$sha1:\.gitmodules/\$sha1:\'\"\${GITMODULES_FILE:-.gitmodules}\"\'/g" \
+    -e "s/cat >\.gitmodules/cat >\"\${GITMODULES_FILE:-.gitmodules}\"/g" \
+    -e "s/sed \(.*\)<\.gitmodules/sed \1<\"\${GITMODULES_FILE:-.gitmodules}\"/g" \
+    -e "s/rm \(.*\)\.gitmodules/rm \1\"\${GITMODULES_FILE:-.gitmodules}\"/g" \
+    -e "s/echo \(.*\)\.gitmodules/echo \1\\\"\${GITMODULES_FILE:-.gitmodules}\\\"/g" \
+    -e "s/\t\(.*\)\.gitmodules$/\t\1\'\"\${GITMODULES_FILE:-.gitmodules}\"\'/g" \
+    -e "s/\t\(.*\)\.gitmodules:/\t\1\'\"\${GITMODULES_FILE:-.gitmodules}\"\':/g" \
+    -e "s/\t\(.*\)\.gitmodules \&\&$/\t\1\'\"\${GITMODULES_FILE:-.gitmodules}\"\' \&\&/g" \
+    -e "s/\t\(.*\)\.gitmodules) \&\&$/\t\1\'\"\${GITMODULES_FILE:-.gitmodules}\"\') \&\&/g" \
+    -e "s/repo\/\.gitmodules /repo\/\'\"\${GITMODULES_FILE:-.gitmodules}\"\' /g" \
+    -e "s/Submodule sm2 a5a65c9..280969a:/Submodule sm2 a5a65c9..\'\$(git -C sm2 rev-list -1 HEAD | cut -c1-7)\':/g" \
+    -e "s/diff --git a\/sm2\/\.gitmodules /diff --git a\/sm2\/\'\"\${GITMODULES_FILE:-.gitmodules}\"\' /g" \
+    -e "s/^M\([ ]\{1,2\}\)\.gitmodules$/M\1\${GITMODULES_FILE:-.gitmodules}/g" \
+    -e "s/^D\([ ]\{1,2\}\)\.gitmodules$/D\1\${GITMODULES_FILE:-.gitmodules}/g" \
+    {} \;
+
+    make clean
+    make
+    touch .patched_test_suite
+fi
+
+echo "Running the tests with the default gitmodules file..."
+make test
+
+echo "Running the tests with a custom gitmodules file..."
+GITMODULES_FILE=.gitmodules_custom make test
+
+echo "Done."
-- 
2.17.0

Re: [RFC 07/10] FIXME: wrap-for-bin.sh: set 'core.submodulesFile' for each git invocation

[Stefan Beller]

Hi Antonio,

On Fri, Apr 13, 2018 at 1:07 AM, Antonio Ospite <ao2@ao2.it> wrote:
> This is to test custom gitmodules file paths. The default path can be
> overridden using the 'GIT_MODULES_FILE' environmental variable.
>
> Maybe In the final patch the option should be set only when running
> tests and not unconditionally in the wrapper script, but as a proof of
> concept the wrapper script was a convenient location.
>
> Also, in the final patch a fixed custom file path could be used instead
> of the environmental variable: to exercise the code it should be enough
> to have a value different from the default one.
>
> The change to 't0001-init.sh' is needed to make the test pass, since now
> a config is set on the command line.

Missing sign off.

So you'd think we'd have to rerun the test suite with GIT_MODULES_FILE set?
That makes for an expensive test. Can we have just a few tests for a few
commands to see that the basics are working correctly?


> ---
>  Makefile        | 3 ++-
>  t/t0001-init.sh | 1 +
>  wrap-for-bin.sh | 2 ++
>  3 files changed, 5 insertions(+), 1 deletion(-)
>  mode change 100644 => 100755 wrap-for-bin.sh
>
> diff --git a/Makefile b/Makefile
> index f18168725..38ee1f6a2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2480,7 +2480,8 @@ bin-wrappers/%: wrap-for-bin.sh
>         @mkdir -p bin-wrappers
>         $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
>              -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
> -            -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))|' < $< > $@ && \
> +            -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))|' \
> +            -e 's|git\"|git\" $$GIT_OPTIONS|' < $< > $@ && \
>         chmod +x $@
>
>  # GNU make supports exporting all variables by "export" without parameters.
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index c413bff9c..6fa3fd24e 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -93,6 +93,7 @@ test_expect_success 'No extra GIT_* on alias scripts' '
>                 sed -n \
>                         -e "/^GIT_PREFIX=/d" \
>                         -e "/^GIT_TEXTDOMAINDIR=/d" \
> +                       -e "/^GIT_CONFIG_PARAMETERS=/d" \
>                         -e "/^GIT_/s/=.*//p" |
>                 sort
>         EOF
> diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
> old mode 100644
> new mode 100755
> index 584240881..02bf41cbd
> --- a/wrap-for-bin.sh
> +++ b/wrap-for-bin.sh
> @@ -20,6 +20,8 @@ PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
>
>  export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
>
> +GIT_OPTIONS="-c core.submodulesfile=${GITMODULES_FILE:-.gitmodules}"
> +
>  if test -n "$GIT_TEST_GDB"
>  then
>         unset GIT_TEST_GDB
> --
> 2.17.0
>

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Antonio Ospite]

On Thu, 12 Apr 2018 16:36:32 -0700
Stefan Beller <sbeller@google.com> wrote:

> Hi Antonio,
>

Hi Stefan,

> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> > Hi,
> >
> > vcsh[1] uses bare git repositories and detached work-trees to manage
> > *distinct* sets of configuration files directly into $HOME.
> >
> > In general, submodules have worked perfectly fine with detached
> > work-trees for some time[2,3,4].
> >
> > However when multiple repositories take turns using the same directory
> > as their work-tree, and more than one of them want to use submodules,
> > there could still be conflicts about the '.gitmodules' file because git
> > hardcodes this path.
> >
[...]
> 
> > So this series proposes a mechanism to set an alternative path for the
> > submodules configuration file (from now on "gitmodules file").
> 
[...]
> > Expanding on that change the first patch in the series makes the path
> > customizable exposing a 'core.submodulesFile' configuration setting.
> 
> I guess the similarity to core.ignoreFile is desired here. Although these
> mechanisms are very different.
>

The option name is similar to 'core.excludesFile' also because, when I
started, I first looked at how multiple ignore files were used, so I
may have been influenced by that.

I acknowledge that the two mechanisms are different, in particular
because git *writes* the gitmodules file itself.

I am not opposed to change the name, something like
'core.submodulesConfigFile' might highlight that the syntax of the file
is the one of git config, but I think it's too long.

> > The new configuration setting can be used to set an *alternative*
> > location for the gitmodules file; IMHO there is no need to provide
> > *additional* locations like in the case of exclude files.
> 
> I think there *may* be a need for additional files.
> Currently there is only the .gitmodules file and the configuration
> in .git/config overriding settings from .gitmodules.
> 
> There was some discussion on the mailing list in the past, which
> presented a intermediate layer in between these two places, in
> a special ref, such that:
>     base is in .gitmodules
>     overwritten via refs/meta/submodules:.gitmodules
>     overwritten via the .git/config
> 
> The intermediate would be a config file that is tracked on another
> ref. This (a) decouples main project history from submodule history
> and (b) makes it easier to distribute as it is part of the repository.
> 
> For example (a) is desired if you dig up an old project and the
> submodules have all moved from one git hosting provider to another.
> Another example would be when you fork a project with submodules
> and don't want to mess with the main history but you just want to
> adjust the submodule URLs. That is possible with such an intermediate
> additional place.
> 
> For (b) you can imagine the fork that you want to distribute in your
> community and you don't want to tell everyone to change the
> submodule URLs, but instead you can provide them with a prepared
> .gitmodules file, that they have to place into that special ref (which
> can be done via fetching).
> 
> I digress as these ideas seem to be orthogonal to your patch series,
> just FYI. prior discussion starting at:
> https://public-inbox.org/git/1462317985-640-1-git-send-email-sbeller@google.com/
> I recall there was a better discussion even prior to that, but have no
> link handy.
>

Just to understand, is that 'refs/meta/submodules:.gitmodules' file
meant to be managed manually? Or do you imagine some options to
instruct "git submodule" to *write* to that file instead of .gitmodules?

In the latter case your idea could cover my use case too, couldn't it?

But most importantly, is this realistically going to be added?
Currently I am not that familiar with the git code base to do it
myself, and I've never explicitly dealt with special refs in git.

The approach of this patch series is a lot simpler, and something I can
work on in my spare time.

Presumably (b) could even be partially supported with my changes, by
having both '.gitmodules' and some custom '.gitmodules-alternative'
files in the repository and tell users to clone with:

  git clone --config core.submodulesFile=.gitmodules-alternative URL

Not as clean as your idea but doable.

[...]
> > Since the gitmodules file is meant to be checked in into the repository,
> > the overridden file path should be relative to the work-tree; is there
> > a way to enforce this constraint at run time (i.e. validate the config
> > value), or is it enough to have it documented?
> 
> I think we'd want to check at run time, if we need this constraint.
> 

I'll look into it once we decide which the way to go.

Thank you.

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 07/10] FIXME: wrap-for-bin.sh: set 'core.submodulesFile' for each git invocation

[Antonio Ospite]

On Fri, 13 Apr 2018 13:05:18 -0700
Stefan Beller <sbeller@google.com> wrote:

> Hi Antonio,
> 
> On Fri, Apr 13, 2018 at 1:07 AM, Antonio Ospite <ao2@ao2.it> wrote:
> > This is to test custom gitmodules file paths. The default path can be
> > overridden using the 'GIT_MODULES_FILE' environmental variable.
> >
> > Maybe In the final patch the option should be set only when running
> > tests and not unconditionally in the wrapper script, but as a proof of
> > concept the wrapper script was a convenient location.
> >
> > Also, in the final patch a fixed custom file path could be used instead
> > of the environmental variable: to exercise the code it should be enough
> > to have a value different from the default one.
> >
> > The change to 't0001-init.sh' is needed to make the test pass, since now
> > a config is set on the command line.
> 
> Missing sign off.
> 
> So you'd think we'd have to rerun the test suite with GIT_MODULES_FILE set?
> That makes for an expensive test. Can we have just a few tests for a few
> commands to see that the basics are working correctly?
>

This approach of running the test suite twice was only "exploratory",
see the script in 10/10: it was never meant as a the final mechanism.

That's also why the FIXME patches do not have a sign-off.

As I tried to comment above a final version could just
replace the references to '.gitmodules' with some fixed non-default
value, provided that the same value is passed to git via
core.submodulesfile 

If the tests pass with a custom value they'll pass with the default
'.gitmodules' one which is a particular case.

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Antonio Ospite]

On Thu, 12 Apr 2018 16:55:15 -0700
Stefan Beller <sbeller@google.com> wrote:

> Hi Antonio,
> 
> the subject line could also be
>   fetch: fix custom submodule location
> as I was not expecting this patch from the subject line.
>

OK.

> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> > Import the default git configuration before accessing the gitmodules
> > file.
> >
> > This is important when a value different from the default one is set via
> > the 'core.submodulesfile' config.
> >
> > Signed-off-by: Antonio Ospite <ao2@ao2.it>
> > ---
> >  builtin/fetch.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index dcdfc66f0..d56636f74 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -1428,8 +1428,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> >         for (i = 1; i < argc; i++)
> >                 strbuf_addf(&default_rla, " %s", argv[i]);
> >
> > -       config_from_gitmodules(gitmodules_fetch_config, NULL);
> >         git_config(git_fetch_config, NULL);
> > +       config_from_gitmodules(gitmodules_fetch_config, NULL);
> 
> Wouldn't this break the overwriting behavior?
> e.g. you configure a submodule URL in .gitmodules and then override it
> in .git/config,
> then we'd currently first load config from the modules file and then override it
> in git_config(git_fetch_config,..), whereas swapping them might have
> unintended consequences? Do the tests pass with this patch?

The patch changes the current behavior indeed, but it does not break
any of the existing tests.

Anyway, to be on the safe side, I could load only the
core.submodulesFile option from the global configuration, maybe even in
config_from_gitmodules() itself, before accessing the gitmodules file,
but I still don't know how to do that.

Is there an API to just load one config setting?

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Antonio Ospite]

On Thu, 12 Apr 2018 16:50:03 -0700
Stefan Beller <sbeller@google.com> wrote:

> Hi Antonio,
> 
> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> > When multiple repositories with detached work-trees take turns using the
> > same directory as their work-tree, and more than one of them want to use
> > submodules, there will be conflicts about the '.gitmodules' file.
> 
> unlike other files which would not conflict?
> There might be file names such as LICENSE, Readme.md etc,
> which are common enough that they would produce conflicts as well?
> I find this argument on its own rather weak. ("Just delete everything in
> the working dir before using it with another repository"). I might be
> missing a crucial bit here?
>

All the vcsh repositories _share_ the same work-tree; they may control
it taking turns but, in general, all files are meant to be checked out
at all times as the basic use case is: *distinct* sets of config files.

Maybe saying that the repositories "take turns" is confusing.
It's an unnecessary information, so I will omit that part form the
commit message.

After your question I've done some research and I've seen other vcsh
users managing conflicting LICENSE and README files using git
sparse-checkouts, to have these files in the single repositories but
not checked out in the shared work-tree:
https://github.com/RichiH/vcsh/issues/120#issuecomment-42639619
https://github.com/jwhitley/vcsh-root/commit/30b0d495c2cbe47ae9617ace9c2c14720d961d78

However I guess that my point here is that the gitmodules file is
something that influences git behavior so it should not be on the user's
shoulder to manage conflicts for it, and most importantly it needs to
be checked out for git to access it, doesn't it?

> > git hardcodes this path so it's not possible to override its location on
> > a per-repository basis to allow such repositories to coexists
> > peacefully.
> >
> > Make the path of the "gitmodules file" customizable exposing
> > a 'core.submodulesFile' configuration setting.
> >
> > The default value will still be '.gitmodules' when 'core.submodulesFile'
> > is not set.
> 
> ok.
> 
> 
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1774,6 +1774,7 @@ extern void prepare_pager_args(struct child_process *, const char *pager);
> >  extern const char *editor_program;
> >  extern const char *askpass_program;
> >  extern const char *excludes_file;
> > +extern const char *submodules_file;
> 
> Could you place this variable in repository.h in struct repository?
> (Some developers currently try to move any global state to that place,
> as that makes working with e.g. nested submodules easier in-process
> and you would not need to spawn processes for submodules)
> 
> Once migrated to the repository struct mentioned above, you'd access
> it via the_repository->submodules_file for the main repository.
>

OK, thanks, I didn't like the global variable either, I was just copying
from excludes_file.

> 
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 24914963c..610fd0dc5 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -71,7 +71,9 @@ get_submodule_config () {
> >         value=$(git config submodule."$name"."$option")
> >         if test -z "$value"
> >         then
> > -               value=$(git config -f .gitmodules submodule."$name"."$option")
> > +               gitmodules_file=$(git config core.submodulesfile)
> > +               : ${gitmodules_file:=.gitmodules}
> > +               value=$(git config -f "$gitmodules_file" submodule."$name"."$option")
> 
> I wonder if it would be cheaper to write a special config lookup now, e.g.
> in builtin/submodule--helper.c we could have a "config-from-gitmodules"
> subcommand that is looking up the modules file and then running the config
> on that file.
>

Can you give an example from the user point of view of such a
"config-from-gitmodules" command?

I might look into it, but that can also be a followup change. 

> I am surprised how little access of the .gitmodules is left in git-submodule.sh
> (which is partially ported to the builtin/submodule--helper.c)
> 
> > diff --git a/submodule-config.c b/submodule-config.c
> > index 3f2075764..8a3396ade 100644
> > --- a/submodule-config.c
> > +++ b/submodule-config.c
> > @@ -468,7 +468,7 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
> >                 return 1;
> >         }
> >
> > -       strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
> > +       strbuf_addf(rev, "%s:%s", oid_to_hex(treeish_name), submodules_file);
> >         if (get_oid(rev->buf, gitmodules_oid) >= 0)
> >                 ret = 1;
> >
> > @@ -583,7 +583,7 @@ void repo_read_gitmodules(struct repository *repo)
> >                 if (repo_read_index(repo) < 0)
> >                         return;
> >
> > -               gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
> > +               gitmodules = repo_worktree_path(repo, submodules_file);
> >
> >                 if (!is_gitmodules_unmerged(repo->index))
> >                         git_config_from_file(gitmodules_cb, gitmodules, repo);
> > diff --git a/submodule.c b/submodule.c
> > index 9a50168b2..2afbdb644 100644
> > --- a/submodule.c
> > +++ b/submodule.c
> > @@ -36,13 +36,13 @@ static struct oid_array ref_tips_after_fetch;
> >   */
> >  int is_gitmodules_unmerged(const struct index_state *istate)
> >  {
> > -       int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
> > +       int pos = index_name_pos(istate, submodules_file, strlen(submodules_file));
> 
> Ah, regarding the coverletter: This clearly assumes the modules
> file is in the tree. So at least here we would make an exception
> for files outside the tree to either not check for un-merged-ness or
> disallow that case entirely.
>

Sorry I am not sure I follow what you are saying here, keep in mind
that I am new to git internals.

Do you mean that, even if we ensure (in
config.c::git_default_core_config) that only paths relative to
the work-tree are allowed, we still have to check here that the
constraint is respected? And is so, why?

> There are quite a few functions in submodule.c which access the new global. :/
> So moving them to the_repository should be fine, but eventually (not
> in this series)
> all these functions would would want to take a repository argument as well
> such that they work on more than the_repository.
> 

I will surely get rid of the global variable, but about changing the
functions signatures I don't feel like promising anything just yet.

Thanks,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Stefan Beller]

Hi Antonio,

> I acknowledge that the two mechanisms are different, in particular
> because git *writes* the gitmodules file itself.
>
> I am not opposed to change the name, something like
> 'core.submodulesConfigFile' might highlight that the syntax of the file
> is the one of git config, but I think it's too long.

I was not speculating on a better name, but on the nature of the
configuration, gitignore is additive and errors are easy to see, but in
gitmodules, there is only one "correct" path+name, so the problem
space is not additive, rather we can have a discussion where we get
the correct config from with low odds of errors.

>> > The new configuration setting can be used to set an *alternative*
>> > location for the gitmodules file; IMHO there is no need to provide
>> > *additional* locations like in the case of exclude files.
>>
>> I think there *may* be a need for additional files.
>> Currently there is only the .gitmodules file and the configuration
>> in .git/config overriding settings from .gitmodules.
>>
>> There was some discussion on the mailing list in the past, which
>> presented a intermediate layer in between these two places, in
>> a special ref, such that:
>>     base is in .gitmodules
>>     overwritten via refs/meta/submodules:.gitmodules
>>     overwritten via the .git/config
>>
>> The intermediate would be a config file that is tracked on another
>> ref. This (a) decouples main project history from submodule history
>> and (b) makes it easier to distribute as it is part of the repository.
>>
>> For example (a) is desired if you dig up an old project and the
>> submodules have all moved from one git hosting provider to another.
>> Another example would be when you fork a project with submodules
>> and don't want to mess with the main history but you just want to
>> adjust the submodule URLs. That is possible with such an intermediate
>> additional place.
>>
>> For (b) you can imagine the fork that you want to distribute in your
>> community and you don't want to tell everyone to change the
>> submodule URLs, but instead you can provide them with a prepared
>> .gitmodules file, that they have to place into that special ref (which
>> can be done via fetching).
>>
>> I digress as these ideas seem to be orthogonal to your patch series,
>> just FYI. prior discussion starting at:
>> https://public-inbox.org/git/1462317985-640-1-git-send-email-sbeller@google.com/
>> I recall there was a better discussion even prior to that, but have no
>> link handy.
>>
>
> Just to understand, is that 'refs/meta/submodules:.gitmodules' file
> meant to be managed manually? Or do you imagine some options to
> instruct "git submodule" to *write* to that file instead of .gitmodules?

I imagined it to be managed manually as it would enable some
workflows for downstream users of superproject repos.

But I'd think a convenient way to write to this location would be
super useful, so we ought to have that eventually.

> In the latter case your idea could cover my use case too, couldn't it?

I would think so, yes.

> But most importantly, is this realistically going to be added?

I plan on adding it eventually. It depends on the priorities and
schedule, no promises, though.

> Currently I am not that familiar with the git code base to do it
> myself, and I've never explicitly dealt with special refs in git.

I think core git only uses them for "actual refs", e.g. remote
tracking branches are used to know about the sha1. This new
special ref would be used to store content outside the main tree,
so we'd have too lookup the blob in that not-checked-out commit
and read and write there.

> The approach of this patch series is a lot simpler, and something I can
> work on in my spare time.
> Presumably (b) could even be partially supported with my changes, by
> having both '.gitmodules' and some custom '.gitmodules-alternative'
> files in the repository and tell users to clone with:
>
>   git clone --config core.submodulesFile=.gitmodules-alternative URL
>
> Not as clean as your idea but doable.

Traditionally Git had a strong stance on not transporting config
in the repo (except submodule URLs :/) itself, so I guess
this .gitmodules-alternative would not be a file in the tree, but
a URL or something?

> [...]
>> > Since the gitmodules file is meant to be checked in into the repository,
>> > the overridden file path should be relative to the work-tree; is there
>> > a way to enforce this constraint at run time (i.e. validate the config
>> > value), or is it enough to have it documented?
>>
>> I think we'd want to check at run time, if we need this constraint.
>>
>
> I'll look into it once we decide which the way to go.
>
> Thank you.
>

Thanks for bringing up this easier approach.
Stefan

Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Stefan Beller]

On Mon, Apr 16, 2018 at 9:18 AM, Antonio Ospite <ao2@ao2.it> wrote:

>
> Is there an API to just load one config setting?

Do you mean to

  git -c key=value foo-command --options ...

Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command

[Antonio Ospite]

On Mon, 16 Apr 2018 12:23:59 -0700
Stefan Beller <sbeller@google.com> wrote:

> On Mon, Apr 16, 2018 at 9:18 AM, Antonio Ospite <ao2@ao2.it> wrote:
> 
> >
> > Is there an API to just load one config setting?
> 
> Do you mean to
> 
>   git -c key=value foo-command --options ...

I meant, instead of:

  git_config(git_fetch_config, NULL);

which updates a series of config settings, something which loads
_in_the_code_ just the one value for 'core.submodulesFile' previously
set either via "git -c" or "git config".

It turns out what I was looking for is git_config_get_string_const().

The new patch will be simply this:

diff --git a/config.c b/config.c
index 6ffb1d501..1ef9801d3 100644
--- a/config.c
+++ b/config.c
@@ -2087,6 +2087,7 @@ int git_config_get_pathname(const char *key, const char **dest)
  */
 void config_from_gitmodules(config_fn_t fn, void *data)
 {
+       git_config_get_string_const("core.submodulesFile", &submodules_file);
        if (the_repository->worktree) {
                char *file = repo_worktree_path(the_repository, submodules_file);
                git_config_from_file(fn, file, data);


Which covers my use case without changing the fetch behavior for previous
users, as "core.submodulesFile" is a new option.

Thanks,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Stefan Beller]

On Mon, Apr 16, 2018 at 9:37 AM, Antonio Ospite <ao2@ao2.it> wrote:
> On Thu, 12 Apr 2018 16:50:03 -0700
> Stefan Beller <sbeller@google.com> wrote:
>
>> Hi Antonio,
>>
>> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
>> > When multiple repositories with detached work-trees take turns using the
>> > same directory as their work-tree, and more than one of them want to use
>> > submodules, there will be conflicts about the '.gitmodules' file.
>>
>> unlike other files which would not conflict?
>> There might be file names such as LICENSE, Readme.md etc,
>> which are common enough that they would produce conflicts as well?
>> I find this argument on its own rather weak. ("Just delete everything in
>> the working dir before using it with another repository"). I might be
>> missing a crucial bit here?
>>
>
> All the vcsh repositories _share_ the same work-tree; they may control
> it taking turns but, in general, all files are meant to be checked out
> at all times as the basic use case is: *distinct* sets of config files.
>
> Maybe saying that the repositories "take turns" is confusing.
> It's an unnecessary information, so I will omit that part form the
> commit message.

So they all have the same workdir, do they track the same set of files
or do they track a disjoint set of files, and ignoring the other repositories
files via the ignore mechanism?

This sounds like an interesting setup. I never though of that as something
useful (in either configuration).

> After your question I've done some research and I've seen other vcsh
> users managing conflicting LICENSE and README files using git
> sparse-checkouts, to have these files in the single repositories but
> not checked out in the shared work-tree:
> https://github.com/RichiH/vcsh/issues/120#issuecomment-42639619
> https://github.com/jwhitley/vcsh-root/commit/30b0d495c2cbe47ae9617ace9c2c14720d961d78
>
> However I guess that my point here is that the gitmodules file is
> something that influences git behavior so it should not be on the user's
> shoulder to manage conflicts for it, and most importantly it needs to
> be checked out for git to access it, doesn't it?

Good point! I wonder if the cleaner solution would be to just
tell git to use HEAD:.gitmodules and not check out the file?
then you would not need to come up with a namespace for names
of the .gitmodules files and scatter them into the worktree as well?


>> > -               value=$(git config -f .gitmodules submodule."$name"."$option")
>> > +               gitmodules_file=$(git config core.submodulesfile)
>> > +               : ${gitmodules_file:=.gitmodules}
>> > +               value=$(git config -f "$gitmodules_file" submodule."$name"."$option")
>>
>> I wonder if it would be cheaper to write a special config lookup now, e.g.
>> in builtin/submodule--helper.c we could have a "config-from-gitmodules"
>> subcommand that is looking up the modules file and then running the config
>> on that file.
>>
>
> Can you give an example from the user point of view of such a
> "config-from-gitmodules" command?
>

    git submodule config <name> <option>

as an 'alias' for

               gitmodules_file=$(git config core.submodulesfile)
               : ${gitmodules_file:=.gitmodules}
               value=$(git config -f "$gitmodules_file"
submodule."$name"."$option")

The helper would figure out which config file to load form
(.gitmodules in tree, HEAD:.gitmodules, your new proposed gitmodules file,
.git/config... or the special ref) and then return the <option> for <name>

So maybe:

    $ git clone https://gerrit.googlesource.com/gerrit && cd gerrit
    # ^ My goto-repo with submodules

    $ git submodule config "plugins/hooks" URL
    ../plugins/hooks



> I might look into it, but that can also be a followup change.


>> > diff --git a/submodule.c b/submodule.c
>> > index 9a50168b2..2afbdb644 100644
>> > --- a/submodule.c
>> > +++ b/submodule.c
>> > @@ -36,13 +36,13 @@ static struct oid_array ref_tips_after_fetch;
>> >   */
>> >  int is_gitmodules_unmerged(const struct index_state *istate)
>> >  {
>> > -       int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
>> > +       int pos = index_name_pos(istate, submodules_file, strlen(submodules_file));
>>
>> Ah, regarding the coverletter: This clearly assumes the modules
>> file is in the tree. So at least here we would make an exception
>> for files outside the tree to either not check for un-merged-ness or
>> disallow that case entirely.
>>
>
> Sorry I am not sure I follow what you are saying here, keep in mind
> that I am new to git internals.
>
> Do you mean that, even if we ensure (in
> config.c::git_default_core_config) that only paths relative to
> the work-tree are allowed, we still have to check here that the
> constraint is respected? And is so, why?

index_name_pos looks up a position of a file in the index,
which would fail for any file not in the index.

So if we give a path outside the tree, the lookup would fail
and we'd treat it as no .gitmodules file would be found,
which implies is_gitmodules_unmerged = false.

That sounds about right, but I think we could make the
distinction clearer, i.e. out-of-tree .gitmodule files *cannot*
be "unmerged" as that requires them to be part of a git
repository with a merge in progress.

>> There are quite a few functions in submodule.c which access the new global. :/
>> So moving them to the_repository should be fine, but eventually (not
>> in this series)
>> all these functions would would want to take a repository argument as well
>> such that they work on more than the_repository.
>>
>
> I will surely get rid of the global variable, but about changing the
> functions signatures I don't feel like promising anything just yet.

Yes, I would just use the_repository->... for now and as said by
"(not in this series)" we can do that later. It was more of a reminder
to myself, sorry for the confusion.

Thanks,
Stefan

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Antonio Ospite]

On Mon, 16 Apr 2018 14:22:35 -0700
Stefan Beller <sbeller@google.com> wrote:

> On Mon, Apr 16, 2018 at 9:37 AM, Antonio Ospite <ao2@ao2.it> wrote:
> > On Thu, 12 Apr 2018 16:50:03 -0700
> > Stefan Beller <sbeller@google.com> wrote:
> >
> >> Hi Antonio,
> >>
> >> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> >> > When multiple repositories with detached work-trees take turns using the
> >> > same directory as their work-tree, and more than one of them want to use
> >> > submodules, there will be conflicts about the '.gitmodules' file.
> >>
> >> unlike other files which would not conflict?
> >> There might be file names such as LICENSE, Readme.md etc,
> >> which are common enough that they would produce conflicts as well?
> >> I find this argument on its own rather weak. ("Just delete everything in
> >> the working dir before using it with another repository"). I might be
> >> missing a crucial bit here?
> >>
> >
> > All the vcsh repositories _share_ the same work-tree; they may control
> > it taking turns but, in general, all files are meant to be checked out
> > at all times as the basic use case is: *distinct* sets of config files.
> >
> > Maybe saying that the repositories "take turns" is confusing.
> > It's an unnecessary information, so I will omit that part form the
> > commit message.
> 
> So they all have the same workdir, do they track the same set of files
> or do they track a disjoint set of files, and ignoring the other repositories
> files via the ignore mechanism?
>

To recap,

vcsh[1] sets $HOME as the work-tree of multiple repositories to track
different sets of dotfiles in distinct repositories, while still having
the files directly available in $HOME. Each repository can ignore
untracked files via the ignore mechanism (namely core.excludesFile).

[1] https://github.com/RichiH/vcsh

For all this to work well, the sets of the tracked files would also need
to be disjoint, and usually they "practically" are, once a few
exceptions are taken care of.

Common intersecting items like LICENSE and README can be handled via
sparse-checkout to have "disjoint checkouts" and this solves most of
the problems, but the same mechanism cannot be used for .gitmodules as
it needs to be checked out.

And the problem cannot be worked around like done with .gitignore
(using core.excludesFile instead) because .gitmodules is unique and
hardcoded.

> This sounds like an interesting setup. I never though of that as something
> useful (in either configuration).
>

Give vcsh a try maybe.

[...]
> > However I guess that my point here is that the gitmodules file is
> > something that influences git behavior so it should not be on the user's
> > shoulder to manage conflicts for it, and most importantly it needs to
> > be checked out for git to access it, doesn't it?
> 
> Good point! I wonder if the cleaner solution would be to just
> tell git to use HEAD:.gitmodules and not check out the file?
> then you would not need to come up with a namespace for names
> of the .gitmodules files and scatter them into the worktree as well?
>

Any solution which:

  1. prevents the gitmodules file to be checked out
  2. but still tracks it in the git repository

OR
  
  1. allows to set the gitmoudles file under some namespace

would work for vcsh I guess.

> 
> >> > -               value=$(git config -f .gitmodules submodule."$name"."$option")
> >> > +               gitmodules_file=$(git config core.submodulesfile)
> >> > +               : ${gitmodules_file:=.gitmodules}
> >> > +               value=$(git config -f "$gitmodules_file" submodule."$name"."$option")
> >>
> >> I wonder if it would be cheaper to write a special config lookup now, e.g.
> >> in builtin/submodule--helper.c we could have a "config-from-gitmodules"
> >> subcommand that is looking up the modules file and then running the config
> >> on that file.
> >>
> >
> > Can you give an example from the user point of view of such a
> > "config-from-gitmodules" command?
> >
> 
>     git submodule config <name> <option>
> 
> as an 'alias' for
> 
>                gitmodules_file=$(git config core.submodulesfile)
>                : ${gitmodules_file:=.gitmodules}
>                value=$(git config -f "$gitmodules_file"
> submodule."$name"."$option")
> 
> The helper would figure out which config file to load form
> (.gitmodules in tree, HEAD:.gitmodules, your new proposed gitmodules file,
> .git/config... or the special ref) and then return the <option> for <name>
> 
> So maybe:
> 
>     $ git clone https://gerrit.googlesource.com/gerrit && cd gerrit
>     # ^ My goto-repo with submodules
> 
>     $ git submodule config "plugins/hooks" URL
>     ../plugins/hooks
> 
>

I may look into such supporting changes once you decide the approach to
take for the bigger problem.

Thank you,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

[Stefan Beller]

Hi Antonio,

>>
>> Good point! I wonder if the cleaner solution would be to just
>> tell git to use HEAD:.gitmodules and not check out the file?
>> then you would not need to come up with a namespace for names
>> of the .gitmodules files and scatter them into the worktree as well?
>>
>
> Any solution which:
>
>   1. prevents the gitmodules file to be checked out
>   2. but still tracks it in the git repository
>
> OR
>
>   1. allows to set the gitmoudles file under some namespace
>
> would work for vcsh I guess.

I personally would tend to rather go for supporting your first solution
(prevent .gitmodules from checked-out, load from sparse HEAD),
but I do not have strong arguments or feeling about this dimension.
I am fine with a namespaced .gtimodules solution, too.

Both solutions can be implemented by either:

A) adding the code where it is (like your patch, e.g. using

> -               value=$(git config -f .gitmodules submodule."$name"."$option")
> +               gitmodules_file=$(git config core.submodulesfile)
> +               : ${gitmodules_file:=.gitmodules}
> +               value=$(git config -f "$gitmodules_file" submodule."$name"."$option")

B) adding a helper, which is a layer of indirection
to load the relevant configuration.

And when it comes to this dimension, I'd strongly favor B over A.
Having this indirection helper in place enables to add more options
later easily as only one place needs to be touched.
(These other options could include the other solution as presented above,
or the idea with the special ref as mentioned in an earlier email)


>> > Can you give an example from the user point of view of such a
>> > "config-from-gitmodules" command?
>> >
>>
>>     git submodule config <name> <option>
>>
>> as an 'alias' for
>>
>>                gitmodules_file=$(git config core.submodulesfile)
>>                : ${gitmodules_file:=.gitmodules}
>>                value=$(git config -f "$gitmodules_file"
>> submodule."$name"."$option")
>>
>> The helper would figure out which config file to load form
>> (.gitmodules in tree, HEAD:.gitmodules, your new proposed gitmodules file,
>> .git/config... or the special ref) and then return the <option> for <name>
>>
>> So maybe:
>>
>>     $ git clone https://gerrit.googlesource.com/gerrit && cd gerrit
>>     # ^ My goto-repo with submodules
>>
>>     $ git submodule config "plugins/hooks" URL
>>     ../plugins/hooks
>>
>>
>
> I may look into such supporting changes once you decide the approach to
> take for the bigger problem.

I think once we have the helper in place you can implement the solution
to the bigger problem as you like?

There are a few pros and cons for namespaced .gitmodules and
non-checked-out sparse HEAD .gitmodules:

How do you modify the .gitmodules config?
============
In the namespaced solution, you can tell users to edit that
file manually or use "git config -f $new_location" to manipulate
that file.

In the sparse solution editing becomes a little bit trickier, as you
need to edit a file in the index (or HEAD).

If you have the special ref, you could just checkout the
special ref in another worktree and make changes and
commit there


How do you change the setup?
============
In case of a sparse gitmodules file, you can just check it out
(make it non-sparse) or vice versa.

In case of a namespaced gitmodules file, you'd change the
config setting and have to move the file to the new location.
as git config is just about configuring, the user is left alone
with moving the file, or would we have a helper for that?
("git submodule relocate-gitmodules" or such)?

If you have the special ref, you could just checkout the
special ref in another worktree and make changes and
commit there.

I hope this helps instead of confusing more,

Thanks,
Stefan

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Jonathan Nieder]

Hi,

Antonio Ospite wrote:

> vcsh[1] uses bare git repositories and detached work-trees to manage
> *distinct* sets of configuration files directly into $HOME.

Cool!  I like the tooling you're creating for this, though keep in mind
that Git has some weaknesses as a tool for deployment.

In particular, keep in mind that when git updates a file, there is a
period of time while it is missing from the filesystem, which can be
problematic for dotfiles.

[...]
> However when multiple repositories take turns using the same directory
> as their work-tree, and more than one of them want to use submodules,
> there could still be conflicts about the '.gitmodules' file because git
> hardcodes this path.
>
> For comparison, in case of '.gitignore' a similar conflict might arise,
> but git has alternative ways to specify exclude files, so vcsh solves
> this by setting core.excludesFile for each repository and track ignored
> files somewhere else (in ~/.gitignore.d/$VCSH_REPO_NAME).

For reference:

	core.excludesFile
		Specifies the pathname to the file that contains
		patterns to describe paths that are not meant to be
		tracked, in addition to .gitignore (per-directory) and
		.git/info/exclude. Defaults to
		$XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is
		either not set or empty, $HOME/.config/git/ignore is
		used instead. See gitignore(5).

Using this as a substitute for <worktree>/.gitignore is a bit of a
hack.  It happens to work, though, so reading on. :)

[...]
> So this series proposes a mechanism to set an alternative path for the
> submodules configuration file (from now on "gitmodules file").

I am nervous about this.  I wonder if there is another way to
accomplish the goal.

One possibility would be to handle the case where .gitmodules is
excluded by a sparse checkout specification and use .gitmodules from
the index in that case.  Would that work for you?

Thanks,
Jonathan

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Antonio Ospite]

On Mon, 23 Apr 2018 10:47:09 -0700
Jonathan Nieder <jrnieder@gmail.com> wrote:

> Hi,
>

Hi Jonathan, thank you for your comment.

> Antonio Ospite wrote:
> 
> > vcsh[1] uses bare git repositories and detached work-trees to manage
> > *distinct* sets of configuration files directly into $HOME.
> 
> Cool!  I like the tooling you're creating for this, though keep in mind
> that Git has some weaknesses as a tool for deployment.
>

I am not the author BTW, just a user trying to address the remaining
corner cases.

> In particular, keep in mind that when git updates a file, there is a
> period of time while it is missing from the filesystem, which can be
> problematic for dotfiles.
>

Thanks for the reminder, it may be worth mentioning this in vcsh
documentation, however I don't have knowledge of users experiencing
problems related to that.

> [...]
> > However when multiple repositories take turns using the same directory
> > as their work-tree, and more than one of them want to use submodules,
> > there could still be conflicts about the '.gitmodules' file because git
> > hardcodes this path.
> >
> > For comparison, in case of '.gitignore' a similar conflict might arise,
> > but git has alternative ways to specify exclude files, so vcsh solves
> > this by setting core.excludesFile for each repository and track ignored
> > files somewhere else (in ~/.gitignore.d/$VCSH_REPO_NAME).
> 
> For reference:
> 
> 	core.excludesFile
> 		Specifies the pathname to the file that contains
> 		patterns to describe paths that are not meant to be
> 		tracked, in addition to .gitignore (per-directory) and
> 		.git/info/exclude. Defaults to
> 		$XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is
> 		either not set or empty, $HOME/.config/git/ignore is
> 		used instead. See gitignore(5).
> 
> Using this as a substitute for <worktree>/.gitignore is a bit of a
> hack.  It happens to work, though, so reading on. :)
> 
> [...]
> > So this series proposes a mechanism to set an alternative path for the
> > submodules configuration file (from now on "gitmodules file").
> 
> I am nervous about this.  I wonder if there is another way to
> accomplish the goal.
>
> One possibility would be to handle the case where .gitmodules is
> excluded by a sparse checkout specification and use .gitmodules from
> the index in that case.  Would that work for you?
> 

Since part of the problem is that .gitmodules *collide* between
repositories, a sparse-checkout approach make sense indeed.

As discussed[1] with Stefan Beller having git use .gitmodules from the
index without the need to have it checked out should work for us.

[1] https://www.spinics.net/lists/git/msg329153.html

Ideally git should also be able to write to that file when it's not
checked out (e.g. when running "git submodule add"), to save the
user from tedious sparse/unsparse rounds when operating with submodules.

As suggested by Stefan I'll first try to remove the hardcoded references
to .gitmodules in git-submodule.sh adding a helper sub-command to
access .gitmodules in a more robust way, and after that git could
be taught to use the file from the index, but this second part
is currently beyond my current git knowledge.

If this mechanism of using unchecked-out files from the index could be
extended to .gitignore (and .gitattributes), then vcsh might even stop
abusing core.excludesFile; sparse checkouts seem the more natural git
way to deal with colliding files in a shared-workdir scenario.

However, having users *write* to .gitignore and .gitattributes while
they are not checked out still sounds quite problematic to me, but maybe
this could be handled by vcsh itself, similarly to what is done for the
file pointed by core.excludesFile.

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?