Re: [PATCH v4 7/8] remote: add remote.*.negotiationInclude config

> From: Derrick Stolee <stolee@gmail.com>
> 
> Add a new 'remote.<name>.negotiationInclude' multi-valued config option that
> provides default values for --negotiation-include when no
> --negotiation-include arguments are specified over the command line.  This
> is a mirror of how 'remote.<name>.negotiationRestrict' specifies defaults
> for the --negotiation-restrict arguments.
> 
> Each value is either an exact ref name or a glob pattern whose tips should
> always be sent as 'have' lines during negotiation. The config values are
> resolved through the same resolve_negotiation_include() codepath as the CLI
> options.
> 
> This option is additive with the normal negotiation process: the negotiation
> algorithm still runs and advertises its own selected commits, but the refs
> matching the config are sent unconditionally on top of those heuristically
> selected commits.
> 
> Similar to the negotiationRestrict config, an empty value resets the value
> list to allow ignoring earlier config values, such as those that might be
> set in system or global config.
> 
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>   Documentation/config/remote.adoc | 27 ++++++++++++++++++
>   Documentation/fetch-options.adoc |  4 +++
>   builtin/fetch.c                  | 11 +++++++
>   remote.c                         |  5 ++++
>   remote.h                         |  1 +
>   t/t5510-fetch.sh                 | 49 ++++++++++++++++++++++++++++++++
>   6 files changed, 97 insertions(+)
> 
> diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
> index 4dcf81fbce..9ae20e4379 100644
> --- a/Documentation/config/remote.adoc
> +++ b/Documentation/config/remote.adoc
> @@ -125,6 +125,33 @@ values are not used.
>   Blank values signal to ignore all previous values, allowing a reset of
>   the list from broader config scenarios.
>   
> +remote.<name>.negotiationInclude::
> +	When negotiating with this remote during `git fetch`, the client
> +	advertises a list of commits that exist locally.  In repos with
> +	many references, this list of "haves" can be truncated. Depending
> +	on data shape, dropping certain references may be expensive. This
> +	multi-valued config option specifies references, commit hashes,
> +	or ref pattern globs whose tips should always be sent as "have"
> +	commits during fetch negotiation with this remote.
> ++
> +Each value is either an exact ref name (e.g. `refs/heads/release`), a
> +commit hash, or a glob pattern (e.g. `refs/heads/release/*`).  The
> +pattern syntax is the same as for `--negotiation-include`.

> +These config values are used as defaults for the `--negotiation-include`
> +command-line option.  If `--negotiation-include` is specified on the
> +command line, then the config values are not used.
> ++
> +This option is additive with the normal negotiation process: the
> +negotiation algorithm still runs and advertises its own selected commits,
> +but the refs matching `remote.<name>.negotiationInclude` are sent
> +unconditionally on top of those heuristically selected commits.  This
> +option is also used during push negotiation when `push.negotiate` is
> +enabled.

