submodule loading: separate code path for .gitmodules and config overlay

stefanbeller · gitster · commit 1d789d089280 · 2017-05-30T14:28:54.000+09:00
The .gitmodules file is not supposed to have all the options available,
that are available in the configuration so separate it out.

A configuration option such as the hypothetical submodule.color.diff
that determines in which color a submodule change is printed,
is a very user specific thing, that the .gitmodules file should
not tamper with.

The .gitmodules file should only be used for settings that required
to setup the project in which the .gitmodules file is tracked. As the
minimum this would only include the name&lt;-&gt;path mapping of the
submodule and its URL and branch.

Any further setting (such as 'fetch.recursesubmodules' or
'submodule.&lt;name&gt;.{update, ignore, shallow}') is not specific
to the project setup requirements, but rather is a distribution
of suggested developer configurations.  In other areas of Git
a suggested developer configuration is not transported in-tree
but via other means.  In an organisation this could be done
by deploying an opinionated system wide config (/etc/gitconfig)
or by putting the settings in the users home directory when
they start at the organisation. In open source projects this
is often accomplished via extensive READMEs (cf. our
SubmittingPatches/CodingGuidlines).

As a later patch in this series wants to introduce
a generic submodule recursion option, we want to make
sure that switch is not exposed via the gitmodules file.

Signed-off-by: Stefan Beller &lt;sbeller@google.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/submodule.c b/submodule.c
@@ -153,7 +153,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 	}
 }
 
-int submodule_config(const char *var, const char *value, void *cb)
+/* For loading from the .gitmodules file. */
+static int git_modules_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "submodule.fetchjobs")) {
 		parallel_jobs = git_config_int(var, value);
@@ -169,6 +170,12 @@ int submodule_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
+/* Loads all submodule settings from the config */
+int submodule_config(const char *var, const char *value, void *cb)
+{
+	return git_modules_config(var, value, cb);
+}
+
 int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
 						     const char *arg, int unset)
 {
@@ -222,7 +229,8 @@ void gitmodules_config(void)
 		}
 
 		if (!gitmodules_is_unmerged)
-			git_config_from_file(submodule_config, gitmodules_path.buf, NULL);
+			git_config_from_file(git_modules_config,
+				gitmodules_path.buf, NULL);
 		strbuf_release(&gitmodules_path);
 	}
 }
@@ -233,7 +241,7 @@ void gitmodules_config_sha1(const unsigned char *commit_sha1)
 	unsigned char sha1[20];
 
 	if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
-		git_config_from_blob_sha1(submodule_config, rev.buf,
+		git_config_from_blob_sha1(git_modules_config, rev.buf,
 					  sha1, NULL);
 	}
 	strbuf_release(&rev);

Original file line number	Diff line number	Diff line change
`@@ -153,7 +153,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,`
`153`	`153`	`}`
`154`	`154`	`}`
`155`	`155`
`156`		`-int submodule_config(const char var, const char value, void *cb)`
	`156`	`+/* For loading from the .gitmodules file. */`
	`157`	`+static int git_modules_config(const char var, const char value, void *cb)`
`157`	`158`	`{`
`158`	`159`	`if (!strcmp(var, "submodule.fetchjobs")) {`
`159`	`160`	`parallel_jobs = git_config_int(var, value);`
`@@ -169,6 +170,12 @@ int submodule_config(const char var, const char value, void *cb)`
`169`	`170`	`return 0;`
`170`	`171`	`}`
`171`	`172`
	`173`	`+/* Loads all submodule settings from the config */`
	`174`	`+int submodule_config(const char var, const char value, void *cb)`
	`175`	`+{`
	`176`	`+ return git_modules_config(var, value, cb);`
	`177`	`+}`
	`178`	`+`
`172`	`179`	`int option_parse_recurse_submodules_worktree_updater(const struct option *opt,`
`173`	`180`	`const char *arg, int unset)`
`174`	`181`	`{`
`@@ -222,7 +229,8 @@ void gitmodules_config(void)`
`222`	`229`	`}`
`223`	`230`
`224`	`231`	`if (!gitmodules_is_unmerged)`
`225`		`- git_config_from_file(submodule_config, gitmodules_path.buf, NULL);`
	`232`	`+ git_config_from_file(git_modules_config,`
	`233`	`+ gitmodules_path.buf, NULL);`
`226`	`234`	`strbuf_release(&gitmodules_path);`
`227`	`235`	`}`
`228`	`236`	`}`
`@@ -233,7 +241,7 @@ void gitmodules_config_sha1(const unsigned char *commit_sha1)`
`233`	`241`	`unsigned char sha1[20];`
`234`	`242`
`235`	`243`	`if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {`
`236`		`- git_config_from_blob_sha1(submodule_config, rev.buf,`
	`244`	`+ git_config_from_blob_sha1(git_modules_config, rev.buf,`
`237`	`245`	`sha1, NULL);`
`238`	`246`	`}`
`239`	`247`	`strbuf_release(&rev);`