{"thread":{"id":"46692","subject":"submodule: --recurse-submodules vs. submodule.recurse=true","startedAt":"2017-09-01T05:44:59Z","lastAt":"2017-09-01T18:15:59Z","messageCount":5,"participants":["Magnus Homann","Nicolas Morey-Chaisemartin","Stefan Beller","Jonathan Nieder","René Scharfe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"327499","messageId":"eba8e727-25ef-b34b-cd2b-e92602709c9b@homann.se","threadId":"46692","inReplyTo":null,"subject":"submodule: --recurse-submodules vs. submodule.recurse=true","fromName":"Magnus Homann","fromEmail":"magnus@homann.se","sentAt":"2017-09-01T05:39:42Z","receivedAt":"2017-09-01T05:44:59Z","isPatch":false,"sender":{"key":"magnus@homann.se","avatar":null},"body":"I'm using git 2.14.1 on cygwin.\n\nUsing --recurse-submodules, I can do 'git pull' and the submodules both get fetched and merged\nautomatically. I was under the impression that setting submodule.recurse to true would have the same\naffect, without needing to write --recurse-submodules every time. But the docs seems a bit vague,\nand I don't understand the git code.\n\nIs there a way to config git pull to automatcially do a \"--recurse-submodules\" ?\n\nThanks,\nMagnus\n"},{"id":"327500","messageId":"cc70ea38-9980-120f-afaa-af7a6e3a8c36@morey-chaisemartin.com","threadId":"46692","inReplyTo":"eba8e727-25ef-b34b-cd2b-e92602709c9b@homann.se","subject":"[PATCH] pull: honor submodule.recurse config option","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-09-01T07:29:38Z","receivedAt":"2017-09-01T07:30:07Z","isPatch":true,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"git pull used to not parse the submodule.recurse config option and simply\nconsider the --recurse-submodules CLI option.\nWhen using the config option, submodules would only be fetched recursively\nwhile the CLi option would tigger both fetch and update/merge.\n\nReported-by: Magnus Homann \nSigned-off-by: Nicolas Morey-Chaisemartin \n---\n builtin/pull.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 7fe281414..e4edf23c5 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -326,6 +326,11 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n \t\tconfig_autostash = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"submodule.recurse\")) {\n+\t\tint r = git_config_bool(var, value) ?\n+\t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n+\t\trecurse_submodules = r;\n+\t}\n \treturn git_default_config(var, value, cb);\n }\n \n-- \n2.14.1.460.g196d2604f\n\n"},{"id":"327504","messageId":"CAGZ79kaxSARkh9+PrYB05+Ln=hngu-9_y+UYi=P+M0OzNdedNw@mail.gmail.com","threadId":"46692","inReplyTo":"cc70ea38-9980-120f-afaa-af7a6e3a8c36@morey-chaisemartin.com","subject":"Re: [PATCH] pull: honor submodule.recurse config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-01T17:11:05Z","receivedAt":"2017-09-01T17:11:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Sep 1, 2017 at 12:29 AM, Nicolas Morey-Chaisemartin\n wrote:\n> git pull used to not parse the submodule.recurse config option and simply\n> consider the --recurse-submodules CLI option.\n> When using the config option, submodules would only be fetched recursively\n> while the CLi option would tigger both fetch and update/merge.\n>\n> Reported-by: Magnus Homann \n> Signed-off-by: Nicolas Morey-Chaisemartin \n\nReviewed-by: Stefan Beller \n\nThanks,\nStefan\n\n\n> ---\n> builtin/pull.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 7fe281414..e4edf23c5 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -326,6 +326,11 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n> config_autostash = git_config_bool(var, value);\n> return 0;\n> }\n> + if (!strcmp(var, \"submodule.recurse\")) {\n> + int r = git_config_bool(var, value) ?\n> + RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n> + recurse_submodules = r;\n> + }\n> return git_default_config(var, value, cb);\n> }\n>\n> --\n> 2.14.1.460.g196d2604f\n>\n"},{"id":"327505","messageId":"20170901172815.GA143138@aiede.mtv.corp.google.com","threadId":"46692","inReplyTo":"cc70ea38-9980-120f-afaa-af7a6e3a8c36@morey-chaisemartin.com","subject":"Re: [PATCH] pull: honor submodule.recurse config option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-09-01T17:28:15Z","receivedAt":"2017-09-01T17:28:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nNicolas Morey-Chaisemartin wrote:\n\n> git pull used to not parse the submodule.recurse config option and simply\n> consider the --recurse-submodules CLI option.\n> When using the config option, submodules would only be fetched recursively\n> while the CLi option would tigger both fetch and update/merge.\n>\n> Reported-by: Magnus Homann \n> Signed-off-by: Nicolas Morey-Chaisemartin \n\nnits:\n\n* Git's commit messages usually use the present tense to describe the\n behavior of Git in absence of a patch, as though writing a bug report.\n They use the imperative mood to describe what the patch will do, as\n though commanding the code to do better.\n* spelling: s/CLi/CLI/; s/tigger/trigger/\n* please also wrap lines consistently\n\nThat would make\n\n\t\"git pull\" supports a --recurse-submodules option but does not parse the\n\tsubmodule.recurse configuration item to set the default for that option.\n\tMeanwhile \"git fetch\" does support submodule.recurse, producing\n\tconfusing behavior: when submodule.recurse is enabled, \"git pull\"\n\trecursively fetches submodules but does not update them after fetch.\n\n\tHandle submodule.recurse in \"git pull\" to fix this.\n\n> ---\n> builtin/pull.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n\nCan you add a test to avoid future changes causing this to regress?\nSee t/t5572-pull-submodule.sh for some existing tests to get\ninspiration from.\n\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 7fe281414..e4edf23c5 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -326,6 +326,11 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n> \t\tconfig_autostash = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> +\tif (!strcmp(var, \"submodule.recurse\")) {\n> +\t\tint r = git_config_bool(var, value) ?\n> +\t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n> +\t\trecurse_submodules = r;\n> +\t}\n> \treturn git_default_config(var, value, cb);\n> }\n> \n\nThe rest looks good.\n\nThanks for working on this,\nJonathan\n"},{"id":"327506","messageId":"6edd3cff-e552-787e-9ca4-0d91df8e51e0@web.de","threadId":"46692","inReplyTo":"CAGZ79kaxSARkh9+PrYB05+Ln=hngu-9_y+UYi=P+M0OzNdedNw@mail.gmail.com","subject":"Re: [PATCH] pull: honor submodule.recurse config option","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-09-01T18:15:40Z","receivedAt":"2017-09-01T18:15:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 01.09.2017 um 19:11 schrieb Stefan Beller:\n> On Fri, Sep 1, 2017 at 12:29 AM, Nicolas Morey-Chaisemartin\n> wrote:\n>> git pull used to not parse the submodule.recurse config option and simply\n>> consider the --recurse-submodules CLI option.\n>> When using the config option, submodules would only be fetched recursively\n>> while the CLi option would tigger both fetch and update/merge.\n>>\n>> Reported-by: Magnus Homann \n>> Signed-off-by: Nicolas Morey-Chaisemartin \n> \n> Reviewed-by: Stefan Beller \n> \n> Thanks,\n> Stefan\n> \n> \n>> ---\n>> builtin/pull.c | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/builtin/pull.c b/builtin/pull.c\n>> index 7fe281414..e4edf23c5 100644\n>> --- a/builtin/pull.c\n>> +++ b/builtin/pull.c\n>> @@ -326,6 +326,11 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n>> config_autostash = git_config_bool(var, value);\n>> return 0;\n>> }\n>> + if (!strcmp(var, \"submodule.recurse\")) {\n>> + int r = git_config_bool(var, value) ?\n>> + RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;>> + recurse_submodules = r;\n\nA few nits to pick:\n\nWhy not assign directly to recurse_submodules? The variable r is only\nset once and read once, and doesn't have a particularly descriptive name\nthat would justify having it.\n\nbuiltin/fetch.c::git_fetch_config(), builtin/push.c::git_push_config()\nand submodule.c::git_default_submodule_config() do the same, and I can't\ninfer why for them either.\n\nAnd why fall through to git_default_config() here even though we know\nthat it won't match \"submodule.recurse\" again? Config functions are\nusually exit early on finding a match, as the \"rebase.autostash\" handler\nabove does.\n\n>> + }\n>> return git_default_config(var, value, cb);\n>> }\n>>\n>> --\n>> 2.14.1.460.g196d2604f\n>>\n"}]}