[PATCH v2 6/7] remote: add negotiationRequire config as default for --negotiation-require

["Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>]

From: Derrick Stolee <stolee@gmail.com>

Add a new 'remote.<name>.negotiationRequire' multi-valued config option
that provides default values for --negotiation-require when no
--negotiation-require 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_require()
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.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/config/remote.adoc | 24 +++++++++++++++++
 Documentation/fetch-options.adoc |  4 +++
 builtin/fetch.c                  | 10 +++++++
 remote.c                         |  6 +++++
 remote.h                         |  1 +
 t/t5510-fetch.sh                 | 46 ++++++++++++++++++++++++++++++++
 6 files changed, 91 insertions(+)

diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 5e8ac6cfdd..9dbe820275 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -123,6 +123,30 @@ command-line option.  If `--negotiation-restrict` (or its synonym
 `--negotiation-tip`) is specified on the command line, then the config
 values are not used.
 
+remote.<name>.negotiationRequire::
+	When negotiating with this remote during `git fetch` and `git push`,
+	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 ref patterns
+	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`) or a
+glob pattern (e.g. `refs/heads/release/*`).  The pattern syntax is the same
+as for `--negotiation-restrict`.
++
+These config values are used as defaults for the `--negotiation-require`
+command-line option.  If `--negotiation-require` 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>.negotiationRequire` are sent
+unconditionally on top of those heuristically selected commits.  This
+option is also used during push negotiation when `push.negotiate` is
+enabled.
+
 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 85ffc5b32b..16c6e8cee9 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -91,6 +91,10 @@ 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-require`.
++
+If this option is not specified on the command line, then any
+`remote.<name>.negotiationRequire` 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 b60652e6b1..a398115fb5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1622,6 +1622,16 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
 		else
 			warning(_("ignoring %s because the protocol does not support it"),
 				"--negotiation-require");
+	} else if (remote->negotiation_require.nr) {
+		if (transport->smart_options) {
+			transport->smart_options->negotiation_require = &remote->negotiation_require;
+		} else {
+			struct strbuf config_name = STRBUF_INIT;
+			strbuf_addf(&config_name, "remote.%s.negotiationRequire", 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 07cdf6434d..53deed7565 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_require);
 
 	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_require, 0);
 }
 
 static void add_merge(struct branch *branch, const char *name)
@@ -568,6 +570,10 @@ static int handle_config(const char *key, const char *value,
 		if (!value)
 			return config_error_nonbool(key);
 		string_list_append(&remote->negotiation_restrict, value);
+	} else if (!strcmp(subkey, "negotiationrequire")) {
+		if (!value)
+			return config_error_nonbool(key);
+		string_list_append(&remote->negotiation_require, value);
 	} else if (!strcmp(subkey, "followremotehead")) {
 		const char *no_warn_branch;
 		if (!strcmp(value, "never"))
diff --git a/remote.h b/remote.h
index e6ec37c393..d986257c78 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_require;
 
 	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 ec30b81c71..0246ac6bc5 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1573,6 +1573,52 @@ test_expect_success '--negotiation-require avoids duplicates with negotiator' '
 	test_line_count = 1 matches
 '
 
+test_expect_success 'remote.<name>.negotiationRequire used as default for --negotiation-require' '
+	test_when_finished rm -f trace &&
+	setup_negotiation_tip server server 0 &&
+
+	git -C client config --add remote.origin.negotiationRequire refs/tags/beta_1 &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+		--negotiation-restrict=alpha_1 \
+		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>.negotiationRequire works with glob patterns' '
+	test_when_finished rm -f trace &&
+	setup_negotiation_tip server server 0 &&
+
+	git -C client config --add remote.origin.negotiationRequire "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-require overrides remote.<name>.negotiationRequire' '
+	test_when_finished rm -f trace &&
+	setup_negotiation_tip server server 0 &&
+
+	git -C client config --add remote.origin.negotiationRequire refs/tags/beta_2 &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+		--negotiation-restrict=alpha_1 \
+		--negotiation-require=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 SYMLINKS 'clone does not get confused by a D/F conflict' '
 	git init df-conflict &&
 	(
-- 
gitgitgadget