Re: [PATCH v3 1/3] repo-settings: fix checking for fetch.negotiationAlgorithm=default

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 1, 2022, 18:21 UTC
Message-ID: <xmqq4k5ifnxg.fsf@gitster.g>
In-Reply-To: <df0ec5ffe98a17e1ec7b572085e733d8748c0379.1643734828.git.gitgitgadget@gmail.com>

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 11 quoted lines

> diff --git a/repo-settings.c b/repo-settings.c
> index 00ca5571a1a..38c10f9977b 100644
> --- a/repo-settings.c
> +++ b/repo-settings.c
> @@ -85,6 +85,8 @@ void prepare_repo_settings(struct repository *r)
>  			r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
>  		else if (!strcasecmp(strval, "noop"))
>  			r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_NOOP;
> +		else if (!strcasecmp(strval, "default"))
> +			r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
>  	}

After this step, this function does:

 * fetch_negotiation_algorithm set to _DEFAULT
 * experimental bit flips it to _SKIPPING
 * config is read and skipping/noop/default overwrites it.

which is better than ignoring "default", but it strikes me as unnatural. If it were done like this instead:

 * fetch_negotiation_algorithm is set to _DEFAULT
 * config is read and skipping/noop/default overwrites it.
 * experimental bit flips it to _SKIPPING only when it is still _DEFAULT

this bug wouldn't have happened, I suspect.

More importantly, those who want to say "I want to keep up with the 'possible future default', which is appropriately chosen by Git for opt-in experimenters" by setting feature.experimental to true, would be able to do so when the flow is reordered like so.

Setting fetch.negotiationAlgorithm=default would be the way to do so.

Let's read on and see later steps of this series achieves the same. Perhaps they may do the same reordering.

Thanks.