{"thread":{"id":"61271","subject":"[PATCH] fetch: return when parsing submodule.recurse","startedAt":"2024-04-05T02:52:25Z","lastAt":"2024-04-05T16:53:48Z","messageCount":2,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"492278","messageId":"pull.1709.git.1712285542303.gitgitgadget@gmail.com","threadId":"61271","inReplyTo":null,"subject":"[PATCH] fetch: return when parsing submodule.recurse","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-04-05T02:52:22Z","receivedAt":"2024-04-05T02:52:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <stolee@gmail.com>\n\nWhen parsing config keys, the normal pattern is to return 0 after\ncompleting the logic for a specific config key, since no other key will\nmatch. One instance, for \"submodule.recurse\", was missing this case in\nbuiltin/fetch.c.\n\nThis is a very minor change, and will have minimal impact to\nperformance. This particular block was edited recently in 56e8bb4fb4\n(fetch: use `fetch_config` to store \"fetch.recurseSubmodules\" value,\n2023-05-17), which led to some hesitation that perhaps this omission was\non purpose.\n\nHowever, no later cases within git_fetch_config() will match the key if\nequal to \"submodule.recurse\" and neither will any key matches within the\ncatch-all git_default_config().\n\nSigned-off-by: Derrick Stolee <stolee@gmail.com>\n---\n    fetch: return when parsing submodule.recurse\n    \n    This is a super-nit that I noticed when looking into this config setting\n    and couldn't help but try to fix.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1709%2Fderrickstolee%2Ffetch-config-parse-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1709/derrickstolee/fetch-config-parse-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1709\n\n builtin/fetch.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 46a793411a4..5857d860dbf 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -138,6 +138,7 @@ static int git_fetch_config(const char *k, const char *v,\n \t\tint r = git_config_bool(k, v) ?\n \t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n \t\tfetch_config->recurse_submodules = r;\n+\t\treturn 0;\n \t}\n \n \tif (!strcmp(k, \"submodule.fetchjobs\")) {\n\nbase-commit: 7774cfed6261ce2900c84e55906da708c711d601\n-- \ngitgitgadget\n"},{"id":"492301","messageId":"xmqqbk6nwy20.fsf@gitster.g","threadId":"61271","inReplyTo":"pull.1709.git.1712285542303.gitgitgadget@gmail.com","subject":"Re: [PATCH] fetch: return when parsing submodule.recurse","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-05T16:53:43Z","receivedAt":"2024-04-05T16:53:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Derrick Stolee <stolee@gmail.com>\n>\n> When parsing config keys, the normal pattern is to return 0 after\n> completing the logic for a specific config key, since no other key will\n> match. One instance, for \"submodule.recurse\", was missing this case in\n> builtin/fetch.c.\n>\n> This is a very minor change, and will have minimal impact to\n> performance.\n\nTrue, and the performance impact should be positive, if it is\nobservable.\n\nWell spotted.  Thanks.\n\n> This particular block was edited recently in 56e8bb4fb4\n> (fetch: use `fetch_config` to store \"fetch.recurseSubmodules\" value,\n> 2023-05-17), which led to some hesitation that perhaps this omission was\n> on purpose.\n>\n> However, no later cases within git_fetch_config() will match the key if\n> equal to \"submodule.recurse\" and neither will any key matches within the\n> catch-all git_default_config().\n>\n> Signed-off-by: Derrick Stolee <stolee@gmail.com>\n> ---\n>     fetch: return when parsing submodule.recurse\n>     \n>     This is a super-nit that I noticed when looking into this config setting\n>     and couldn't help but try to fix.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1709%2Fderrickstolee%2Ffetch-config-parse-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1709/derrickstolee/fetch-config-parse-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1709\n>\n>  builtin/fetch.c | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index 46a793411a4..5857d860dbf 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -138,6 +138,7 @@ static int git_fetch_config(const char *k, const char *v,\n>  \t\tint r = git_config_bool(k, v) ?\n>  \t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n>  \t\tfetch_config->recurse_submodules = r;\n> +\t\treturn 0;\n>  \t}\n>  \n>  \tif (!strcmp(k, \"submodule.fetchjobs\")) {\n>\n> base-commit: 7774cfed6261ce2900c84e55906da708c711d601\n"}]}