Re: [PATCH v2] clone: use submodules.recurse option for automatically clone submodules

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> So, I would sort-of understand if you pretend --recurse-submodules
> was given from the command line when submodule.recurse is set to
> true (which would mean that you'd append "." to the string list).
> But I do not understand why appending "true" is a good thing at all
> here.
>
> Another thing I noticed.
>
> If you have "[submodule] recurse" in your $HOME/.gitconfig, you'd
> want to be able to countermand from the command line with
>
>     git clone --no-recurse-submodules ...
>
> so that the clone would not go recursive.  And that should be
> tested.  
>
> You'd also want the opposite, i.e. with "[submodule] recurse=no" in
> your $HOME/.gitconfig and running
>
>     git clone --recurse-submodules ...
>
> should countermand the configuration.


Totally untested, but just to illustrate the approach, here is a
sample patch to implement "Pretend --recurse-submodules=. is set on
the command line when submodule.recurse is set (in the 'last one
wins' sense) and there is no --recurse-submodules command line
option."  It should outline the right interactions between the
command line options and configuration variable, like allowing "git
clone --no-recurse-submodules" to defeat submodule.recurse
configuration.

Not that I agree that "[submodules] recurse" set in the
$HOME/.gitconfig should affect "git clone".  It is merely to
illustrate how it could be done, if it were a good idea.

 builtin/clone.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 0fc89ae2b9..163803d89e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -32,6 +32,7 @@
 #include "connected.h"
 #include "packfile.h"
 #include "list-objects-filter-options.h"
+#include "submodule.h"
 
 /*
  * Overall FIXMEs:
@@ -71,6 +72,8 @@ static struct list_objects_filter_options filter_options;
 static struct string_list server_options = STRING_LIST_INIT_NODUP;
 static int option_remote_submodules;
 
+static int recurse_submodules_option_given;
+
 static int recurse_submodules_cb(const struct option *opt,
 				 const char *arg, int unset)
 {
@@ -81,7 +84,7 @@ static int recurse_submodules_cb(const struct option *opt,
 	else
 		string_list_append((struct string_list *)opt->value,
 				   (const char *)opt->defval);
-
+	recurse_submodules_option_given = 1;
 	return 0;
 }
 
@@ -929,6 +932,13 @@ static int path_exists(const char *path)
 	return !stat(path, &sb);
 }
 
+static int git_clone_config(const char *var, const char *value, void *cb)
+{
+	if (starts_with(var, "submodule."))
+		return git_default_submodule_config(var, value, NULL);
+	return git_default_config(var, value, cb);
+}
+
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int is_bundle = 0, is_local;
@@ -1103,7 +1113,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	write_config(&option_config);
 
-	git_config(git_default_config, NULL);
+	git_config(git_clone_config, NULL);
+	if (!recurse_submodules_option_given && should_update_submodules())
+		string_list_append(&option_recurse_submodules, ".");
 
 	if (option_bare) {
 		if (option_mirror)