Re: [RFC PATCH] clone: add clone.recursesubmodules config option

From

JMJeremy Morton <admin@game-point.net>

Date

Oct 3, 2016, 15:36 UTC

Message-ID

<57F27B02.8080803@game-point.net>

In-Reply-To

<1401874256-13332-1-git-send-email-judge.packham@gmail.com>

-- 
Best regards,
Jeremy Morton (Jez)

On 04/06/2014 10:30, Chris Packham wrote:
> Add a config option that will cause clone to recurse into submodules as
> if the --recurse-submodules option had been specified on the command
> line. This can be overridden with the --no-recurse-submodules option.
>
> Signed-off-by: Chris Packham<judge.packham@gmail.com>
> ---
> On 04/06/14 09:05, Junio C Hamano wrote:
>>> Mara Kim<mara.kim@vanderbilt.edu>  writes:
>>>
>>>> Apologies if this question has been asked already, but what is the
>>>> reasoning behind making git clone not recursive (--recursive) by
>>>> default?
>>>
>>> The primary reason why submodules are separate repositories is not
>>> to require people to have everything.  Some people want recursive,
>>> some others don't, and the world is not always "majority wins" (not
>>> that I am saying that majority will want recursive).
>>>
>>> Inertia, aka backward compatibility and not surprising existing
>>> users, plays some role when deciding the default.
>>>
>>> Also, going --recursive when the user did not want is a lot more
>>> expensive mistake to fix than not being --recursive when the user
>>> wanted to.
>>
>> Having said all that, I do not mean to say that I am opposed to
>> introduce some mechanism to let the users express their preference
>> between recursive and non-recursive better, so that "git clone"
>> without an explicit --recursive (or --no-recursive) can work to
>> their taste.  A configuration in $HOME/.gitconfig might be a place
>> to start, even though that has the downside of assuming that the
>> given user would want to use the same settings for all his projects,
>> which may not be the case in practice.
>
> And here's a quick proof of concept. Not sure about the config variable name
> and it could probably do with a negative test as well.
>
>   builtin/clone.c              |  9 +++++++++
>   t/t7407-submodule-foreach.sh | 17 +++++++++++++++++
>   2 files changed, 26 insertions(+)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index b12989d..92aea81 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -734,6 +734,14 @@ static void write_refspec_config(const char* src_ref_prefix,
>   	strbuf_release(&value);
>   }
>
> +static int git_clone_config(const char *key, const char *value, void *data)
> +{
> +	if (!strcmp(key, "clone.recursesubmodules"))
> +		option_recursive = git_config_bool(key, value);
> +
> +	return 0;
> +}
> +
>   int cmd_clone(int argc, const char **argv, const char *prefix)
>   {
>   	int is_bundle = 0, is_local;
> @@ -759,6 +767,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>   	junk_pid = getpid();
>
>   	packet_trace_identity("clone");
> +	git_config(git_clone_config, NULL);
>   	argc = parse_options(argc, argv, prefix, builtin_clone_options,
>   			     builtin_clone_usage, 0);
>
> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
> index 7ca10b8..fc2c189 100755
> --- a/t/t7407-submodule-foreach.sh
> +++ b/t/t7407-submodule-foreach.sh
> @@ -307,6 +307,23 @@ test_expect_success 'use "update --recursive nested1" to checkout all submodules
>   	)
>   '
>
> +test_expect_success 'use "git clone" with clone.recursesubmodules to checkout all submodules' '
> +	git config --local clone.recursesubmodules true&&
> +	git clone super clone7&&
> +	(
> +		cd clone7&&
> +		git rev-parse --resolve-git-dir .git&&
> +		git rev-parse --resolve-git-dir sub1/.git&&
> +		git rev-parse --resolve-git-dir sub2/.git&&
> +		git rev-parse --resolve-git-dir sub3/.git&&
> +		git rev-parse --resolve-git-dir nested1/.git&&
> +		git rev-parse --resolve-git-dir nested1/nested2/.git&&
> +		git rev-parse --resolve-git-dir nested1/nested2/nested3/.git&&
> +		git rev-parse --resolve-git-dir nested1/nested2/nested3/submodule/.git
> +	)&&
> +	git config --local --unset clone.recursesubmodules
> +'
> +
>   test_expect_success 'command passed to foreach retains notion of stdin' '
>   	(
>   		cd super&&