> ++
> +Blank values signal to ignore all previous values, allowing a reset of
> +the list from broader config scenarios.
> +
>   remote.<name>.followRemoteHEAD::
>   	How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
>   	when fetching using the configured refspecs of a remote.
> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index 7b897a7202..8074004377 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -91,6 +91,10 @@ The pattern syntax is the same as for `--negotiation-restrict`.
>   If `--negotiation-restrict` is used, the have set is first restricted by
>   that option and then increased to include the tips specified by
>   `--negotiation-include`.
> ++
> +If this option is not specified on the command line, then any
> +`remote.<name>.negotiationInclude` config values for the current remote
> +are used instead.
>   
>   `--negotiate-only`::
>   	Do not fetch anything from the server, and instead print the
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 6b456b3689..2308cab377 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1630,6 +1630,17 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
>   		else
>   			warning(_("ignoring %s because the protocol does not support it"),
>   				"--negotiation-include");
> +	} else if (remote->negotiation_include.nr) {
> +		if (transport->smart_options) {
> +			add_negotiation_tips(&remote->negotiation_include,
> +					     &transport->smart_options->negotiation_include_tips);
> +		} else {
> +			struct strbuf config_name = STRBUF_INIT;
> +			strbuf_addf(&config_name, "remote.%s.negotiationInclude", remote->name);
> +			warning(_("ignoring %s because the protocol does not support it"),
> +				config_name.buf);
> +			strbuf_release(&config_name);
> +		}
>   	}
>   	return transport;
>   }
> diff --git a/remote.c b/remote.c
> index 620086e16e..6fb5758820 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -153,6 +153,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
>   	refspec_init_fetch(&ret->fetch);
>   	string_list_init_dup(&ret->server_options);
>   	string_list_init_dup(&ret->negotiation_restrict);
> +	string_list_init_dup(&ret->negotiation_include);
>   
>   	ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
>   		   remote_state->remotes_alloc);
> @@ -181,6 +182,7 @@ static void remote_clear(struct remote *remote)
>   	FREE_AND_NULL(remote->http_proxy_authmethod);
>   	string_list_clear(&remote->server_options, 0);
>   	string_list_clear(&remote->negotiation_restrict, 0);
> +	string_list_clear(&remote->negotiation_include, 0);
>   }
>   
>   static void add_merge(struct branch *branch, const char *name)
> @@ -567,6 +569,9 @@ static int handle_config(const char *key, const char *value,
>   	} else if (!strcmp(subkey, "negotiationrestrict")) {
>   		return parse_transport_option(key, value,
>   					      &remote->negotiation_restrict);
> +	} else if (!strcmp(subkey, "negotiationinclude")) {
> +		return parse_transport_option(key, value,
> +					      &remote->negotiation_include);
>   	} else if (!strcmp(subkey, "followremotehead")) {
>   		const char *no_warn_branch;
>   		if (!strcmp(value, "never"))

> diff --git a/remote.h b/remote.h
> index e6ec37c393..d8809b6991 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -118,6 +118,7 @@ struct remote {
>   
>   	struct string_list server_options;
>   	struct string_list negotiation_restrict;
> +	struct string_list negotiation_include;
>   
>   	enum follow_remote_head_settings follow_remote_head;
>   	const char *no_warn_branch;
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index bc2e2af959..33f61ac12a 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -1587,6 +1587,55 @@ test_expect_success '--negotiation-include avoids duplicates with negotiator' '
>   	test_line_count = 1 matches
>   '
>   
> +test_expect_success 'remote.<name>.negotiationInclude used as default for --negotiation-include' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	# test the reset of the list on an empty value
> +	git -C client config --add remote.origin.negotiationInclude refs/tags/alpha_1 &&
> +	git -C client config --add remote.origin.negotiationInclude "" &&
> +	git -C client config --add remote.origin.negotiationInclude refs/tags/beta_1 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=beta_2 \
> +		origin alpha_s beta_s &&
> +
> +	ALPHA_1=$(git -C client rev-parse alpha_1) &&
> +	test_grep ! "fetch> have $ALPHA_1" trace &&
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace
> +'

> +
> +test_expect_success 'remote.<name>.negotiationInclude works with glob patterns' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	git -C client config --add remote.origin.negotiationInclude "refs/tags/beta_*" &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		origin alpha_s beta_s &&
> +
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace &&
> +	BETA_2=$(git -C client rev-parse beta_2) &&
> +	test_grep "fetch> have $BETA_2" trace
> +'
> +
> +test_expect_success 'CLI --negotiation-include overrides remote.<name>.negotiationInclude' '
> +	test_when_finished rm -f trace &&
> +	setup_negotiation_tip server server 0 &&
> +
> +	git -C client config --add remote.origin.negotiationInclude refs/tags/beta_2 &&
> +	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
> +		--negotiation-restrict=alpha_1 \
> +		--negotiation-include=refs/tags/beta_1 \
> +		origin alpha_s beta_s &&
> +
> +	BETA_1=$(git -C client rev-parse beta_1) &&
> +	test_grep "fetch> have $BETA_1" trace &&
> +	BETA_2=$(git -C client rev-parse beta_2) &&
> +	test_grep ! "fetch> have $BETA_2" trace
> +'
> +
>   test_expect_success '--negotiation-include avoids duplicates with v0' '
>   	test_when_finished rm -f trace &&
>   	setup_negotiation_tip server server 0 &&