Support server option from configuration

Currently, server options for Git protocol v2 can only be specified via the command line option "--server-option ", which is inconvenient for users who want to specify a list of default options.

This patch series introduces a new multi-valued configuration, fetch.serverOption, to specify default server options. Designed similarly to push.pushOption:

1. Server options set in lower-priority configuration files (e.g., /etc/gitconfig or $HOME/.gitconfig) can be overridden or unset in more specific repository configurations using an empty string.
2. Command-line specified server options take precedence over those from the configuration.

All commands involving server options, including git-fetch, git-clone, git-ls-remote, and git-pull, have been updated to recognize the new configuration.

Xing Xin (4):
transport: add parse_transport_option() method builtin/fetch.c: add fetch.serverOption configuration builtin/clone.c: recognize fetch.serverOption configuration builtin/ls-remote.c: recognize fetch.serverOption configuration

 Documentation/config/fetch.txt  | 29 ++++++++++
 Documentation/fetch-options.txt |  3 ++
 Documentation/git-clone.txt     |  3 ++
 Documentation/git-ls-remote.txt |  3 ++
 builtin/clone.c                 | 22 +++++---
 builtin/fetch.c                 | 31 ++++++++---
 builtin/ls-remote.c             | 32 ++++++++++--
 builtin/push.c                  |  6 +--
 t/t5702-protocol-v2.sh          | 93 ++++++++++++++++++++++++++++-----
 transport.c                     |  8 +++
 transport.h                     |  3 ++
 11 files changed, 196 insertions(+), 37 deletions(-)

base-commit: 4590f2e9412378c61eac95966709c78766d326ba
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1776%2Fblanet%2Fxx%2Fadd-server-option-from-config-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1776/blanet/xx/add-server-option-from-config-v1
Pull-Request: https://github.com/git/git/pull/1776

gitgitgadget

From: Xing Xin <[email protected]>

Introduce the parse_transport_option() method used to parse push.pushOption configuration values. This method will be further utilized in subsequent commits to parse a newly added fetch.serverOption configuration for fetches, which aligns with the design pattern of push.pushOption.

---
 builtin/push.c | 6 +-----
 transport.c    | 8 ++++++++
 transport.h    | 3 +++
 3 files changed, 12 insertions(+), 5 deletions(-)

index 7a67398124f..63d19acfb27 100644
@@ -519,11 +519,7 @@ static int git_push_config(const char *k, const char *v,
 	} else if (!strcmp(k, "push.pushoption")) {
 		if (!v)
 			return config_error_nonbool(k);
-		else
-			if (!*v)
-				string_list_clear(&push_options_config, 0);
-			else
-				string_list_append(&push_options_config, v);
+		parse_transport_option(v, &push_options_config);
 		return 0;
 	} else if (!strcmp(k, "color.push")) {
 		push_use_color = git_config_colorbool(k, v);

index bab28965f96..ca6cd5b3436 100644
@@ -1091,6 +1091,14 @@ int is_transport_allowed(const char *type, int from_user)
 	BUG("invalid protocol_allow_config type");
 }
 
+void parse_transport_option(const char *option, struct string_list *transport_options)
+{
+	if (!*option)
+		string_list_clear(transport_options, 0);
+	else
+		string_list_append(transport_options, option);
+}
+
 void transport_check_allowed(const char *type)
 {
 	if (!is_transport_allowed(type, -1))

index 6393cd9823c..1b8735e2ca4 100644
@@ -342,4 +342,7 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 /* common method used by transport-helper.c and send-pack.c */
 void reject_atomic_push(struct ref *refs, int mirror_mode);
 
+/* common method to parse push-option for pushes or server-option for fetches */
+void parse_transport_option(const char *option, struct string_list *transport_options);
+
 #endif

gitgitgadget

From: Xing Xin <[email protected]>

Currently, server options for Git protocol v2 can only be passed via the command line using "--server-option <option>", which is inconvenient when we want to specify a list of default options to send.

This commit introduces a new multi-valued configuration fetch.serverOption, which is designed similar to push.pushOption. It allows specifying default server options to be sent automatically if not explicitly set. Subsequent commits will extend this configuration to other commands that involve server options, such as git-ls-remote and git-clone.

When server options are set in lower-priority configuration files (e.g., /etc/gitconfig or $HOME/.gitconfig), they can be overridden or unset in more specific repository configurations using an empty string.

Tests and documentation have been updated accordingly.

---
 Documentation/config/fetch.txt  | 29 +++++++++++++++++++++++
 Documentation/fetch-options.txt |  3 +++
 builtin/fetch.c                 | 31 +++++++++++++++++-------
 t/t5702-protocol-v2.sh          | 42 +++++++++++++++++++++++++++------
 4 files changed, 90 insertions(+), 15 deletions(-)

index d7dc461bd16..9987d2a7425 100644
@@ -126,3 +126,32 @@ fetch.bundleCreationToken::
 The creation token values are chosen by the provider serving the specific
 bundle URI. If you modify the URI at `fetch.bundleURI`, then be sure to
 remove the value for the `fetch.bundleCreationToken` value before fetching.
+
+fetch.serverOption::
+	When no `--server-option=<option>` argument is given from the
+	command line, fetch behaves as if each <value> of this variable is
+	given as `--server-option=<value>`.
++
+This is a multi-valued variable, and an empty value can be used in a higher
+priority configuration file (e.g. `.git/config` in a repository) to clear
+the values inherited from a lower priority configuration files (e.g.
+`$HOME/.gitconfig`).
++
+----
+
+Example:
+
+/etc/gitconfig
+  fetch.serverOption = a
+  fetch.serverOption = b
+
+~/.gitconfig
+  fetch.serverOption = c
+
+repo/.git/config
+  fetch.serverOption =
+  fetch.serverOption = b
+
+This will result in only b (a and c are cleared).
+
+----

index e22b217fba9..39969aeb326 100644
@@ -304,6 +304,9 @@ endif::git-pull[]
 	unknown ones, is server-specific.
 	When multiple `--server-option=<option>` are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no `--server-option=<option>` is given from the command
+	line, the values of configuration variable `fetch.serverOption`
+	are used instead.
 
 --show-forced-updates::
 	By default, git checks if a branch is force-updated during

index c297569a473..d49dff8a76a 100644
@@ -95,7 +95,7 @@ static struct transport *gtransport;
 static struct transport *gsecondary;
 static struct refspec refmap = REFSPEC_INIT_FETCH;
 static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
-static struct string_list server_options = STRING_LIST_INIT_DUP;
+static struct string_list config_server_options = STRING_LIST_INIT_DUP;
 static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
 
 struct fetch_config {

@@ -170,6 +170,13 @@ static int git_fetch_config(const char *k, const char *v,
 			    "fetch.output", v);
 	}
 
+	if (!strcmp(k, "fetch.serveroption")) {
+		if (!v)
+			return config_error_nonbool(k);
+		parse_transport_option(v, &config_server_options);
+		return 0;
+	}
+
 	return git_default_config(k, v, ctx, cb);
 }
 

@@ -2063,7 +2070,8 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 
 static int fetch_one(struct remote *remote, int argc, const char **argv,
 		     int prune_tags_ok, int use_stdin_refspecs,
-		     const struct fetch_config *config)
+		     const struct fetch_config *config,
+		     struct string_list *server_options)
 {
 	struct refspec rs = REFSPEC_INIT_FETCH;
 	int i;

@@ -2124,8 +2132,8 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
 		strbuf_release(&line);
 	}
 
-	if (server_options.nr)
-		gtransport->server_options = &server_options;
+	if (server_options && server_options->nr)
+		gtransport->server_options = server_options;
 
 	sigchain_push_common(unlock_pack_on_signal);
 	atexit(unlock_pack_atexit);

@@ -2152,6 +2160,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	const char *submodule_prefix = "";
 	const char *bundle_uri;
 	struct string_list list = STRING_LIST_INIT_DUP;
+	struct string_list option_server_options = STRING_LIST_INIT_DUP;
+	struct string_list *server_options = NULL;
 	struct remote *remote = NULL;
 	int all = -1, multiple = 0;
 	int result = 0;

@@ -2231,7 +2241,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 			 N_("accept refs that update .git/shallow")),
 		OPT_CALLBACK_F(0, "refmap", &refmap, N_("refmap"),
 			       N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
-		OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
+		OPT_STRING_LIST('o', "server-option", &option_server_options, N_("server-specific"),
+				N_("option to transmit")),
 		OPT_IPVERSION(&family),
 		OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
 				N_("report that we have only objects reachable from this object")),

@@ -2272,6 +2283,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);
 
+	server_options = option_server_options.nr ?
+			 &option_server_options : &config_server_options;
+
 	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
 		config.recurse_submodules = recurse_submodules_cli;
 

@@ -2418,8 +2432,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 			result = 1;
 			goto cleanup;
 		}
-		if (server_options.nr)
-			gtransport->server_options = &server_options;
+		if (server_options && server_options->nr)
+			gtransport->server_options = server_options;
 		result = transport_fetch_refs(gtransport, NULL);
 
 		oidset_iter_init(&acked_commits, &iter);

@@ -2430,7 +2444,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		if (filter_options.choice || repo_has_promisor_remote(the_repository))
 			fetch_one_setup_partial(remote);
 		result = fetch_one(remote, argc, argv, prune_tags_ok, stdin_refspecs,
-				   &config);
+				   &config, server_options);
 	} else {
 		int max_children = max_jobs;
 

@@ -2529,5 +2543,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
  cleanup:
 	string_list_clear(&list, 0);
+	string_list_clear(&option_server_options, 0);
 	return result;
 }

gitgitgadget

From: Xing Xin <[email protected]>

Teach git-clone to recognize the fetch.serverOption configuration as a default list of server options to send for Git protocol v2, if server options are not explicitly set via the command line.

Note that builtin/clone.c:cmd_clone originally read the git config twice via builtin/clone.c:git_clone_config, which would duplicate server options if parsing logic were added there. Upon investigation, it was found that the first config read is unnecessary since all the global variables it sets are actually used after the second config read. Therefore, the first config read is replaced with a simple config.c:git_default_config.

Tests and documentation have been updated accordingly.

---
 Documentation/git-clone.txt |  3 +++
 builtin/clone.c             | 22 ++++++++++++++++------
 t/t5702-protocol-v2.sh      | 22 ++++++++++++++++++++--
 3 files changed, 39 insertions(+), 8 deletions(-)

index 8e925db7e9c..105645ed685 100644
@@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.
 	unknown ones, is server-specific.
 	When multiple ++--server-option=++__<option>__ are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no ++--server-option=++__<option>__ is given from the command
+	line, the values of configuration variable `fetch.serverOption`
+	are used instead.
 
 `-n`::
 `--no-checkout`::

index 269b6e18a4e..5a1e2e769af 100644
@@ -85,7 +85,8 @@ static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;
 static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
 static int option_filter_submodules = -1;    /* unspecified */
 static int config_filter_submodules = -1;    /* unspecified */
-static struct string_list server_options = STRING_LIST_INIT_NODUP;
+static struct string_list config_server_options = STRING_LIST_INIT_DUP;
+static struct string_list option_server_options = STRING_LIST_INIT_DUP;
 static int option_remote_submodules;
 static const char *bundle_uri;
 

@@ -160,7 +161,7 @@ static struct option builtin_clone_options[] = {
 		   N_("specify the reference format to use")),
 	OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
 			N_("set config inside the new repository")),
-	OPT_STRING_LIST(0, "server-option", &server_options,
+	OPT_STRING_LIST(0, "server-option", &option_server_options,
 			N_("server-specific"), N_("option to transmit")),
 	OPT_IPVERSION(&family),
 	OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),

@@ -847,6 +848,12 @@ static int git_clone_config(const char *k, const char *v,
 		config_reject_shallow = git_config_bool(k, v);
 	if (!strcmp(k, "clone.filtersubmodules"))
 		config_filter_submodules = git_config_bool(k, v);
+	if (!strcmp(k, "fetch.serveroption")) {
+		if (!v)
+			return config_error_nonbool(k);
+		parse_transport_option(v, &config_server_options);
+		return 0;
+	}
 
 	return git_default_config(k, v, ctx, cb);
 }

@@ -982,17 +989,20 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	int hash_algo;
 	enum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
 	const int do_not_override_repo_unix_permissions = -1;
-
+	struct string_list *server_options = NULL;
 	struct transport_ls_refs_options transport_ls_refs_options =
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
 
 	packet_trace_identity("clone");
 
-	git_config(git_clone_config, NULL);
+	git_config(git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
 
+	server_options = option_server_options.nr ?
+			 &option_server_options : &config_server_options;
+
 	if (argc > 2)
 		usage_msg_opt(_("Too many arguments."),
 			builtin_clone_usage, builtin_clone_options);

@@ -1359,8 +1369,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK,
 				     option_upload_pack);
 
-	if (server_options.nr)
-		transport->server_options = &server_options;
+	if (server_options && server_options->nr)
+		transport->server_options = server_options;
 
 	if (filter_options.choice) {
 		const char *spec =

index ae25400010e..3bf31fb570d 100755
@@ -424,12 +424,30 @@ test_expect_success 'warn if using server-option with fetch with legacy protocol
 test_expect_success 'server-options are sent when cloning' '
 	test_when_finished "rm -rf log myclone" &&
 
+	# Specify server options from command line
 	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
 		clone --server-option=hello --server-option=world \
 		"file://$(pwd)/file_parent" myclone &&
+	test_grep "server-option=hello" log &&
+	test_grep "server-option=world" log &&
+	rm -rf log myclone &&
 
-	grep "server-option=hello" log &&
-	grep "server-option=world" log
+	# Specify server options from fetch.serverOption config
+	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
+		-c fetch.serverOption=hello -c fetch.serverOption=world \
+		clone "file://$(pwd)/file_parent" myclone &&
+	test_grep "server-option=hello" log &&
+	test_grep "server-option=world" log &&
+	rm -rf log myclone &&
+
+	# Cmdline server options take a higher priority
+	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
+		-c fetch.serverOption=hello -c fetch.serverOption=world \
+		clone --server-option=foo=bar \
+		"file://$(pwd)/file_parent" myclone &&
+	test_grep ! "server-option=hello" log &&
+	test_grep ! "server-option=world" log &&
+	test_grep "server-option=foo=bar" log
 '
 
 test_expect_success 'warn if using server-option with clone with legacy protocol' '

gitgitgadget

From: Xing Xin <[email protected]>

Teach git-ls-remote to recognize the fetch.serverOption configuration as a default list of server options to send for Git protocol v2, if server options are not explicitly set via the command line.

Tests and documentation have been updated accordingly.

---
 Documentation/git-ls-remote.txt |  3 +++
 builtin/ls-remote.c             | 32 ++++++++++++++++++++++++++++----
 t/t5702-protocol-v2.sh          | 29 ++++++++++++++++++++++++-----
 3 files changed, 55 insertions(+), 9 deletions(-)

index 76c86c3ce4f..fc9930193f8 100644
@@ -81,6 +81,9 @@ OPTIONS
 	character.
 	When multiple `--server-option=<option>` are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no `--server-option=<option>` is given from the command
+	line, the values of configuration variable `fetch.serverOption`
+	are used instead.
 
 <repository>::
 	The "remote" repository to query.  This parameter can be

index 0a491595ca8..87afd69828c 100644
@@ -1,4 +1,5 @@
 #include "builtin.h"
+#include "config.h"
 #include "gettext.h"
 #include "hex.h"
 #include "transport.h"

@@ -8,6 +9,8 @@
 #include "parse-options.h"
 #include "wildmatch.h"
 
+static struct string_list config_server_options = STRING_LIST_INIT_DUP;
+
 static const char * const ls_remote_usage[] = {
 	N_("git ls-remote [--branches] [--tags] [--refs] [--upload-pack=<exec>]\n"
 	   "              [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]\n"

@@ -37,6 +40,18 @@ static int tail_match(const struct strvec *pattern, const char *path)
 	return 0;
 }
 
+static int git_ls_remote_config(const char *k, const char *v,
+			    const struct config_context *ctx, void *cb)
+{
+	if (!strcmp(k, "fetch.serveroption")) {
+		if (!v)
+			return config_error_nonbool(k);
+		parse_transport_option(v, &config_server_options);
+		return 0;
+	}
+	return git_default_config(k, v, ctx, cb);
+}
+
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	const char *dest = NULL;

@@ -49,8 +64,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	struct strvec pattern = STRVEC_INIT;
 	struct transport_ls_refs_options transport_options =
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
+	struct string_list option_server_options = STRING_LIST_INIT_DUP;
+	struct string_list *server_options = NULL;
 	int i;
-	struct string_list server_options = STRING_LIST_INIT_DUP;
 
 	struct remote *remote;
 	struct transport *transport;

@@ -80,16 +96,23 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			      2, PARSE_OPT_NOCOMPLETE),
 		OPT_BOOL(0, "symref", &show_symref_target,
 			 N_("show underlying ref in addition to the object pointed by it")),
-		OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
+		OPT_STRING_LIST('o', "server-option", &option_server_options,
+				N_("server-specific"),
+				N_("option to transmit")),
 		OPT_END()
 	};
 
+	git_config(git_ls_remote_config, NULL);
+
 	memset(&ref_array, 0, sizeof(ref_array));
 
 	argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	dest = argv[0];
 
+	server_options = option_server_options.nr ?
+			 &option_server_options : &config_server_options;
+
 	/*
 	 * TODO: This is buggy, but required for transport helpers. When a
 	 * transport helper advertises a "refspec", then we'd add that to a

@@ -130,8 +153,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	transport = transport_get(remote, NULL);
 	if (uploadpack)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
-	if (server_options.nr)
-		transport->server_options = &server_options;
+	if (server_options && server_options->nr)
+		transport->server_options = server_options;
 
 	ref = transport_get_remote_refs(transport, &transport_options);
 	if (ref) {

@@ -169,5 +192,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	transport_ls_refs_options_release(&transport_options);
 
 	strvec_clear(&pattern);
+	string_list_clear(&option_server_options, 0);
 	return status;
 }

index 3bf31fb570d..da6241afa22 100755
@@ -173,16 +173,35 @@ test_expect_success 'ref advertisement is filtered with ls-remote using protocol
 test_expect_success 'server-options are sent when using ls-remote' '
 	test_when_finished "rm -f log" &&
 
-	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
-		ls-remote -o hello -o world "file://$(pwd)/file_parent" main >actual &&
-
 	cat >expect <<-EOF &&
 	$(git -C file_parent rev-parse refs/heads/main)$(printf "\t")refs/heads/main
 	EOF
 
+	# Specify server options from command line
+	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
+		ls-remote -o hello -o world "file://$(pwd)/file_parent" main >actual &&
+	test_cmp expect actual &&
+	test_grep "server-option=hello" log &&
+	test_grep "server-option=world" log &&
+	rm -f log &&
+
+	# Specify server options from fetch.serverOption config
+	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
+		-c fetch.serverOption=hello -c fetch.serverOption=world \
+		ls-remote "file://$(pwd)/file_parent" main >actual &&
+	test_cmp expect actual &&
+	test_grep "server-option=hello" log &&
+	test_grep "server-option=world" log &&
+	rm -f log &&
+
+	# Cmdline server options take a higher priority
+	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
+		-c fetch.serverOption=hello -c fetch.serverOption=world \
+		ls-remote -o foo=bar "file://$(pwd)/file_parent" main >actual &&
 	test_cmp expect actual &&
-	grep "server-option=hello" log &&
-	grep "server-option=world" log
+	test_grep ! "server-option=hello" log &&
+	test_grep ! "server-option=world" log &&
+	test_grep "server-option=foo=bar" log
 '
 
 test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '

gitgitgadget

It would be nice to learn about the context this comes from. In which scenario does it make sense to specify options by default? What is the intended usecase? I'm sure that this feature proposal comes from a specific usecase that you have at your employer, so learning a bit about it would help to decide whether it makes sense to have or not.

Patrick

Sorry for not explaining our use case clearly. We have several internal repositories configured with numerous CI tasks, each requiring code preparation (sometimes via clone, sometimes via fetch). These CI tasks are ususally triggered by post-receive hook, so the concurrent tasks are actually fetching the same copy of code.

On git server, we want to deploy a special pack-objects-hook to mitigate the performance impacts caused by these CI tasks (so the packfile produced by git-pack-objects can be reused). Since not all clone/fetch operations can benefit from this caching mechanism (e.g. pulls from users' dev environment), we need the client to pass a special identifier to inform the server whether caching support should be enabled for that clone/fetch. Clearly, using server options is a good choice.

To achieve our design, we need to add two patch series to git:

1. Support injecting server options to identify environments via configuration, because adding the --server-option parameter would require too many script modifications, making it difficult to deploy. This is what this patch series does.
2. Git server should pass the received server options as environment variables (similar to push options) to the pack-objects-hook.

I named the new configuration fetch.serverOption mainly to follow the push.pushOption pattern. Since which server options to support is actually server-specific, using remote.<name>.serverOption is a good idea for git-fetch. However, how should we design the configuration for git-ls-remote or git-clone, if we wanna provide all of them with a default list of server options to send?

Xing Xin

When you talk about client, do you mean that the actual users will have to configure this? That sounds somewhat unmaintainable on your side from the surface. I guess I ain't got enough knowledge around the environment you operate in though, so I probably shouldn't judge.

As mentioned in another reply, I think that putting this into the remote configuration "remote.*.serverOption" might be a better solution, as it also brings you the ability to set this per remote by default.

Patrick

Yes, that is indeed our design goal. We want specially configured git clients to benefit from caching acceleration when cloning code, while clients without special configurations follow the regular logic. This is because code fetching in CI environments is often homogeneous, making it worthwhile to implement caching logic to speed up the process and reduce server load. In contrast, code fetching from users is usually diverse, making caching less valuable.

By linking pack-objects-hook with server options, we believe we can effectively differentiate between various clone behaviors. In fact, I think a similar design can also be extended to GitHub Actions or GitLab CI to save CPU on git servers, although I'm not sure whether a similar mechanism is already available.

CI environments are usually provided by the infrastructure team, making this solution easier to maintain, and we do not expect any changes on real users.

I agree that using "remote.*.serverOption" is better. In fact, I also think "push.pushOption" would be better as "remote.*.pushOption". What I'm contemplating is whether we need to add a configuration for a default list of server options, so that when "remote.origin.serverOption" is not present, we can fall back to use that as default.

Xing Xin

Junio proposed in [1] to introduce [remote "*"] syntax to supply default values to all remotes. You could pick up that proposal and implement it as part of your patch series.

I also have to agree that "push.pushOption" would be way more sensible if it was configured per remote. I think it would be sensible to also introduce "remote.*.pushOption" in the same way and have it override the default value of "push.pushOption" if present. So the precedence order would become (from high to low):

- remote.someRemote.pushOption
- remote."*".pushOption
- push.PushOption

This should be backwards compatible, too.

Patrick

[1]: <[email protected]>

;-) Sounds sensible.

At 2024-09-05 21:44:41, "Patrick Steinhardt" <[email protected]> wrote

[snip]

Given the addition of a remote.*.<subkey> configuration in Git's global settings, such as:

    git config --global remote."*".demoConfigKey demoConfigValue

The current versions of Git may produce errors with certain commands. For example, running git fetch --all will result in:

    $ git fetch --all
    Fetching *
    fatal: '*' does not appear to be a git repository
    fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists.
error: could not fetch *
Fetching origin

This raises the question of whether this can be defined as an incompatibility or whether this is acceptable. If it isn't, I prefer using a remote.defaultServerOption instead, as it aligns with our existing use of remote.pushDefault anyway.

[snip]

Xing Xin

We do not currently handle "remote.*.something", so we'd first have to add support for this syntax.

While for this config key it may work alright, I think supporting "remote.*.something" is preferable. Ideally, it would be generic enough to apply to all per-remote settings such that we don't have to add support for every single config key that applies to remotes.

Right now we only care about push options and server options. But somebody may want to globally configure proxies, tag options, partial clone filters or something else. And if we got all of that without having to introduce remote.defaultPruneTags, remote.defaultPartialCloneFilter and similar options I'd consider it a win.

Patrick

Ah, good point. Anything that wants to enumerate the subkeys under remote. hierarchy MUST be aware of the new convention.

So such a code must of course need to be updated to treat "*" as a virtual thing and exclude from enumeration (I suspect "git remote" has the same property), and delivered to the end-users at the same time.

An alternative is to use remote.<key> as a fallback default for remote.<name>.<key>, which has been done successfully for other hierarchies like http.<url>.<key> would override http.<key> for any defined <key>s. So if we have remote.<name>.skipFetchAll per remote, we could use remote.skipFetchAll as its fallback default value.

We have multiple internal repositories configured with a huge number of CI tasks, each requiring code preparation (either via clone or fetch). These CI tasks, triggered by post-receive hooks, often fetch the same code (usually using --depth=1) concurrently.

To reduce performance impacts on the git server caused by these tasks, we want to deploy a special designed pack-objects-hook. This hook would allow the packs generated by git-pack-objects to be reused. Since not all clone/fetch operations will benefit from this caching (e.g., pulls from developers' environments), clients need to pass a special identifier to indicate whether caching should be enabled for a particular operation. Using server options is a suitable method for this.

However, server options can only be specified via the command line option (via --server-option or -o), which is inconvenient and requires CI script modifications. A configuration-based approach is easier to propagate (by changing the global configuration ~/.gitconfig) and avoids issues with older Git versions that don't support --server-option.

This patch series introduces a new multi-valued configuration, remote.<name>.serverOption, similar to push.pushOption, to specify default server options for the corresponding remote.

* Patch 1~3 is the main change for introducing the new configuration.
* Patch 4 fixes a issue for git-fetch not sending server-options when fetching multiple remotes.
* Patch 5 is a minor fix for a server options-related memory leak.

Xing Xin (5):
transport: introduce parse_transport_option() method
remote: introduce remote.<name>.serverOption configuration transport.c::handshake: make use of server options from remote
fetch: respect --server-option when fetching multiple remotes
ls-remote: leakfix for not clearing server_options

 Documentation/config/remote.txt |  10 +++
 Documentation/fetch-options.txt |   3 +
 Documentation/git-clone.txt     |   3 +
 Documentation/git-ls-remote.txt |   3 +
 builtin/fetch.c                 |   2 +
 builtin/ls-remote.c             |   1 +
 builtin/push.c                  |   9 +--
 remote.c                        |   7 ++
 remote.h                        |   3 +
 t/t5702-protocol-v2.sh          | 133 ++++++++++++++++++++++++++++++++
 transport.c                     |  15 ++++
 transport.h                     |   4 +
 12 files changed, 185 insertions(+), 8 deletions(-)

base-commit: 4590f2e9412378c61eac95966709c78766d326ba
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1776%2Fblanet%2Fxx%2Fadd-server-option-from-config-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1776/blanet/xx/add-server-option-from-config-v2
Pull-Request: https://github.com/git/git/pull/1776

Range-diff vs v1:

1: 5c8f3c166a5 ! 1: c95ed5e0dd5 transport: add parse_transport_option() method

     @@ Metadata
      Author: Xing Xin <[email protected]>
      
       ## Commit message ##
     -    transport: add parse_transport_option() method
     +    transport: introduce parse_transport_option() method
      
     -    Introduce the `parse_transport_option()` method used to parse
     -    `push.pushOption` configuration values. This method will be further
     -    utilized in subsequent commits to parse a newly added
     -    `fetch.serverOption` configuration for fetches, which aligns with the
     -    design pattern of `push.pushOption`.
     +    Add the `parse_transport_option()` method to parse the `push.pushOption`
     +    configuration. This method will also be used in the next commit to
     +    handle the new `remote.<name>.serverOption` configuration for setting
     +    server options in Git protocol v2.
      
          Signed-off-by: Xing Xin <[email protected]>
      
       ## builtin/push.c ##
      @@ builtin/push.c: static int git_push_config(const char *k, const char *v,
     + 			RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
     + 		recurse_submodules = val;
       	} else if (!strcmp(k, "push.pushoption")) {
     - 		if (!v)
     - 			return config_error_nonbool(k);
     +-		if (!v)
     +-			return config_error_nonbool(k);
      -		else
      -			if (!*v)
      -				string_list_clear(&push_options_config, 0);
      -			else
      -				string_list_append(&push_options_config, v);
     -+		parse_transport_option(v, &push_options_config);
     - 		return 0;
     +-		return 0;
     ++		return parse_transport_option(k, v, &push_options_config);
       	} else if (!strcmp(k, "color.push")) {
       		push_use_color = git_config_colorbool(k, v);
     + 		return 0;
      
       ## transport.c ##
      @@ transport.c: int is_transport_allowed(const char *type, int from_user)
       	BUG("invalid protocol_allow_config type");
       }
       
     -+void parse_transport_option(const char *option, struct string_list *transport_options)
     ++int parse_transport_option(const char *var, const char *value,
     ++			   struct string_list *transport_options)
      +{
     -+	if (!*option)
     ++	if (!value)
     ++		return config_error_nonbool(var);
     ++	if (!*value)
      +		string_list_clear(transport_options, 0);
      +	else
     -+		string_list_append(transport_options, option);
     ++		string_list_append(transport_options, value);
     ++	return 0;
      +}
      +
       void transport_check_allowed(const char *type)
     @@ transport.h: void transport_print_push_status(const char *dest, struct ref *refs
       /* common method used by transport-helper.c and send-pack.c */
       void reject_atomic_push(struct ref *refs, int mirror_mode);
       
     -+/* common method to parse push-option for pushes or server-option for fetches */
     -+void parse_transport_option(const char *option, struct string_list *transport_options);
     ++/* common method to parse push-option or server-option from config */
     ++int parse_transport_option(const char *var, const char *value,
     ++			   struct string_list *transport_options);
      +
       #endif
 2:  7e2d5c504d7 < -:  ----------- builtin/fetch.c: add fetch.serverOption configuration
 -:  ----------- > 2:  2474b4c69d6 remote: introduce remote.<name>.serverOption configuration
 3:  7c3ebda513d ! 3:  a7f3e458501 builtin/clone.c: recognize fetch.serverOption configuration
     @@ Metadata
      Author: Xing Xin <[email protected]>
      
       ## Commit message ##
     -    builtin/clone.c: recognize fetch.serverOption configuration
     +    transport.c::handshake: make use of server options from remote
      
     -    Teach git-clone to recognize the `fetch.serverOption` configuration as a
     -    default list of server options to send for Git protocol v2, if server
     -    options are not explicitly set via the command line.
     +    Utilize the `server_options` from the corresponding remote during the
     +    handshake in `transport.c` when Git protocol v2 is detected. This helps
     +    initialize the `server_options` in `transport.h:transport` if no server
     +    options are set for the transport (typically via `--server-option` or
     +    `-o`).
      
     -    Note that `builtin/clone.c:cmd_clone` originally read the git config
     -    twice via `builtin/clone.c:git_clone_config`, which would duplicate
     -    server options if parsing logic were added there. Upon investigation, it
     -    was found that the first config read is unnecessary since all the global
     -    variables it sets are actually used after the second config read.
     -    Therefore, the first config read is replaced with a simple
     -    `config.c:git_default_config`.
     +    While another potential place to incorporate server options from the
     +    remote is in `transport.c:transport_get`, setting server options for a
     +    transport using a protocol other than v2 could lead to unexpected errors
     +    (see `transport.c:die_if_server_options`).
      
     -    Tests and documentation have been updated accordingly.
     +    Relevant tests and documentation have been updated accordingly.
      
          Signed-off-by: Xing Xin <[email protected]>
      
     + ## Documentation/fetch-options.txt ##
     +@@ Documentation/fetch-options.txt: endif::git-pull[]
     + 	unknown ones, is server-specific.
     + 	When multiple `--server-option=<option>` are given, they are all
     + 	sent to the other side in the order listed on the command line.
     ++	When no `--server-option=<option>` is given from the command line,
     ++	the values of configuration variable `remote.<name>.serverOption`
     ++	are used instead.
     + 
     + --show-forced-updates::
     + 	By default, git checks if a branch is force-updated during
     +
       ## Documentation/git-clone.txt ##
      @@ Documentation/git-clone.txt: objects from the source repository into a pack in the cloned repository.
       	unknown ones, is server-specific.
       	When multiple ++--server-option=++__<option>__ are given, they are all
       	sent to the other side in the order listed on the command line.
      +	When no ++--server-option=++__<option>__ is given from the command
     -+	line, the values of configuration variable `fetch.serverOption`
     ++	line, the values of configuration variable `remote.<name>.serverOption`
      +	are used instead.
       
       `-n`::
       `--no-checkout`::
      
     - ## builtin/clone.c ##
     -@@ builtin/clone.c: static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;
     - static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
     - static int option_filter_submodules = -1;    /* unspecified */
     - static int config_filter_submodules = -1;    /* unspecified */
     --static struct string_list server_options = STRING_LIST_INIT_NODUP;
     -+static struct string_list config_server_options = STRING_LIST_INIT_DUP;
     -+static struct string_list option_server_options = STRING_LIST_INIT_DUP;
     - static int option_remote_submodules;
     - static const char *bundle_uri;
     - 
     -@@ builtin/clone.c: static struct option builtin_clone_options[] = {
     - 		   N_("specify the reference format to use")),
     - 	OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
     - 			N_("set config inside the new repository")),
     --	OPT_STRING_LIST(0, "server-option", &server_options,
     -+	OPT_STRING_LIST(0, "server-option", &option_server_options,
     - 			N_("server-specific"), N_("option to transmit")),
     - 	OPT_IPVERSION(&family),
     - 	OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
     -@@ builtin/clone.c: static int git_clone_config(const char *k, const char *v,
     - 		config_reject_shallow = git_config_bool(k, v);
     - 	if (!strcmp(k, "clone.filtersubmodules"))
     - 		config_filter_submodules = git_config_bool(k, v);
     -+	if (!strcmp(k, "fetch.serveroption")) {
     -+		if (!v)
     -+			return config_error_nonbool(k);
     -+		parse_transport_option(v, &config_server_options);
     -+		return 0;
     -+	}
     - 
     - 	return git_default_config(k, v, ctx, cb);
     - }
     -@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
     - 	int hash_algo;
     - 	enum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
     - 	const int do_not_override_repo_unix_permissions = -1;
     --
     -+	struct string_list *server_options = NULL;
     - 	struct transport_ls_refs_options transport_ls_refs_options =
     - 		TRANSPORT_LS_REFS_OPTIONS_INIT;
     - 
     - 	packet_trace_identity("clone");
     - 
     --	git_config(git_clone_config, NULL);
     -+	git_config(git_default_config, NULL);
     + ## Documentation/git-ls-remote.txt ##
     +@@ Documentation/git-ls-remote.txt: OPTIONS
     + 	character.
     + 	When multiple `--server-option=<option>` are given, they are all
     + 	sent to the other side in the order listed on the command line.
     ++	When no `--server-option=<option>` is given from the command line,
     ++	the values of configuration variable `remote.<name>.serverOption`
     ++	are used instead.
       
     - 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
     - 			     builtin_clone_usage, 0);
     + <repository>::
     + 	The "remote" repository to query.  This parameter can be
     +
     + ## t/t5702-protocol-v2.sh ##
     +@@ t/t5702-protocol-v2.sh: test_expect_success 'server-options are sent when using ls-remote' '
     + 	grep "server-option=world" log
     + '
       
     -+	server_options = option_server_options.nr ?
     -+			 &option_server_options : &config_server_options;
     -+
     - 	if (argc > 2)
     - 		usage_msg_opt(_("Too many arguments."),
     - 			builtin_clone_usage, builtin_clone_options);
     -@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
     - 		transport_set_option(transport, TRANS_OPT_UPLOADPACK,
     - 				     option_upload_pack);
     ++test_expect_success 'server-options from configuration are used by ls-remote' '
     ++	test_when_finished "rm -rf log myclone" &&
     ++	git clone "file://$(pwd)/file_parent" myclone &&
     ++	cat >expect <<-EOF &&
     ++	$(git -C file_parent rev-parse refs/heads/main)$(printf "\t")refs/heads/main
     ++	EOF
     ++
     ++	# Default server options from configuration are used
     ++	git -C myclone config --add remote.origin.serverOption foo &&
     ++	git -C myclone config --add remote.origin.serverOption bar &&
     ++	GIT_TRACE_PACKET="$(pwd)/log" git -C myclone -c protocol.version=2 \
     ++		ls-remote origin main >actual &&
     ++	test_cmp expect actual &&
     ++	test_grep "ls-remote> server-option=foo" log &&
     ++	test_grep "ls-remote> server-option=bar" log &&
     ++	rm -f log &&
     ++
     ++	# Empty value of remote.<name>.serverOption clears the list
     ++	git -C myclone config --add remote.origin.serverOption "" &&
     ++	git -C myclone config --add remote.origin.serverOption tar &&
     ++	GIT_TRACE_PACKET="$(pwd)/log" git -C myclone -c protocol.version=2 \
     ++		ls-remote origin main >actual &&
     ++	test_cmp expect actual &&
     ++	test_grep "ls-remote> server-option=tar" log &&
     ++	test_grep ! "ls-remote> server-option=foo" log &&
     ++	test_grep ! "ls-remote> server-option=bar" log &&
     ++	rm -f log &&
     ++
     ++	# Server option from command line overrides those from configuration
     ++	GIT_TRACE_PACKET="$(pwd)/log" git -C myclone -c protocol.version=2 \
     ++		ls-remote -o hello -o world origin main >actual &&
     ++	test_cmp expect actual &&
     ++	test_grep "ls-remote> server-option=hello" log &&
     ++	test_grep "ls-remote> server-option=world" log &&
     ++	test_grep ! "ls-remote> server-option=tar" log
     ++'
     ++
     + test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '
     + 	test_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \
     + 		ls-remote -o hello -o world "file://$(pwd)/file_parent" main 2>err &&
     +@@ t/t5702-protocol-v2.sh: test_expect_success 'server-options are sent when fetching' '
     + 	grep "server-option=world" log
     + '
       
     --	if (server_options.nr)
     --		transport->server_options = &server_options;
     -+	if (server_options && server_options->nr)
     -+		transport->server_options = server_options;
     ++test_expect_success 'server-options from configuration are used by git-fetch' '
     ++	test_when_finished "rm -rf log myclone" &&
     ++	git clone "file://$(pwd)/file_parent" myclone &&
     ++	git -C file_parent log -1 --format=%s >expect &&
     ++
     ++	# Default server options from configuration are used
     ++	git -C myclone config --add remote.origin.serverOption foo &&
     ++	git -C myclone config --add remote.origin.serverOption bar &&
     ++	GIT_TRACE_PACKET="$(pwd)/log" git -C myclone -c protocol.version=2 \
     ++		fetch origin main &&
     ++	git -C myclone log -1 --format=%s origin/main >actual &&
     ++	test_cmp expect actual &&
     ++	test_grep "fetch> server-option=foo" log &&
     ++	test_grep "fetch> server-option=bar" log &&
     ++	rm -f log &&
     ++
     ++	# Empty value of remote.<name>.serverOption clears the list
     ++	git -C myclone config --add remote.origin.serverOption "" &&
     ++	git -C myclone config --add remote.origin.serverOption tar &&
     ++	GIT_TRACE_PACKET="$(pwd)/log" git -C myclone -c protocol.version=2 \
     ++		fetch origin main &&
     ++	git -C myclone log -1 --format=%s origin/main >actual &&
     ++	test_cmp expect actual &&
     ++	test_grep "fetch> server-option=tar" log &&
     ++	test_grep ! "fetch> server-option=foo" log &&
     ++	test_grep ! "fetch> server-option=bar" log &&
     ++	rm -f log &&
     ++
     ++	# Server option from command line overrides those from configuration
     ++	GIT_TRACE_PACKET="$(pwd)/log" git -C myclone -c protocol.version=2 \
     ++		fetch -o hello -o world origin main &&
     ++	git -C myclone log -1 --format=%s origin/main >actual &&
     ++	test_cmp expect actual &&
     ++	test_grep "fetch> server-option=hello" log &&
     ++	test_grep "fetch> server-option=world" log &&
     ++	test_grep ! "fetch> server-option=tar" log
     ++'
     ++
     + test_expect_success 'warn if using server-option with fetch with legacy protocol' '
     + 	test_when_finished "rm -rf temp_child" &&
       
     - 	if (filter_options.choice) {
     - 		const char *spec =
     -
     - ## t/t5702-protocol-v2.sh ##
     -@@ t/t5702-protocol-v2.sh: test_expect_success 'warn if using server-option with fetch with legacy protocol
     - test_expect_success 'server-options are sent when cloning' '
     - 	test_when_finished "rm -rf log myclone" &&
     +@@ t/t5702-protocol-v2.sh: test_expect_success 'server-options are sent when cloning' '
     + 	grep "server-option=world" log
     + '
       
     -+	# Specify server options from command line
     - 	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
     - 		clone --server-option=hello --server-option=world \
     - 		"file://$(pwd)/file_parent" myclone &&
     -+	test_grep "server-option=hello" log &&
     -+	test_grep "server-option=world" log &&
     ++test_expect_success 'server-options from configuration are used by git-clone' '
     ++	test_when_finished "rm -rf log myclone" &&
     ++
     ++	# Default server options from configuration are used
     ++	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
     ++		-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \
     ++		clone "file://$(pwd)/file_parent" myclone &&
     ++	test_grep "clone> server-option=foo" log &&
     ++	test_grep "clone> server-option=bar" log &&
      +	rm -rf log myclone &&
     - 
     --	grep "server-option=hello" log &&
     --	grep "server-option=world" log
     -+	# Specify server options from fetch.serverOption config
     ++
     ++	# Empty value of remote.<name>.serverOption clears the list
      +	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
     -+		-c fetch.serverOption=hello -c fetch.serverOption=world \
     ++		-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \
     ++		-c remote.origin.serverOption= -c remote.origin.serverOption=tar \
      +		clone "file://$(pwd)/file_parent" myclone &&
     -+	test_grep "server-option=hello" log &&
     -+	test_grep "server-option=world" log &&
     ++	test_grep "clone> server-option=tar" log &&
     ++	test_grep ! "clone> server-option=foo" log &&
     ++	test_grep ! "clone> server-option=bar" log &&
      +	rm -rf log myclone &&
      +
     -+	# Cmdline server options take a higher priority
     ++	# Server option from command line overrides those from configuration
      +	GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
     -+		-c fetch.serverOption=hello -c fetch.serverOption=world \
     -+		clone --server-option=foo=bar \
     ++		-c remote.origin.serverOption=tar \
     ++		clone --server-option=hello --server-option=world \
      +		"file://$(pwd)/file_parent" myclone &&
     -+	test_grep ! "server-option=hello" log &&
     -+	test_grep ! "server-option=world" log &&
     -+	test_grep "server-option=foo=bar" log
     ++	test_grep "clone> server-option=hello" log &&
     ++	test_grep "clone> server-option=world" log &&
     ++	test_grep ! "clone> server-option=tar" log
     ++'
     ++
     + test_expect_success 'warn if using server-option with clone with legacy protocol' '
     + 	test_when_finished "rm -rf myclone" &&
     + 
     +@@ t/t5702-protocol-v2.sh: test_expect_success 'warn if using server-option with clone with legacy protocol
     + 	test_grep "server options require protocol version 2 or later" err
       '
       
     - test_expect_success 'warn if using server-option with clone with legacy protocol' '
     ++test_expect_success 'server-option configuration with legacy protocol is ok' '
     ++	test_when_finished "rm -rf myclone" &&
     ++
     ++	env GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \
     ++		-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \
     ++		clone "file://$(pwd)/file_parent" myclone
     ++'
     ++
     ++test_expect_success 'invalid server-option configuration' '
     ++	test_when_finished "rm -rf myclone" &&
     ++
     ++	test_must_fail git -c protocol.version=2 \
     ++		-c remote.origin.serverOption \
     ++		clone "file://$(pwd)/file_parent" myclone 2>err &&
     ++	test_grep "error: missing value for '\''remote.origin.serveroption'\''" err
     ++'
     ++
     + test_expect_success 'upload-pack respects config using protocol v2' '
     + 	git init server &&
     + 	write_script server/.git/hook <<-\EOF &&
     +
     + ## transport.c ##
     +@@ transport.c: static struct ref *handshake(struct transport *transport, int for_push,
     + 	data->version = discover_version(&reader);
     + 	switch (data->version) {
     + 	case protocol_v2:
     ++		if ((!transport->server_options || !transport->server_options->nr) &&
     ++		    transport->remote->server_options.nr)
     ++			transport->server_options = &transport->remote->server_options;
     + 		if (server_feature_v2("session-id", &server_sid))
     + 			trace2_data_string("transfer", NULL, "server-sid", server_sid);
     + 		if (must_list_refs)
 4:  8962031f999 < -:  ----------- builtin/ls-remote.c: recognize fetch.serverOption configuration
 -:  ----------- > 4:  39ee8dbef78 fetch: respect --server-option when fetching multiple remotes
 -:  ----------- > 5:  39c07a6c8ee ls-remote: leakfix for not clearing server_options

gitgitgadget

From: Xing Xin <[email protected]>

Add the parse_transport_option() method to parse the push.pushOption configuration. This method will also be used in the next commit to handle the new remote.<name>.serverOption configuration for setting server options in Git protocol v2.

---
 builtin/push.c |  9 +--------
 transport.c    | 12 ++++++++++++
 transport.h    |  4 ++++
 3 files changed, 17 insertions(+), 8 deletions(-)

index 7a67398124f..290a08f6ef2 100644
@@ -517,14 +517,7 @@ static int git_push_config(const char *k, const char *v,
 			RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
 		recurse_submodules = val;
 	} else if (!strcmp(k, "push.pushoption")) {
-		if (!v)
-			return config_error_nonbool(k);
-		else
-			if (!*v)
-				string_list_clear(&push_options_config, 0);
-			else
-				string_list_append(&push_options_config, v);
-		return 0;
+		return parse_transport_option(k, v, &push_options_config);
 	} else if (!strcmp(k, "color.push")) {
 		push_use_color = git_config_colorbool(k, v);
 		return 0;

index bab28965f96..c8947415eec 100644
@@ -1091,6 +1091,18 @@ int is_transport_allowed(const char *type, int from_user)
 	BUG("invalid protocol_allow_config type");
 }
 
+int parse_transport_option(const char *var, const char *value,
+			   struct string_list *transport_options)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	if (!*value)
+		string_list_clear(transport_options, 0);
+	else
+		string_list_append(transport_options, value);
+	return 0;
+}
+
 void transport_check_allowed(const char *type)
 {
 	if (!is_transport_allowed(type, -1))

index 6393cd9823c..44100fa9b7f 100644
@@ -342,4 +342,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 /* common method used by transport-helper.c and send-pack.c */
 void reject_atomic_push(struct ref *refs, int mirror_mode);
 
+/* common method to parse push-option or server-option from config */
+int parse_transport_option(const char *var, const char *value,
+			   struct string_list *transport_options);
+
 #endif

gitgitgadget

From: Xing Xin <[email protected]>

Currently, server options for Git protocol v2 can only be specified via the command line option "--server-option" or "-o", which is inconvenient when users want to specify a list of default options to send. Therefore, we are introducing a new configuration to hold a list of default server options, akin to the push.pushOption configuration for push options.

Initially, I named the new configuration fetch.serverOption to align with push.pushOption. However, after discussing with Patrick, it was renamed to remote.<name>.serverOption as suggested, because:

1. Server options are designed to be server-specific, making it more logical to use a per-remote configuration.
2. Using "fetch." prefixed configurations in git-clone or git-ls-remote seems out of place and inconsistent in design.

The parsing logic for remote.<name>.serverOption also relies on transport.c:parse_transport_option, similar to push.pushOption, and they follow the same priority design:

1. Server options set in lower-priority configuration files (e.g., /etc/gitconfig or $HOME/.gitconfig) can be overridden or unset in more specific repository configurations using an empty string.
2. Command-line specified server options take precedence over those from the configuration.

Server options from configuration are stored to the corresponding remote.h:remote as a new field server_options. The field will be utilized in the subsequent commit to help initialize the server_options of transport.h:transport.

And documentation have been updated accordingly.

---
 Documentation/config/remote.txt | 10 ++++++++++
 remote.c                        |  7 +++++++
 remote.h                        |  3 +++
 3 files changed, 20 insertions(+)

index 8efc53e836d..8ba48573fb6 100644
@@ -95,3 +95,13 @@ remote.<name>.partialclonefilter::
 	Changing or clearing this value will only affect fetches for new commits.
 	To fetch associated objects for commits already present in the local object
 	database, use the `--refetch` option of linkgit:git-fetch[1].
+
+remote.<name>.serverOption::
+	When no `--server-option=<option>` argument is given from the command
+	line, git will use the values from this configuration as a default list of
+	server options for this remote.
++
+This is a multi-valued variable, and an empty value can be used in a higher
+priority configuration file (e.g. `.git/config` in a repository) to clear
+the values inherited from a lower priority configuration files (e.g.
+`$HOME/.gitconfig`).

index 7d5b8f750d8..fe11ead27ad 100644
@@ -24,6 +24,7 @@
 #include "advice.h"
 #include "connect.h"
 #include "parse-options.h"
+#include "transport.h"
 
 enum map_direction { FROM_SRC, FROM_DST };
 

@@ -125,6 +126,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
 	struct remote *ret;
 	struct remotes_hash_key lookup;
 	struct hashmap_entry lookup_entry, *e;
+	struct string_list server_options = STRING_LIST_INIT_DUP;
 
 	if (!len)
 		len = strlen(name);

@@ -143,6 +145,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
 	ret->name = xstrndup(name, len);
 	refspec_init(&ret->push, REFSPEC_PUSH);
 	refspec_init(&ret->fetch, REFSPEC_FETCH);
+	ret->server_options = server_options;
 
 	ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
 		   remote_state->remotes_alloc);

@@ -166,6 +169,7 @@ static void remote_clear(struct remote *remote)
 	free((char *)remote->uploadpack);
 	FREE_AND_NULL(remote->http_proxy);
 	FREE_AND_NULL(remote->http_proxy_authmethod);
+	string_list_clear(&remote->server_options, 0);
 }
 
 static void add_merge(struct branch *branch, const char *name)

@@ -482,6 +486,9 @@ static int handle_config(const char *key, const char *value,
 					 key, value);
 	} else if (!strcmp(subkey, "vcs")) {
 		return git_config_string(&remote->foreign_vcs, key, value);
+	} else if (!strcmp(subkey, "serveroption")) {
+		return parse_transport_option(key, value,
+					      &remote->server_options);
 	}
 	return 0;
 }

