Re: git pull defaults for recursesubmodules

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2919 bytes --]

On Wed, Oct 24, 2018 at 12:04:06AM +0300, Tommi Vainikainen wrote:
> I configured my local git to fetch with recurseSubmodules = on-demand,
> which I found the most convenient setting. However then I noticed that
> I mostly use git pull actually to fetch from remotes, but git pull
> does not utilize any recurseSubmoddules setting now, or at least I
> could not find such.
> 
> I would expect that if git-config has fetch.recurseSubmodules set,
> also git pull should use this setting, or at least similar option such
> as pull.recurseSubmodules should be available. I'd prefer sharing
> fetch.recurseSubmodules setting here.
> 
> I've attached a minimal patch, which I believe implements this
> configuration usage, and a test case to show my expected behavior for
> git pull.

Typically, we prefer patches to be inline; descriptive content like this
goes after the --- line in the patch, or in a cover letter.

> From e4483ec5b3d9c38a6e30aa0ab9fa521cba582345 Mon Sep 17 00:00:00 2001
> From: Tommi Vainikainen <thv@iki.fi>
> Date: Tue, 23 Oct 2018 23:47:58 +0300
> Subject: [PATCH 1/1] pull: obey fetch.recurseSubmodules when fetching
> 
> "git pull" now uses same recurse-submodules config for fetching as "git
> fetch" by default if not overridden from command line.1

You have an extra '1" at the end of this line.

Also, missing sign-off.  See Documentation/SubmittingPatches.

> diff --git a/builtin/pull.c b/builtin/pull.c
> index 798ecf7faf..ed39b0e8ed 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -347,6 +347,9 @@ static int git_pull_config(const char *var, const char *value, void *cb)
>  		recurse_submodules = git_config_bool(var, value) ?
>  			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
>  		return 0;
> +	} else if (!strcmp(var, "fetch.recursesubmodules")) {
> +		recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);
> +		return 0;
>  	}
>  	return git_default_config(var, value, cb);
>  }
> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
> index f916729a12..579ae5c374 100755
> --- a/t/t5572-pull-submodule.sh
> +++ b/t/t5572-pull-submodule.sh
> @@ -75,6 +75,17 @@ test_expect_success "submodule.recurse option triggers recursive pull" '
>  	test_path_is_file super/sub/merge_strategy_2.t
>  '
>  
> +test_expect_success "fetch.recurseSubmodules=true triggers recursive pull" '
> +	test_commit -C child fetch_recurse_submodules &&
> +	git -C parent submodule update --remote &&
> +	git -C parent add sub &&
> +	git -C parent commit -m "update submodule" &&
> +
> +	git -C super config fetch.recurseSubmodules true &&
> +	git -C super pull --no-rebase &&
> +	test_path_is_file super/sub/fetch_recurse_submodules.t
> +'

Can we have a test that --no-recurse-submodules overrides
fetch.recurseSubmodules?
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]