[4/4] builtin/ls-remote.c: recognize fetch.serverOption configuration

Commit Message

Xing Xin Sept. 2, 2024, 12:13 p.m. UTC

From: Xing Xin <[e-mail]>

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.

Signed-off-by: Xing Xin <[e-mail]>
---
 Documentation/git-ls-remote.txt |  3 +++
 builtin/ls-remote.c             | 32 ++++++++++++++++++++++++++++----
 t/t5702-protocol-v2.sh          | 29 ++++++++++++++++++++++++-----
 3 files changed, 55 insertions(+), 9 deletions(-)

Comments

Patrick Steinhardt Sept. 3, 2024, 10:09 a.m. UTC | #1

On Mon, Sep 02, 2024 at 12:13:56PM +0000, Xing Xin via GitGitGadget wrote:
> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 76c86c3ce4f..fc9930193f8 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -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

Hm. So `fetch.serverOption` now applies to git-fetch(1), git-clone(1)
and git-ls-remote(1). That feels somewhat unexpected to me given its
name, so I'm further leaning into the direction by now that it should be
a per-remote setting `remote.<name>.serverOption`.

Patrick

Patch

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 76c86c3ce4f..fc9930193f8 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -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
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 0a491595ca8..87afd69828c 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -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;
 }
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 3bf31fb570d..da6241afa22 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -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' '