index b901b56746d..d9926a76d23 100644
@@ -4,6 +4,7 @@
 #include "hash.h"
 #include "hashmap.h"
 #include "refspec.h"
+#include "string-list.h"
 #include "strvec.h"
 
 struct option;

@@ -104,6 +105,8 @@ struct remote {
 
 	/* The method used for authenticating against `http_proxy`. */
 	char *http_proxy_authmethod;
+
+	struct string_list server_options;
 };
 
 /**

gitgitgadget

From: Xing Xin <[email protected]>

Utilize the server_options from the corresponding remote during the handshake in transport.c when Git protocol v2 is detected. This helps initialize the server_options in transport.h:transport if no server options are set for the transport (typically via --server-option or -o).

While another potential place to incorporate server options from the remote is in transport.c:transport_get, setting server options for a transport using a protocol other than v2 could lead to unexpected errors (see transport.c:die_if_server_options).

Relevant tests and documentation have been updated accordingly.

---
 Documentation/fetch-options.txt |   3 +
 Documentation/git-clone.txt     |   3 +
 Documentation/git-ls-remote.txt |   3 +
 t/t5702-protocol-v2.sh          | 123 ++++++++++++++++++++++++++++++++
 transport.c                     |   3 +
 5 files changed, 135 insertions(+)

index e22b217fba9..424cdeafcdd 100644
@@ -304,6 +304,9 @@ endif::git-pull[]
 	unknown ones, is server-specific.
 	When multiple `--server-option=<option>` are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no `--server-option=<option>` is given from the command line,
+	the values of configuration variable `remote.<name>.serverOption`
+	are used instead.
 
 --show-forced-updates::
 	By default, git checks if a branch is force-updated during

index 8e925db7e9c..409fb3a3af6 100644
@@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.
 	unknown ones, is server-specific.
 	When multiple ++--server-option=++__<option>__ are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no ++--server-option=++__<option>__ is given from the command
+	line, the values of configuration variable `remote.<name>.serverOption`
+	are used instead.
 
 `-n`::
 `--no-checkout`::

index 76c86c3ce4f..d71c4ab3e29 100644
@@ -81,6 +81,9 @@ OPTIONS
 	character.
 	When multiple `--server-option=<option>` are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no `--server-option=<option>` is given from the command line,
+	the values of configuration variable `remote.<name>.serverOption`
+	are used instead.
 
 <repository>::
 	The "remote" repository to query.  This parameter can be

index c8947415eec..bcfd4199c87 100644
@@ -332,6 +332,9 @@ static struct ref *handshake(struct transport *transport, int for_push,
 	data->version = discover_version(&reader);
 	switch (data->version) {
 	case protocol_v2:
+		if ((!transport->server_options || !transport->server_options->nr) &&
+		    transport->remote->server_options.nr)
+			transport->server_options = &transport->remote->server_options;
 		if (server_feature_v2("session-id", &server_sid))
 			trace2_data_string("transfer", NULL, "server-sid", server_sid);
 		if (must_list_refs)

gitgitgadget

From: Xing Xin <[email protected]>

Fix an issue where server options specified via the command line (--server-option or -o) were not sent when fetching from multiple remotes using Git protocol v2.

To reproduce the issue with a repository containing multiple remotes:

  GIT_TRACE_PACKET=1 git -c protocol.version=2 fetch --server-option=demo --all

Observe that no server options are sent to any remote.

The root cause was identified in builtin/fetch.c:fetch_multiple, which is invoked when fetching from more than one remote. This function forks a git-fetch subprocess for each remote but did not include the specified server options in the subprocess arguments.

This commit ensures that command-line specified server options are properly passed to each subprocess. Relevant tests have been added.

---
 builtin/fetch.c        |  2 ++
 t/t5702-protocol-v2.sh | 10 ++++++++++
 2 files changed, 12 insertions(+)

index c297569a473..c1b3aea7745 100644
@@ -1980,6 +1980,8 @@ static int fetch_multiple(struct string_list *list, int max_children,
 	strvec_pushl(&argv, "-c", "fetch.bundleURI=",
 		     "fetch", "--append", "--no-auto-gc",
 		     "--no-write-commit-graph", NULL);
+	for (i = 0; i < server_options.nr; i++)
+		strvec_pushf(&argv, "--server-option=%s", server_options.items[i].string);
 	add_options_to_argv(&argv, config);
 
 	if (max_children != 1 && list->nr != 1) {

index 5cec2061d28..d3df81e7852 100755
@@ -418,6 +418,16 @@ test_expect_success 'server-options are sent when fetching' '
 	grep "server-option=world" log
 '
 
+test_expect_success 'server-options are sent when fetch multiple remotes' '
+	test_when_finished "rm -f log server_options_sent" &&
+	git clone "file://$(pwd)/file_parent" child_multi_remotes &&
+	git -C child_multi_remotes remote add another "file://$(pwd)/file_parent" &&
+	GIT_TRACE_PACKET="$(pwd)/log" git -C child_multi_remotes -c protocol.version=2 \
+		fetch -o hello --all &&
+	grep "fetch> server-option=hello" log >server_options_sent &&
+	test_line_count = 2 server_options_sent
+'
+
 test_expect_success 'server-options from configuration are used by git-fetch' '
 	test_when_finished "rm -rf log myclone" &&
 	git clone "file://$(pwd)/file_parent" myclone &&

gitgitgadget

From: Xing Xin <[email protected]>

Ensure server_options is properly cleared using string_list_clear() in builtin/ls-remote.c:cmd_ls_remote.

Although we cannot yet enable TEST_PASSES_SANITIZE_LEAK=true for t/t5702-protocol-v2.sh due to other existing leaks, this fix ensures that "git-ls-remote" related server options tests pass the sanitize leak check:

...
ok 12 - server-options are sent when using ls-remote ok 13 - server-options from configuration are used by ls-remote
...

---
 builtin/ls-remote.c | 1 +
 1 file changed, 1 insertion(+)

index 0a491595ca8..c3fdda08409 100644
@@ -169,5 +169,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	transport_ls_refs_options_release(&transport_options);
 
 	strvec_clear(&pattern);
+	string_list_clear(&server_options, 0);
 	return status;
 }

gitgitgadget

After some consideration, I tend not to implement a remote.<subkey> or remote.*.<subkey> as a fallback default in this patch series.

In fact I attempted to use remote.serverOption and extended a new server_options field in remote.h:remote_state to store its parsing result. However, I think this approach isn't general enough (it only differs in name from the remote.defaultServerOption).

Implementing a mechanism similar to http.<url>.<subkey> and http.<subkey>(the fallback default) seems a better approach, but it involves more refactoring, which is too much to do at this time.

I have just submitted my v2 series, which only adds the remote.<name>.serverOption configuration.

Xing Xin

Nice catch.

Patrick

Sorry for taking this long to get back to this patch series. I've given it another careful read and have a single proposal for improved docs that you may want to address. But other than those nits this version looks great to me, thanks!

Patrick

We manage some internal repositories with numerous CI tasks, each requiring code preparation through git-clone or git-fetch. These tasks, triggered by post-receive hooks, often fetch the same copy of code concurrently using --depth=1, causing extremely high load spikes on our Git servers.

To reduce performance impacts caused by these tasks, we plan to deploy a specially designed pack-objects-hook [1]. This hook would allow the packs generated by git-pack-objects(during git-clone or git-fetch) to be reused. Since not all clone/fetch operations will benefit from this caching (e.g., pulls from developer environments), clients need to pass a special identifier to indicate whether caching should be enabled. Using server options [2] is suitable for this purpose.

However, server options can only be specified via the command line option (via --server-option or -o), which is inconvenient and requires modifications to CI scripts. A configuration-based approach is preferable, as it can be propagated through global configuration (e.g. ~/.gitconfig) and avoids compatibility issues with older Git versions that don't support --server-option.

This patch series introduces a new multi-valued configuration, remote.<name>.serverOption, similar to push.pushOption, to specify default server options for the corresponding remote.

* Patches 1~3 contain the main changes for introducing the new configuration.
* Patch 4 fixes a issue for git-fetch not sending server-options when fetching from multiple remotes.
* Patch 5 is a minor fix for a server options-related memory leak.

1. https://git-scm.com/docs/git-config#Documentation/git-config.txt-uploadpackpackObjectsHook
2. https://git-scm.com/docs/gitprotocol-v2#_server_option

Xing Xin (5):
transport: introduce parse_transport_option() method
remote: introduce remote.<name>.serverOption configuration transport.c::handshake: make use of server options from remote
fetch: respect --server-option when fetching multiple remotes
ls-remote: leakfix for not clearing server_options

 Documentation/config/remote.txt |  10 +++
 Documentation/fetch-options.txt |   3 +
 Documentation/git-clone.txt     |   3 +
 Documentation/git-ls-remote.txt |   3 +
 builtin/fetch.c                 |   2 +
 builtin/ls-remote.c             |   1 +
 builtin/push.c                  |   9 +--
 remote.c                        |   6 ++
 remote.h                        |   3 +
 t/t5702-protocol-v2.sh          | 133 ++++++++++++++++++++++++++++++++
 transport.c                     |  15 ++++
 transport.h                     |   4 +
 12 files changed, 184 insertions(+), 8 deletions(-)

base-commit: 6258f68c3c1092c901337895c864073dcdea9213
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1776%2Fblanet%2Fxx%2Fadd-server-option-from-config-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1776/blanet/xx/add-server-option-from-config-v3
Pull-Request: https://github.com/git/git/pull/1776

Range-diff vs v2:

 1:  c95ed5e0dd5 = 1:  b44face42e1 transport: introduce parse_transport_option() method
 2:  2474b4c69d6 ! 2:  3c6b129d368 remote: introduce remote.<name>.serverOption configuration

@@ Documentation/config/remote.txt: remote.<name>.partialclonefilter::
database, use the --refetch option of linkgit:git-fetch[1].
+
+remote.<name>.serverOption::
-+ When no --server-option=<option> argument is given from the command
-+ line, git will use the values from this configuration as a default list of
-+ server options for this remote.
++ The default set of server options used when fetching from this remote.
++ These server options can be overridden by the --server-option= command
++ line arguments.
++
+This is a multi-valued variable, and an empty value can be used in a higher +priority configuration file (e.g. .git/config in a repository) to clear
@@ remote.c

enum map_direction { FROM_SRC, FROM_DST };

-@@ remote.c: static struct remote *make_remote(struct remote_state *remote_state,
- struct remote *ret;
- struct remotes_hash_key lookup;
- struct hashmap_entry lookup_entry, *e;
-+ struct string_list server_options = STRING_LIST_INIT_DUP;
-
- if (!len)
- len = strlen(name);
@@ remote.c: static struct remote *make_remote(struct remote_state *remote_state,
ret->name = xstrndup(name, len);
refspec_init(&ret->push, REFSPEC_PUSH);
refspec_init(&ret->fetch, REFSPEC_FETCH);
-+ ret->server_options = server_options;
++ string_list_init_dup(&ret->server_options);

ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
remote_state->remotes_alloc);
@@ remote.c: static void remote_clear(struct remote *remote)

static void add_merge(struct branch *branch, const char *name)
@@ remote.c: static int handle_config(const char *key, const char *value,
- key, value);
} else if (!strcmp(subkey, "vcs")) {
+ FREE_AND_NULL(remote->foreign_vcs);
return git_config_string(&remote->foreign_vcs, key, value);
+ } else if (!strcmp(subkey, "serveroption")) {
+ return parse_transport_option(key, value,

 3:  a7f3e458501 = 3:  f0835259b06 transport.c::handshake: make use of server options from remote
 4:  39ee8dbef78 = 4:  420b15d9f37 fetch: respect --server-option when fetching multiple remotes
 5:  39c07a6c8ee ! 5:  2528d929c7e ls-remote: leakfix for not clearing server_options

@@ Commit message

## builtin/ls-remote.c ##
-@@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *prefix) +@@ builtin/ls-remote.c: int cmd_ls_remote(int argc,
transport_ls_refs_options_release(&transport_options);

strvec_clear(&pattern);

gitgitgadget

From: Xing Xin <[email protected]>

Add the parse_transport_option() method to parse the push.pushOption configuration. This method will also be used in the next commit to handle the new remote.<name>.serverOption configuration for setting server options in Git protocol v2.

---
 builtin/push.c |  9 +--------
 transport.c    | 12 ++++++++++++
 transport.h    |  4 ++++
 3 files changed, 17 insertions(+), 8 deletions(-)

index e6f48969b81..e0ff60d48e5 100644
@@ -519,14 +519,7 @@ static int git_push_config(const char *k, const char *v,
 			RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
 		recurse_submodules = val;
 	} else if (!strcmp(k, "push.pushoption")) {
-		if (!v)
-			return config_error_nonbool(k);
-		else
-			if (!*v)
-				string_list_clear(&push_options_config, 0);
-			else
-				string_list_append(&push_options_config, v);
-		return 0;
+		return parse_transport_option(k, v, &push_options_config);
 	} else if (!strcmp(k, "color.push")) {
 		push_use_color = git_config_colorbool(k, v);
 		return 0;

index 3c4714581f5..4d9e605b273 100644
@@ -1099,6 +1099,18 @@ int is_transport_allowed(const char *type, int from_user)
 	BUG("invalid protocol_allow_config type");
 }
 
+int parse_transport_option(const char *var, const char *value,
+			   struct string_list *transport_options)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	if (!*value)
+		string_list_clear(transport_options, 0);
+	else
+		string_list_append(transport_options, value);
+	return 0;
+}
+
 void transport_check_allowed(const char *type)
 {
 	if (!is_transport_allowed(type, -1))

index 6393cd9823c..44100fa9b7f 100644
@@ -342,4 +342,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 /* common method used by transport-helper.c and send-pack.c */
 void reject_atomic_push(struct ref *refs, int mirror_mode);
 
