Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch: return when parsing submodule.recurse #1709

Closed

Conversation

derrickstolee
Copy link

This is a super-nit that I noticed when looking into this config setting and couldn't help but try to fix.

When parsing config keys, the normal pattern is to return 0 after
completing the logic for a specific config key, since no other key will
match. One instance, for "submodule.recurse", was missing this case in
builtin/fetch.c.

This is a very minor change, and will have minimal impact to
performance. This particular block was edited recently in 56e8bb4
(fetch: use `fetch_config` to store "fetch.recurseSubmodules" value,
2023-05-17), which led to some hesitation that perhaps this omission was
on purpose.

However, no later cases within git_fetch_config() will match the key if
equal to "submodule.recurse" and neither will any key matches within the
catch-all git_default_config().

Signed-off-by: Derrick Stolee <stolee@gmail.com>
@derrickstolee
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Apr 5, 2024

Submitted as pull.1709.git.1712285542303.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1709/derrickstolee/fetch-config-parse-v1

To fetch this version to local tag pr-1709/derrickstolee/fetch-config-parse-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1709/derrickstolee/fetch-config-parse-v1

Copy link

gitgitgadget bot commented Apr 5, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <stolee@gmail.com>
>
> When parsing config keys, the normal pattern is to return 0 after
> completing the logic for a specific config key, since no other key will
> match. One instance, for "submodule.recurse", was missing this case in
> builtin/fetch.c.
>
> This is a very minor change, and will have minimal impact to
> performance.

True, and the performance impact should be positive, if it is
observable.

Well spotted.  Thanks.

> This particular block was edited recently in 56e8bb4fb4
> (fetch: use `fetch_config` to store "fetch.recurseSubmodules" value,
> 2023-05-17), which led to some hesitation that perhaps this omission was
> on purpose.
>
> However, no later cases within git_fetch_config() will match the key if
> equal to "submodule.recurse" and neither will any key matches within the
> catch-all git_default_config().
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>     fetch: return when parsing submodule.recurse
>     
>     This is a super-nit that I noticed when looking into this config setting
>     and couldn't help but try to fix.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1709%2Fderrickstolee%2Ffetch-config-parse-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1709/derrickstolee/fetch-config-parse-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1709
>
>  builtin/fetch.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 46a793411a4..5857d860dbf 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -138,6 +138,7 @@ static int git_fetch_config(const char *k, const char *v,
>  		int r = git_config_bool(k, v) ?
>  			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
>  		fetch_config->recurse_submodules = r;
> +		return 0;
>  	}
>  
>  	if (!strcmp(k, "submodule.fetchjobs")) {
>
> base-commit: 7774cfed6261ce2900c84e55906da708c711d601

Copy link

gitgitgadget bot commented Apr 5, 2024

This branch is now known as ds/fetch-config-parse-microfix.

Copy link

gitgitgadget bot commented Apr 5, 2024

This patch series was integrated into seen via git@10416ae.

@gitgitgadget gitgitgadget bot added the seen label Apr 5, 2024
Copy link

gitgitgadget bot commented Apr 5, 2024

There was a status update in the "New Topics" section about the branch ds/fetch-config-parse-microfix on the Git mailing list:

A config parser callback function fell through instead of returning
after recognising and processing a variable, wasting cycles, which
has been corrected.

Will merge to 'next'.
source: <pull.1709.git.1712285542303.gitgitgadget@gmail.com>

Copy link

gitgitgadget bot commented Apr 5, 2024

This patch series was integrated into seen via git@cbaed11.

Copy link

gitgitgadget bot commented Apr 9, 2024

This patch series was integrated into seen via git@989941e.

Copy link

gitgitgadget bot commented Apr 9, 2024

This patch series was integrated into seen via git@05f92b3.

Copy link

gitgitgadget bot commented Apr 9, 2024

This patch series was integrated into next via git@585dcad.

@gitgitgadget gitgitgadget bot added the next label Apr 9, 2024
Copy link

gitgitgadget bot commented Apr 9, 2024

There was a status update in the "Cooking" section about the branch ds/fetch-config-parse-microfix on the Git mailing list:

A config parser callback function fell through instead of returning
after recognising and processing a variable, wasting cycles, which
has been corrected.

Will merge to 'master'.
source: <pull.1709.git.1712285542303.gitgitgadget@gmail.com>

Copy link

gitgitgadget bot commented Apr 10, 2024

This patch series was integrated into seen via git@5bbec71.

Copy link

gitgitgadget bot commented Apr 10, 2024

This patch series was integrated into seen via git@9073028.

Copy link

gitgitgadget bot commented Apr 12, 2024

This patch series was integrated into seen via git@3646987.

Copy link

gitgitgadget bot commented Apr 12, 2024

This patch series was integrated into seen via git@ad87840.

Copy link

gitgitgadget bot commented Apr 13, 2024

There was a status update in the "Cooking" section about the branch ds/fetch-config-parse-microfix on the Git mailing list:

A config parser callback function fell through instead of returning
after recognising and processing a variable, wasting cycles, which
has been corrected.

Will merge to 'master'.
source: <pull.1709.git.1712285542303.gitgitgadget@gmail.com>

Copy link

gitgitgadget bot commented Apr 15, 2024

This patch series was integrated into seen via git@306c407.

Copy link

gitgitgadget bot commented Apr 15, 2024

This patch series was integrated into seen via git@6c142bc.

Copy link

gitgitgadget bot commented Apr 15, 2024

This patch series was integrated into master via git@6c142bc.

Copy link

gitgitgadget bot commented Apr 15, 2024

This patch series was integrated into next via git@6c142bc.

@gitgitgadget gitgitgadget bot added the master label Apr 15, 2024
@gitgitgadget gitgitgadget bot closed this Apr 15, 2024
Copy link

gitgitgadget bot commented Apr 15, 2024

Closed via 6c142bc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant