{"thread":{"id":"62037","subject":"[PATCH 0/4] Support server option from configuration","startedAt":"2024-09-02T12:14:00Z","lastAt":"2024-10-08T17:57:15Z","messageCount":39,"participants":["blanet via GitGitGadget","Xing Xin via GitGitGadget","Patrick Steinhardt","Xing Xin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"501974","messageId":"pull.1776.git.git.1725279236.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":null,"subject":"[PATCH 0/4] Support server option from configuration","fromName":"blanet via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-02T12:13:52Z","receivedAt":"2024-09-02T12:14:00Z","isPatch":true,"sender":{"key":"name:blanet","avatar":null},"body":"Currently, server options for Git protocol v2 can only be specified via the\ncommand line option \"--server-option \", which is inconvenient for users who\nwant to specify a list of default options.\n\nThis patch series introduces a new multi-valued configuration,\nfetch.serverOption, to specify default server options. Designed similarly to\npush.pushOption:\n\n 1. Server options set in lower-priority configuration files (e.g.,\n    /etc/gitconfig or $HOME/.gitconfig) can be overridden or unset in more\n    specific repository configurations using an empty string.\n 2. Command-line specified server options take precedence over those from\n    the configuration.\n\nAll commands involving server options, including git-fetch, git-clone,\ngit-ls-remote, and git-pull, have been updated to recognize the new\nconfiguration.\n\nXing Xin (4):\n  transport: add parse_transport_option() method\n  builtin/fetch.c: add fetch.serverOption configuration\n  builtin/clone.c: recognize fetch.serverOption configuration\n  builtin/ls-remote.c: recognize fetch.serverOption configuration\n\n Documentation/config/fetch.txt  | 29 ++++++++++\n Documentation/fetch-options.txt |  3 ++\n Documentation/git-clone.txt     |  3 ++\n Documentation/git-ls-remote.txt |  3 ++\n builtin/clone.c                 | 22 +++++---\n builtin/fetch.c                 | 31 ++++++++---\n builtin/ls-remote.c             | 32 ++++++++++--\n builtin/push.c                  |  6 +--\n t/t5702-protocol-v2.sh          | 93 ++++++++++++++++++++++++++++-----\n transport.c                     |  8 +++\n transport.h                     |  3 ++\n 11 files changed, 196 insertions(+), 37 deletions(-)\n\n\nbase-commit: 4590f2e9412378c61eac95966709c78766d326ba\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1776%2Fblanet%2Fxx%2Fadd-server-option-from-config-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1776/blanet/xx/add-server-option-from-config-v1\nPull-Request: https://github.com/git/git/pull/1776\n-- \ngitgitgadget\n"},{"id":"501975","messageId":"5c8f3c166a59c9d1e6dfa3116a5403ac752efdba.1725279236.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.git.git.1725279236.gitgitgadget@gmail.com","subject":"[PATCH 1/4] transport: add parse_transport_option() method","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-02T12:13:53Z","receivedAt":"2024-09-02T12:14:01Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nIntroduce the `parse_transport_option()` method used to parse\n`push.pushOption` configuration values. This method will be further\nutilized in subsequent commits to parse a newly added\n`fetch.serverOption` configuration for fetches, which aligns with the\ndesign pattern of `push.pushOption`.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n builtin/push.c | 6 +-----\n transport.c    | 8 ++++++++\n transport.h    | 3 +++\n 3 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 7a67398124f..63d19acfb27 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -519,11 +519,7 @@ static int git_push_config(const char *k, const char *v,\n \t} else if (!strcmp(k, \"push.pushoption\")) {\n \t\tif (!v)\n \t\t\treturn config_error_nonbool(k);\n-\t\telse\n-\t\t\tif (!*v)\n-\t\t\t\tstring_list_clear(&push_options_config, 0);\n-\t\t\telse\n-\t\t\t\tstring_list_append(&push_options_config, v);\n+\t\tparse_transport_option(v, &push_options_config);\n \t\treturn 0;\n \t} else if (!strcmp(k, \"color.push\")) {\n \t\tpush_use_color = git_config_colorbool(k, v);\ndiff --git a/transport.c b/transport.c\nindex bab28965f96..ca6cd5b3436 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1091,6 +1091,14 @@ int is_transport_allowed(const char *type, int from_user)\n \tBUG(\"invalid protocol_allow_config type\");\n }\n \n+void parse_transport_option(const char *option, struct string_list *transport_options)\n+{\n+\tif (!*option)\n+\t\tstring_list_clear(transport_options, 0);\n+\telse\n+\t\tstring_list_append(transport_options, option);\n+}\n+\n void transport_check_allowed(const char *type)\n {\n \tif (!is_transport_allowed(type, -1))\ndiff --git a/transport.h b/transport.h\nindex 6393cd9823c..1b8735e2ca4 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -342,4 +342,7 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n /* common method used by transport-helper.c and send-pack.c */\n void reject_atomic_push(struct ref *refs, int mirror_mode);\n \n+/* common method to parse push-option for pushes or server-option for fetches */\n+void parse_transport_option(const char *option, struct string_list *transport_options);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"501976","messageId":"7e2d5c504d762bb9cdb9ca008b92d349d81c194b.1725279236.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.git.git.1725279236.gitgitgadget@gmail.com","subject":"[PATCH 2/4] builtin/fetch.c: add fetch.serverOption configuration","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-02T12:13:54Z","receivedAt":"2024-09-02T12:14:02Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nCurrently, server options for Git protocol v2 can only be passed via the\ncommand line using \"--server-option <option>\", which is inconvenient\nwhen we want to specify a list of default options to send.\n\nThis commit introduces a new multi-valued configuration\n`fetch.serverOption`, which is designed similar to `push.pushOption`. It\nallows specifying default server options to be sent automatically if not\nexplicitly set. Subsequent commits will extend this configuration to\nother commands that involve server options, such as git-ls-remote and\ngit-clone.\n\nWhen server options are set in lower-priority configuration files (e.g.,\n/etc/gitconfig or $HOME/.gitconfig), they can be overridden or unset in\nmore specific repository configurations using an empty string.\n\nTests and documentation have been updated accordingly.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n Documentation/config/fetch.txt  | 29 +++++++++++++++++++++++\n Documentation/fetch-options.txt |  3 +++\n builtin/fetch.c                 | 31 +++++++++++++++++-------\n t/t5702-protocol-v2.sh          | 42 +++++++++++++++++++++++++++------\n 4 files changed, 90 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt\nindex d7dc461bd16..9987d2a7425 100644\n--- a/Documentation/config/fetch.txt\n+++ b/Documentation/config/fetch.txt\n@@ -126,3 +126,32 @@ fetch.bundleCreationToken::\n The creation token values are chosen by the provider serving the specific\n bundle URI. If you modify the URI at `fetch.bundleURI`, then be sure to\n remove the value for the `fetch.bundleCreationToken` value before fetching.\n+\n+fetch.serverOption::\n+\tWhen no `--server-option=<option>` argument is given from the\n+\tcommand line, fetch behaves as if each <value> of this variable is\n+\tgiven as `--server-option=<value>`.\n++\n+This is a multi-valued variable, and an empty value can be used in a higher\n+priority configuration file (e.g. `.git/config` in a repository) to clear\n+the values inherited from a lower priority configuration files (e.g.\n+`$HOME/.gitconfig`).\n++\n+----\n+\n+Example:\n+\n+/etc/gitconfig\n+  fetch.serverOption = a\n+  fetch.serverOption = b\n+\n+~/.gitconfig\n+  fetch.serverOption = c\n+\n+repo/.git/config\n+  fetch.serverOption =\n+  fetch.serverOption = b\n+\n+This will result in only b (a and c are cleared).\n+\n+----\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex e22b217fba9..39969aeb326 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -304,6 +304,9 @@ endif::git-pull[]\n \tunknown ones, is server-specific.\n \tWhen multiple `--server-option=<option>` are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no `--server-option=<option>` is given from the command\n+\tline, the values of configuration variable `fetch.serverOption`\n+\tare used instead.\n \n --show-forced-updates::\n \tBy default, git checks if a branch is force-updated during\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex c297569a473..d49dff8a76a 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -95,7 +95,7 @@ static struct transport *gtransport;\n static struct transport *gsecondary;\n static struct refspec refmap = REFSPEC_INIT_FETCH;\n static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;\n-static struct string_list server_options = STRING_LIST_INIT_DUP;\n+static struct string_list config_server_options = STRING_LIST_INIT_DUP;\n static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;\n \n struct fetch_config {\n@@ -170,6 +170,13 @@ static int git_fetch_config(const char *k, const char *v,\n \t\t\t    \"fetch.output\", v);\n \t}\n \n+\tif (!strcmp(k, \"fetch.serveroption\")) {\n+\t\tif (!v)\n+\t\t\treturn config_error_nonbool(k);\n+\t\tparse_transport_option(v, &config_server_options);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(k, v, ctx, cb);\n }\n \n@@ -2063,7 +2070,8 @@ static inline void fetch_one_setup_partial(struct remote *remote)\n \n static int fetch_one(struct remote *remote, int argc, const char **argv,\n \t\t     int prune_tags_ok, int use_stdin_refspecs,\n-\t\t     const struct fetch_config *config)\n+\t\t     const struct fetch_config *config,\n+\t\t     struct string_list *server_options)\n {\n \tstruct refspec rs = REFSPEC_INIT_FETCH;\n \tint i;\n@@ -2124,8 +2132,8 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,\n \t\tstrbuf_release(&line);\n \t}\n \n-\tif (server_options.nr)\n-\t\tgtransport->server_options = &server_options;\n+\tif (server_options && server_options->nr)\n+\t\tgtransport->server_options = server_options;\n \n \tsigchain_push_common(unlock_pack_on_signal);\n \tatexit(unlock_pack_atexit);\n@@ -2152,6 +2160,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tconst char *submodule_prefix = \"\";\n \tconst char *bundle_uri;\n \tstruct string_list list = STRING_LIST_INIT_DUP;\n+\tstruct string_list option_server_options = STRING_LIST_INIT_DUP;\n+\tstruct string_list *server_options = NULL;\n \tstruct remote *remote = NULL;\n \tint all = -1, multiple = 0;\n \tint result = 0;\n@@ -2231,7 +2241,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"accept refs that update .git/shallow\")),\n \t\tOPT_CALLBACK_F(0, \"refmap\", &refmap, N_(\"refmap\"),\n \t\t\t       N_(\"specify fetch refmap\"), PARSE_OPT_NONEG, parse_refmap_arg),\n-\t\tOPT_STRING_LIST('o', \"server-option\", &server_options, N_(\"server-specific\"), N_(\"option to transmit\")),\n+\t\tOPT_STRING_LIST('o', \"server-option\", &option_server_options, N_(\"server-specific\"),\n+\t\t\t\tN_(\"option to transmit\")),\n \t\tOPT_IPVERSION(&family),\n \t\tOPT_STRING_LIST(0, \"negotiation-tip\", &negotiation_tip, N_(\"revision\"),\n \t\t\t\tN_(\"report that we have only objects reachable from this object\")),\n@@ -2272,6 +2283,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix,\n \t\t\t     builtin_fetch_options, builtin_fetch_usage, 0);\n \n+\tserver_options = option_server_options.nr ?\n+\t\t\t &option_server_options : &config_server_options;\n+\n \tif (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)\n \t\tconfig.recurse_submodules = recurse_submodules_cli;\n \n@@ -2418,8 +2432,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\t\tresult = 1;\n \t\t\tgoto cleanup;\n \t\t}\n-\t\tif (server_options.nr)\n-\t\t\tgtransport->server_options = &server_options;\n+\t\tif (server_options && server_options->nr)\n+\t\t\tgtransport->server_options = server_options;\n \t\tresult = transport_fetch_refs(gtransport, NULL);\n \n \t\toidset_iter_init(&acked_commits, &iter);\n@@ -2430,7 +2444,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tif (filter_options.choice || repo_has_promisor_remote(the_repository))\n \t\t\tfetch_one_setup_partial(remote);\n \t\tresult = fetch_one(remote, argc, argv, prune_tags_ok, stdin_refspecs,\n-\t\t\t\t   &config);\n+\t\t\t\t   &config, server_options);\n \t} else {\n \t\tint max_children = max_jobs;\n \n@@ -2529,5 +2543,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \n  cleanup:\n \tstring_list_clear(&list, 0);\n+\tstring_list_clear(&option_server_options, 0);\n \treturn result;\n }\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1ef540f73d3..ae25400010e 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -368,17 +368,45 @@ test_expect_success 'ref advertisement is filtered during fetch using protocol v\n test_expect_success 'server-options are sent when fetching' '\n \ttest_when_finished \"rm -f log\" &&\n \n-\ttest_commit -C file_parent four &&\n-\n+\t# Specify server options from command line\n \tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C file_child -c protocol.version=2 \\\n \t\tfetch -o hello -o world origin main &&\n+\ttest_grep \"server-option=hello\" log &&\n+\ttest_grep \"server-option=world\" log &&\n+\trm -f log &&\n \n-\tgit -C file_child log -1 --format=%s origin/main >actual &&\n-\tgit -C file_parent log -1 --format=%s >expect &&\n-\ttest_cmp expect actual &&\n+\t# Specify server options from fetch.serverOption config\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C file_child -c protocol.version=2 \\\n+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n+\t\tfetch origin main &&\n+\ttest_grep \"server-option=hello\" log &&\n+\ttest_grep \"server-option=world\" log &&\n+\trm -f log &&\n \n-\tgrep \"server-option=hello\" log &&\n-\tgrep \"server-option=world\" log\n+\t# Cmdline server options take a higher priority\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C file_child -c protocol.version=2 \\\n+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n+\t\tfetch -o foo=bar origin main &&\n+\ttest_grep ! \"server-option=hello\" log &&\n+\ttest_grep ! \"server-option=world\" log &&\n+\ttest_grep \"server-option=foo=bar\" log\n+'\n+\n+test_expect_success 'empty value of fetch.serverOption in config clears the list' '\n+\ttest_when_finished \"rm -f log\" &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C file_child -c protocol.version=2 \\\n+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n+\t\t-c fetch.serverOption= -c fetch.serverOption=foo=bar \\\n+\t\tfetch origin main &&\n+\ttest_grep ! \"server-option=hello\" log &&\n+\ttest_grep ! \"server-option=world\" log &&\n+\ttest_grep \"server-option=foo=bar\" log\n+'\n+\n+test_expect_success 'invalid fetch.serverOption in config' '\n+\ttest_when_finished \"rm -f log\" &&\n+\ttest_must_fail git -C file_child -c protocol.version=2 \\\n+\t\t-c fetch.serverOption fetch origin main\n '\n \n test_expect_success 'warn if using server-option with fetch with legacy protocol' '\n-- \ngitgitgadget\n\n"},{"id":"501977","messageId":"7c3ebda513d872a2ab2aa0cff5887757de4cde0a.1725279236.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.git.git.1725279236.gitgitgadget@gmail.com","subject":"[PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-02T12:13:55Z","receivedAt":"2024-09-02T12:14:03Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nTeach git-clone to recognize the `fetch.serverOption` configuration as a\ndefault list of server options to send for Git protocol v2, if server\noptions are not explicitly set via the command line.\n\nNote that `builtin/clone.c:cmd_clone` originally read the git config\ntwice via `builtin/clone.c:git_clone_config`, which would duplicate\nserver options if parsing logic were added there. Upon investigation, it\nwas found that the first config read is unnecessary since all the global\nvariables it sets are actually used after the second config read.\nTherefore, the first config read is replaced with a simple\n`config.c:git_default_config`.\n\nTests and documentation have been updated accordingly.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n Documentation/git-clone.txt |  3 +++\n builtin/clone.c             | 22 ++++++++++++++++------\n t/t5702-protocol-v2.sh      | 22 ++++++++++++++++++++--\n 3 files changed, 39 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 8e925db7e9c..105645ed685 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.\n \tunknown ones, is server-specific.\n \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no ++--server-option=++__<option>__ is given from the command\n+\tline, the values of configuration variable `fetch.serverOption`\n+\tare used instead.\n \n `-n`::\n `--no-checkout`::\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 269b6e18a4e..5a1e2e769af 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -85,7 +85,8 @@ static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;\n static int option_filter_submodules = -1;    /* unspecified */\n static int config_filter_submodules = -1;    /* unspecified */\n-static struct string_list server_options = STRING_LIST_INIT_NODUP;\n+static struct string_list config_server_options = STRING_LIST_INIT_DUP;\n+static struct string_list option_server_options = STRING_LIST_INIT_DUP;\n static int option_remote_submodules;\n static const char *bundle_uri;\n \n@@ -160,7 +161,7 @@ static struct option builtin_clone_options[] = {\n \t\t   N_(\"specify the reference format to use\")),\n \tOPT_STRING_LIST('c', \"config\", &option_config, N_(\"key=value\"),\n \t\t\tN_(\"set config inside the new repository\")),\n-\tOPT_STRING_LIST(0, \"server-option\", &server_options,\n+\tOPT_STRING_LIST(0, \"server-option\", &option_server_options,\n \t\t\tN_(\"server-specific\"), N_(\"option to transmit\")),\n \tOPT_IPVERSION(&family),\n \tOPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),\n@@ -847,6 +848,12 @@ static int git_clone_config(const char *k, const char *v,\n \t\tconfig_reject_shallow = git_config_bool(k, v);\n \tif (!strcmp(k, \"clone.filtersubmodules\"))\n \t\tconfig_filter_submodules = git_config_bool(k, v);\n+\tif (!strcmp(k, \"fetch.serveroption\")) {\n+\t\tif (!v)\n+\t\t\treturn config_error_nonbool(k);\n+\t\tparse_transport_option(v, &config_server_options);\n+\t\treturn 0;\n+\t}\n \n \treturn git_default_config(k, v, ctx, cb);\n }\n@@ -982,17 +989,20 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tint hash_algo;\n \tenum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;\n \tconst int do_not_override_repo_unix_permissions = -1;\n-\n+\tstruct string_list *server_options = NULL;\n \tstruct transport_ls_refs_options transport_ls_refs_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \n \tpacket_trace_identity(\"clone\");\n \n-\tgit_config(git_clone_config, NULL);\n+\tgit_config(git_default_config, NULL);\n \n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t     builtin_clone_usage, 0);\n \n+\tserver_options = option_server_options.nr ?\n+\t\t\t &option_server_options : &config_server_options;\n+\n \tif (argc > 2)\n \t\tusage_msg_opt(_(\"Too many arguments.\"),\n \t\t\tbuiltin_clone_usage, builtin_clone_options);\n@@ -1359,8 +1369,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\n \t\t\t\t     option_upload_pack);\n \n-\tif (server_options.nr)\n-\t\ttransport->server_options = &server_options;\n+\tif (server_options && server_options->nr)\n+\t\ttransport->server_options = server_options;\n \n \tif (filter_options.choice) {\n \t\tconst char *spec =\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex ae25400010e..3bf31fb570d 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -424,12 +424,30 @@ test_expect_success 'warn if using server-option with fetch with legacy protocol\n test_expect_success 'server-options are sent when cloning' '\n \ttest_when_finished \"rm -rf log myclone\" &&\n \n+\t# Specify server options from command line\n \tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n \t\tclone --server-option=hello --server-option=world \\\n \t\t\"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"server-option=hello\" log &&\n+\ttest_grep \"server-option=world\" log &&\n+\trm -rf log myclone &&\n \n-\tgrep \"server-option=hello\" log &&\n-\tgrep \"server-option=world\" log\n+\t# Specify server options from fetch.serverOption config\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"server-option=hello\" log &&\n+\ttest_grep \"server-option=world\" log &&\n+\trm -rf log myclone &&\n+\n+\t# Cmdline server options take a higher priority\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n+\t\tclone --server-option=foo=bar \\\n+\t\t\"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep ! \"server-option=hello\" log &&\n+\ttest_grep ! \"server-option=world\" log &&\n+\ttest_grep \"server-option=foo=bar\" log\n '\n \n test_expect_success 'warn if using server-option with clone with legacy protocol' '\n-- \ngitgitgadget\n\n"},{"id":"501978","messageId":"8962031f9997c4030cf32652908fc431f3bf9976.1725279236.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.git.git.1725279236.gitgitgadget@gmail.com","subject":"[PATCH 4/4] builtin/ls-remote.c: recognize fetch.serverOption configuration","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-02T12:13:56Z","receivedAt":"2024-09-02T12:14:04Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nTeach git-ls-remote to recognize the `fetch.serverOption` configuration\nas a default list of server options to send for Git protocol v2, if\nserver options are not explicitly set via the command line.\n\nTests and documentation have been updated accordingly.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n Documentation/git-ls-remote.txt |  3 +++\n builtin/ls-remote.c             | 32 ++++++++++++++++++++++++++++----\n t/t5702-protocol-v2.sh          | 29 ++++++++++++++++++++++++-----\n 3 files changed, 55 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\nindex 76c86c3ce4f..fc9930193f8 100644\n--- a/Documentation/git-ls-remote.txt\n+++ b/Documentation/git-ls-remote.txt\n@@ -81,6 +81,9 @@ OPTIONS\n \tcharacter.\n \tWhen multiple `--server-option=<option>` are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no `--server-option=<option>` is given from the command\n+\tline, the values of configuration variable `fetch.serverOption`\n+\tare used instead.\n \n <repository>::\n \tThe \"remote\" repository to query.  This parameter can be\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 0a491595ca8..87afd69828c 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -1,4 +1,5 @@\n #include \"builtin.h\"\n+#include \"config.h\"\n #include \"gettext.h\"\n #include \"hex.h\"\n #include \"transport.h\"\n@@ -8,6 +9,8 @@\n #include \"parse-options.h\"\n #include \"wildmatch.h\"\n \n+static struct string_list config_server_options = STRING_LIST_INIT_DUP;\n+\n static const char * const ls_remote_usage[] = {\n \tN_(\"git ls-remote [--branches] [--tags] [--refs] [--upload-pack=<exec>]\\n\"\n \t   \"              [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]\\n\"\n@@ -37,6 +40,18 @@ static int tail_match(const struct strvec *pattern, const char *path)\n \treturn 0;\n }\n \n+static int git_ls_remote_config(const char *k, const char *v,\n+\t\t\t    const struct config_context *ctx, void *cb)\n+{\n+\tif (!strcmp(k, \"fetch.serveroption\")) {\n+\t\tif (!v)\n+\t\t\treturn config_error_nonbool(k);\n+\t\tparse_transport_option(v, &config_server_options);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(k, v, ctx, cb);\n+}\n+\n int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n {\n \tconst char *dest = NULL;\n@@ -49,8 +64,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tstruct strvec pattern = STRVEC_INIT;\n \tstruct transport_ls_refs_options transport_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n+\tstruct string_list option_server_options = STRING_LIST_INIT_DUP;\n+\tstruct string_list *server_options = NULL;\n \tint i;\n-\tstruct string_list server_options = STRING_LIST_INIT_DUP;\n \n \tstruct remote *remote;\n \tstruct transport *transport;\n@@ -80,16 +96,23 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t\t      2, PARSE_OPT_NOCOMPLETE),\n \t\tOPT_BOOL(0, \"symref\", &show_symref_target,\n \t\t\t N_(\"show underlying ref in addition to the object pointed by it\")),\n-\t\tOPT_STRING_LIST('o', \"server-option\", &server_options, N_(\"server-specific\"), N_(\"option to transmit\")),\n+\t\tOPT_STRING_LIST('o', \"server-option\", &option_server_options,\n+\t\t\t\tN_(\"server-specific\"),\n+\t\t\t\tN_(\"option to transmit\")),\n \t\tOPT_END()\n \t};\n \n+\tgit_config(git_ls_remote_config, NULL);\n+\n \tmemset(&ref_array, 0, sizeof(ref_array));\n \n \targc = parse_options(argc, argv, prefix, options, ls_remote_usage,\n \t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n \tdest = argv[0];\n \n+\tserver_options = option_server_options.nr ?\n+\t\t\t &option_server_options : &config_server_options;\n+\n \t/*\n \t * TODO: This is buggy, but required for transport helpers. When a\n \t * transport helper advertises a \"refspec\", then we'd add that to a\n@@ -130,8 +153,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \ttransport = transport_get(remote, NULL);\n \tif (uploadpack)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);\n-\tif (server_options.nr)\n-\t\ttransport->server_options = &server_options;\n+\tif (server_options && server_options->nr)\n+\t\ttransport->server_options = server_options;\n \n \tref = transport_get_remote_refs(transport, &transport_options);\n \tif (ref) {\n@@ -169,5 +192,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \ttransport_ls_refs_options_release(&transport_options);\n \n \tstrvec_clear(&pattern);\n+\tstring_list_clear(&option_server_options, 0);\n \treturn status;\n }\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 3bf31fb570d..da6241afa22 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -173,16 +173,35 @@ test_expect_success 'ref advertisement is filtered with ls-remote using protocol\n test_expect_success 'server-options are sent when using ls-remote' '\n \ttest_when_finished \"rm -f log\" &&\n \n-\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n-\t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" main >actual &&\n-\n \tcat >expect <<-EOF &&\n \t$(git -C file_parent rev-parse refs/heads/main)$(printf \"\\t\")refs/heads/main\n \tEOF\n \n+\t# Specify server options from command line\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"server-option=hello\" log &&\n+\ttest_grep \"server-option=world\" log &&\n+\trm -f log &&\n+\n+\t# Specify server options from fetch.serverOption config\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n+\t\tls-remote \"file://$(pwd)/file_parent\" main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"server-option=hello\" log &&\n+\ttest_grep \"server-option=world\" log &&\n+\trm -f log &&\n+\n+\t# Cmdline server options take a higher priority\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n+\t\tls-remote -o foo=bar \"file://$(pwd)/file_parent\" main >actual &&\n \ttest_cmp expect actual &&\n-\tgrep \"server-option=hello\" log &&\n-\tgrep \"server-option=world\" log\n+\ttest_grep ! \"server-option=hello\" log &&\n+\ttest_grep ! \"server-option=world\" log &&\n+\ttest_grep \"server-option=foo=bar\" log\n '\n \n test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n-- \ngitgitgadget\n"},{"id":"502023","messageId":"ZtbgYniS6UKT9i-O@pks.im","threadId":"62037","inReplyTo":"pull.1776.git.git.1725279236.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/4] Support server option from configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-03T10:09:38Z","receivedAt":"2024-09-03T10:09:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 02, 2024 at 12:13:52PM +0000, blanet via GitGitGadget wrote:\n> Currently, server options for Git protocol v2 can only be specified via the\n> command line option \"--server-option \", which is inconvenient for users who\n> want to specify a list of default options.\n> \n> This patch series introduces a new multi-valued configuration,\n> fetch.serverOption, to specify default server options. Designed similarly to\n> push.pushOption:\n> \n>  1. Server options set in lower-priority configuration files (e.g.,\n>     /etc/gitconfig or $HOME/.gitconfig) can be overridden or unset in more\n>     specific repository configurations using an empty string.\n>  2. Command-line specified server options take precedence over those from\n>     the configuration.\n> \n> All commands involving server options, including git-fetch, git-clone,\n> git-ls-remote, and git-pull, have been updated to recognize the new\n> configuration.\n\nIt would be nice to learn about the context this comes from. In which\nscenario does it make sense to specify options by default? What is the\nintended usecase? I'm sure that this feature proposal comes from a\nspecific usecase that you have at your employer, so learning a bit about\nit would help to decide whether it makes sense to have or not.\n\nPatrick\n"},{"id":"502024","messageId":"ZtbgZiVNrWVMBIWr@pks.im","threadId":"62037","inReplyTo":"7e2d5c504d762bb9cdb9ca008b92d349d81c194b.1725279236.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/4] builtin/fetch.c: add fetch.serverOption configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-03T10:09:42Z","receivedAt":"2024-09-03T10:09:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 02, 2024 at 12:13:54PM +0000, Xing Xin via GitGitGadget wrote:\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index c297569a473..d49dff8a76a 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -2231,7 +2241,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n>  \t\t\t N_(\"accept refs that update .git/shallow\")),\n>  \t\tOPT_CALLBACK_F(0, \"refmap\", &refmap, N_(\"refmap\"),\n>  \t\t\t       N_(\"specify fetch refmap\"), PARSE_OPT_NONEG, parse_refmap_arg),\n> -\t\tOPT_STRING_LIST('o', \"server-option\", &server_options, N_(\"server-specific\"), N_(\"option to transmit\")),\n> +\t\tOPT_STRING_LIST('o', \"server-option\", &option_server_options, N_(\"server-specific\"),\n> +\t\t\t\tN_(\"option to transmit\")),\n>  \t\tOPT_IPVERSION(&family),\n>  \t\tOPT_STRING_LIST(0, \"negotiation-tip\", &negotiation_tip, N_(\"revision\"),\n>  \t\t\t\tN_(\"report that we have only objects reachable from this object\")),\n> @@ -2272,6 +2283,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n>  \targc = parse_options(argc, argv, prefix,\n>  \t\t\t     builtin_fetch_options, builtin_fetch_usage, 0);\n>  \n> +\tserver_options = option_server_options.nr ?\n> +\t\t\t &option_server_options : &config_server_options;\n> +\n>  \tif (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)\n>  \t\tconfig.recurse_submodules = recurse_submodules_cli;\n>  \n\nDoesn't this mean that `server_options` will never be `NULL`? Why do we\nhave the new checks for whether or not `server_options` is set, like\ne.g. in the next hunk.\n\n> @@ -2418,8 +2432,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n>  \t\t\tresult = 1;\n>  \t\t\tgoto cleanup;\n>  \t\t}\n> -\t\tif (server_options.nr)\n> -\t\t\tgtransport->server_options = &server_options;\n> +\t\tif (server_options && server_options->nr)\n> +\t\t\tgtransport->server_options = server_options;\n>  \t\tresult = transport_fetch_refs(gtransport, NULL);\n>  \n>  \t\toidset_iter_init(&acked_commits, &iter);\n> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\n> index 1ef540f73d3..ae25400010e 100755\n> --- a/t/t5702-protocol-v2.sh\n> +++ b/t/t5702-protocol-v2.sh\n> @@ -368,17 +368,45 @@ test_expect_success 'ref advertisement is filtered during fetch using protocol v\n>  test_expect_success 'server-options are sent when fetching' '\n>  \ttest_when_finished \"rm -f log\" &&\n>  \n> -\ttest_commit -C file_parent four &&\n> -\n> +\t# Specify server options from command line\n>  \tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C file_child -c protocol.version=2 \\\n>  \t\tfetch -o hello -o world origin main &&\n> +\ttest_grep \"server-option=hello\" log &&\n> +\ttest_grep \"server-option=world\" log &&\n> +\trm -f log &&\n>  \n> -\tgit -C file_child log -1 --format=%s origin/main >actual &&\n> -\tgit -C file_parent log -1 --format=%s >expect &&\n> -\ttest_cmp expect actual &&\n> +\t# Specify server options from fetch.serverOption config\n> +\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C file_child -c protocol.version=2 \\\n> +\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n> +\t\tfetch origin main &&\n> +\ttest_grep \"server-option=hello\" log &&\n> +\ttest_grep \"server-option=world\" log &&\n> +\trm -f log &&\n>  \n> -\tgrep \"server-option=hello\" log &&\n> -\tgrep \"server-option=world\" log\n> +\t# Cmdline server options take a higher priority\n> +\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C file_child -c protocol.version=2 \\\n> +\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n> +\t\tfetch -o foo=bar origin main &&\n> +\ttest_grep ! \"server-option=hello\" log &&\n> +\ttest_grep ! \"server-option=world\" log &&\n> +\ttest_grep \"server-option=foo=bar\" log\n> +'\n\nI think it makes more sense to check for the new behaviour in new tests\nexclusively. Otherwise one is left wondering whether any of the old\nbehaviour changed.\n\nPatrick\n"},{"id":"502025","messageId":"ZtbgafDQwbU9tBoq@pks.im","threadId":"62037","inReplyTo":"7c3ebda513d872a2ab2aa0cff5887757de4cde0a.1725279236.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-03T10:09:45Z","receivedAt":"2024-09-03T10:09:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 02, 2024 at 12:13:55PM +0000, Xing Xin via GitGitGadget wrote:\n> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n> index 8e925db7e9c..105645ed685 100644\n> --- a/Documentation/git-clone.txt\n> +++ b/Documentation/git-clone.txt\n> @@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.\n>  \tunknown ones, is server-specific.\n>  \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n>  \tsent to the other side in the order listed on the command line.\n> +\tWhen no ++--server-option=++__<option>__ is given from the command\n> +\tline, the values of configuration variable `fetch.serverOption`\n> +\tare used instead.\n>  \n>  `-n`::\n>  `--no-checkout`::\n\nI'm not a 100% sure, but I don't think that `fetch.*` configs typically\nimpact git-clone(1). So this here is a tad surprising to me.\n\nIt makes me wonder whether it is actually sensible to implement this as\npart of the `fetch` namespace in the first place. I'm not yet quite sure\nwhere this whole series slots in, that is why one would want to set up\ndefault server options in the first place. So what I'm wondering right\nnow is whether the server options are something that you want to apply\nglobally for all remotes, or whether you'd rather want to set them up\nper remote.\n\nIn the latter case I could see that it may make sense to instead make\nthis `remote.<name>.serverOption`. This would also remove the unclean\ndesign that a fetch-related config now impacts clones, even though it\nnow works differently than our push options.\n\nI guess this depends on the actual usecase.\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 269b6e18a4e..5a1e2e769af 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -847,6 +848,12 @@ static int git_clone_config(const char *k, const char *v,\n>  \t\tconfig_reject_shallow = git_config_bool(k, v);\n>  \tif (!strcmp(k, \"clone.filtersubmodules\"))\n>  \t\tconfig_filter_submodules = git_config_bool(k, v);\n> +\tif (!strcmp(k, \"fetch.serveroption\")) {\n> +\t\tif (!v)\n> +\t\t\treturn config_error_nonbool(k);\n> +\t\tparse_transport_option(v, &config_server_options);\n> +\t\treturn 0;\n> +\t}\n>  \n>  \treturn git_default_config(k, v, ctx, cb);\n>  }\n\nSeeing that we always have the `config_error_nonbool()` call, would it\nmake sense to also move that into `parse_transport_option()` and have it\nreturn an error code for us?\n\nPatrick\n"},{"id":"502026","messageId":"ZtbgbIOxOB2_MzOA@pks.im","threadId":"62037","inReplyTo":"8962031f9997c4030cf32652908fc431f3bf9976.1725279236.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/4] builtin/ls-remote.c: recognize fetch.serverOption configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-03T10:09:48Z","receivedAt":"2024-09-03T10:09:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 02, 2024 at 12:13:56PM +0000, Xing Xin via GitGitGadget wrote:\n> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\n> index 76c86c3ce4f..fc9930193f8 100644\n> --- a/Documentation/git-ls-remote.txt\n> +++ b/Documentation/git-ls-remote.txt\n> @@ -81,6 +81,9 @@ OPTIONS\n>  \tcharacter.\n>  \tWhen multiple `--server-option=<option>` are given, they are all\n>  \tsent to the other side in the order listed on the command line.\n> +\tWhen no `--server-option=<option>` is given from the command\n> +\tline, the values of configuration variable `fetch.serverOption`\n> +\tare used instead.\n>  \n>  <repository>::\n>  \tThe \"remote\" repository to query.  This parameter can be\n\nHm. So `fetch.serverOption` now applies to git-fetch(1), git-clone(1)\nand git-ls-remote(1). That feels somewhat unexpected to me given its\nname, so I'm further leaning into the direction by now that it should be\na per-remote setting `remote.<name>.serverOption`.\n\nPatrick\n"},{"id":"502100","messageId":"366dd64e.7b53.191bc028796.Coremail.bupt_xingxin@163.com","threadId":"62037","inReplyTo":"ZtbgafDQwbU9tBoq@pks.im","subject":"Re:Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Xing Xin","fromEmail":"bupt_xingxin@163.com","sentAt":"2024-09-04T07:49:28Z","receivedAt":"2024-09-04T07:52:34Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"At 2024-09-03 18:09:45, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n>On Mon, Sep 02, 2024 at 12:13:55PM +0000, Xing Xin via GitGitGadget wrote:\n>> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n>> index 8e925db7e9c..105645ed685 100644\n>> --- a/Documentation/git-clone.txt\n>> +++ b/Documentation/git-clone.txt\n>> @@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.\n>>  \tunknown ones, is server-specific.\n>>  \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n>>  \tsent to the other side in the order listed on the command line.\n>> +\tWhen no ++--server-option=++__<option>__ is given from the command\n>> +\tline, the values of configuration variable `fetch.serverOption`\n>> +\tare used instead.\n>>  \n>>  `-n`::\n>>  `--no-checkout`::\n>\n>I'm not a 100% sure, but I don't think that `fetch.*` configs typically\n>impact git-clone(1). So this here is a tad surprising to me.\n>\n>It makes me wonder whether it is actually sensible to implement this as\n>part of the `fetch` namespace in the first place. I'm not yet quite sure\n>where this whole series slots in, that is why one would want to set up\n>default server options in the first place. So what I'm wondering right\n>now is whether the server options are something that you want to apply\n>globally for all remotes, or whether you'd rather want to set them up\n>per remote.\n\nSorry for not explaining our use case clearly. We have several internal\nrepositories configured with numerous CI tasks, each requiring code\npreparation (sometimes via clone, sometimes via fetch). These CI tasks\nare ususally triggered by post-receive hook, so the concurrent tasks are\nactually fetching the same copy of code.\n\nOn git server, we want to deploy a special pack-objects-hook to mitigate\nthe performance impacts caused by these CI tasks (so the packfile\nproduced by git-pack-objects can be reused).  Since not all clone/fetch\noperations can benefit from this caching mechanism (e.g. pulls from\nusers' dev environment), we need the client to pass a special identifier\nto inform the server whether caching support should be enabled for that\nclone/fetch. Clearly, using server options is a good choice.\n\nTo achieve our design, we need to add two patch series to git:\n\n1. Support injecting server options to identify environments via\n   configuration, because adding the --server-option parameter would\n   require too many script modifications, making it difficult to deploy.\n   This is what this patch series does.\n2. Git server should pass the received server options as environment\n   variables (similar to push options) to the pack-objects-hook.\n\n>In the latter case I could see that it may make sense to instead make\n>this `remote.<name>.serverOption`. This would also remove the unclean\n\nI named the new configuration `fetch.serverOption` mainly to follow the\n`push.pushOption` pattern.  Since which server options to support is\nactually server-specific, using `remote.<name>.serverOption` is a good\nidea for git-fetch. However, how should we design the configuration for\ngit-ls-remote or git-clone, if we wanna provide all of them with a\ndefault list of server options to send?\n\n>design that a fetch-related config now impacts clones, even though it\n>now works differently than our push options.\n>\n>I guess this depends on the actual usecase.\n>\n\nXing Xin\n"},{"id":"502235","messageId":"ZtmQZQHnwYLNvT0F@pks.im","threadId":"62037","inReplyTo":"366dd64e.7b53.191bc028796.Coremail.bupt_xingxin@163.com","subject":"Re: Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-05T11:05:15Z","receivedAt":"2024-09-05T11:05:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Sep 04, 2024 at 03:49:28PM +0800, Xing Xin wrote:\n> At 2024-09-03 18:09:45, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n> >On Mon, Sep 02, 2024 at 12:13:55PM +0000, Xing Xin via GitGitGadget wrote:\n> >> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n> >> index 8e925db7e9c..105645ed685 100644\n> >> --- a/Documentation/git-clone.txt\n> >> +++ b/Documentation/git-clone.txt\n> >> @@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.\n> >>  \tunknown ones, is server-specific.\n> >>  \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n> >>  \tsent to the other side in the order listed on the command line.\n> >> +\tWhen no ++--server-option=++__<option>__ is given from the command\n> >> +\tline, the values of configuration variable `fetch.serverOption`\n> >> +\tare used instead.\n> >>  \n> >>  `-n`::\n> >>  `--no-checkout`::\n> >\n> >I'm not a 100% sure, but I don't think that `fetch.*` configs typically\n> >impact git-clone(1). So this here is a tad surprising to me.\n> >\n> >It makes me wonder whether it is actually sensible to implement this as\n> >part of the `fetch` namespace in the first place. I'm not yet quite sure\n> >where this whole series slots in, that is why one would want to set up\n> >default server options in the first place. So what I'm wondering right\n> >now is whether the server options are something that you want to apply\n> >globally for all remotes, or whether you'd rather want to set them up\n> >per remote.\n> \n> Sorry for not explaining our use case clearly. We have several internal\n> repositories configured with numerous CI tasks, each requiring code\n> preparation (sometimes via clone, sometimes via fetch). These CI tasks\n> are ususally triggered by post-receive hook, so the concurrent tasks are\n> actually fetching the same copy of code.\n> \n> On git server, we want to deploy a special pack-objects-hook to mitigate\n> the performance impacts caused by these CI tasks (so the packfile\n> produced by git-pack-objects can be reused).  Since not all clone/fetch\n> operations can benefit from this caching mechanism (e.g. pulls from\n> users' dev environment), we need the client to pass a special identifier\n> to inform the server whether caching support should be enabled for that\n> clone/fetch. Clearly, using server options is a good choice.\n> \n> To achieve our design, we need to add two patch series to git:\n> \n> 1. Support injecting server options to identify environments via\n>    configuration, because adding the --server-option parameter would\n>    require too many script modifications, making it difficult to deploy.\n>    This is what this patch series does.\n> 2. Git server should pass the received server options as environment\n>    variables (similar to push options) to the pack-objects-hook.\n\nWhen you talk about client, do you mean that the actual users will have\nto configure this? That sounds somewhat unmaintainable on your side from\nthe surface. I guess I ain't got enough knowledge around the environment\nyou operate in though, so I probably shouldn't judge.\n\n> >In the latter case I could see that it may make sense to instead make\n> >this `remote.<name>.serverOption`. This would also remove the unclean\n> \n> I named the new configuration `fetch.serverOption` mainly to follow the\n> `push.pushOption` pattern.  Since which server options to support is\n> actually server-specific, using `remote.<name>.serverOption` is a good\n> idea for git-fetch. However, how should we design the configuration for\n> git-ls-remote or git-clone, if we wanna provide all of them with a\n> default list of server options to send?\n\nAs mentioned in another reply, I think that putting this into the remote\nconfiguration \"remote.*.serverOption\" might be a better solution, as it\nalso brings you the ability to set this per remote by default.\n\nPatrick\n"},{"id":"502236","messageId":"3ce04aef.b004.191c21a2159.Coremail.bupt_xingxin@163.com","threadId":"62037","inReplyTo":"ZtmQZQHnwYLNvT0F@pks.im","subject":"Re:Re: Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Xing Xin","fromEmail":"bupt_xingxin@163.com","sentAt":"2024-09-05T12:12:58Z","receivedAt":"2024-09-05T12:13:23Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"At 2024-09-05 19:05:15, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n>On Wed, Sep 04, 2024 at 03:49:28PM +0800, Xing Xin wrote:\n>> At 2024-09-03 18:09:45, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n>> >On Mon, Sep 02, 2024 at 12:13:55PM +0000, Xing Xin via GitGitGadget wrote:\n>> >> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n>> >> index 8e925db7e9c..105645ed685 100644\n>> >> --- a/Documentation/git-clone.txt\n>> >> +++ b/Documentation/git-clone.txt\n>> >> @@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.\n>> >>  \tunknown ones, is server-specific.\n>> >>  \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n>> >>  \tsent to the other side in the order listed on the command line.\n>> >> +\tWhen no ++--server-option=++__<option>__ is given from the command\n>> >> +\tline, the values of configuration variable `fetch.serverOption`\n>> >> +\tare used instead.\n>> >>  \n>> >>  `-n`::\n>> >>  `--no-checkout`::\n>> >\n>> >I'm not a 100% sure, but I don't think that `fetch.*` configs typically\n>> >impact git-clone(1). So this here is a tad surprising to me.\n>> >\n>> >It makes me wonder whether it is actually sensible to implement this as\n>> >part of the `fetch` namespace in the first place. I'm not yet quite sure\n>> >where this whole series slots in, that is why one would want to set up\n>> >default server options in the first place. So what I'm wondering right\n>> >now is whether the server options are something that you want to apply\n>> >globally for all remotes, or whether you'd rather want to set them up\n>> >per remote.\n>> \n>> Sorry for not explaining our use case clearly. We have several internal\n>> repositories configured with numerous CI tasks, each requiring code\n>> preparation (sometimes via clone, sometimes via fetch). These CI tasks\n>> are ususally triggered by post-receive hook, so the concurrent tasks are\n>> actually fetching the same copy of code.\n>> \n>> On git server, we want to deploy a special pack-objects-hook to mitigate\n>> the performance impacts caused by these CI tasks (so the packfile\n>> produced by git-pack-objects can be reused).  Since not all clone/fetch\n>> operations can benefit from this caching mechanism (e.g. pulls from\n>> users' dev environment), we need the client to pass a special identifier\n>> to inform the server whether caching support should be enabled for that\n>> clone/fetch. Clearly, using server options is a good choice.\n>> \n>> To achieve our design, we need to add two patch series to git:\n>> \n>> 1. Support injecting server options to identify environments via\n>>    configuration, because adding the --server-option parameter would\n>>    require too many script modifications, making it difficult to deploy.\n>>    This is what this patch series does.\n>> 2. Git server should pass the received server options as environment\n>>    variables (similar to push options) to the pack-objects-hook.\n>\n>When you talk about client, do you mean that the actual users will have\n>to configure this? That sounds somewhat unmaintainable on your side from\n>the surface. I guess I ain't got enough knowledge around the environment\n>you operate in though, so I probably shouldn't judge.\n\nYes, that is indeed our design goal. We want specially configured git\nclients to benefit from caching acceleration when cloning code, while\nclients without special configurations follow the regular logic. This is\nbecause code fetching in CI environments is often homogeneous, making it\nworthwhile to implement caching logic to speed up the process and reduce\nserver load. In contrast, code fetching from users is usually diverse,\nmaking caching less valuable.\n\nBy linking pack-objects-hook with server options, we believe we can\neffectively differentiate between various clone behaviors. In fact, I\nthink a similar design can also be extended to GitHub Actions or GitLab\nCI to save CPU on git servers, although I'm not sure whether a similar\nmechanism is already available.\n\nCI environments are usually provided by the infrastructure team, making this\nsolution easier to maintain, and we do not expect any changes on real users.\n\n>> >In the latter case I could see that it may make sense to instead make\n>> >this `remote.<name>.serverOption`. This would also remove the unclean\n>> \n>> I named the new configuration `fetch.serverOption` mainly to follow the\n>> `push.pushOption` pattern.  Since which server options to support is\n>> actually server-specific, using `remote.<name>.serverOption` is a good\n>> idea for git-fetch. However, how should we design the configuration for\n>> git-ls-remote or git-clone, if we wanna provide all of them with a\n>> default list of server options to send?\n>\n>As mentioned in another reply, I think that putting this into the remote\n>configuration \"remote.*.serverOption\" might be a better solution, as it\n>also brings you the ability to set this per remote by default.\n\nI agree that using \"remote.*.serverOption\" is better. In fact, I also\nthink \"push.pushOption\" would be better as \"remote.*.pushOption\". What I'm\ncontemplating is whether we need to add a configuration for a default\nlist of server options, so that when \"remote.origin.serverOption\" is not\npresent, we can fall back to use that as default.\n\n\nXing Xin\n\n"},{"id":"502237","messageId":"Ztm1w_61vjEkiC4C@pks.im","threadId":"62037","inReplyTo":"3ce04aef.b004.191c21a2159.Coremail.bupt_xingxin@163.com","subject":"Re: Re: Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-05T13:44:41Z","receivedAt":"2024-09-05T13:44:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Sep 05, 2024 at 08:12:58PM +0800, Xing Xin wrote:\n> At 2024-09-05 19:05:15, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n> >On Wed, Sep 04, 2024 at 03:49:28PM +0800, Xing Xin wrote:\n> >> At 2024-09-03 18:09:45, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n> >> >In the latter case I could see that it may make sense to instead make\n> >> >this `remote.<name>.serverOption`. This would also remove the unclean\n> >> \n> >> I named the new configuration `fetch.serverOption` mainly to follow the\n> >> `push.pushOption` pattern.  Since which server options to support is\n> >> actually server-specific, using `remote.<name>.serverOption` is a good\n> >> idea for git-fetch. However, how should we design the configuration for\n> >> git-ls-remote or git-clone, if we wanna provide all of them with a\n> >> default list of server options to send?\n> >\n> >As mentioned in another reply, I think that putting this into the remote\n> >configuration \"remote.*.serverOption\" might be a better solution, as it\n> >also brings you the ability to set this per remote by default.\n> \n> I agree that using \"remote.*.serverOption\" is better. In fact, I also\n> think \"push.pushOption\" would be better as \"remote.*.pushOption\". What I'm\n> contemplating is whether we need to add a configuration for a default\n> list of server options, so that when \"remote.origin.serverOption\" is not\n> present, we can fall back to use that as default.\n\nJunio proposed in [1] to introduce `[remote \"*\"]` syntax to supply\ndefault values to all remotes. You could pick up that proposal and\nimplement it as part of your patch series.\n\nI also have to agree that \"push.pushOption\" would be way more sensible\nif it was configured per remote. I think it would be sensible to also\nintroduce \"remote.*.pushOption\" in the same way and have it override the\ndefault value of \"push.pushOption\" if present. So the precedence order\nwould become (from high to low):\n\n  - remote.someRemote.pushOption\n  - remote.\"*\".pushOption\n  - push.PushOption\n\nThis should be backwards compatible, too.\n\nPatrick\n\n[1]: <xmqq5xrcn2k1.fsf@gitster.g>\n"},{"id":"502271","messageId":"xmqq7cbq2dn0.fsf@gitster.g","threadId":"62037","inReplyTo":"Ztm1w_61vjEkiC4C@pks.im","subject":"Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-05T17:50:43Z","receivedAt":"2024-09-05T17:50:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> I also have to agree that \"push.pushOption\" would be way more sensible\n> if it was configured per remote. I think it would be sensible to also\n> introduce \"remote.*.pushOption\" in the same way and have it override the\n> default value of \"push.pushOption\" if present. So the precedence order\n> would become (from high to low):\n>\n>   - remote.someRemote.pushOption\n>   - remote.\"*\".pushOption\n>   - push.PushOption\n>\n> This should be backwards compatible, too.\n\n;-)  Sounds sensible.\n"},{"id":"502420","messageId":"3a7b1ced.364e.191d4b07a1a.Coremail.bupt_xingxin@163.com","threadId":"62037","inReplyTo":"Ztm1w_61vjEkiC4C@pks.im","subject":"Re:Re: Re: Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Xing Xin","fromEmail":"bupt_xingxin@163.com","sentAt":"2024-09-09T02:50:21Z","receivedAt":"2024-09-09T02:50:59Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"At 2024-09-05 21:44:41, \"Patrick Steinhardt\" <ps@pks.im> wrote\n\n[snip]\n\n>> >> I named the new configuration `fetch.serverOption` mainly to follow the\n>> >> `push.pushOption` pattern.  Since which server options to support is\n>> >> actually server-specific, using `remote.<name>.serverOption` is a good\n>> >> idea for git-fetch. However, how should we design the configuration for\n>> >> git-ls-remote or git-clone, if we wanna provide all of them with a\n>> >> default list of server options to send?\n>> >\n>> >As mentioned in another reply, I think that putting this into the remote\n>> >configuration \"remote.*.serverOption\" might be a better solution, as it\n>> >also brings you the ability to set this per remote by default.\n>> \n>> I agree that using \"remote.*.serverOption\" is better. In fact, I also\n>> think \"push.pushOption\" would be better as \"remote.*.pushOption\". What I'm\n>> contemplating is whether we need to add a configuration for a default\n>> list of server options, so that when \"remote.origin.serverOption\" is not\n>> present, we can fall back to use that as default.\n>\n>Junio proposed in [1] to introduce `[remote \"*\"]` syntax to supply\n>default values to all remotes. You could pick up that proposal and\n>implement it as part of your patch series.\n\nGiven the addition of a remote.*.<subkey> configuration in Git's global\nsettings, such as:\n\n    git config --global remote.\"*\".demoConfigKey demoConfigValue\n\nThe current versions of Git may produce errors with certain commands.\nFor example, running `git fetch --all` will result in:\n\n    $ git fetch --all\n    Fetching *\n    fatal: '*' does not appear to be a git repository\n    fatal: Could not read from remote repository.\n\n    Please make sure you have the correct access rights\n    and the repository exists.\n    error: could not fetch *\n    Fetching origin\n\nThis raises the question of whether this can be defined as an\nincompatibility or whether this is acceptable. If it isn't, I prefer\nusing a `remote.defaultServerOption` instead, as it aligns with our\nexisting use of `remote.pushDefault` anyway.\n\n[snip]\n\nXing Xin\n"},{"id":"502441","messageId":"Zt7gufVmnMIGraNx@pks.im","threadId":"62037","inReplyTo":"3a7b1ced.364e.191d4b07a1a.Coremail.bupt_xingxin@163.com","subject":"Re: Re: Re: Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-09T11:49:16Z","receivedAt":"2024-09-09T11:49:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 09, 2024 at 10:50:21AM +0800, Xing Xin wrote:\n> At 2024-09-05 21:44:41, \"Patrick Steinhardt\" <ps@pks.im> wrote\n> \n> [snip]\n> \n> >> >> I named the new configuration `fetch.serverOption` mainly to follow the\n> >> >> `push.pushOption` pattern.  Since which server options to support is\n> >> >> actually server-specific, using `remote.<name>.serverOption` is a good\n> >> >> idea for git-fetch. However, how should we design the configuration for\n> >> >> git-ls-remote or git-clone, if we wanna provide all of them with a\n> >> >> default list of server options to send?\n> >> >\n> >> >As mentioned in another reply, I think that putting this into the remote\n> >> >configuration \"remote.*.serverOption\" might be a better solution, as it\n> >> >also brings you the ability to set this per remote by default.\n> >> \n> >> I agree that using \"remote.*.serverOption\" is better. In fact, I also\n> >> think \"push.pushOption\" would be better as \"remote.*.pushOption\". What I'm\n> >> contemplating is whether we need to add a configuration for a default\n> >> list of server options, so that when \"remote.origin.serverOption\" is not\n> >> present, we can fall back to use that as default.\n> >\n> >Junio proposed in [1] to introduce `[remote \"*\"]` syntax to supply\n> >default values to all remotes. You could pick up that proposal and\n> >implement it as part of your patch series.\n> \n> Given the addition of a remote.*.<subkey> configuration in Git's global\n> settings, such as:\n> \n>     git config --global remote.\"*\".demoConfigKey demoConfigValue\n> \n> The current versions of Git may produce errors with certain commands.\n> For example, running `git fetch --all` will result in:\n> \n>     $ git fetch --all\n>     Fetching *\n>     fatal: '*' does not appear to be a git repository\n>     fatal: Could not read from remote repository.\n> \n>     Please make sure you have the correct access rights\n>     and the repository exists.\n>     error: could not fetch *\n>     Fetching origin\n\nWe do not currently handle \"remote.*.something\", so we'd first have to\nadd support for this syntax.\n\n> This raises the question of whether this can be defined as an\n> incompatibility or whether this is acceptable. If it isn't, I prefer\n> using a `remote.defaultServerOption` instead, as it aligns with our\n> existing use of `remote.pushDefault` anyway.\n\nWhile for this config key it may work alright, I think supporting\n\"remote.*.something\" is preferable. Ideally, it would be generic enough\nto apply to all per-remote settings such that we don't have to add\nsupport for every single config key that applies to remotes.\n\nRight now we only care about push options and server options. But\nsomebody may want to globally configure proxies, tag options, partial\nclone filters or something else. And if we got all of that without\nhaving to introduce `remote.defaultPruneTags`,\n`remote.defaultPartialCloneFilter` and similar options I'd consider it a\nwin.\n\nPatrick\n"},{"id":"502457","messageId":"xmqq7cbkvnk8.fsf@gitster.g","threadId":"62037","inReplyTo":"3a7b1ced.364e.191d4b07a1a.Coremail.bupt_xingxin@163.com","subject":"Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-09T15:44:55Z","receivedAt":"2024-09-09T15:44:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Xing Xin\" <bupt_xingxin@163.com> writes:\n\n> Given the addition of a remote.*.<subkey> configuration in Git's global\n> settings, such as:\n>\n>     git config --global remote.\"*\".demoConfigKey demoConfigValue\n>\n> The current versions of Git may produce errors with certain commands.\n> For example, running `git fetch --all` will result in:\n>\n>     $ git fetch --all\n>     Fetching *\n>     fatal: '*' does not appear to be a git repository\n>     fatal: Could not read from remote repository.\n\nAh, good point.  Anything that wants to enumerate the subkeys under\nremote. hierarchy MUST be aware of the new convention.\n\nSo such a code must of course need to be updated to treat \"*\" as a\nvirtual thing and exclude from enumeration (I suspect \"git remote\"\nhas the same property), and delivered to the end-users at the same\ntime.\n\nAn alternative is to use remote.<key> as a fallback default for\nremote.<name>.<key>, which has been done successfully for other\nhierarchies like http.<url>.<key> would override http.<key> for any\ndefined <key>s.  So if we have remote.<name>.skipFetchAll per\nremote, we could use remote.skipFetchAll as its fallback default\nvalue.\n"},{"id":"503250","messageId":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.git.git.1725279236.gitgitgadget@gmail.com","subject":"[PATCH v2 0/5] Support server option from configuration","fromName":"blanet via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-23T12:17:53Z","receivedAt":"2024-09-23T12:18:02Z","isPatch":true,"sender":{"key":"name:blanet","avatar":null},"body":"We have multiple internal repositories configured with a huge number of CI\ntasks, each requiring code preparation (either via clone or fetch). These CI\ntasks, triggered by post-receive hooks, often fetch the same code (usually\nusing --depth=1) concurrently.\n\nTo reduce performance impacts on the git server caused by these tasks, we\nwant to deploy a special designed pack-objects-hook. This hook would allow\nthe packs generated by git-pack-objects to be reused. Since not all\nclone/fetch operations will benefit from this caching (e.g., pulls from\ndevelopers' environments), clients need to pass a special identifier to\nindicate whether caching should be enabled for a particular operation. Using\nserver options is a suitable method for this.\n\nHowever, server options can only be specified via the command line option\n(via --server-option or -o), which is inconvenient and requires CI script\nmodifications. A configuration-based approach is easier to propagate (by\nchanging the global configuration ~/.gitconfig) and avoids issues with older\nGit versions that don't support --server-option.\n\nThis patch series introduces a new multi-valued configuration,\nremote.<name>.serverOption, similar to push.pushOption, to specify default\nserver options for the corresponding remote.\n\n * Patch 1~3 is the main change for introducing the new configuration.\n * Patch 4 fixes a issue for git-fetch not sending server-options when\n   fetching multiple remotes.\n * Patch 5 is a minor fix for a server options-related memory leak.\n\nXing Xin (5):\n  transport: introduce parse_transport_option() method\n  remote: introduce remote.<name>.serverOption configuration\n  transport.c::handshake: make use of server options from remote\n  fetch: respect --server-option when fetching multiple remotes\n  ls-remote: leakfix for not clearing server_options\n\n Documentation/config/remote.txt |  10 +++\n Documentation/fetch-options.txt |   3 +\n Documentation/git-clone.txt     |   3 +\n Documentation/git-ls-remote.txt |   3 +\n builtin/fetch.c                 |   2 +\n builtin/ls-remote.c             |   1 +\n builtin/push.c                  |   9 +--\n remote.c                        |   7 ++\n remote.h                        |   3 +\n t/t5702-protocol-v2.sh          | 133 ++++++++++++++++++++++++++++++++\n transport.c                     |  15 ++++\n transport.h                     |   4 +\n 12 files changed, 185 insertions(+), 8 deletions(-)\n\n\nbase-commit: 4590f2e9412378c61eac95966709c78766d326ba\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1776%2Fblanet%2Fxx%2Fadd-server-option-from-config-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1776/blanet/xx/add-server-option-from-config-v2\nPull-Request: https://github.com/git/git/pull/1776\n\nRange-diff vs v1:\n\n 1:  5c8f3c166a5 ! 1:  c95ed5e0dd5 transport: add parse_transport_option() method\n     @@ Metadata\n      Author: Xing Xin <xingxin.xx@bytedance.com>\n      \n       ## Commit message ##\n     -    transport: add parse_transport_option() method\n     +    transport: introduce parse_transport_option() method\n      \n     -    Introduce the `parse_transport_option()` method used to parse\n     -    `push.pushOption` configuration values. This method will be further\n     -    utilized in subsequent commits to parse a newly added\n     -    `fetch.serverOption` configuration for fetches, which aligns with the\n     -    design pattern of `push.pushOption`.\n     +    Add the `parse_transport_option()` method to parse the `push.pushOption`\n     +    configuration. This method will also be used in the next commit to\n     +    handle the new `remote.<name>.serverOption` configuration for setting\n     +    server options in Git protocol v2.\n      \n          Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>\n      \n       ## builtin/push.c ##\n      @@ builtin/push.c: static int git_push_config(const char *k, const char *v,\n     + \t\t\tRECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;\n     + \t\trecurse_submodules = val;\n       \t} else if (!strcmp(k, \"push.pushoption\")) {\n     - \t\tif (!v)\n     - \t\t\treturn config_error_nonbool(k);\n     +-\t\tif (!v)\n     +-\t\t\treturn config_error_nonbool(k);\n      -\t\telse\n      -\t\t\tif (!*v)\n      -\t\t\t\tstring_list_clear(&push_options_config, 0);\n      -\t\t\telse\n      -\t\t\t\tstring_list_append(&push_options_config, v);\n     -+\t\tparse_transport_option(v, &push_options_config);\n     - \t\treturn 0;\n     +-\t\treturn 0;\n     ++\t\treturn parse_transport_option(k, v, &push_options_config);\n       \t} else if (!strcmp(k, \"color.push\")) {\n       \t\tpush_use_color = git_config_colorbool(k, v);\n     + \t\treturn 0;\n      \n       ## transport.c ##\n      @@ transport.c: int is_transport_allowed(const char *type, int from_user)\n       \tBUG(\"invalid protocol_allow_config type\");\n       }\n       \n     -+void parse_transport_option(const char *option, struct string_list *transport_options)\n     ++int parse_transport_option(const char *var, const char *value,\n     ++\t\t\t   struct string_list *transport_options)\n      +{\n     -+\tif (!*option)\n     ++\tif (!value)\n     ++\t\treturn config_error_nonbool(var);\n     ++\tif (!*value)\n      +\t\tstring_list_clear(transport_options, 0);\n      +\telse\n     -+\t\tstring_list_append(transport_options, option);\n     ++\t\tstring_list_append(transport_options, value);\n     ++\treturn 0;\n      +}\n      +\n       void transport_check_allowed(const char *type)\n     @@ transport.h: void transport_print_push_status(const char *dest, struct ref *refs\n       /* common method used by transport-helper.c and send-pack.c */\n       void reject_atomic_push(struct ref *refs, int mirror_mode);\n       \n     -+/* common method to parse push-option for pushes or server-option for fetches */\n     -+void parse_transport_option(const char *option, struct string_list *transport_options);\n     ++/* common method to parse push-option or server-option from config */\n     ++int parse_transport_option(const char *var, const char *value,\n     ++\t\t\t   struct string_list *transport_options);\n      +\n       #endif\n 2:  7e2d5c504d7 < -:  ----------- builtin/fetch.c: add fetch.serverOption configuration\n -:  ----------- > 2:  2474b4c69d6 remote: introduce remote.<name>.serverOption configuration\n 3:  7c3ebda513d ! 3:  a7f3e458501 builtin/clone.c: recognize fetch.serverOption configuration\n     @@ Metadata\n      Author: Xing Xin <xingxin.xx@bytedance.com>\n      \n       ## Commit message ##\n     -    builtin/clone.c: recognize fetch.serverOption configuration\n     +    transport.c::handshake: make use of server options from remote\n      \n     -    Teach git-clone to recognize the `fetch.serverOption` configuration as a\n     -    default list of server options to send for Git protocol v2, if server\n     -    options are not explicitly set via the command line.\n     +    Utilize the `server_options` from the corresponding remote during the\n     +    handshake in `transport.c` when Git protocol v2 is detected. This helps\n     +    initialize the `server_options` in `transport.h:transport` if no server\n     +    options are set for the transport (typically via `--server-option` or\n     +    `-o`).\n      \n     -    Note that `builtin/clone.c:cmd_clone` originally read the git config\n     -    twice via `builtin/clone.c:git_clone_config`, which would duplicate\n     -    server options if parsing logic were added there. Upon investigation, it\n     -    was found that the first config read is unnecessary since all the global\n     -    variables it sets are actually used after the second config read.\n     -    Therefore, the first config read is replaced with a simple\n     -    `config.c:git_default_config`.\n     +    While another potential place to incorporate server options from the\n     +    remote is in `transport.c:transport_get`, setting server options for a\n     +    transport using a protocol other than v2 could lead to unexpected errors\n     +    (see `transport.c:die_if_server_options`).\n      \n     -    Tests and documentation have been updated accordingly.\n     +    Relevant tests and documentation have been updated accordingly.\n      \n          Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>\n      \n     + ## Documentation/fetch-options.txt ##\n     +@@ Documentation/fetch-options.txt: endif::git-pull[]\n     + \tunknown ones, is server-specific.\n     + \tWhen multiple `--server-option=<option>` are given, they are all\n     + \tsent to the other side in the order listed on the command line.\n     ++\tWhen no `--server-option=<option>` is given from the command line,\n     ++\tthe values of configuration variable `remote.<name>.serverOption`\n     ++\tare used instead.\n     + \n     + --show-forced-updates::\n     + \tBy default, git checks if a branch is force-updated during\n     +\n       ## Documentation/git-clone.txt ##\n      @@ Documentation/git-clone.txt: objects from the source repository into a pack in the cloned repository.\n       \tunknown ones, is server-specific.\n       \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n       \tsent to the other side in the order listed on the command line.\n      +\tWhen no ++--server-option=++__<option>__ is given from the command\n     -+\tline, the values of configuration variable `fetch.serverOption`\n     ++\tline, the values of configuration variable `remote.<name>.serverOption`\n      +\tare used instead.\n       \n       `-n`::\n       `--no-checkout`::\n      \n     - ## builtin/clone.c ##\n     -@@ builtin/clone.c: static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n     - static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;\n     - static int option_filter_submodules = -1;    /* unspecified */\n     - static int config_filter_submodules = -1;    /* unspecified */\n     --static struct string_list server_options = STRING_LIST_INIT_NODUP;\n     -+static struct string_list config_server_options = STRING_LIST_INIT_DUP;\n     -+static struct string_list option_server_options = STRING_LIST_INIT_DUP;\n     - static int option_remote_submodules;\n     - static const char *bundle_uri;\n     - \n     -@@ builtin/clone.c: static struct option builtin_clone_options[] = {\n     - \t\t   N_(\"specify the reference format to use\")),\n     - \tOPT_STRING_LIST('c', \"config\", &option_config, N_(\"key=value\"),\n     - \t\t\tN_(\"set config inside the new repository\")),\n     --\tOPT_STRING_LIST(0, \"server-option\", &server_options,\n     -+\tOPT_STRING_LIST(0, \"server-option\", &option_server_options,\n     - \t\t\tN_(\"server-specific\"), N_(\"option to transmit\")),\n     - \tOPT_IPVERSION(&family),\n     - \tOPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),\n     -@@ builtin/clone.c: static int git_clone_config(const char *k, const char *v,\n     - \t\tconfig_reject_shallow = git_config_bool(k, v);\n     - \tif (!strcmp(k, \"clone.filtersubmodules\"))\n     - \t\tconfig_filter_submodules = git_config_bool(k, v);\n     -+\tif (!strcmp(k, \"fetch.serveroption\")) {\n     -+\t\tif (!v)\n     -+\t\t\treturn config_error_nonbool(k);\n     -+\t\tparse_transport_option(v, &config_server_options);\n     -+\t\treturn 0;\n     -+\t}\n     - \n     - \treturn git_default_config(k, v, ctx, cb);\n     - }\n     -@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)\n     - \tint hash_algo;\n     - \tenum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;\n     - \tconst int do_not_override_repo_unix_permissions = -1;\n     --\n     -+\tstruct string_list *server_options = NULL;\n     - \tstruct transport_ls_refs_options transport_ls_refs_options =\n     - \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n     - \n     - \tpacket_trace_identity(\"clone\");\n     - \n     --\tgit_config(git_clone_config, NULL);\n     -+\tgit_config(git_default_config, NULL);\n     + ## Documentation/git-ls-remote.txt ##\n     +@@ Documentation/git-ls-remote.txt: OPTIONS\n     + \tcharacter.\n     + \tWhen multiple `--server-option=<option>` are given, they are all\n     + \tsent to the other side in the order listed on the command line.\n     ++\tWhen no `--server-option=<option>` is given from the command line,\n     ++\tthe values of configuration variable `remote.<name>.serverOption`\n     ++\tare used instead.\n       \n     - \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n     - \t\t\t     builtin_clone_usage, 0);\n     + <repository>::\n     + \tThe \"remote\" repository to query.  This parameter can be\n     +\n     + ## t/t5702-protocol-v2.sh ##\n     +@@ t/t5702-protocol-v2.sh: test_expect_success 'server-options are sent when using ls-remote' '\n     + \tgrep \"server-option=world\" log\n     + '\n       \n     -+\tserver_options = option_server_options.nr ?\n     -+\t\t\t &option_server_options : &config_server_options;\n     -+\n     - \tif (argc > 2)\n     - \t\tusage_msg_opt(_(\"Too many arguments.\"),\n     - \t\t\tbuiltin_clone_usage, builtin_clone_options);\n     -@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)\n     - \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\n     - \t\t\t\t     option_upload_pack);\n     ++test_expect_success 'server-options from configuration are used by ls-remote' '\n     ++\ttest_when_finished \"rm -rf log myclone\" &&\n     ++\tgit clone \"file://$(pwd)/file_parent\" myclone &&\n     ++\tcat >expect <<-EOF &&\n     ++\t$(git -C file_parent rev-parse refs/heads/main)$(printf \"\\t\")refs/heads/main\n     ++\tEOF\n     ++\n     ++\t# Default server options from configuration are used\n     ++\tgit -C myclone config --add remote.origin.serverOption foo &&\n     ++\tgit -C myclone config --add remote.origin.serverOption bar &&\n     ++\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n     ++\t\tls-remote origin main >actual &&\n     ++\ttest_cmp expect actual &&\n     ++\ttest_grep \"ls-remote> server-option=foo\" log &&\n     ++\ttest_grep \"ls-remote> server-option=bar\" log &&\n     ++\trm -f log &&\n     ++\n     ++\t# Empty value of remote.<name>.serverOption clears the list\n     ++\tgit -C myclone config --add remote.origin.serverOption \"\" &&\n     ++\tgit -C myclone config --add remote.origin.serverOption tar &&\n     ++\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n     ++\t\tls-remote origin main >actual &&\n     ++\ttest_cmp expect actual &&\n     ++\ttest_grep \"ls-remote> server-option=tar\" log &&\n     ++\ttest_grep ! \"ls-remote> server-option=foo\" log &&\n     ++\ttest_grep ! \"ls-remote> server-option=bar\" log &&\n     ++\trm -f log &&\n     ++\n     ++\t# Server option from command line overrides those from configuration\n     ++\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n     ++\t\tls-remote -o hello -o world origin main >actual &&\n     ++\ttest_cmp expect actual &&\n     ++\ttest_grep \"ls-remote> server-option=hello\" log &&\n     ++\ttest_grep \"ls-remote> server-option=world\" log &&\n     ++\ttest_grep ! \"ls-remote> server-option=tar\" log\n     ++'\n     ++\n     + test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n     + \ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n     + \t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" main 2>err &&\n     +@@ t/t5702-protocol-v2.sh: test_expect_success 'server-options are sent when fetching' '\n     + \tgrep \"server-option=world\" log\n     + '\n       \n     --\tif (server_options.nr)\n     --\t\ttransport->server_options = &server_options;\n     -+\tif (server_options && server_options->nr)\n     -+\t\ttransport->server_options = server_options;\n     ++test_expect_success 'server-options from configuration are used by git-fetch' '\n     ++\ttest_when_finished \"rm -rf log myclone\" &&\n     ++\tgit clone \"file://$(pwd)/file_parent\" myclone &&\n     ++\tgit -C file_parent log -1 --format=%s >expect &&\n     ++\n     ++\t# Default server options from configuration are used\n     ++\tgit -C myclone config --add remote.origin.serverOption foo &&\n     ++\tgit -C myclone config --add remote.origin.serverOption bar &&\n     ++\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n     ++\t\tfetch origin main &&\n     ++\tgit -C myclone log -1 --format=%s origin/main >actual &&\n     ++\ttest_cmp expect actual &&\n     ++\ttest_grep \"fetch> server-option=foo\" log &&\n     ++\ttest_grep \"fetch> server-option=bar\" log &&\n     ++\trm -f log &&\n     ++\n     ++\t# Empty value of remote.<name>.serverOption clears the list\n     ++\tgit -C myclone config --add remote.origin.serverOption \"\" &&\n     ++\tgit -C myclone config --add remote.origin.serverOption tar &&\n     ++\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n     ++\t\tfetch origin main &&\n     ++\tgit -C myclone log -1 --format=%s origin/main >actual &&\n     ++\ttest_cmp expect actual &&\n     ++\ttest_grep \"fetch> server-option=tar\" log &&\n     ++\ttest_grep ! \"fetch> server-option=foo\" log &&\n     ++\ttest_grep ! \"fetch> server-option=bar\" log &&\n     ++\trm -f log &&\n     ++\n     ++\t# Server option from command line overrides those from configuration\n     ++\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n     ++\t\tfetch -o hello -o world origin main &&\n     ++\tgit -C myclone log -1 --format=%s origin/main >actual &&\n     ++\ttest_cmp expect actual &&\n     ++\ttest_grep \"fetch> server-option=hello\" log &&\n     ++\ttest_grep \"fetch> server-option=world\" log &&\n     ++\ttest_grep ! \"fetch> server-option=tar\" log\n     ++'\n     ++\n     + test_expect_success 'warn if using server-option with fetch with legacy protocol' '\n     + \ttest_when_finished \"rm -rf temp_child\" &&\n       \n     - \tif (filter_options.choice) {\n     - \t\tconst char *spec =\n     -\n     - ## t/t5702-protocol-v2.sh ##\n     -@@ t/t5702-protocol-v2.sh: test_expect_success 'warn if using server-option with fetch with legacy protocol\n     - test_expect_success 'server-options are sent when cloning' '\n     - \ttest_when_finished \"rm -rf log myclone\" &&\n     +@@ t/t5702-protocol-v2.sh: test_expect_success 'server-options are sent when cloning' '\n     + \tgrep \"server-option=world\" log\n     + '\n       \n     -+\t# Specify server options from command line\n     - \tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n     - \t\tclone --server-option=hello --server-option=world \\\n     - \t\t\"file://$(pwd)/file_parent\" myclone &&\n     -+\ttest_grep \"server-option=hello\" log &&\n     -+\ttest_grep \"server-option=world\" log &&\n     ++test_expect_success 'server-options from configuration are used by git-clone' '\n     ++\ttest_when_finished \"rm -rf log myclone\" &&\n     ++\n     ++\t# Default server options from configuration are used\n     ++\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n     ++\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n     ++\t\tclone \"file://$(pwd)/file_parent\" myclone &&\n     ++\ttest_grep \"clone> server-option=foo\" log &&\n     ++\ttest_grep \"clone> server-option=bar\" log &&\n      +\trm -rf log myclone &&\n     - \n     --\tgrep \"server-option=hello\" log &&\n     --\tgrep \"server-option=world\" log\n     -+\t# Specify server options from fetch.serverOption config\n     ++\n     ++\t# Empty value of remote.<name>.serverOption clears the list\n      +\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n     -+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n     ++\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n     ++\t\t-c remote.origin.serverOption= -c remote.origin.serverOption=tar \\\n      +\t\tclone \"file://$(pwd)/file_parent\" myclone &&\n     -+\ttest_grep \"server-option=hello\" log &&\n     -+\ttest_grep \"server-option=world\" log &&\n     ++\ttest_grep \"clone> server-option=tar\" log &&\n     ++\ttest_grep ! \"clone> server-option=foo\" log &&\n     ++\ttest_grep ! \"clone> server-option=bar\" log &&\n      +\trm -rf log myclone &&\n      +\n     -+\t# Cmdline server options take a higher priority\n     ++\t# Server option from command line overrides those from configuration\n      +\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n     -+\t\t-c fetch.serverOption=hello -c fetch.serverOption=world \\\n     -+\t\tclone --server-option=foo=bar \\\n     ++\t\t-c remote.origin.serverOption=tar \\\n     ++\t\tclone --server-option=hello --server-option=world \\\n      +\t\t\"file://$(pwd)/file_parent\" myclone &&\n     -+\ttest_grep ! \"server-option=hello\" log &&\n     -+\ttest_grep ! \"server-option=world\" log &&\n     -+\ttest_grep \"server-option=foo=bar\" log\n     ++\ttest_grep \"clone> server-option=hello\" log &&\n     ++\ttest_grep \"clone> server-option=world\" log &&\n     ++\ttest_grep ! \"clone> server-option=tar\" log\n     ++'\n     ++\n     + test_expect_success 'warn if using server-option with clone with legacy protocol' '\n     + \ttest_when_finished \"rm -rf myclone\" &&\n     + \n     +@@ t/t5702-protocol-v2.sh: test_expect_success 'warn if using server-option with clone with legacy protocol\n     + \ttest_grep \"server options require protocol version 2 or later\" err\n       '\n       \n     - test_expect_success 'warn if using server-option with clone with legacy protocol' '\n     ++test_expect_success 'server-option configuration with legacy protocol is ok' '\n     ++\ttest_when_finished \"rm -rf myclone\" &&\n     ++\n     ++\tenv GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n     ++\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n     ++\t\tclone \"file://$(pwd)/file_parent\" myclone\n     ++'\n     ++\n     ++test_expect_success 'invalid server-option configuration' '\n     ++\ttest_when_finished \"rm -rf myclone\" &&\n     ++\n     ++\ttest_must_fail git -c protocol.version=2 \\\n     ++\t\t-c remote.origin.serverOption \\\n     ++\t\tclone \"file://$(pwd)/file_parent\" myclone 2>err &&\n     ++\ttest_grep \"error: missing value for '\\''remote.origin.serveroption'\\''\" err\n     ++'\n     ++\n     + test_expect_success 'upload-pack respects config using protocol v2' '\n     + \tgit init server &&\n     + \twrite_script server/.git/hook <<-\\EOF &&\n     +\n     + ## transport.c ##\n     +@@ transport.c: static struct ref *handshake(struct transport *transport, int for_push,\n     + \tdata->version = discover_version(&reader);\n     + \tswitch (data->version) {\n     + \tcase protocol_v2:\n     ++\t\tif ((!transport->server_options || !transport->server_options->nr) &&\n     ++\t\t    transport->remote->server_options.nr)\n     ++\t\t\ttransport->server_options = &transport->remote->server_options;\n     + \t\tif (server_feature_v2(\"session-id\", &server_sid))\n     + \t\t\ttrace2_data_string(\"transfer\", NULL, \"server-sid\", server_sid);\n     + \t\tif (must_list_refs)\n 4:  8962031f999 < -:  ----------- builtin/ls-remote.c: recognize fetch.serverOption configuration\n -:  ----------- > 4:  39ee8dbef78 fetch: respect --server-option when fetching multiple remotes\n -:  ----------- > 5:  39c07a6c8ee ls-remote: leakfix for not clearing server_options\n\n-- \ngitgitgadget\n"},{"id":"503251","messageId":"c95ed5e0dd58f01db6a390b23c5dcbaaa7a4e43e.1727093878.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","subject":"[PATCH v2 1/5] transport: introduce parse_transport_option() method","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-23T12:17:54Z","receivedAt":"2024-09-23T12:18:03Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nAdd the `parse_transport_option()` method to parse the `push.pushOption`\nconfiguration. This method will also be used in the next commit to\nhandle the new `remote.<name>.serverOption` configuration for setting\nserver options in Git protocol v2.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n builtin/push.c |  9 +--------\n transport.c    | 12 ++++++++++++\n transport.h    |  4 ++++\n 3 files changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 7a67398124f..290a08f6ef2 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -517,14 +517,7 @@ static int git_push_config(const char *k, const char *v,\n \t\t\tRECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;\n \t\trecurse_submodules = val;\n \t} else if (!strcmp(k, \"push.pushoption\")) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\telse\n-\t\t\tif (!*v)\n-\t\t\t\tstring_list_clear(&push_options_config, 0);\n-\t\t\telse\n-\t\t\t\tstring_list_append(&push_options_config, v);\n-\t\treturn 0;\n+\t\treturn parse_transport_option(k, v, &push_options_config);\n \t} else if (!strcmp(k, \"color.push\")) {\n \t\tpush_use_color = git_config_colorbool(k, v);\n \t\treturn 0;\ndiff --git a/transport.c b/transport.c\nindex bab28965f96..c8947415eec 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1091,6 +1091,18 @@ int is_transport_allowed(const char *type, int from_user)\n \tBUG(\"invalid protocol_allow_config type\");\n }\n \n+int parse_transport_option(const char *var, const char *value,\n+\t\t\t   struct string_list *transport_options)\n+{\n+\tif (!value)\n+\t\treturn config_error_nonbool(var);\n+\tif (!*value)\n+\t\tstring_list_clear(transport_options, 0);\n+\telse\n+\t\tstring_list_append(transport_options, value);\n+\treturn 0;\n+}\n+\n void transport_check_allowed(const char *type)\n {\n \tif (!is_transport_allowed(type, -1))\ndiff --git a/transport.h b/transport.h\nindex 6393cd9823c..44100fa9b7f 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -342,4 +342,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n /* common method used by transport-helper.c and send-pack.c */\n void reject_atomic_push(struct ref *refs, int mirror_mode);\n \n+/* common method to parse push-option or server-option from config */\n+int parse_transport_option(const char *var, const char *value,\n+\t\t\t   struct string_list *transport_options);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"503252","messageId":"2474b4c69d68576e901cdd6381aac9410cf21922.1727093878.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","subject":"[PATCH v2 2/5] remote: introduce remote.<name>.serverOption configuration","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-23T12:17:55Z","receivedAt":"2024-09-23T12:18:07Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nCurrently, server options for Git protocol v2 can only be specified via\nthe command line option \"--server-option\" or \"-o\", which is inconvenient\nwhen users want to specify a list of default options to send. Therefore,\nwe are introducing a new configuration to hold a list of default server\noptions, akin to the `push.pushOption` configuration for push options.\n\nInitially, I named the new configuration `fetch.serverOption` to align\nwith `push.pushOption`. However, after discussing with Patrick, it was\nrenamed to `remote.<name>.serverOption` as suggested, because:\n\n1. Server options are designed to be server-specific, making it more\n   logical to use a per-remote configuration.\n2. Using \"fetch.\" prefixed configurations in git-clone or git-ls-remote\n   seems out of place and inconsistent in design.\n\nThe parsing logic for `remote.<name>.serverOption` also relies on\n`transport.c:parse_transport_option`, similar to `push.pushOption`, and\nthey follow the same priority design:\n\n1. Server options set in lower-priority configuration files (e.g.,\n   /etc/gitconfig or $HOME/.gitconfig) can be overridden or unset in\n   more specific repository configurations using an empty string.\n2. Command-line specified server options take precedence over those from\n   the configuration.\n\nServer options from configuration are stored to the corresponding\n`remote.h:remote` as a new field `server_options`.  The field will be\nutilized in the subsequent commit to help initialize the\n`server_options` of `transport.h:transport`.\n\nAnd documentation have been updated accordingly.\n\nHelped-by: Patrick Steinhardt <ps@pks.im>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nReported-by: Liu Zhongbo <liuzhongbo.6666@bytedance.com>\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n Documentation/config/remote.txt | 10 ++++++++++\n remote.c                        |  7 +++++++\n remote.h                        |  3 +++\n 3 files changed, 20 insertions(+)\n\ndiff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt\nindex 8efc53e836d..8ba48573fb6 100644\n--- a/Documentation/config/remote.txt\n+++ b/Documentation/config/remote.txt\n@@ -95,3 +95,13 @@ remote.<name>.partialclonefilter::\n \tChanging or clearing this value will only affect fetches for new commits.\n \tTo fetch associated objects for commits already present in the local object\n \tdatabase, use the `--refetch` option of linkgit:git-fetch[1].\n+\n+remote.<name>.serverOption::\n+\tWhen no `--server-option=<option>` argument is given from the command\n+\tline, git will use the values from this configuration as a default list of\n+\tserver options for this remote.\n++\n+This is a multi-valued variable, and an empty value can be used in a higher\n+priority configuration file (e.g. `.git/config` in a repository) to clear\n+the values inherited from a lower priority configuration files (e.g.\n+`$HOME/.gitconfig`).\ndiff --git a/remote.c b/remote.c\nindex 7d5b8f750d8..fe11ead27ad 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -24,6 +24,7 @@\n #include \"advice.h\"\n #include \"connect.h\"\n #include \"parse-options.h\"\n+#include \"transport.h\"\n \n enum map_direction { FROM_SRC, FROM_DST };\n \n@@ -125,6 +126,7 @@ static struct remote *make_remote(struct remote_state *remote_state,\n \tstruct remote *ret;\n \tstruct remotes_hash_key lookup;\n \tstruct hashmap_entry lookup_entry, *e;\n+\tstruct string_list server_options = STRING_LIST_INIT_DUP;\n \n \tif (!len)\n \t\tlen = strlen(name);\n@@ -143,6 +145,7 @@ static struct remote *make_remote(struct remote_state *remote_state,\n \tret->name = xstrndup(name, len);\n \trefspec_init(&ret->push, REFSPEC_PUSH);\n \trefspec_init(&ret->fetch, REFSPEC_FETCH);\n+\tret->server_options = server_options;\n \n \tALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,\n \t\t   remote_state->remotes_alloc);\n@@ -166,6 +169,7 @@ static void remote_clear(struct remote *remote)\n \tfree((char *)remote->uploadpack);\n \tFREE_AND_NULL(remote->http_proxy);\n \tFREE_AND_NULL(remote->http_proxy_authmethod);\n+\tstring_list_clear(&remote->server_options, 0);\n }\n \n static void add_merge(struct branch *branch, const char *name)\n@@ -482,6 +486,9 @@ static int handle_config(const char *key, const char *value,\n \t\t\t\t\t key, value);\n \t} else if (!strcmp(subkey, \"vcs\")) {\n \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n+\t} else if (!strcmp(subkey, \"serveroption\")) {\n+\t\treturn parse_transport_option(key, value,\n+\t\t\t\t\t      &remote->server_options);\n \t}\n \treturn 0;\n }\ndiff --git a/remote.h b/remote.h\nindex b901b56746d..d9926a76d23 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -4,6 +4,7 @@\n #include \"hash.h\"\n #include \"hashmap.h\"\n #include \"refspec.h\"\n+#include \"string-list.h\"\n #include \"strvec.h\"\n \n struct option;\n@@ -104,6 +105,8 @@ struct remote {\n \n \t/* The method used for authenticating against `http_proxy`. */\n \tchar *http_proxy_authmethod;\n+\n+\tstruct string_list server_options;\n };\n \n /**\n-- \ngitgitgadget\n\n"},{"id":"503253","messageId":"a7f3e458501ab22f51269d6e63ceeeb7ea488d91.1727093878.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","subject":"[PATCH v2 3/5] transport.c::handshake: make use of server options from remote","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-23T12:17:56Z","receivedAt":"2024-09-23T12:18:08Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nUtilize the `server_options` from the corresponding remote during the\nhandshake in `transport.c` when Git protocol v2 is detected. This helps\ninitialize the `server_options` in `transport.h:transport` if no server\noptions are set for the transport (typically via `--server-option` or\n`-o`).\n\nWhile another potential place to incorporate server options from the\nremote is in `transport.c:transport_get`, setting server options for a\ntransport using a protocol other than v2 could lead to unexpected errors\n(see `transport.c:die_if_server_options`).\n\nRelevant tests and documentation have been updated accordingly.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n Documentation/fetch-options.txt |   3 +\n Documentation/git-clone.txt     |   3 +\n Documentation/git-ls-remote.txt |   3 +\n t/t5702-protocol-v2.sh          | 123 ++++++++++++++++++++++++++++++++\n transport.c                     |   3 +\n 5 files changed, 135 insertions(+)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex e22b217fba9..424cdeafcdd 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -304,6 +304,9 @@ endif::git-pull[]\n \tunknown ones, is server-specific.\n \tWhen multiple `--server-option=<option>` are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no `--server-option=<option>` is given from the command line,\n+\tthe values of configuration variable `remote.<name>.serverOption`\n+\tare used instead.\n \n --show-forced-updates::\n \tBy default, git checks if a branch is force-updated during\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 8e925db7e9c..409fb3a3af6 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.\n \tunknown ones, is server-specific.\n \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no ++--server-option=++__<option>__ is given from the command\n+\tline, the values of configuration variable `remote.<name>.serverOption`\n+\tare used instead.\n \n `-n`::\n `--no-checkout`::\ndiff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\nindex 76c86c3ce4f..d71c4ab3e29 100644\n--- a/Documentation/git-ls-remote.txt\n+++ b/Documentation/git-ls-remote.txt\n@@ -81,6 +81,9 @@ OPTIONS\n \tcharacter.\n \tWhen multiple `--server-option=<option>` are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no `--server-option=<option>` is given from the command line,\n+\tthe values of configuration variable `remote.<name>.serverOption`\n+\tare used instead.\n \n <repository>::\n \tThe \"remote\" repository to query.  This parameter can be\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1ef540f73d3..5cec2061d28 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -185,6 +185,43 @@ test_expect_success 'server-options are sent when using ls-remote' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options from configuration are used by ls-remote' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\tgit clone \"file://$(pwd)/file_parent\" myclone &&\n+\tcat >expect <<-EOF &&\n+\t$(git -C file_parent rev-parse refs/heads/main)$(printf \"\\t\")refs/heads/main\n+\tEOF\n+\n+\t# Default server options from configuration are used\n+\tgit -C myclone config --add remote.origin.serverOption foo &&\n+\tgit -C myclone config --add remote.origin.serverOption bar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tls-remote origin main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"ls-remote> server-option=foo\" log &&\n+\ttest_grep \"ls-remote> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Empty value of remote.<name>.serverOption clears the list\n+\tgit -C myclone config --add remote.origin.serverOption \"\" &&\n+\tgit -C myclone config --add remote.origin.serverOption tar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tls-remote origin main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"ls-remote> server-option=tar\" log &&\n+\ttest_grep ! \"ls-remote> server-option=foo\" log &&\n+\ttest_grep ! \"ls-remote> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Server option from command line overrides those from configuration\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tls-remote -o hello -o world origin main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"ls-remote> server-option=hello\" log &&\n+\ttest_grep \"ls-remote> server-option=world\" log &&\n+\ttest_grep ! \"ls-remote> server-option=tar\" log\n+'\n+\n test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n \ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n \t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" main 2>err &&\n@@ -381,6 +418,44 @@ test_expect_success 'server-options are sent when fetching' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options from configuration are used by git-fetch' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\tgit clone \"file://$(pwd)/file_parent\" myclone &&\n+\tgit -C file_parent log -1 --format=%s >expect &&\n+\n+\t# Default server options from configuration are used\n+\tgit -C myclone config --add remote.origin.serverOption foo &&\n+\tgit -C myclone config --add remote.origin.serverOption bar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tfetch origin main &&\n+\tgit -C myclone log -1 --format=%s origin/main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"fetch> server-option=foo\" log &&\n+\ttest_grep \"fetch> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Empty value of remote.<name>.serverOption clears the list\n+\tgit -C myclone config --add remote.origin.serverOption \"\" &&\n+\tgit -C myclone config --add remote.origin.serverOption tar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tfetch origin main &&\n+\tgit -C myclone log -1 --format=%s origin/main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"fetch> server-option=tar\" log &&\n+\ttest_grep ! \"fetch> server-option=foo\" log &&\n+\ttest_grep ! \"fetch> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Server option from command line overrides those from configuration\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tfetch -o hello -o world origin main &&\n+\tgit -C myclone log -1 --format=%s origin/main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"fetch> server-option=hello\" log &&\n+\ttest_grep \"fetch> server-option=world\" log &&\n+\ttest_grep ! \"fetch> server-option=tar\" log\n+'\n+\n test_expect_success 'warn if using server-option with fetch with legacy protocol' '\n \ttest_when_finished \"rm -rf temp_child\" &&\n \n@@ -404,6 +479,37 @@ test_expect_success 'server-options are sent when cloning' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options from configuration are used by git-clone' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\n+\t# Default server options from configuration are used\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"clone> server-option=foo\" log &&\n+\ttest_grep \"clone> server-option=bar\" log &&\n+\trm -rf log myclone &&\n+\n+\t# Empty value of remote.<name>.serverOption clears the list\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n+\t\t-c remote.origin.serverOption= -c remote.origin.serverOption=tar \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"clone> server-option=tar\" log &&\n+\ttest_grep ! \"clone> server-option=foo\" log &&\n+\ttest_grep ! \"clone> server-option=bar\" log &&\n+\trm -rf log myclone &&\n+\n+\t# Server option from command line overrides those from configuration\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption=tar \\\n+\t\tclone --server-option=hello --server-option=world \\\n+\t\t\"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"clone> server-option=hello\" log &&\n+\ttest_grep \"clone> server-option=world\" log &&\n+\ttest_grep ! \"clone> server-option=tar\" log\n+'\n+\n test_expect_success 'warn if using server-option with clone with legacy protocol' '\n \ttest_when_finished \"rm -rf myclone\" &&\n \n@@ -415,6 +521,23 @@ test_expect_success 'warn if using server-option with clone with legacy protocol\n \ttest_grep \"server options require protocol version 2 or later\" err\n '\n \n+test_expect_success 'server-option configuration with legacy protocol is ok' '\n+\ttest_when_finished \"rm -rf myclone\" &&\n+\n+\tenv GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n+\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone\n+'\n+\n+test_expect_success 'invalid server-option configuration' '\n+\ttest_when_finished \"rm -rf myclone\" &&\n+\n+\ttest_must_fail git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone 2>err &&\n+\ttest_grep \"error: missing value for '\\''remote.origin.serveroption'\\''\" err\n+'\n+\n test_expect_success 'upload-pack respects config using protocol v2' '\n \tgit init server &&\n \twrite_script server/.git/hook <<-\\EOF &&\ndiff --git a/transport.c b/transport.c\nindex c8947415eec..bcfd4199c87 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -332,6 +332,9 @@ static struct ref *handshake(struct transport *transport, int for_push,\n \tdata->version = discover_version(&reader);\n \tswitch (data->version) {\n \tcase protocol_v2:\n+\t\tif ((!transport->server_options || !transport->server_options->nr) &&\n+\t\t    transport->remote->server_options.nr)\n+\t\t\ttransport->server_options = &transport->remote->server_options;\n \t\tif (server_feature_v2(\"session-id\", &server_sid))\n \t\t\ttrace2_data_string(\"transfer\", NULL, \"server-sid\", server_sid);\n \t\tif (must_list_refs)\n-- \ngitgitgadget\n\n"},{"id":"503254","messageId":"39ee8dbef783f425e6a8b01a68ac01fde304b698.1727093878.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","subject":"[PATCH v2 4/5] fetch: respect --server-option when fetching multiple remotes","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-23T12:17:57Z","receivedAt":"2024-09-23T12:18:09Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nFix an issue where server options specified via the command line\n(`--server-option` or `-o`) were not sent when fetching from multiple\nremotes using Git protocol v2.\n\nTo reproduce the issue with a repository containing multiple remotes:\n\n  GIT_TRACE_PACKET=1 git -c protocol.version=2 fetch --server-option=demo --all\n\nObserve that no server options are sent to any remote.\n\nThe root cause was identified in `builtin/fetch.c:fetch_multiple`, which\nis invoked when fetching from more than one remote. This function forks\na `git-fetch` subprocess for each remote but did not include the\nspecified server options in the subprocess arguments.\n\nThis commit ensures that command-line specified server options are\nproperly passed to each subprocess. Relevant tests have been added.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n builtin/fetch.c        |  2 ++\n t/t5702-protocol-v2.sh | 10 ++++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex c297569a473..c1b3aea7745 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1980,6 +1980,8 @@ static int fetch_multiple(struct string_list *list, int max_children,\n \tstrvec_pushl(&argv, \"-c\", \"fetch.bundleURI=\",\n \t\t     \"fetch\", \"--append\", \"--no-auto-gc\",\n \t\t     \"--no-write-commit-graph\", NULL);\n+\tfor (i = 0; i < server_options.nr; i++)\n+\t\tstrvec_pushf(&argv, \"--server-option=%s\", server_options.items[i].string);\n \tadd_options_to_argv(&argv, config);\n \n \tif (max_children != 1 && list->nr != 1) {\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 5cec2061d28..d3df81e7852 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -418,6 +418,16 @@ test_expect_success 'server-options are sent when fetching' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options are sent when fetch multiple remotes' '\n+\ttest_when_finished \"rm -f log server_options_sent\" &&\n+\tgit clone \"file://$(pwd)/file_parent\" child_multi_remotes &&\n+\tgit -C child_multi_remotes remote add another \"file://$(pwd)/file_parent\" &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C child_multi_remotes -c protocol.version=2 \\\n+\t\tfetch -o hello --all &&\n+\tgrep \"fetch> server-option=hello\" log >server_options_sent &&\n+\ttest_line_count = 2 server_options_sent\n+'\n+\n test_expect_success 'server-options from configuration are used by git-fetch' '\n \ttest_when_finished \"rm -rf log myclone\" &&\n \tgit clone \"file://$(pwd)/file_parent\" myclone &&\n-- \ngitgitgadget\n\n"},{"id":"503255","messageId":"39c07a6c8eed645e22ae55affd015ee55ec59571.1727093878.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","subject":"[PATCH v2 5/5] ls-remote: leakfix for not clearing server_options","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-23T12:17:58Z","receivedAt":"2024-09-23T12:18:09Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nEnsure `server_options` is properly cleared using `string_list_clear()`\nin `builtin/ls-remote.c:cmd_ls_remote`.\n\nAlthough we cannot yet enable `TEST_PASSES_SANITIZE_LEAK=true` for\n`t/t5702-protocol-v2.sh` due to other existing leaks, this fix ensures\nthat \"git-ls-remote\" related server options tests pass the sanitize leak\ncheck:\n\n  ...\n  ok 12 - server-options are sent when using ls-remote\n  ok 13 - server-options from configuration are used by ls-remote\n  ...\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n builtin/ls-remote.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 0a491595ca8..c3fdda08409 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -169,5 +169,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \ttransport_ls_refs_options_release(&transport_options);\n \n \tstrvec_clear(&pattern);\n+\tstring_list_clear(&server_options, 0);\n \treturn status;\n }\n-- \ngitgitgadget\n"},{"id":"503257","messageId":"7b01a69b.bdf6.1921efb807c.Coremail.bupt_xingxin@163.com","threadId":"62037","inReplyTo":"Zt7gufVmnMIGraNx@pks.im","subject":"Re:Re: Re: Re: Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration","fromName":"Xing Xin","fromEmail":"bupt_xingxin@163.com","sentAt":"2024-09-23T13:04:11Z","receivedAt":"2024-09-23T13:06:30Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"At 2024-09-09 19:49:16, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n>On Mon, Sep 09, 2024 at 10:50:21AM +0800, Xing Xin wrote:\n>> At 2024-09-05 21:44:41, \"Patrick Steinhardt\" <ps@pks.im> wrote\n>> \n>> [snip]\n>> \n>> >> >> I named the new configuration `fetch.serverOption` mainly to follow the\n>> >> >> `push.pushOption` pattern.  Since which server options to support is\n>> >> >> actually server-specific, using `remote.<name>.serverOption` is a good\n>> >> >> idea for git-fetch. However, how should we design the configuration for\n>> >> >> git-ls-remote or git-clone, if we wanna provide all of them with a\n>> >> >> default list of server options to send?\n>> >> >\n>> >> >As mentioned in another reply, I think that putting this into the remote\n>> >> >configuration \"remote.*.serverOption\" might be a better solution, as it\n>> >> >also brings you the ability to set this per remote by default.\n>> >> \n>> >> I agree that using \"remote.*.serverOption\" is better. In fact, I also\n>> >> think \"push.pushOption\" would be better as \"remote.*.pushOption\". What I'm\n>> >> contemplating is whether we need to add a configuration for a default\n>> >> list of server options, so that when \"remote.origin.serverOption\" is not\n>> >> present, we can fall back to use that as default.\n>> >\n>> >Junio proposed in [1] to introduce `[remote \"*\"]` syntax to supply\n>> >default values to all remotes. You could pick up that proposal and\n>> >implement it as part of your patch series.\n>> \n>> Given the addition of a remote.*.<subkey> configuration in Git's global\n>> settings, such as:\n>> \n>>     git config --global remote.\"*\".demoConfigKey demoConfigValue\n>> \n>> The current versions of Git may produce errors with certain commands.\n>> For example, running `git fetch --all` will result in:\n>> \n>>     $ git fetch --all\n>>     Fetching *\n>>     fatal: '*' does not appear to be a git repository\n>>     fatal: Could not read from remote repository.\n>> \n>>     Please make sure you have the correct access rights\n>>     and the repository exists.\n>>     error: could not fetch *\n>>     Fetching origin\n>\n>We do not currently handle \"remote.*.something\", so we'd first have to\n>add support for this syntax.\n>\n>> This raises the question of whether this can be defined as an\n>> incompatibility or whether this is acceptable. If it isn't, I prefer\n>> using a `remote.defaultServerOption` instead, as it aligns with our\n>> existing use of `remote.pushDefault` anyway.\n>\n>While for this config key it may work alright, I think supporting\n>\"remote.*.something\" is preferable. Ideally, it would be generic enough\n>to apply to all per-remote settings such that we don't have to add\n>support for every single config key that applies to remotes.\n>\n>Right now we only care about push options and server options. But\n>somebody may want to globally configure proxies, tag options, partial\n>clone filters or something else. And if we got all of that without\n>having to introduce `remote.defaultPruneTags`,\n>`remote.defaultPartialCloneFilter` and similar options I'd consider it a\n>win.\n\nAfter some consideration, I tend not to implement a `remote.<subkey>`\nor `remote.*.<subkey>` as a fallback default in this patch series.\n\nIn fact I attempted to use `remote.serverOption` and extended a new\n`server_options` field in `remote.h:remote_state` to store its parsing\nresult. However, I think this approach isn't general enough (it only\ndiffers in name from the `remote.defaultServerOption`).\n\nImplementing a mechanism similar to `http.<url>.<subkey>` and\n`http.<subkey>`(the fallback default) seems a better approach, but it\ninvolves more refactoring, which is too much to do at this time.\n\nI have just submitted my v2 series, which only adds the\n`remote.<name>.serverOption` configuration.\n\n\nXing Xin\n"},{"id":"504266","messageId":"ZwOaR5OqdLaS4cu6@pks.im","threadId":"62037","inReplyTo":"2474b4c69d68576e901cdd6381aac9410cf21922.1727093878.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/5] remote: introduce remote.<name>.serverOption configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T08:22:31Z","receivedAt":"2024-10-07T08:22:37Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 23, 2024 at 12:17:55PM +0000, Xing Xin via GitGitGadget wrote:\n> diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt\n> index 8efc53e836d..8ba48573fb6 100644\n> --- a/Documentation/config/remote.txt\n> +++ b/Documentation/config/remote.txt\n> @@ -95,3 +95,13 @@ remote.<name>.partialclonefilter::\n>  \tChanging or clearing this value will only affect fetches for new commits.\n>  \tTo fetch associated objects for commits already present in the local object\n>  \tdatabase, use the `--refetch` option of linkgit:git-fetch[1].\n> +\n> +remote.<name>.serverOption::\n> +\tWhen no `--server-option=<option>` argument is given from the command\n> +\tline, git will use the values from this configuration as a default list of\n\nThis description feels a bit inelegant to me. How about the following:\n\n    The default set of server options used when fetching from the\n    remote. These server options are overridden by the\n    `--server-option=` command line arguments.\n\n> @@ -125,6 +126,7 @@ static struct remote *make_remote(struct remote_state *remote_state,\n>  \tstruct remote *ret;\n>  \tstruct remotes_hash_key lookup;\n>  \tstruct hashmap_entry lookup_entry, *e;\n> +\tstruct string_list server_options = STRING_LIST_INIT_DUP;\n>  \n>  \tif (!len)\n>  \t\tlen = strlen(name);\n> @@ -143,6 +145,7 @@ static struct remote *make_remote(struct remote_state *remote_state,\n>  \tret->name = xstrndup(name, len);\n>  \trefspec_init(&ret->push, REFSPEC_PUSH);\n>  \trefspec_init(&ret->fetch, REFSPEC_FETCH);\n> +\tret->server_options = server_options;\n\nI was wondering at first where we actually populate the\n`server_options`, until I realized that this is only done to initialize\nthe `ret->server_options`. I think it would be less confusing if this\nused `string_list_init_dup()` instead of the local variable.\n\nPatrick\n"},{"id":"504267","messageId":"ZwOaTJ95r3Jc_9n3@pks.im","threadId":"62037","inReplyTo":"39ee8dbef783f425e6a8b01a68ac01fde304b698.1727093878.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 4/5] fetch: respect --server-option when fetching multiple remotes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T08:22:36Z","receivedAt":"2024-10-07T08:22:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 23, 2024 at 12:17:57PM +0000, Xing Xin via GitGitGadget wrote:\n> From: Xing Xin <xingxin.xx@bytedance.com>\n> \n> Fix an issue where server options specified via the command line\n> (`--server-option` or `-o`) were not sent when fetching from multiple\n> remotes using Git protocol v2.\n> \n> To reproduce the issue with a repository containing multiple remotes:\n> \n>   GIT_TRACE_PACKET=1 git -c protocol.version=2 fetch --server-option=demo --all\n> \n> Observe that no server options are sent to any remote.\n> \n> The root cause was identified in `builtin/fetch.c:fetch_multiple`, which\n> is invoked when fetching from more than one remote. This function forks\n> a `git-fetch` subprocess for each remote but did not include the\n> specified server options in the subprocess arguments.\n> \n> This commit ensures that command-line specified server options are\n> properly passed to each subprocess. Relevant tests have been added.\n\nNice catch.\n\nPatrick\n"},{"id":"504268","messageId":"ZwOaUA5pCc6k0k5d@pks.im","threadId":"62037","inReplyTo":"39c07a6c8eed645e22ae55affd015ee55ec59571.1727093878.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 5/5] ls-remote: leakfix for not clearing server_options","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T08:22:40Z","receivedAt":"2024-10-07T08:22:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 23, 2024 at 12:17:58PM +0000, Xing Xin via GitGitGadget wrote:\n> From: Xing Xin <xingxin.xx@bytedance.com>\n> \n> Ensure `server_options` is properly cleared using `string_list_clear()`\n> in `builtin/ls-remote.c:cmd_ls_remote`.\n> \n> Although we cannot yet enable `TEST_PASSES_SANITIZE_LEAK=true` for\n> `t/t5702-protocol-v2.sh` due to other existing leaks, this fix ensures\n> that \"git-ls-remote\" related server options tests pass the sanitize leak\n> check:\n> \n>   ...\n>   ok 12 - server-options are sent when using ls-remote\n>   ok 13 - server-options from configuration are used by ls-remote\n>   ...\n> \n> Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>\n> ---\n>  builtin/ls-remote.c | 1 +\n>  1 file changed, 1 insertion(+)\n> \n> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\n> index 0a491595ca8..c3fdda08409 100644\n> --- a/builtin/ls-remote.c\n> +++ b/builtin/ls-remote.c\n> @@ -169,5 +169,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n>  \ttransport_ls_refs_options_release(&transport_options);\n>  \n>  \tstrvec_clear(&pattern);\n> +\tstring_list_clear(&server_options, 0);\n>  \treturn status;\n>  }\n\nIndeed. I've also got a patch for this pending as part of my memory leak\nseries, but didn't yet send it out. So I'm happy when this lands\nindependently so that I have to carry one patch less.\n\nPatrick\n"},{"id":"504269","messageId":"ZwOaZkxETwdId9Qg@pks.im","threadId":"62037","inReplyTo":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/5] Support server option from configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T08:23:02Z","receivedAt":"2024-10-07T08:23:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 23, 2024 at 12:17:53PM +0000, blanet via GitGitGadget wrote:\n> We have multiple internal repositories configured with a huge number of CI\n> tasks, each requiring code preparation (either via clone or fetch). These CI\n> tasks, triggered by post-receive hooks, often fetch the same code (usually\n> using --depth=1) concurrently.\n> \n> To reduce performance impacts on the git server caused by these tasks, we\n> want to deploy a special designed pack-objects-hook. This hook would allow\n> the packs generated by git-pack-objects to be reused. Since not all\n> clone/fetch operations will benefit from this caching (e.g., pulls from\n> developers' environments), clients need to pass a special identifier to\n> indicate whether caching should be enabled for a particular operation. Using\n> server options is a suitable method for this.\n> \n> However, server options can only be specified via the command line option\n> (via --server-option or -o), which is inconvenient and requires CI script\n> modifications. A configuration-based approach is easier to propagate (by\n> changing the global configuration ~/.gitconfig) and avoids issues with older\n> Git versions that don't support --server-option.\n> \n> This patch series introduces a new multi-valued configuration,\n> remote.<name>.serverOption, similar to push.pushOption, to specify default\n> server options for the corresponding remote.\n> \n>  * Patch 1~3 is the main change for introducing the new configuration.\n>  * Patch 4 fixes a issue for git-fetch not sending server-options when\n>    fetching multiple remotes.\n>  * Patch 5 is a minor fix for a server options-related memory leak.\n\nSorry for taking this long to get back to this patch series. I've given\nit another careful read and have a single proposal for improved docs\nthat you may want to address. But other than those nits this version\nlooks great to me, thanks!\n\nPatrick\n"},{"id":"504378","messageId":"pull.1776.v3.git.git.1728358699.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v2.git.git.1727093878.gitgitgadget@gmail.com","subject":"[PATCH v3 0/5] Support server option from configuration","fromName":"blanet via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-08T03:38:14Z","receivedAt":"2024-10-08T03:38:23Z","isPatch":true,"sender":{"key":"name:blanet","avatar":null},"body":"We manage some internal repositories with numerous CI tasks, each requiring\ncode preparation through git-clone or git-fetch. These tasks, triggered by\npost-receive hooks, often fetch the same copy of code concurrently using\n--depth=1, causing extremely high load spikes on our Git servers.\n\nTo reduce performance impacts caused by these tasks, we plan to deploy a\nspecially designed pack-objects-hook [1]. This hook would allow the packs\ngenerated by git-pack-objects(during git-clone or git-fetch) to be reused.\nSince not all clone/fetch operations will benefit from this caching (e.g.,\npulls from developer environments), clients need to pass a special\nidentifier to indicate whether caching should be enabled. Using server\noptions [2] is suitable for this purpose.\n\nHowever, server options can only be specified via the command line option\n(via --server-option or -o), which is inconvenient and requires\nmodifications to CI scripts. A configuration-based approach is preferable,\nas it can be propagated through global configuration (e.g. ~/.gitconfig) and\navoids compatibility issues with older Git versions that don't support\n--server-option.\n\nThis patch series introduces a new multi-valued configuration,\nremote.<name>.serverOption, similar to push.pushOption, to specify default\nserver options for the corresponding remote.\n\n * Patches 1~3 contain the main changes for introducing the new\n   configuration.\n * Patch 4 fixes a issue for git-fetch not sending server-options when\n   fetching from multiple remotes.\n * Patch 5 is a minor fix for a server options-related memory leak.\n\n 1. https://git-scm.com/docs/git-config#Documentation/git-config.txt-uploadpackpackObjectsHook\n 2. https://git-scm.com/docs/gitprotocol-v2#_server_option\n\nXing Xin (5):\n  transport: introduce parse_transport_option() method\n  remote: introduce remote.<name>.serverOption configuration\n  transport.c::handshake: make use of server options from remote\n  fetch: respect --server-option when fetching multiple remotes\n  ls-remote: leakfix for not clearing server_options\n\n Documentation/config/remote.txt |  10 +++\n Documentation/fetch-options.txt |   3 +\n Documentation/git-clone.txt     |   3 +\n Documentation/git-ls-remote.txt |   3 +\n builtin/fetch.c                 |   2 +\n builtin/ls-remote.c             |   1 +\n builtin/push.c                  |   9 +--\n remote.c                        |   6 ++\n remote.h                        |   3 +\n t/t5702-protocol-v2.sh          | 133 ++++++++++++++++++++++++++++++++\n transport.c                     |  15 ++++\n transport.h                     |   4 +\n 12 files changed, 184 insertions(+), 8 deletions(-)\n\n\nbase-commit: 6258f68c3c1092c901337895c864073dcdea9213\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1776%2Fblanet%2Fxx%2Fadd-server-option-from-config-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1776/blanet/xx/add-server-option-from-config-v3\nPull-Request: https://github.com/git/git/pull/1776\n\nRange-diff vs v2:\n\n 1:  c95ed5e0dd5 = 1:  b44face42e1 transport: introduce parse_transport_option() method\n 2:  2474b4c69d6 ! 2:  3c6b129d368 remote: introduce remote.<name>.serverOption configuration\n     @@ Documentation/config/remote.txt: remote.<name>.partialclonefilter::\n       \tdatabase, use the `--refetch` option of linkgit:git-fetch[1].\n      +\n      +remote.<name>.serverOption::\n     -+\tWhen no `--server-option=<option>` argument is given from the command\n     -+\tline, git will use the values from this configuration as a default list of\n     -+\tserver options for this remote.\n     ++\tThe default set of server options used when fetching from this remote.\n     ++\tThese server options can be overridden by the `--server-option=` command\n     ++\tline arguments.\n      ++\n      +This is a multi-valued variable, and an empty value can be used in a higher\n      +priority configuration file (e.g. `.git/config` in a repository) to clear\n     @@ remote.c\n       \n       enum map_direction { FROM_SRC, FROM_DST };\n       \n     -@@ remote.c: static struct remote *make_remote(struct remote_state *remote_state,\n     - \tstruct remote *ret;\n     - \tstruct remotes_hash_key lookup;\n     - \tstruct hashmap_entry lookup_entry, *e;\n     -+\tstruct string_list server_options = STRING_LIST_INIT_DUP;\n     - \n     - \tif (!len)\n     - \t\tlen = strlen(name);\n      @@ remote.c: static struct remote *make_remote(struct remote_state *remote_state,\n       \tret->name = xstrndup(name, len);\n       \trefspec_init(&ret->push, REFSPEC_PUSH);\n       \trefspec_init(&ret->fetch, REFSPEC_FETCH);\n     -+\tret->server_options = server_options;\n     ++\tstring_list_init_dup(&ret->server_options);\n       \n       \tALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,\n       \t\t   remote_state->remotes_alloc);\n     @@ remote.c: static void remote_clear(struct remote *remote)\n       \n       static void add_merge(struct branch *branch, const char *name)\n      @@ remote.c: static int handle_config(const char *key, const char *value,\n     - \t\t\t\t\t key, value);\n       \t} else if (!strcmp(subkey, \"vcs\")) {\n     + \t\tFREE_AND_NULL(remote->foreign_vcs);\n       \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n      +\t} else if (!strcmp(subkey, \"serveroption\")) {\n      +\t\treturn parse_transport_option(key, value,\n 3:  a7f3e458501 = 3:  f0835259b06 transport.c::handshake: make use of server options from remote\n 4:  39ee8dbef78 = 4:  420b15d9f37 fetch: respect --server-option when fetching multiple remotes\n 5:  39c07a6c8ee ! 5:  2528d929c7e ls-remote: leakfix for not clearing server_options\n     @@ Commit message\n          Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>\n      \n       ## builtin/ls-remote.c ##\n     -@@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n     +@@ builtin/ls-remote.c: int cmd_ls_remote(int argc,\n       \ttransport_ls_refs_options_release(&transport_options);\n       \n       \tstrvec_clear(&pattern);\n\n-- \ngitgitgadget\n"},{"id":"504379","messageId":"b44face42e16f2044b36ed4ff64316a23c0caba8.1728358699.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v3.git.git.1728358699.gitgitgadget@gmail.com","subject":"[PATCH v3 1/5] transport: introduce parse_transport_option() method","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-08T03:38:15Z","receivedAt":"2024-10-08T03:38:24Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nAdd the `parse_transport_option()` method to parse the `push.pushOption`\nconfiguration. This method will also be used in the next commit to\nhandle the new `remote.<name>.serverOption` configuration for setting\nserver options in Git protocol v2.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n builtin/push.c |  9 +--------\n transport.c    | 12 ++++++++++++\n transport.h    |  4 ++++\n 3 files changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex e6f48969b81..e0ff60d48e5 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -519,14 +519,7 @@ static int git_push_config(const char *k, const char *v,\n \t\t\tRECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;\n \t\trecurse_submodules = val;\n \t} else if (!strcmp(k, \"push.pushoption\")) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\telse\n-\t\t\tif (!*v)\n-\t\t\t\tstring_list_clear(&push_options_config, 0);\n-\t\t\telse\n-\t\t\t\tstring_list_append(&push_options_config, v);\n-\t\treturn 0;\n+\t\treturn parse_transport_option(k, v, &push_options_config);\n \t} else if (!strcmp(k, \"color.push\")) {\n \t\tpush_use_color = git_config_colorbool(k, v);\n \t\treturn 0;\ndiff --git a/transport.c b/transport.c\nindex 3c4714581f5..4d9e605b273 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1099,6 +1099,18 @@ int is_transport_allowed(const char *type, int from_user)\n \tBUG(\"invalid protocol_allow_config type\");\n }\n \n+int parse_transport_option(const char *var, const char *value,\n+\t\t\t   struct string_list *transport_options)\n+{\n+\tif (!value)\n+\t\treturn config_error_nonbool(var);\n+\tif (!*value)\n+\t\tstring_list_clear(transport_options, 0);\n+\telse\n+\t\tstring_list_append(transport_options, value);\n+\treturn 0;\n+}\n+\n void transport_check_allowed(const char *type)\n {\n \tif (!is_transport_allowed(type, -1))\ndiff --git a/transport.h b/transport.h\nindex 6393cd9823c..44100fa9b7f 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -342,4 +342,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n /* common method used by transport-helper.c and send-pack.c */\n void reject_atomic_push(struct ref *refs, int mirror_mode);\n \n+/* common method to parse push-option or server-option from config */\n+int parse_transport_option(const char *var, const char *value,\n+\t\t\t   struct string_list *transport_options);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"504380","messageId":"3c6b129d3689f1920c4f06220c74820822537076.1728358699.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v3.git.git.1728358699.gitgitgadget@gmail.com","subject":"[PATCH v3 2/5] remote: introduce remote.<name>.serverOption configuration","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-08T03:38:16Z","receivedAt":"2024-10-08T03:38:24Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nCurrently, server options for Git protocol v2 can only be specified via\nthe command line option \"--server-option\" or \"-o\", which is inconvenient\nwhen users want to specify a list of default options to send. Therefore,\nwe are introducing a new configuration to hold a list of default server\noptions, akin to the `push.pushOption` configuration for push options.\n\nInitially, I named the new configuration `fetch.serverOption` to align\nwith `push.pushOption`. However, after discussing with Patrick, it was\nrenamed to `remote.<name>.serverOption` as suggested, because:\n\n1. Server options are designed to be server-specific, making it more\n   logical to use a per-remote configuration.\n2. Using \"fetch.\" prefixed configurations in git-clone or git-ls-remote\n   seems out of place and inconsistent in design.\n\nThe parsing logic for `remote.<name>.serverOption` also relies on\n`transport.c:parse_transport_option`, similar to `push.pushOption`, and\nthey follow the same priority design:\n\n1. Server options set in lower-priority configuration files (e.g.,\n   /etc/gitconfig or $HOME/.gitconfig) can be overridden or unset in\n   more specific repository configurations using an empty string.\n2. Command-line specified server options take precedence over those from\n   the configuration.\n\nServer options from configuration are stored to the corresponding\n`remote.h:remote` as a new field `server_options`.  The field will be\nutilized in the subsequent commit to help initialize the\n`server_options` of `transport.h:transport`.\n\nAnd documentation have been updated accordingly.\n\nHelped-by: Patrick Steinhardt <ps@pks.im>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nReported-by: Liu Zhongbo <liuzhongbo.6666@bytedance.com>\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n Documentation/config/remote.txt | 10 ++++++++++\n remote.c                        |  6 ++++++\n remote.h                        |  3 +++\n 3 files changed, 19 insertions(+)\n\ndiff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt\nindex 36e771556c6..b194dff07b9 100644\n--- a/Documentation/config/remote.txt\n+++ b/Documentation/config/remote.txt\n@@ -96,3 +96,13 @@ remote.<name>.partialclonefilter::\n \tChanging or clearing this value will only affect fetches for new commits.\n \tTo fetch associated objects for commits already present in the local object\n \tdatabase, use the `--refetch` option of linkgit:git-fetch[1].\n+\n+remote.<name>.serverOption::\n+\tThe default set of server options used when fetching from this remote.\n+\tThese server options can be overridden by the `--server-option=` command\n+\tline arguments.\n++\n+This is a multi-valued variable, and an empty value can be used in a higher\n+priority configuration file (e.g. `.git/config` in a repository) to clear\n+the values inherited from a lower priority configuration files (e.g.\n+`$HOME/.gitconfig`).\ndiff --git a/remote.c b/remote.c\nindex 390a03c2643..fa65549aa44 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -24,6 +24,7 @@\n #include \"advice.h\"\n #include \"connect.h\"\n #include \"parse-options.h\"\n+#include \"transport.h\"\n \n enum map_direction { FROM_SRC, FROM_DST };\n \n@@ -143,6 +144,7 @@ static struct remote *make_remote(struct remote_state *remote_state,\n \tret->name = xstrndup(name, len);\n \trefspec_init(&ret->push, REFSPEC_PUSH);\n \trefspec_init(&ret->fetch, REFSPEC_FETCH);\n+\tstring_list_init_dup(&ret->server_options);\n \n \tALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,\n \t\t   remote_state->remotes_alloc);\n@@ -166,6 +168,7 @@ static void remote_clear(struct remote *remote)\n \tfree((char *)remote->uploadpack);\n \tFREE_AND_NULL(remote->http_proxy);\n \tFREE_AND_NULL(remote->http_proxy_authmethod);\n+\tstring_list_clear(&remote->server_options, 0);\n }\n \n static void add_merge(struct branch *branch, const char *name)\n@@ -508,6 +511,9 @@ static int handle_config(const char *key, const char *value,\n \t} else if (!strcmp(subkey, \"vcs\")) {\n \t\tFREE_AND_NULL(remote->foreign_vcs);\n \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n+\t} else if (!strcmp(subkey, \"serveroption\")) {\n+\t\treturn parse_transport_option(key, value,\n+\t\t\t\t\t      &remote->server_options);\n \t}\n \treturn 0;\n }\ndiff --git a/remote.h b/remote.h\nindex a58713f20ad..e95d30946c0 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -4,6 +4,7 @@\n #include \"hash.h\"\n #include \"hashmap.h\"\n #include \"refspec.h\"\n+#include \"string-list.h\"\n #include \"strvec.h\"\n \n struct option;\n@@ -104,6 +105,8 @@ struct remote {\n \n \t/* The method used for authenticating against `http_proxy`. */\n \tchar *http_proxy_authmethod;\n+\n+\tstruct string_list server_options;\n };\n \n /**\n-- \ngitgitgadget\n\n"},{"id":"504381","messageId":"f0835259b06aed6130e6ee7c5dc37cfebdc77393.1728358699.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v3.git.git.1728358699.gitgitgadget@gmail.com","subject":"[PATCH v3 3/5] transport.c::handshake: make use of server options from remote","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-08T03:38:17Z","receivedAt":"2024-10-08T03:38:25Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nUtilize the `server_options` from the corresponding remote during the\nhandshake in `transport.c` when Git protocol v2 is detected. This helps\ninitialize the `server_options` in `transport.h:transport` if no server\noptions are set for the transport (typically via `--server-option` or\n`-o`).\n\nWhile another potential place to incorporate server options from the\nremote is in `transport.c:transport_get`, setting server options for a\ntransport using a protocol other than v2 could lead to unexpected errors\n(see `transport.c:die_if_server_options`).\n\nRelevant tests and documentation have been updated accordingly.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n Documentation/fetch-options.txt |   3 +\n Documentation/git-clone.txt     |   3 +\n Documentation/git-ls-remote.txt |   3 +\n t/t5702-protocol-v2.sh          | 123 ++++++++++++++++++++++++++++++++\n transport.c                     |   3 +\n 5 files changed, 135 insertions(+)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex 80838fe37ef..9dc7ac8dbdc 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -305,6 +305,9 @@ endif::git-pull[]\n \tunknown ones, is server-specific.\n \tWhen multiple `--server-option=<option>` are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no `--server-option=<option>` is given from the command line,\n+\tthe values of configuration variable `remote.<name>.serverOption`\n+\tare used instead.\n \n --show-forced-updates::\n \tBy default, git checks if a branch is force-updated during\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 8e925db7e9c..409fb3a3af6 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -149,6 +149,9 @@ objects from the source repository into a pack in the cloned repository.\n \tunknown ones, is server-specific.\n \tWhen multiple ++--server-option=++__<option>__ are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no ++--server-option=++__<option>__ is given from the command\n+\tline, the values of configuration variable `remote.<name>.serverOption`\n+\tare used instead.\n \n `-n`::\n `--no-checkout`::\ndiff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\nindex 76c86c3ce4f..d71c4ab3e29 100644\n--- a/Documentation/git-ls-remote.txt\n+++ b/Documentation/git-ls-remote.txt\n@@ -81,6 +81,9 @@ OPTIONS\n \tcharacter.\n \tWhen multiple `--server-option=<option>` are given, they are all\n \tsent to the other side in the order listed on the command line.\n+\tWhen no `--server-option=<option>` is given from the command line,\n+\tthe values of configuration variable `remote.<name>.serverOption`\n+\tare used instead.\n \n <repository>::\n \tThe \"remote\" repository to query.  This parameter can be\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1ef540f73d3..5cec2061d28 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -185,6 +185,43 @@ test_expect_success 'server-options are sent when using ls-remote' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options from configuration are used by ls-remote' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\tgit clone \"file://$(pwd)/file_parent\" myclone &&\n+\tcat >expect <<-EOF &&\n+\t$(git -C file_parent rev-parse refs/heads/main)$(printf \"\\t\")refs/heads/main\n+\tEOF\n+\n+\t# Default server options from configuration are used\n+\tgit -C myclone config --add remote.origin.serverOption foo &&\n+\tgit -C myclone config --add remote.origin.serverOption bar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tls-remote origin main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"ls-remote> server-option=foo\" log &&\n+\ttest_grep \"ls-remote> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Empty value of remote.<name>.serverOption clears the list\n+\tgit -C myclone config --add remote.origin.serverOption \"\" &&\n+\tgit -C myclone config --add remote.origin.serverOption tar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tls-remote origin main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"ls-remote> server-option=tar\" log &&\n+\ttest_grep ! \"ls-remote> server-option=foo\" log &&\n+\ttest_grep ! \"ls-remote> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Server option from command line overrides those from configuration\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tls-remote -o hello -o world origin main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"ls-remote> server-option=hello\" log &&\n+\ttest_grep \"ls-remote> server-option=world\" log &&\n+\ttest_grep ! \"ls-remote> server-option=tar\" log\n+'\n+\n test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n \ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n \t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" main 2>err &&\n@@ -381,6 +418,44 @@ test_expect_success 'server-options are sent when fetching' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options from configuration are used by git-fetch' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\tgit clone \"file://$(pwd)/file_parent\" myclone &&\n+\tgit -C file_parent log -1 --format=%s >expect &&\n+\n+\t# Default server options from configuration are used\n+\tgit -C myclone config --add remote.origin.serverOption foo &&\n+\tgit -C myclone config --add remote.origin.serverOption bar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tfetch origin main &&\n+\tgit -C myclone log -1 --format=%s origin/main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"fetch> server-option=foo\" log &&\n+\ttest_grep \"fetch> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Empty value of remote.<name>.serverOption clears the list\n+\tgit -C myclone config --add remote.origin.serverOption \"\" &&\n+\tgit -C myclone config --add remote.origin.serverOption tar &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tfetch origin main &&\n+\tgit -C myclone log -1 --format=%s origin/main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"fetch> server-option=tar\" log &&\n+\ttest_grep ! \"fetch> server-option=foo\" log &&\n+\ttest_grep ! \"fetch> server-option=bar\" log &&\n+\trm -f log &&\n+\n+\t# Server option from command line overrides those from configuration\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C myclone -c protocol.version=2 \\\n+\t\tfetch -o hello -o world origin main &&\n+\tgit -C myclone log -1 --format=%s origin/main >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"fetch> server-option=hello\" log &&\n+\ttest_grep \"fetch> server-option=world\" log &&\n+\ttest_grep ! \"fetch> server-option=tar\" log\n+'\n+\n test_expect_success 'warn if using server-option with fetch with legacy protocol' '\n \ttest_when_finished \"rm -rf temp_child\" &&\n \n@@ -404,6 +479,37 @@ test_expect_success 'server-options are sent when cloning' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options from configuration are used by git-clone' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\n+\t# Default server options from configuration are used\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"clone> server-option=foo\" log &&\n+\ttest_grep \"clone> server-option=bar\" log &&\n+\trm -rf log myclone &&\n+\n+\t# Empty value of remote.<name>.serverOption clears the list\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n+\t\t-c remote.origin.serverOption= -c remote.origin.serverOption=tar \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"clone> server-option=tar\" log &&\n+\ttest_grep ! \"clone> server-option=foo\" log &&\n+\ttest_grep ! \"clone> server-option=bar\" log &&\n+\trm -rf log myclone &&\n+\n+\t# Server option from command line overrides those from configuration\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption=tar \\\n+\t\tclone --server-option=hello --server-option=world \\\n+\t\t\"file://$(pwd)/file_parent\" myclone &&\n+\ttest_grep \"clone> server-option=hello\" log &&\n+\ttest_grep \"clone> server-option=world\" log &&\n+\ttest_grep ! \"clone> server-option=tar\" log\n+'\n+\n test_expect_success 'warn if using server-option with clone with legacy protocol' '\n \ttest_when_finished \"rm -rf myclone\" &&\n \n@@ -415,6 +521,23 @@ test_expect_success 'warn if using server-option with clone with legacy protocol\n \ttest_grep \"server options require protocol version 2 or later\" err\n '\n \n+test_expect_success 'server-option configuration with legacy protocol is ok' '\n+\ttest_when_finished \"rm -rf myclone\" &&\n+\n+\tenv GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n+\t\t-c remote.origin.serverOption=foo -c remote.origin.serverOption=bar \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone\n+'\n+\n+test_expect_success 'invalid server-option configuration' '\n+\ttest_when_finished \"rm -rf myclone\" &&\n+\n+\ttest_must_fail git -c protocol.version=2 \\\n+\t\t-c remote.origin.serverOption \\\n+\t\tclone \"file://$(pwd)/file_parent\" myclone 2>err &&\n+\ttest_grep \"error: missing value for '\\''remote.origin.serveroption'\\''\" err\n+'\n+\n test_expect_success 'upload-pack respects config using protocol v2' '\n \tgit init server &&\n \twrite_script server/.git/hook <<-\\EOF &&\ndiff --git a/transport.c b/transport.c\nindex 4d9e605b273..b416fc84567 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -334,6 +334,9 @@ static struct ref *handshake(struct transport *transport, int for_push,\n \tdata->version = discover_version(&reader);\n \tswitch (data->version) {\n \tcase protocol_v2:\n+\t\tif ((!transport->server_options || !transport->server_options->nr) &&\n+\t\t    transport->remote->server_options.nr)\n+\t\t\ttransport->server_options = &transport->remote->server_options;\n \t\tif (server_feature_v2(\"session-id\", &server_sid))\n \t\t\ttrace2_data_string(\"transfer\", NULL, \"server-sid\", server_sid);\n \t\tif (must_list_refs)\n-- \ngitgitgadget\n\n"},{"id":"504382","messageId":"420b15d9f37d2510d0e4f5390a4b93a5ead7c966.1728358699.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v3.git.git.1728358699.gitgitgadget@gmail.com","subject":"[PATCH v3 4/5] fetch: respect --server-option when fetching multiple remotes","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-08T03:38:18Z","receivedAt":"2024-10-08T03:38:26Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nFix an issue where server options specified via the command line\n(`--server-option` or `-o`) were not sent when fetching from multiple\nremotes using Git protocol v2.\n\nTo reproduce the issue with a repository containing multiple remotes:\n\n  GIT_TRACE_PACKET=1 git -c protocol.version=2 fetch --server-option=demo --all\n\nObserve that no server options are sent to any remote.\n\nThe root cause was identified in `builtin/fetch.c:fetch_multiple`, which\nis invoked when fetching from more than one remote. This function forks\na `git-fetch` subprocess for each remote but did not include the\nspecified server options in the subprocess arguments.\n\nThis commit ensures that command-line specified server options are\nproperly passed to each subprocess. Relevant tests have been added.\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n builtin/fetch.c        |  2 ++\n t/t5702-protocol-v2.sh | 10 ++++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex c900f577219..54be2e7ca9f 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1980,6 +1980,8 @@ static int fetch_multiple(struct string_list *list, int max_children,\n \tstrvec_pushl(&argv, \"-c\", \"fetch.bundleURI=\",\n \t\t     \"fetch\", \"--append\", \"--no-auto-gc\",\n \t\t     \"--no-write-commit-graph\", NULL);\n+\tfor (i = 0; i < server_options.nr; i++)\n+\t\tstrvec_pushf(&argv, \"--server-option=%s\", server_options.items[i].string);\n \tadd_options_to_argv(&argv, config);\n \n \tif (max_children != 1 && list->nr != 1) {\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 5cec2061d28..d3df81e7852 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -418,6 +418,16 @@ test_expect_success 'server-options are sent when fetching' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'server-options are sent when fetch multiple remotes' '\n+\ttest_when_finished \"rm -f log server_options_sent\" &&\n+\tgit clone \"file://$(pwd)/file_parent\" child_multi_remotes &&\n+\tgit -C child_multi_remotes remote add another \"file://$(pwd)/file_parent\" &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/log\" git -C child_multi_remotes -c protocol.version=2 \\\n+\t\tfetch -o hello --all &&\n+\tgrep \"fetch> server-option=hello\" log >server_options_sent &&\n+\ttest_line_count = 2 server_options_sent\n+'\n+\n test_expect_success 'server-options from configuration are used by git-fetch' '\n \ttest_when_finished \"rm -rf log myclone\" &&\n \tgit clone \"file://$(pwd)/file_parent\" myclone &&\n-- \ngitgitgadget\n\n"},{"id":"504383","messageId":"2528d929c7e37f592a12967e6a2d86b57dc38293.1728358699.git.gitgitgadget@gmail.com","threadId":"62037","inReplyTo":"pull.1776.v3.git.git.1728358699.gitgitgadget@gmail.com","subject":"[PATCH v3 5/5] ls-remote: leakfix for not clearing server_options","fromName":"Xing Xin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-08T03:38:19Z","receivedAt":"2024-10-08T03:38:27Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"From: Xing Xin <xingxin.xx@bytedance.com>\n\nEnsure `server_options` is properly cleared using `string_list_clear()`\nin `builtin/ls-remote.c:cmd_ls_remote`.\n\nAlthough we cannot yet enable `TEST_PASSES_SANITIZE_LEAK=true` for\n`t/t5702-protocol-v2.sh` due to other existing leaks, this fix ensures\nthat \"git-ls-remote\" related server options tests pass the sanitize leak\ncheck:\n\n  ...\n  ok 12 - server-options are sent when using ls-remote\n  ok 13 - server-options from configuration are used by ls-remote\n  ...\n\nSigned-off-by: Xing Xin <xingxin.xx@bytedance.com>\n---\n builtin/ls-remote.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex f723b3bf3bb..423318f87ec 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -173,5 +173,6 @@ int cmd_ls_remote(int argc,\n \ttransport_ls_refs_options_release(&transport_options);\n \n \tstrvec_clear(&pattern);\n+\tstring_list_clear(&server_options, 0);\n \treturn status;\n }\n-- \ngitgitgadget\n"},{"id":"504384","messageId":"e927c65.4aed.1926a354fdb.Coremail.bupt_xingxin@163.com","threadId":"62037","inReplyTo":"ZwOaR5OqdLaS4cu6@pks.im","subject":"Re:Re: [PATCH v2 2/5] remote: introduce remote.<name>.serverOption configuration","fromName":"Xing Xin","fromEmail":"bupt_xingxin@163.com","sentAt":"2024-10-08T03:38:51Z","receivedAt":"2024-10-08T03:39:29Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"At 2024-10-07 16:22:31, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n>On Mon, Sep 23, 2024 at 12:17:55PM +0000, Xing Xin via GitGitGadget wrote:\n>> diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt\n>> index 8efc53e836d..8ba48573fb6 100644\n>> --- a/Documentation/config/remote.txt\n>> +++ b/Documentation/config/remote.txt\n>> @@ -95,3 +95,13 @@ remote.<name>.partialclonefilter::\n>>  \tChanging or clearing this value will only affect fetches for new commits.\n>>  \tTo fetch associated objects for commits already present in the local object\n>>  \tdatabase, use the `--refetch` option of linkgit:git-fetch[1].\n>> +\n>> +remote.<name>.serverOption::\n>> +\tWhen no `--server-option=<option>` argument is given from the command\n>> +\tline, git will use the values from this configuration as a default list of\n>\n>This description feels a bit inelegant to me. How about the following:\n>\n>    The default set of server options used when fetching from the\n>    remote. These server options are overridden by the\n>    `--server-option=` command line arguments.\n\nThanks for the help! Applied to the new patch (with some minor modifications).\n\n>> @@ -125,6 +126,7 @@ static struct remote *make_remote(struct remote_state *remote_state,\n>>  \tstruct remote *ret;\n>>  \tstruct remotes_hash_key lookup;\n>>  \tstruct hashmap_entry lookup_entry, *e;\n>> +\tstruct string_list server_options = STRING_LIST_INIT_DUP;\n>>  \n>>  \tif (!len)\n>>  \t\tlen = strlen(name);\n>> @@ -143,6 +145,7 @@ static struct remote *make_remote(struct remote_state *remote_state,\n>>  \tret->name = xstrndup(name, len);\n>>  \trefspec_init(&ret->push, REFSPEC_PUSH);\n>>  \trefspec_init(&ret->fetch, REFSPEC_FETCH);\n>> +\tret->server_options = server_options;\n>\n>I was wondering at first where we actually populate the\n>`server_options`, until I realized that this is only done to initialize\n>the `ret->server_options`. I think it would be less confusing if this\n>used `string_list_init_dup()` instead of the local variable.\n\nMake sense.\n\nXing Xin\n"},{"id":"504385","messageId":"2d736d9e.4c15.1926a38e006.Coremail.bupt_xingxin@163.com","threadId":"62037","inReplyTo":"ZwOaZkxETwdId9Qg@pks.im","subject":"Re:Re: [PATCH v2 0/5] Support server option from configuration","fromName":"Xing Xin","fromEmail":"bupt_xingxin@163.com","sentAt":"2024-10-08T03:42:45Z","receivedAt":"2024-10-08T03:43:08Z","isPatch":true,"sender":{"key":"bupt_xingxin@163.com","avatar":null},"body":"At 2024-10-07 16:23:02, \"Patrick Steinhardt\" <ps@pks.im> wrote:\n>On Mon, Sep 23, 2024 at 12:17:53PM +0000, blanet via GitGitGadget wrote:\n>> We have multiple internal repositories configured with a huge number of CI\n>> tasks, each requiring code preparation (either via clone or fetch). These CI\n>> tasks, triggered by post-receive hooks, often fetch the same code (usually\n>> using --depth=1) concurrently.\n>> \n>> To reduce performance impacts on the git server caused by these tasks, we\n>> want to deploy a special designed pack-objects-hook. This hook would allow\n>> the packs generated by git-pack-objects to be reused. Since not all\n>> clone/fetch operations will benefit from this caching (e.g., pulls from\n>> developers' environments), clients need to pass a special identifier to\n>> indicate whether caching should be enabled for a particular operation. Using\n>> server options is a suitable method for this.\n>> \n>> However, server options can only be specified via the command line option\n>> (via --server-option or -o), which is inconvenient and requires CI script\n>> modifications. A configuration-based approach is easier to propagate (by\n>> changing the global configuration ~/.gitconfig) and avoids issues with older\n>> Git versions that don't support --server-option.\n>> \n>> This patch series introduces a new multi-valued configuration,\n>> remote.<name>.serverOption, similar to push.pushOption, to specify default\n>> server options for the corresponding remote.\n>> \n>>  * Patch 1~3 is the main change for introducing the new configuration.\n>>  * Patch 4 fixes a issue for git-fetch not sending server-options when\n>>    fetching multiple remotes.\n>>  * Patch 5 is a minor fix for a server options-related memory leak.\n>\n>Sorry for taking this long to get back to this patch series. I've given\n>it another careful read and have a single proposal for improved docs\n>that you may want to address. But other than those nits this version\n>looks great to me, thanks!\n\n\nThanks for your kind help! I just got back from holiday, so your reply was\nperfectly timed. :-)\n\nXing Xin\n"},{"id":"504387","messageId":"ZwSuRa_2OeAAv3pQ@pks.im","threadId":"62037","inReplyTo":"pull.1776.v3.git.git.1728358699.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/5] Support server option from configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-08T04:00:05Z","receivedAt":"2024-10-08T04:00:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Oct 08, 2024 at 03:38:14AM +0000, blanet via GitGitGadget wrote:\n> We manage some internal repositories with numerous CI tasks, each requiring\n> code preparation through git-clone or git-fetch. These tasks, triggered by\n> post-receive hooks, often fetch the same copy of code concurrently using\n> --depth=1, causing extremely high load spikes on our Git servers.\n> \n> To reduce performance impacts caused by these tasks, we plan to deploy a\n> specially designed pack-objects-hook [1]. This hook would allow the packs\n> generated by git-pack-objects(during git-clone or git-fetch) to be reused.\n> Since not all clone/fetch operations will benefit from this caching (e.g.,\n> pulls from developer environments), clients need to pass a special\n> identifier to indicate whether caching should be enabled. Using server\n> options [2] is suitable for this purpose.\n> \n> However, server options can only be specified via the command line option\n> (via --server-option or -o), which is inconvenient and requires\n> modifications to CI scripts. A configuration-based approach is preferable,\n> as it can be propagated through global configuration (e.g. ~/.gitconfig) and\n> avoids compatibility issues with older Git versions that don't support\n> --server-option.\n> \n> This patch series introduces a new multi-valued configuration,\n> remote.<name>.serverOption, similar to push.pushOption, to specify default\n> server options for the corresponding remote.\n> \n>  * Patches 1~3 contain the main changes for introducing the new\n>    configuration.\n>  * Patch 4 fixes a issue for git-fetch not sending server-options when\n>    fetching from multiple remotes.\n>  * Patch 5 is a minor fix for a server options-related memory leak.\n> \n>  1. https://git-scm.com/docs/git-config#Documentation/git-config.txt-uploadpackpackObjectsHook\n>  2. https://git-scm.com/docs/gitprotocol-v2#_server_option\n\nThe range-diff looks as expected to me, so this should be ready\nto go from my point of view. Thanks!\n\nPatrick\n"},{"id":"504459","messageId":"xmqqiku233w1.fsf@gitster.g","threadId":"62037","inReplyTo":"ZwSuRa_2OeAAv3pQ@pks.im","subject":"Re: [PATCH v3 0/5] Support server option from configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-08T17:23:26Z","receivedAt":"2024-10-08T17:23:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Tue, Oct 08, 2024 at 03:38:14AM +0000, blanet via GitGitGadget wrote:\n>> We manage some internal repositories with numerous CI tasks, each requiring\n>> code preparation through git-clone or git-fetch. These tasks, triggered by\n>> post-receive hooks, often fetch the same copy of code concurrently using\n>> --depth=1, causing extremely high load spikes on our Git servers.\n>> ...\n>> This patch series introduces a new multi-valued configuration,\n>> remote.<name>.serverOption, similar to push.pushOption, to specify default\n>> server options for the corresponding remote.\n>> \n>>  * Patches 1~3 contain the main changes for introducing the new\n>>    configuration.\n>>  * Patch 4 fixes a issue for git-fetch not sending server-options when\n>>    fetching from multiple remotes.\n>>  * Patch 5 is a minor fix for a server options-related memory leak.\n>> \n>>  1. https://git-scm.com/docs/git-config#Documentation/git-config.txt-uploadpackpackObjectsHook\n>>  2. https://git-scm.com/docs/gitprotocol-v2#_server_option\n>\n> The range-diff looks as expected to me, so this should be ready\n> to go from my point of view. Thanks!\n\nThanks, both of you.  Queued.\n"},{"id":"504461","messageId":"xmqqv7y21nra.fsf@gitster.g","threadId":"62037","inReplyTo":"420b15d9f37d2510d0e4f5390a4b93a5ead7c966.1728358699.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 4/5] fetch: respect --server-option when fetching multiple remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-08T17:57:13Z","receivedAt":"2024-10-08T17:57:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Xing Xin via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Xing Xin <xingxin.xx@bytedance.com>\n>\n> Fix an issue where server options specified via the command line\n> (`--server-option` or `-o`) were not sent when fetching from multiple\n> remotes using Git protocol v2.\n>\n> To reproduce the issue with a repository containing multiple remotes:\n>\n>   GIT_TRACE_PACKET=1 git -c protocol.version=2 fetch --server-option=demo --all\n>\n> Observe that no server options are sent to any remote.\n>\n> The root cause was identified in `builtin/fetch.c:fetch_multiple`, which\n> is invoked when fetching from more than one remote. This function forks\n> a `git-fetch` subprocess for each remote but did not include the\n> specified server options in the subprocess arguments.\n>\n> This commit ensures that command-line specified server options are\n> properly passed to each subprocess. Relevant tests have been added.\n\nA more interesting use case, as there is no reason to expect that a\nsingle server-option command line option may apply to all remotes,\nwould be to configure different server options for different remotes\nvia the new serverOption configuration, so that different server\noptions are used for different remotes.\n\nIf it happens that the same server option is applicable for all\nremotes, it is reasonable to give --server-option from the command\nline and expect it to be propagated down to subfetches, so\nregardless of the \"what happens when different remotes have\ndifferent options configured?\" above, the change in this step looks\nreasonable to me.\n"}]}