+/* common method to parse push-option or server-option from config */
+int parse_transport_option(const char *var, const char *value,
+			   struct string_list *transport_options);
+
 #endif

gitgitgadget

From: Xing Xin <[email protected]>

Currently, server options for Git protocol v2 can only be specified via the command line option "--server-option" or "-o", which is inconvenient when users want to specify a list of default options to send. Therefore, we are introducing a new configuration to hold a list of default server options, akin to the push.pushOption configuration for push options.

Initially, I named the new configuration fetch.serverOption to align with push.pushOption. However, after discussing with Patrick, it was renamed to remote.<name>.serverOption as suggested, because:

1. Server options are designed to be server-specific, making it more logical to use a per-remote configuration.
2. Using "fetch." prefixed configurations in git-clone or git-ls-remote seems out of place and inconsistent in design.

The parsing logic for remote.<name>.serverOption also relies on transport.c:parse_transport_option, similar to push.pushOption, and they follow the same priority design:

1. Server options set in lower-priority configuration files (e.g., /etc/gitconfig or $HOME/.gitconfig) can be overridden or unset in more specific repository configurations using an empty string.
2. Command-line specified server options take precedence over those from the configuration.

Server options from configuration are stored to the corresponding remote.h:remote as a new field server_options. The field will be utilized in the subsequent commit to help initialize the server_options of transport.h:transport.

