{"thread":{"id":"49654","subject":"git pull defaults for recursesubmodules","startedAt":"2018-10-23T21:04:21Z","lastAt":"2018-10-25T10:09:24Z","messageCount":6,"participants":["Tommi Vainikainen","Stefan Beller","brian m. carlson","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"361328","messageId":"CAGshahkvn3fcyuqtD-WQE9tn+7rSad84+mtA_cfkz+t42xqPdw@mail.gmail.com","threadId":"49654","inReplyTo":null,"subject":"git pull defaults for recursesubmodules","fromName":"Tommi Vainikainen","fromEmail":"tvainika@gmail.com","sentAt":"2018-10-23T21:04:06Z","receivedAt":"2018-10-23T21:04:21Z","isPatch":false,"sender":{"key":"tvainika@gmail.com","avatar":null},"body":"I configured my local git to fetch with recurseSubmodules = on-demand,\nwhich I found the most convenient setting. However then I noticed that\nI mostly use git pull actually to fetch from remotes, but git pull\ndoes not utilize any recurseSubmoddules setting now, or at least I\ncould not find such.\n\nI would expect that if git-config has fetch.recurseSubmodules set,\nalso git pull should use this setting, or at least similar option such\nas pull.recurseSubmodules should be available. I'd prefer sharing\nfetch.recurseSubmodules setting here.\n\nI've attached a minimal patch, which I believe implements this\nconfiguration usage, and a test case to show my expected behavior for\ngit pull.\n\n--\nTommi Vainikainen\n\n\nFrom e4483ec5b3d9c38a6e30aa0ab9fa521cba582345 Mon Sep 17 00:00:00 2001\nFrom: Tommi Vainikainen <thv@iki.fi>\nDate: Tue, 23 Oct 2018 23:47:58 +0300\nSubject: [PATCH 1/1] pull: obey fetch.recurseSubmodules when fetching\n\n\"git pull\" now uses same recurse-submodules config for fetching as \"git\nfetch\" by default if not overridden from command line.1\n---\n builtin/pull.c            |  3 +++\n t/t5572-pull-submodule.sh | 11 +++++++++++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 798ecf7faf..ed39b0e8ed 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -347,6 +347,9 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n \t\trecurse_submodules = git_config_bool(var, value) ?\n \t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n \t\treturn 0;\n+\t} else if (!strcmp(var, \"fetch.recursesubmodules\")) {\n+\t\trecurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n+\t\treturn 0;\n \t}\n \treturn git_default_config(var, value, cb);\n }\ndiff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\nindex f916729a12..579ae5c374 100755\n--- a/t/t5572-pull-submodule.sh\n+++ b/t/t5572-pull-submodule.sh\n@@ -75,6 +75,17 @@ test_expect_success \"submodule.recurse option triggers recursive pull\" '\n \ttest_path_is_file super/sub/merge_strategy_2.t\n '\n \n+test_expect_success \"fetch.recurseSubmodules=true triggers recursive pull\" '\n+\ttest_commit -C child fetch_recurse_submodules &&\n+\tgit -C parent submodule update --remote &&\n+\tgit -C parent add sub &&\n+\tgit -C parent commit -m \"update submodule\" &&\n+\n+\tgit -C super config fetch.recurseSubmodules true &&\n+\tgit -C super pull --no-rebase &&\n+\ttest_path_is_file super/sub/fetch_recurse_submodules.t\n+'\n+\n test_expect_success \" --[no-]recurse-submodule and submodule.recurse\" '\n \ttest_commit -C child merge_strategy_3 &&\n \tgit -C parent submodule update --remote &&\n-- \n2.19.1\n\n"},{"id":"361332","messageId":"CAGZ79kZf8owJUkYF2SsCr_2q8hKxZW_Z3SbUQr6YMgNh8THfXQ@mail.gmail.com","threadId":"49654","inReplyTo":"CAGshahkvn3fcyuqtD-WQE9tn+7rSad84+mtA_cfkz+t42xqPdw@mail.gmail.com","subject":"Re: git pull defaults for recursesubmodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-23T21:57:13Z","receivedAt":"2018-10-23T21:57:27Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 23, 2018 at 2:04 PM Tommi Vainikainen <tvainika@gmail.com> wrote:\n>\n> I configured my local git to fetch with recurseSubmodules = on-demand,\n> which I found the most convenient setting. However then I noticed that\n> I mostly use git pull actually to fetch from remotes, but git pull\n> does not utilize any recurseSubmoddules setting now, or at least I\n> could not find such.\n>\n> I would expect that if git-config has fetch.recurseSubmodules set,\n> also git pull should use this setting, or at least similar option such\n> as pull.recurseSubmodules should be available. I'd prefer sharing\n> fetch.recurseSubmodules setting here.\n>\n> I've attached a minimal patch, which I believe implements this\n> configuration usage, and a test case to show my expected behavior for\n> git pull.\n\nThis makes sense to me and the patch looks good to me.\nIt is unclear to me if this is a regression or an oversight of\nof a6d7eb2c7a (pull: optionally rebase submodules (remote\nsubmodule changes only), 2017-06-23)\n\nThanks,\nStefan\n"},{"id":"361340","messageId":"20181023230324.GC6119@genre.crustytoothpaste.net","threadId":"49654","inReplyTo":"CAGshahkvn3fcyuqtD-WQE9tn+7rSad84+mtA_cfkz+t42xqPdw@mail.gmail.com","subject":"Re: git pull defaults for recursesubmodules","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-23T23:03:24Z","receivedAt":"2018-10-23T23:03:31Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Oct 24, 2018 at 12:04:06AM +0300, Tommi Vainikainen wrote:\n> I configured my local git to fetch with recurseSubmodules = on-demand,\n> which I found the most convenient setting. However then I noticed that\n> I mostly use git pull actually to fetch from remotes, but git pull\n> does not utilize any recurseSubmoddules setting now, or at least I\n> could not find such.\n> \n> I would expect that if git-config has fetch.recurseSubmodules set,\n> also git pull should use this setting, or at least similar option such\n> as pull.recurseSubmodules should be available. I'd prefer sharing\n> fetch.recurseSubmodules setting here.\n> \n> I've attached a minimal patch, which I believe implements this\n> configuration usage, and a test case to show my expected behavior for\n> git pull.\n\nTypically, we prefer patches to be inline; descriptive content like this\ngoes after the --- line in the patch, or in a cover letter.\n\n> From e4483ec5b3d9c38a6e30aa0ab9fa521cba582345 Mon Sep 17 00:00:00 2001\n> From: Tommi Vainikainen <thv@iki.fi>\n> Date: Tue, 23 Oct 2018 23:47:58 +0300\n> Subject: [PATCH 1/1] pull: obey fetch.recurseSubmodules when fetching\n> \n> \"git pull\" now uses same recurse-submodules config for fetching as \"git\n> fetch\" by default if not overridden from command line.1\n\nYou have an extra '1\" at the end of this line.\n\nAlso, missing sign-off.  See Documentation/SubmittingPatches.\n\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 798ecf7faf..ed39b0e8ed 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -347,6 +347,9 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n>  \t\trecurse_submodules = git_config_bool(var, value) ?\n>  \t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n>  \t\treturn 0;\n> +\t} else if (!strcmp(var, \"fetch.recursesubmodules\")) {\n> +\t\trecurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n> +\t\treturn 0;\n>  \t}\n>  \treturn git_default_config(var, value, cb);\n>  }\n> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\n> index f916729a12..579ae5c374 100755\n> --- a/t/t5572-pull-submodule.sh\n> +++ b/t/t5572-pull-submodule.sh\n> @@ -75,6 +75,17 @@ test_expect_success \"submodule.recurse option triggers recursive pull\" '\n>  \ttest_path_is_file super/sub/merge_strategy_2.t\n>  '\n>  \n> +test_expect_success \"fetch.recurseSubmodules=true triggers recursive pull\" '\n> +\ttest_commit -C child fetch_recurse_submodules &&\n> +\tgit -C parent submodule update --remote &&\n> +\tgit -C parent add sub &&\n> +\tgit -C parent commit -m \"update submodule\" &&\n> +\n> +\tgit -C super config fetch.recurseSubmodules true &&\n> +\tgit -C super pull --no-rebase &&\n> +\ttest_path_is_file super/sub/fetch_recurse_submodules.t\n> +'\n\nCan we have a test that --no-recurse-submodules overrides\nfetch.recurseSubmodules?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"361366","messageId":"CAGshahmzPyC65g6dWSqucDckoxyv=uKQLuHR8aKRFdbSp3KaJQ@mail.gmail.com","threadId":"49654","inReplyTo":"CAGZ79kZf8owJUkYF2SsCr_2q8hKxZW_Z3SbUQr6YMgNh8THfXQ@mail.gmail.com","subject":"Re: git pull defaults for recursesubmodules","fromName":"Tommi Vainikainen","fromEmail":"tvainika@gmail.com","sentAt":"2018-10-24T05:32:56Z","receivedAt":"2018-10-24T05:33:10Z","isPatch":false,"sender":{"key":"tvainika@gmail.com","avatar":null},"body":"ke 24. lokak. 2018 klo 0.57 Stefan Beller (sbeller@google.com) kirjoitti:\n> On Tue, Oct 23, 2018 at 2:04 PM Tommi Vainikainen <tvainika@gmail.com> wrote:\n> > I would expect that if git-config has fetch.recurseSubmodules set,\n> > also git pull should use this setting, or at least similar option such\n>\n> This makes sense to me and the patch looks good to me.\n> It is unclear to me if this is a regression or an oversight of\n> of a6d7eb2c7a (pull: optionally rebase submodules (remote\n> submodule changes only), 2017-06-23)\n\nWith my testing, no it was not regression at least from that commit.\nIt did not work as I expected before that commit.\n\nWhat was unclear to me is why this config needs to be read as\npull calls fetch, why fetch does not use this configuration as is?\nIf fetch.prune=1, should git pull command also prune or not during\nfetch? There does not seem to be test case for that behavior.\n\n-- \nTommi Vainikainen\n"},{"id":"361372","messageId":"CAGshahk7pY4rW5SQu73AdHMmxsbDCo5UP5LGF67FQYBws492TA@mail.gmail.com","threadId":"49654","inReplyTo":"20181023230324.GC6119@genre.crustytoothpaste.net","subject":"Re: git pull defaults for recursesubmodules","fromName":"Tommi Vainikainen","fromEmail":"tvainika@gmail.com","sentAt":"2018-10-24T06:57:18Z","receivedAt":"2018-10-24T06:57:33Z","isPatch":false,"sender":{"key":"tvainika@gmail.com","avatar":null},"body":"ke 24. lokak. 2018 klo 2.03 brian m. carlson\n(sandals@crustytoothpaste.net) kirjoitti:\n> You have an extra '1\" at the end of this line.\n> Also, missing sign-off.  See Documentation/SubmittingPatches.\n\nAfter reading SubmittingPatches I didn't find if I should now send a\nfresh patch with\nchanges squashed together or new commits appended after first commit in that\npatch. Patch is updated accordingly as fresh patch.\n\n> Can we have a test that --no-recurse-submodules overrides\n> fetch.recurseSubmodules?\n\nAttached (sorry about that, I've no access now to more convenient mail\nsetup) is refreshed\npatch which also tests that. I included it in same patch to follow\nstyle of other tests\nin t5572-pull-submodule.\n\n-- \nTommi Vainikainen\n\n\nFrom 5d4b495f7c68a2e99314fef90e538e0d44ec1056 Mon Sep 17 00:00:00 2001\nFrom: Tommi Vainikainen <thv@iki.fi>\nDate: Wed, 24 Oct 2018 09:06:54 +0300\nSubject: [PATCH] pull: obey fetch.recurseSubmodules when fetching\n\n\"git pull\" now uses same recurse-submodules config for fetching as \"git\nfetch\" by default if not overridden from command line.\n\nThe command line arg --recurse-submodules=no overrides\nfetch.recurseSubmodules configuration.\n\nSigned-off-by: Tommi Vainikainen <thv@iki.fi>\n---\n builtin/pull.c            |  3 +++\n t/t5572-pull-submodule.sh | 13 +++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 681c127a07..bb51cb531d 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -346,6 +346,9 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n \t\trecurse_submodules = git_config_bool(var, value) ?\n \t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n \t\treturn 0;\n+\t} else if (!strcmp(var, \"fetch.recursesubmodules\")) {\n+\t\trecurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n+\t\treturn 0;\n \t}\n \treturn git_default_config(var, value, cb);\n }\ndiff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\nindex f916729a12..66c479a135 100755\n--- a/t/t5572-pull-submodule.sh\n+++ b/t/t5572-pull-submodule.sh\n@@ -75,6 +75,19 @@ test_expect_success \"submodule.recurse option triggers recursive pull\" '\n \ttest_path_is_file super/sub/merge_strategy_2.t\n '\n \n+test_expect_success \"fetch.recurseSubmodules=true triggers recursive pull\" '\n+\ttest_commit -C child fetch_recurse_submodules &&\n+\tgit -C parent submodule update --remote &&\n+\tgit -C parent add sub &&\n+\tgit -C parent commit -m \"update submodule\" &&\n+\n+\tgit -C super config fetch.recurseSubmodules true &&\n+\tgit -C super pull --recurse-submodules=no --no-rebase &&\n+\ttest_path_is_missing super/sub/fetch_recurse_submodules.t &&\n+\tgit -C super pull --no-rebase &&\n+\ttest_path_is_file super/sub/fetch_recurse_submodules.t\n+'\n+\n test_expect_success \" --[no-]recurse-submodule and submodule.recurse\" '\n \ttest_commit -C child merge_strategy_3 &&\n \tgit -C parent submodule update --remote &&\n-- \n2.19.1\n\n"},{"id":"361502","messageId":"xmqqy3amficw.fsf@gitster-ct.c.googlers.com","threadId":"49654","inReplyTo":"CAGshahk7pY4rW5SQu73AdHMmxsbDCo5UP5LGF67FQYBws492TA@mail.gmail.com","subject":"Re: git pull defaults for recursesubmodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T10:09:19Z","receivedAt":"2018-10-25T10:09:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tommi Vainikainen <tvainika@gmail.com> writes:\n\n> After reading SubmittingPatches I didn't find if I should now send a\n> fresh patch with\n> changes squashed together or new commits appended after first commit in that\n> patch. Patch is updated accordingly as fresh patch.\n\n(just on mechanics, not on the contents of your actual patch)\n\nYou can and should treat any topic that is not yet in 'next' as if\nit did not exist.  If you refined based on a v1 patch, pretend as if\nyou were a perfect developer and you came up with that refined\nversion without producing any problematic things that were pointed\nont in your v1.  Pretend mistakes in v1 never happened.  Pretend\nthat you are perfect! ;-)\n\nIf you can limit the signs that an earlier rounds ever existed to\n\n(1) The In-reply-to: header of the message you send your updated\n    version of the patch in, so that people can find the older\n    version and its discussion thread, and\n\n(2) The cover letter that describes what you improved in the\n    updated version relative to the last round, in addition to the\n    overview of the series [note: this only exists for a larger\n    patch series, and not usually done for a single patch]\n\nyou achieved your goal.\n"}]}