git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* git pull defaults for recursesubmodules
@ 2018-10-23 21:04 Tommi Vainikainen
  2018-10-23 21:57 ` Stefan Beller
  2018-10-23 23:03 ` brian m. carlson
  0 siblings, 2 replies; 6+ messages in thread
From: Tommi Vainikainen @ 2018-10-23 21:04 UTC (permalink / raw)
  To: git

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

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.

--
Tommi Vainikainen

[-- Attachment #2: 0001-pull-obey-fetch.recurseSubmodules-when-fetching.patch --]
[-- Type: text/x-patch, Size: 1881 bytes --]

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
---
 builtin/pull.c            |  3 +++
 t/t5572-pull-submodule.sh | 11 +++++++++++
 2 files changed, 14 insertions(+)

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
+'
+
 test_expect_success " --[no-]recurse-submodule and submodule.recurse" '
 	test_commit -C child merge_strategy_3 &&
 	git -C parent submodule update --remote &&
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: git pull defaults for recursesubmodules
  2018-10-23 21:04 git pull defaults for recursesubmodules Tommi Vainikainen
@ 2018-10-23 21:57 ` Stefan Beller
  2018-10-24  5:32   ` Tommi Vainikainen
  2018-10-23 23:03 ` brian m. carlson
  1 sibling, 1 reply; 6+ messages in thread
From: Stefan Beller @ 2018-10-23 21:57 UTC (permalink / raw)
  To: tvainika; +Cc: git

On Tue, Oct 23, 2018 at 2:04 PM Tommi Vainikainen <tvainika@gmail.com> 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.

This makes sense to me and the patch looks good to me.
It is unclear to me if this is a regression or an oversight of
of a6d7eb2c7a (pull: optionally rebase submodules (remote
submodule changes only), 2017-06-23)

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git pull defaults for recursesubmodules
  2018-10-23 21:04 git pull defaults for recursesubmodules Tommi Vainikainen
  2018-10-23 21:57 ` Stefan Beller
@ 2018-10-23 23:03 ` brian m. carlson
  2018-10-24  6:57   ` Tommi Vainikainen
  1 sibling, 1 reply; 6+ messages in thread
From: brian m. carlson @ 2018-10-23 23:03 UTC (permalink / raw)
  To: Tommi Vainikainen; +Cc: git

[-- 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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git pull defaults for recursesubmodules
  2018-10-23 21:57 ` Stefan Beller
@ 2018-10-24  5:32   ` Tommi Vainikainen
  0 siblings, 0 replies; 6+ messages in thread
From: Tommi Vainikainen @ 2018-10-24  5:32 UTC (permalink / raw)
  To: sbeller; +Cc: git

ke 24. lokak. 2018 klo 0.57 Stefan Beller (sbeller@google.com) kirjoitti:
> On Tue, Oct 23, 2018 at 2:04 PM Tommi Vainikainen <tvainika@gmail.com> wrote:
> > I would expect that if git-config has fetch.recurseSubmodules set,
> > also git pull should use this setting, or at least similar option such
>
> This makes sense to me and the patch looks good to me.
> It is unclear to me if this is a regression or an oversight of
> of a6d7eb2c7a (pull: optionally rebase submodules (remote
> submodule changes only), 2017-06-23)

With my testing, no it was not regression at least from that commit.
It did not work as I expected before that commit.

What was unclear to me is why this config needs to be read as
pull calls fetch, why fetch does not use this configuration as is?
If fetch.prune=1, should git pull command also prune or not during
fetch? There does not seem to be test case for that behavior.

-- 
Tommi Vainikainen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git pull defaults for recursesubmodules
  2018-10-23 23:03 ` brian m. carlson
@ 2018-10-24  6:57   ` Tommi Vainikainen
  2018-10-25 10:09     ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Tommi Vainikainen @ 2018-10-24  6:57 UTC (permalink / raw)
  To: sandals, git

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

ke 24. lokak. 2018 klo 2.03 brian m. carlson
(sandals@crustytoothpaste.net) kirjoitti:
> You have an extra '1" at the end of this line.
> Also, missing sign-off.  See Documentation/SubmittingPatches.

After reading SubmittingPatches I didn't find if I should now send a
fresh patch with
changes squashed together or new commits appended after first commit in that
patch. Patch is updated accordingly as fresh patch.

> Can we have a test that --no-recurse-submodules overrides
> fetch.recurseSubmodules?

Attached (sorry about that, I've no access now to more convenient mail
setup) is refreshed
patch which also tests that. I included it in same patch to follow
style of other tests
in t5572-pull-submodule.

-- 
Tommi Vainikainen

[-- Attachment #2: 0001-pull-obey-fetch.recurseSubmodules-when-fetching.patch --]
[-- Type: text/x-patch, Size: 2142 bytes --]

From 5d4b495f7c68a2e99314fef90e538e0d44ec1056 Mon Sep 17 00:00:00 2001
From: Tommi Vainikainen <thv@iki.fi>
Date: Wed, 24 Oct 2018 09:06:54 +0300
Subject: [PATCH] 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.

The command line arg --recurse-submodules=no overrides
fetch.recurseSubmodules configuration.

Signed-off-by: Tommi Vainikainen <thv@iki.fi>
---
 builtin/pull.c            |  3 +++
 t/t5572-pull-submodule.sh | 13 +++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 681c127a07..bb51cb531d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -346,6 +346,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..66c479a135 100755
--- a/t/t5572-pull-submodule.sh
+++ b/t/t5572-pull-submodule.sh
@@ -75,6 +75,19 @@ 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 --recurse-submodules=no --no-rebase &&
+	test_path_is_missing super/sub/fetch_recurse_submodules.t &&
+	git -C super pull --no-rebase &&
+	test_path_is_file super/sub/fetch_recurse_submodules.t
+'
+
 test_expect_success " --[no-]recurse-submodule and submodule.recurse" '
 	test_commit -C child merge_strategy_3 &&
 	git -C parent submodule update --remote &&
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: git pull defaults for recursesubmodules
  2018-10-24  6:57   ` Tommi Vainikainen
@ 2018-10-25 10:09     ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2018-10-25 10:09 UTC (permalink / raw)
  To: Tommi Vainikainen; +Cc: sandals, git

Tommi Vainikainen <tvainika@gmail.com> writes:

> After reading SubmittingPatches I didn't find if I should now send a
> fresh patch with
> changes squashed together or new commits appended after first commit in that
> patch. Patch is updated accordingly as fresh patch.

(just on mechanics, not on the contents of your actual patch)

You can and should treat any topic that is not yet in 'next' as if
it did not exist.  If you refined based on a v1 patch, pretend as if
you were a perfect developer and you came up with that refined
version without producing any problematic things that were pointed
ont in your v1.  Pretend mistakes in v1 never happened.  Pretend
that you are perfect! ;-)

If you can limit the signs that an earlier rounds ever existed to

(1) The In-reply-to: header of the message you send your updated
    version of the patch in, so that people can find the older
    version and its discussion thread, and

(2) The cover letter that describes what you improved in the
    updated version relative to the last round, in addition to the
    overview of the series [note: this only exists for a larger
    patch series, and not usually done for a single patch]

you achieved your goal.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-10-25 10:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-23 21:04 git pull defaults for recursesubmodules Tommi Vainikainen
2018-10-23 21:57 ` Stefan Beller
2018-10-24  5:32   ` Tommi Vainikainen
2018-10-23 23:03 ` brian m. carlson
2018-10-24  6:57   ` Tommi Vainikainen
2018-10-25 10:09     ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).