And documentation have been updated accordingly.

---
 Documentation/config/remote.txt | 10 ++++++++++
 remote.c                        |  6 ++++++
 remote.h                        |  3 +++
 3 files changed, 19 insertions(+)

index 36e771556c6..b194dff07b9 100644
@@ -96,3 +96,13 @@ remote.<name>.partialclonefilter::
 	Changing or clearing this value will only affect fetches for new commits.
 	To fetch associated objects for commits already present in the local object
 	database, use the `--refetch` option of linkgit:git-fetch[1].
+
+remote.<name>.serverOption::
+	The default set of server options used when fetching from this remote.
+	These server options can be overridden by the `--server-option=` command
+	line arguments.
++
+This is a multi-valued variable, and an empty value can be used in a higher
+priority configuration file (e.g. `.git/config` in a repository) to clear
+the values inherited from a lower priority configuration files (e.g.
+`$HOME/.gitconfig`).

index 390a03c2643..fa65549aa44 100644
@@ -24,6 +24,7 @@
 #include "advice.h"
 #include "connect.h"
 #include "parse-options.h"
+#include "transport.h"
 
 enum map_direction { FROM_SRC, FROM_DST };
 

@@ -143,6 +144,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
 	ret->name = xstrndup(name, len);
 	refspec_init(&ret->push, REFSPEC_PUSH);
 	refspec_init(&ret->fetch, REFSPEC_FETCH);
