Message ID | CAGshahkvn3fcyuqtD-WQE9tn+7rSad84+mtA_cfkz+t42xqPdw@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | git pull defaults for recursesubmodules | expand |
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
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?
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.
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 <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.
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