+	string_list_init_dup(&ret->server_options);
 
 	ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
 		   remote_state->remotes_alloc);

@@ -166,6 +168,7 @@ static void remote_clear(struct remote *remote)
 	free((char *)remote->uploadpack);
 	FREE_AND_NULL(remote->http_proxy);
 	FREE_AND_NULL(remote->http_proxy_authmethod);
+	string_list_clear(&remote->server_options, 0);
 }
 
 static void add_merge(struct branch *branch, const char *name)

@@ -508,6 +511,9 @@ static int handle_config(const char *key, const char *value,
 	} else if (!strcmp(subkey, "vcs")) {
 		FREE_AND_NULL(remote->foreign_vcs);
 		return git_config_string(&remote->foreign_vcs, key, value);
+	} else if (!strcmp(subkey, "serveroption")) {
+		return parse_transport_option(key, value,
+					      &remote->server_options);
 	}
 	return 0;
 }

index a58713f20ad..e95d30946c0 100644
@@ -4,6 +4,7 @@
 #include "hash.h"
 #include "hashmap.h"
 #include "refspec.h"
+#include "string-list.h"
 #include "strvec.h"
 
 struct option;

@@ -104,6 +105,8 @@ struct remote {
 
 	/* The method used for authenticating against `http_proxy`. */
 	char *http_proxy_authmethod;
+
+	struct string_list server_options;
 };
 
 /**

gitgitgadget

From: Xing Xin <[email protected]>

Utilize the server_options from the corresponding remote during the handshake in transport.c when Git protocol v2 is detected. This helps initialize the server_options in transport.h:transport if no server options are set for the transport (typically via --server-option or -o).

While another potential place to incorporate server options from the remote is in transport.c:transport_get, setting server options for a transport using a protocol other than v2 could lead to unexpected errors (see transport.c:die_if_server_options).

Relevant tests and documentation have been updated accordingly.

---
 Documentation/fetch-options.txt |   3 +
 Documentation/git-clone.txt     |   3 +
 Documentation/git-ls-remote.txt |   3 +
 t/t5702-protocol-v2.sh          | 123 ++++++++++++++++++++++++++++++++
 transport.c                     |   3 +
 5 files changed, 135 insertions(+)

index 80838fe37ef..9dc7ac8dbdc 100644
@@ -305,6 +305,9 @@ endif::git-pull[]
 	unknown ones, is server-specific.
 	When multiple `--server-option=<option>` are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no `--server-option=<option>` is given from the command line,
+	the values of configuration variable `remote.<name>.serverOption`
+	are used instead.
 
 --show-forced-updates::
 	By default, git checks if a branch is force-updated during

index 8e925db7e9c..409fb3a3af6 100644
@@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.
 	unknown ones, is server-specific.
 	When multiple ++--server-option=++__<option>__ are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no ++--server-option=++__<option>__ is given from the command
+	line, the values of configuration variable `remote.<name>.serverOption`
+	are used instead.
 
 `-n`::
 `--no-checkout`::

index 76c86c3ce4f..d71c4ab3e29 100644
@@ -81,6 +81,9 @@ OPTIONS
 	character.
 	When multiple `--server-option=<option>` are given, they are all
 	sent to the other side in the order listed on the command line.
+	When no `--server-option=<option>` is given from the command line,
+	the values of configuration variable `remote.<name>.serverOption`
+	are used instead.
 
 <repository>::
 	The "remote" repository to query.  This parameter can be

index 4d9e605b273..b416fc84567 100644
@@ -334,6 +334,9 @@ static struct ref *handshake(struct transport *transport, int for_push,
 	data->version = discover_version(&reader);
 	switch (data->version) {
 	case protocol_v2:
+		if ((!transport->server_options || !transport->server_options->nr) &&
+		    transport->remote->server_options.nr)
+			transport->server_options = &transport->remote->server_options;
 		if (server_feature_v2("session-id", &server_sid))
 			trace2_data_string("transfer", NULL, "server-sid", server_sid);
 		if (must_list_refs)

gitgitgadget

From: Xing Xin <[email protected]>

Fix an issue where server options specified via the command line (--server-option or -o) were not sent when fetching from multiple remotes using Git protocol v2.

To reproduce the issue with a repository containing multiple remotes:

  GIT_TRACE_PACKET=1 git -c protocol.version=2 fetch --server-option=demo --all

Observe that no server options are sent to any remote.

The root cause was identified in builtin/fetch.c:fetch_multiple, which is invoked when fetching from more than one remote. This function forks a git-fetch subprocess for each remote but did not include the specified server options in the subprocess arguments.

This commit ensures that command-line specified server options are properly passed to each subprocess. Relevant tests have been added.

---
 builtin/fetch.c        |  2 ++
 t/t5702-protocol-v2.sh | 10 ++++++++++
 2 files changed, 12 insertions(+)

index c900f577219..54be2e7ca9f 100644
@@ -1980,6 +1980,8 @@ static int fetch_multiple(struct string_list *list, int max_children,
 	strvec_pushl(&argv, "-c", "fetch.bundleURI=",
 		     "fetch", "--append", "--no-auto-gc",
 		     "--no-write-commit-graph", NULL);
+	for (i = 0; i < server_options.nr; i++)
+		strvec_pushf(&argv, "--server-option=%s", server_options.items[i].string);
 	add_options_to_argv(&argv, config);
 
 	if (max_children != 1 && list->nr != 1) {

index 5cec2061d28..d3df81e7852 100755
@@ -418,6 +418,16 @@ test_expect_success 'server-options are sent when fetching' '
 	grep "server-option=world" log
 '
 
+test_expect_success 'server-options are sent when fetch multiple remotes' '
+	test_when_finished "rm -f log server_options_sent" &&
+	git clone "file://$(pwd)/file_parent" child_multi_remotes &&
+	git -C child_multi_remotes remote add another "file://$(pwd)/file_parent" &&
+	GIT_TRACE_PACKET="$(pwd)/log" git -C child_multi_remotes -c protocol.version=2 \
+		fetch -o hello --all &&
+	grep "fetch> server-option=hello" log >server_options_sent &&
+	test_line_count = 2 server_options_sent
+'
+
 test_expect_success 'server-options from configuration are used by git-fetch' '
 	test_when_finished "rm -rf log myclone" &&
 	git clone "file://$(pwd)/file_parent" myclone &&

gitgitgadget

From: Xing Xin <[email protected]>

Ensure server_options is properly cleared using string_list_clear() in builtin/ls-remote.c:cmd_ls_remote.

Although we cannot yet enable TEST_PASSES_SANITIZE_LEAK=true for t/t5702-protocol-v2.sh due to other existing leaks, this fix ensures that "git-ls-remote" related server options tests pass the sanitize leak check:

...
ok 12 - server-options are sent when using ls-remote ok 13 - server-options from configuration are used by ls-remote
...

---
 builtin/ls-remote.c | 1 +
 1 file changed, 1 insertion(+)

index f723b3bf3bb..423318f87ec 100644
@@ -173,5 +173,6 @@ int cmd_ls_remote(int argc,
 	transport_ls_refs_options_release(&transport_options);
 
 	strvec_clear(&pattern);
+	string_list_clear(&server_options, 0);
 	return status;
 }

gitgitgadget

Thanks for the help! Applied to the new patch (with some minor modifications).

Make sense.

Xing Xin

Thanks for your kind help! I just got back from holiday, so your reply was perfectly timed. :-)

Xing Xin

The range-diff looks as expected to me, so this should be ready to go from my point of view. Thanks!

Patrick

Thanks, both of you. Queued.

A more interesting use case, as there is no reason to expect that a single server-option command line option may apply to all remotes, would be to configure different server options for different remotes via the new serverOption configuration, so that different server options are used for different remotes.

If it happens that the same server option is applicable for all remotes, it is reasonable to give --server-option from the command line and expect it to be propagated down to subfetches, so regardless of the "what happens when different remotes have different options configured?" above, the change in this step looks reasonable to me.