{"thread":{"id":"50881","subject":"[PATCH] clone: send server options when using protocol v2","startedAt":"2019-04-05T20:44:18Z","lastAt":"2019-04-15T09:43:54Z","messageCount":22,"participants":["Jonathan Tan","Jonathan Nieder","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"373259","messageId":"20190405204413.93900-1-jonathantanmy@google.com","threadId":"50881","inReplyTo":null,"subject":"[PATCH] clone: send server options when using protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-05T20:44:13Z","receivedAt":"2019-04-05T20:44:18Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Commit 5e3548ef16 (\"fetch: send server options when using protocol v2\",\n2018-04-24) taught \"fetch\" the ability to send server options when using\nprotocol v2, but not \"clone\". This ability is triggered by \"-o\" or\n\"--server-option\".\n\nTeach \"clone\" the same ability, except that because \"clone\" already\nhas \"-o\" for another parameter, teach \"clone\" only to receive\n\"--server-option\".\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Documentation/git-clone.txt |  7 +++++++\n builtin/clone.c             |  6 ++++++\n t/t5702-protocol-v2.sh      | 11 +++++++++++\n 3 files changed, 24 insertions(+)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 2fd12524f9..5b79da359c 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -131,6 +131,13 @@ objects from the source repository into a pack in the cloned repository.\n \tis specified. This flag forces progress status even if the\n \tstandard error stream is not directed to a terminal.\n \n+--server-option=<option>::\n+\tTransmit the given string to the server when communicating using\n+\tprotocol version 2.  The given string must not contain a NUL or LF\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+\n --no-checkout::\n -n::\n \tNo checkout of HEAD is performed after the clone is complete.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 50bde99618..625d47c1f4 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -66,6 +66,7 @@ static int option_dissociate;\n static int max_jobs = -1;\n static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options;\n+static struct string_list server_options = STRING_LIST_INIT_DUP;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -137,6 +138,8 @@ static struct option builtin_clone_options[] = {\n \t\t   N_(\"separate git dir from working tree\")),\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_(\"server-specific\"),\n+\t\t\tN_(\"option to transmit\")),\n \tOPT_SET_INT('4', \"ipv4\", &family, N_(\"use IPv4 addresses only\"),\n \t\t\tTRANSPORT_FAMILY_IPV4),\n \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n@@ -1136,6 +1139,9 @@ 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+\n \tif (filter_options.choice) {\n \t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n \t\texpand_list_objects_filter_spec(&filter_options,\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex e112b6086c..5b28d88401 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -251,6 +251,17 @@ 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 cloning' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\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+\n+\tgrep \"server-option=hello\" log &&\n+\tgrep \"server-option=world\" log\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-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373319","messageId":"20190406115728.GB219876@google.com","threadId":"50881","inReplyTo":"20190405204413.93900-1-jonathantanmy@google.com","subject":"Re: [PATCH] clone: send server options when using protocol v2","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-04-06T11:57:28Z","receivedAt":"2019-04-06T11:57:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Tan wrote:\n\n> Commit 5e3548ef16 (\"fetch: send server options when using protocol v2\",\n> 2018-04-24) taught \"fetch\" the ability to send server options when using\n> protocol v2, but not \"clone\". This ability is triggered by \"-o\" or\n> \"--server-option\".\n>\n> Teach \"clone\" the same ability, except that because \"clone\" already\n> has \"-o\" for another parameter, teach \"clone\" only to receive\n> \"--server-option\".\n\nCan you give an example of what this would be used for?  An example I\ncan think of might be\n\n\tgit clone --server-option=priority=batch <url>\n\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> ---\n>  Documentation/git-clone.txt |  7 +++++++\n>  builtin/clone.c             |  6 ++++++\n>  t/t5702-protocol-v2.sh      | 11 +++++++++++\n>  3 files changed, 24 insertions(+)\n\nThanks.\n\n[...]\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -66,6 +66,7 @@ static int option_dissociate;\n>  static int max_jobs = -1;\n>  static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n>  static struct list_objects_filter_options filter_options;\n> +static struct string_list server_options = STRING_LIST_INIT_DUP;\n\nThe other string-list options in this file all use NODUP.  Is there a\nreason this one uses DUP instead?  (Just curious --- I suspect either\nwould work fine, since nothing here does tricks with modifying argv\nentries after option parsing.)\n\nThe same question applies to the corresponding option in\nbuiltin/fetch.c, so while it is not likely to matter in practice, it\nwould be nice for readability to find out.\n\n>  \n>  static int recurse_submodules_cb(const struct option *opt,\n>  \t\t\t\t const char *arg, int unset)\n> @@ -137,6 +138,8 @@ static struct option builtin_clone_options[] = {\n>  \t\t   N_(\"separate git dir from working tree\")),\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_(\"server-specific\"),\n\nnit: long line\n\n> +\t\t\tN_(\"option to transmit\")),\n[...]\n> --- a/t/t5702-protocol-v2.sh\n> +++ b/t/t5702-protocol-v2.sh\n> @@ -251,6 +251,17 @@ 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 cloning' '\n> +\ttest_when_finished \"rm -rf log myclone\" &&\n> +\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> +\n> +\tgrep \"server-option=hello\" log &&\n> +\tgrep \"server-option=world\" log\n> +'\n\nNice.  Thanks for including this.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks for a pleasant read.\n"},{"id":"373436","messageId":"20190408171149.125872-1-jonathantanmy@google.com","threadId":"50881","inReplyTo":"20190406115728.GB219876@google.com","subject":"Re: [PATCH] clone: send server options when using protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-08T17:11:49Z","receivedAt":"2019-04-08T17:11:55Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> > Teach \"clone\" the same ability, except that because \"clone\" already\n> > has \"-o\" for another parameter, teach \"clone\" only to receive\n> > \"--server-option\".\n> \n> Can you give an example of what this would be used for?  An example I\n> can think of might be\n> \n> \tgit clone --server-option=priority=batch <url>\n\nprotocol-v2.txt says that it is server-specific, so I don't think I can\ngive any meaningful examples here.\n\n> > --- a/builtin/clone.c\n> > +++ b/builtin/clone.c\n> > @@ -66,6 +66,7 @@ static int option_dissociate;\n> >  static int max_jobs = -1;\n> >  static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n> >  static struct list_objects_filter_options filter_options;\n> > +static struct string_list server_options = STRING_LIST_INIT_DUP;\n> \n> The other string-list options in this file all use NODUP.  Is there a\n> reason this one uses DUP instead?  (Just curious --- I suspect either\n> would work fine, since nothing here does tricks with modifying argv\n> entries after option parsing.)\n> \n> The same question applies to the corresponding option in\n> builtin/fetch.c, so while it is not likely to matter in practice, it\n> would be nice for readability to find out.\n\nI guess it could be either. It's true that I just copied it from\nbuiltin/fetch.c. I'll change it to NODUP if a reroll is needed for\nanother reason.\n\n> > +\tOPT_STRING_LIST(0, \"server-option\", &server_options, N_(\"server-specific\"),\n> \n> nit: long line\n\nI'll change this if a reroll is needed for another reason.\n\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n> \n> Thanks for a pleasant read.\n\nThanks for your review.\n"},{"id":"373496","messageId":"xmqqk1g342xi.fsf@gitster-ct.c.googlers.com","threadId":"50881","inReplyTo":"20190408171149.125872-1-jonathantanmy@google.com","subject":"Re: [PATCH] clone: send server options when using protocol v2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-09T15:24:25Z","receivedAt":"2019-04-09T15:24:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan <jonathantanmy@google.com> writes:\n\n>> > Teach \"clone\" the same ability, except that because \"clone\" already\n>> > has \"-o\" for another parameter, teach \"clone\" only to receive\n>> > \"--server-option\".\n>> \n>> Can you give an example of what this would be used for?  An example I\n>> can think of might be\n>> \n>> \tgit clone --server-option=priority=batch <url>\n>\n> protocol-v2.txt says that it is server-specific, so I don't think I can\n> give any meaningful examples here.\n\nDoes the code behave sensibly when the --server-option=... option is\ngiven and\n\n (a) the given option is not understood by the other side that talks\n     protocol v2?  Or\n\n (b) it turns out that the other side does not talk protocol v2?\n\nIn the former case, I would expect that there would be a way to\nrespond with a failure for the other side and \"git clone\" should\nnotice and respond by die()-ing at the end.\n\nIn the latter case, I would expect at least that a warning about\naccepted but ignored server-option to be given, or \"git clone\" to\nfail with an error \"cannot honor the server-option\".\n\nIf the code does not match the above expectations, at least that\nshould be documented, I think.  If \"git fetch\" shares the same\nissue, this would be a good time to address it, too.\n\nThanks.\n\n"},{"id":"373527","messageId":"20190409184949.207114-1-jonathantanmy@google.com","threadId":"50881","inReplyTo":"xmqqk1g342xi.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] clone: send server options when using protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-09T18:49:49Z","receivedAt":"2019-04-09T18:49:55Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> Does the code behave sensibly when the --server-option=... option is\n> given and\n> \n>  (a) the given option is not understood by the other side that talks\n>      protocol v2?  Or\n> \n>  (b) it turns out that the other side does not talk protocol v2?\n> \n> In the former case, I would expect that there would be a way to\n> respond with a failure for the other side and \"git clone\" should\n> notice and respond by die()-ing at the end.\n\nRight now, the server ignores it (it collects them as \"keys\" in\nprocess_request() in serve.c and then passes them to the individual\ncommands, but they don't do anything about it).\n\nRight now it's documented as:\n\n\t-o <option>::\n\t--server-option=<option>::\n\t\tTransmit the given string to the server when communicating using\n\t\tprotocol version 2.  The given string must not contain a NUL or LF\n\t\tcharacter.\n\t\tWhen multiple `--server-option=<option>` are given, they are all\n\t\tsent to the other side in the order listed on the command line.\n\nI'm inclined to add:\n\n  The server's handling of server options, including unknown ones, is\n  server-specific.\n\n> In the latter case, I would expect at least that a warning about\n> accepted but ignored server-option to be given, or \"git clone\" to\n> fail with an error \"cannot honor the server-option\".\n\nRight now there's no warning, but I'll add one in the next reroll to\nboth \"fetch\" and \"clone\".\n\n> If the code does not match the above expectations, at least that\n> should be documented, I think.  If \"git fetch\" shares the same\n> issue, this would be a good time to address it, too.\n"},{"id":"373531","messageId":"cover.1554841624.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"20190405204413.93900-1-jonathantanmy@google.com","subject":"[PATCH v2 0/2] Server options when cloning","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-09T20:31:32Z","receivedAt":"2019-04-09T20:31:40Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"I created v2 by rewriting jt/clone-server-option, so you'll see some\nReviewed-by lines and Signed-off-by lines with another name than mine.\n\nChanges from v1:\n - Broke long line. (DUP->NODUP was already done by Junio in\n   jt/clone-server-option, so you don't see it in the range-diff below.)\n - Warn if server-option is specified but not sent to the server (in\n   fetch and clone).\n - Document that server's handling of server-option is server-specific.\n\nThese patches are on maint, matching jt/clone-server-option, but should\napply cleanly on master too.\n\nJonathan Tan (2):\n  transport: warn if server options are unsupported\n  clone: send server options when using protocol v2\n\n Documentation/fetch-options.txt |  3 ++-\n Documentation/git-clone.txt     |  8 +++++++\n builtin/clone.c                 |  6 ++++++\n t/t5702-protocol-v2.sh          | 38 +++++++++++++++++++++++++++++++++\n transport.c                     |  8 +++++++\n 5 files changed, 62 insertions(+), 1 deletion(-)\n\nRange-diff against v1:\n-:  ---------- > 1:  af3cc05324 transport: warn if server options are unsupported\n1:  90ce94e039 ! 2:  142c25abd2 clone: send server options when using protocol v2\n    @@ -11,10 +11,27 @@\n         has \"-o\" for another parameter, teach \"clone\" only to receive\n         \"--server-option\".\n     \n    +    Explain in the documentation, both for clone and for fetch, that server\n    +    handling of server options are server-specific.\n    +\n         Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n         Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n         Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     \n    + diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\n    + --- a/Documentation/fetch-options.txt\n    + +++ b/Documentation/fetch-options.txt\n    +@@\n    + --server-option=<option>::\n    + \tTransmit the given string to the server when communicating using\n    + \tprotocol version 2.  The given string must not contain a NUL or LF\n    +-\tcharacter.\n    ++\tcharacter.  The server's handling of server options, including\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    + \n    +\n      diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n      --- a/Documentation/git-clone.txt\n      +++ b/Documentation/git-clone.txt\n    @@ -25,7 +42,8 @@\n     +--server-option=<option>::\n     +\tTransmit the given string to the server when communicating using\n     +\tprotocol version 2.  The given string must not contain a NUL or LF\n    -+\tcharacter.\n    ++\tcharacter.  The server's handling of server options, including\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     +\n    @@ -48,8 +66,8 @@\n      \t\t   N_(\"separate git dir from working tree\")),\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_(\"server-specific\"),\n    -+\t\t\tN_(\"option to transmit\")),\n    ++\tOPT_STRING_LIST(0, \"server-option\", &server_options,\n    ++\t\t\tN_(\"server-specific\"), N_(\"option to transmit\")),\n      \tOPT_SET_INT('4', \"ipv4\", &family, N_(\"use IPv4 addresses only\"),\n      \t\t\tTRANSPORT_FAMILY_IPV4),\n      \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n    @@ -68,7 +86,7 @@\n      --- a/t/t5702-protocol-v2.sh\n      +++ b/t/t5702-protocol-v2.sh\n     @@\n    - \tgrep \"server-option=world\" log\n    + \tgrep \"Ignoring server options\" err\n      '\n      \n     +test_expect_success 'server-options are sent when cloning' '\n    @@ -81,6 +99,16 @@\n     +\tgrep \"server-option=hello\" log &&\n     +\tgrep \"server-option=world\" 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    ++\tGIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n    ++\t\tclone --server-option=hello --server-option=world \\\n    ++\t\t\"file://$(pwd)/file_parent\" myclone 2>err &&\n    ++\n    ++\tgrep \"Ignoring server options\" err\n    ++'\n     +\n      test_expect_success 'upload-pack respects config using protocol v2' '\n      \tgit init server &&\n-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373532","messageId":"af3cc05324f53316eedb2f437789eacb24c11489.1554841624.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"cover.1554841624.git.jonathantanmy@google.com","subject":"[PATCH v2 1/2] transport: warn if server options are unsupported","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-09T20:31:33Z","receivedAt":"2019-04-09T20:31:42Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Server options were added in commit 5e3548ef16 (\"fetch: send server\noptions when using protocol v2\", 2018-04-24), supported only for\nprotocol version 2. Add a warning if server options are specified for\nthe user if a legacy protocol is used instead.\n\nAn effort is made to avoid printing the same warning more than once by\nclearing transport->server_options after the warning, but this does not\nfully avoid double-printing (for example, when backfulling tags using\nanother fetch with a non-reusable transport).\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5702-protocol-v2.sh | 17 +++++++++++++++++\n transport.c            |  8 ++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex db4ae09f2f..f1a3ff1021 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -182,6 +182,12 @@ test_expect_success 'server-options are sent when using ls-remote' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n+\tGIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n+\t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n+\n+\tgrep \"Ignoring server options\" err\n+'\n \n test_expect_success 'clone with file:// using protocol v2' '\n \ttest_when_finished \"rm -f log\" &&\n@@ -251,6 +257,17 @@ test_expect_success 'server-options are sent when fetching' '\n \tgrep \"server-option=world\" 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+\tgit init temp_child &&\n+\n+\tGIT_TEST_PROTOCOL_VERSION=0 git -C temp_child -c protocol.version=0 \\\n+\t\tfetch -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n+\n+\tgrep \"Ignoring server options\" 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 e078812897..14f14ef93c 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -252,6 +252,12 @@ static int connect_setup(struct transport *transport, int for_push)\n \treturn 0;\n }\n \n+static void warn_server_options_unsupported(struct transport *transport)\n+{\n+\twarning(_(\"Ignoring server options because protocol version does not support it\"));\n+\ttransport->server_options = NULL;\n+}\n+\n /*\n  * Obtains the protocol version from the transport and writes it to\n  * transport->data->version, first connecting if not already connected.\n@@ -286,6 +292,7 @@ static struct ref *handshake(struct transport *transport, int for_push,\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n+\t\twarn_server_options_unsupported(transport);\n \t\tget_remote_heads(&reader, &refs,\n \t\t\t\t for_push ? REF_NORMAL : 0,\n \t\t\t\t &data->extra_have,\n@@ -363,6 +370,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n+\t\twarn_server_options_unsupported(transport);\n \t\trefs = fetch_pack(&args, data->fd, data->conn,\n \t\t\t\t  refs_tmp ? refs_tmp : transport->remote_refs,\n \t\t\t\t  dest, to_fetch, nr_heads, &data->shallow,\n-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373533","messageId":"142c25abd2c5c5c477c44d0e47cb20a3d504ab1c.1554841624.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"cover.1554841624.git.jonathantanmy@google.com","subject":"[PATCH v2 2/2] clone: send server options when using protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-09T20:31:34Z","receivedAt":"2019-04-09T20:31:49Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Commit 5e3548ef16 (\"fetch: send server options when using protocol v2\",\n2018-04-24) taught \"fetch\" the ability to send server options when using\nprotocol v2, but not \"clone\". This ability is triggered by \"-o\" or\n\"--server-option\".\n\nTeach \"clone\" the same ability, except that because \"clone\" already\nhas \"-o\" for another parameter, teach \"clone\" only to receive\n\"--server-option\".\n\nExplain in the documentation, both for clone and for fetch, that server\nhandling of server options are server-specific.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/fetch-options.txt |  3 ++-\n Documentation/git-clone.txt     |  8 ++++++++\n builtin/clone.c                 |  6 ++++++\n t/t5702-protocol-v2.sh          | 21 +++++++++++++++++++++\n 4 files changed, 37 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex fa0a3151b3..91c47752ec 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -216,7 +216,8 @@ endif::git-pull[]\n --server-option=<option>::\n \tTransmit the given string to the server when communicating using\n \tprotocol version 2.  The given string must not contain a NUL or LF\n-\tcharacter.\n+\tcharacter.  The server's handling of server options, including\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 \ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 2fd12524f9..a0f14b51f2 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -131,6 +131,14 @@ objects from the source repository into a pack in the cloned repository.\n \tis specified. This flag forces progress status even if the\n \tstandard error stream is not directed to a terminal.\n \n+--server-option=<option>::\n+\tTransmit the given string to the server when communicating using\n+\tprotocol version 2.  The given string must not contain a NUL or LF\n+\tcharacter.  The server's handling of server options, including\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+\n --no-checkout::\n -n::\n \tNo checkout of HEAD is performed after the clone is complete.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 50bde99618..31a47d190a 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -66,6 +66,7 @@ static int option_dissociate;\n static int max_jobs = -1;\n static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options;\n+static struct string_list server_options = STRING_LIST_INIT_NODUP;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -137,6 +138,8 @@ static struct option builtin_clone_options[] = {\n \t\t   N_(\"separate git dir from working tree\")),\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+\t\t\tN_(\"server-specific\"), N_(\"option to transmit\")),\n \tOPT_SET_INT('4', \"ipv4\", &family, N_(\"use IPv4 addresses only\"),\n \t\t\tTRANSPORT_FAMILY_IPV4),\n \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n@@ -1136,6 +1139,9 @@ 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+\n \tif (filter_options.choice) {\n \t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n \t\texpand_list_objects_filter_spec(&filter_options,\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex f1a3ff1021..7ecef5f768 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -268,6 +268,27 @@ test_expect_success 'warn if using server-option with fetch with legacy protocol\n \tgrep \"Ignoring server options\" err\n '\n \n+test_expect_success 'server-options are sent when cloning' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\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+\n+\tgrep \"server-option=hello\" log &&\n+\tgrep \"server-option=world\" 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+\tGIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n+\t\tclone --server-option=hello --server-option=world \\\n+\t\t\"file://$(pwd)/file_parent\" myclone 2>err &&\n+\n+\tgrep \"Ignoring server options\" 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-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373534","messageId":"20190409204525.GA92879@google.com","threadId":"50881","inReplyTo":"af3cc05324f53316eedb2f437789eacb24c11489.1554841624.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 1/2] transport: warn if server options are unsupported","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-04-09T20:45:25Z","receivedAt":"2019-04-09T20:45:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJonathan Tan wrote:\n\n> Server options were added in commit 5e3548ef16 (\"fetch: send server\n> options when using protocol v2\", 2018-04-24), supported only for\n> protocol version 2. Add a warning if server options are specified for\n> the user if a legacy protocol is used instead.\n>\n> An effort is made to avoid printing the same warning more than once by\n> clearing transport->server_options after the warning, but this does not\n> fully avoid double-printing (for example, when backfulling tags using\n> another fetch with a non-reusable transport).\n>\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> ---\n>  t/t5702-protocol-v2.sh | 17 +++++++++++++++++\n>  transport.c            |  8 ++++++++\n>  2 files changed, 25 insertions(+)\n\nThanks for writing this.  I'd be in favor of making this die().\nCompare what we already do in push:\n\n\tif (args->push_options && !push_options_supported)\n\t\tdie(_(\"the receiving end does not support push options\"));\n\nWhat happens in the case of push with protocol v0, where server options\nare supported?\n\n[...]\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -252,6 +252,12 @@ static int connect_setup(struct transport *transport, int for_push)\n>  \treturn 0;\n>  }\n>  \n> +static void warn_server_options_unsupported(struct transport *transport)\n> +{\n> +\twarning(_(\"Ignoring server options because protocol version does not support it\"));\n\nnits:\n- error and warning messages tend to use lowercase\n- the user running into this may want to know how to switch to a better\n  protocol version.  Is there e.g. a manpage we can point them to?\n\nFor example:\n\n\tfatal: server options require protocol version 2 or later\n\thint: see protocol.version in \"git help config\" for more details\n\n> +\ttransport->server_options = NULL;\n\nShould this use a static to also warn only once in the tag-catchup case\nyou mentioned?\n\n> +}\n> +\n>  /*\n>   * Obtains the protocol version from the transport and writes it to\n>   * transport->data->version, first connecting if not already connected.\n> @@ -286,6 +292,7 @@ static struct ref *handshake(struct transport *transport, int for_push,\n>  \t\tbreak;\n>  \tcase protocol_v1:\n>  \tcase protocol_v0:\n> +\t\twarn_server_options_unsupported(transport);\n>  \t\tget_remote_heads(&reader, &refs,\n>  \t\t\t\t for_push ? REF_NORMAL : 0,\n>  \t\t\t\t &data->extra_have,\n> @@ -363,6 +370,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n>  \t\tbreak;\n>  \tcase protocol_v1:\n>  \tcase protocol_v0:\n> +\t\twarn_server_options_unsupported(transport);\n>  \t\trefs = fetch_pack(&args, data->fd, data->conn,\n\nLooks like this only affects fetch, so the question above about push\nis only about the commit message.\n\nWith whatever subset of the suggested changes makes sense,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks.\n"},{"id":"373535","messageId":"20190409204616.GB92879@google.com","threadId":"50881","inReplyTo":"142c25abd2c5c5c477c44d0e47cb20a3d504ab1c.1554841624.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 2/2] clone: send server options when using protocol v2","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-04-09T20:46:16Z","receivedAt":"2019-04-09T20:46:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Tan wrote:\n\n> Commit 5e3548ef16 (\"fetch: send server options when using protocol v2\",\n> 2018-04-24) taught \"fetch\" the ability to send server options when using\n> protocol v2, but not \"clone\". This ability is triggered by \"-o\" or\n> \"--server-option\".\n>\n> Teach \"clone\" the same ability, except that because \"clone\" already\n> has \"-o\" for another parameter, teach \"clone\" only to receive\n> \"--server-option\".\n>\n> Explain in the documentation, both for clone and for fetch, that server\n> handling of server options are server-specific.\n>\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  Documentation/fetch-options.txt |  3 ++-\n>  Documentation/git-clone.txt     |  8 ++++++++\n>  builtin/clone.c                 |  6 ++++++\n>  t/t5702-protocol-v2.sh          | 21 +++++++++++++++++++++\n>  4 files changed, 37 insertions(+), 1 deletion(-)\n\nThis is indeed\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks.\n"},{"id":"373565","messageId":"xmqq4l7634d6.fsf@gitster-ct.c.googlers.com","threadId":"50881","inReplyTo":"20190409204525.GA92879@google.com","subject":"Re: [PATCH v2 1/2] transport: warn if server options are unsupported","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-10T03:51:01Z","receivedAt":"2019-04-10T03:51:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Thanks for writing this.  I'd be in favor of making this die().\n> Compare what we already do in push:\n>\n> \tif (args->push_options && !push_options_supported)\n> \t\tdie(_(\"the receiving end does not support push options\"));\n\nThat's a good point.\n\n> What happens in the case of push with protocol v0, where server options\n> are supported?\n\nAre, or are not?\n\nIn any case, all other suggestions in your reviews are worth\nfollowing up, I would think.  Thanks for a quick review.\n"},{"id":"373683","messageId":"cover.1555014408.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"20190405204413.93900-1-jonathantanmy@google.com","subject":"[PATCH v3 0/2] Server options when cloning","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-11T20:30:14Z","receivedAt":"2019-04-11T20:30:22Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Thanks, Jonathan Nieder, for your review.\n\n> Thanks for writing this.  I'd be in favor of making this die().\n> Compare what we already do in push:\n> \n> \tif (args->push_options && !push_options_supported)\n> \t\tdie(_(\"the receiving end does not support push options\"));\n\nThanks for pointing out what \"push\" does, and done.\n\n> What happens in the case of push with protocol v0, where server options\n> are supported?\n\nThey are just sent to the pre-receive and post-receive hooks, according\nto the \"git push\" documentation. I added a mention of the push option\nbehavior in the 2nd commit's message.\n\n> For example:\n> \n> \tfatal: server options require protocol version 2 or later\n> \thint: see protocol.version in \"git help config\" for more details\n\nThanks - used your example (except I put the hint first, since the fatal\nline is fatal).\n\n> Should this use a static to also warn only once in the tag-catchup case\n> you mentioned?\n\nSince we're dying, this is no longer needed.\n\nJonathan Tan (2):\n  transport: warn if server options are unsupported\n  clone: send server options when using protocol v2\n\n Documentation/fetch-options.txt |  3 ++-\n Documentation/git-clone.txt     |  8 +++++++\n builtin/clone.c                 |  6 +++++\n t/t5702-protocol-v2.sh          | 41 +++++++++++++++++++++++++++++++++\n transport.c                     | 10 ++++++++\n 5 files changed, 67 insertions(+), 1 deletion(-)\n\nRange-diff against v2:\n1:  af3cc05324 ! 1:  63049081c9 transport: warn if server options are unsupported\n    @@ -4,13 +4,13 @@\n     \n         Server options were added in commit 5e3548ef16 (\"fetch: send server\n         options when using protocol v2\", 2018-04-24), supported only for\n    -    protocol version 2. Add a warning if server options are specified for\n    -    the user if a legacy protocol is used instead.\n    +    protocol version 2. But if the user specifies server options, and the\n    +    protocol version being used doesn't support them, the server options are\n    +    silently ignored.\n     \n    -    An effort is made to avoid printing the same warning more than once by\n    -    clearing transport->server_options after the warning, but this does not\n    -    fully avoid double-printing (for example, when backfulling tags using\n    -    another fetch with a non-reusable transport).\n    +    Teach any transport users to die instead in this situation, just like\n    +    how \"push\" dies if push options are provided when the server doesn't\n    +    support them.\n     \n         Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n     \n    @@ -22,10 +22,11 @@\n      '\n      \n     +test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n    -+\tGIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n    ++\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \\\n     +\t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n     +\n    -+\tgrep \"Ignoring server options\" err\n    ++\tgrep \"see protocol.version in\" err &&\n    ++\tgrep \"server options require protocol version 2 or later\" err\n     +'\n      \n      test_expect_success 'clone with file:// using protocol v2' '\n    @@ -39,10 +40,11 @@\n     +\n     +\tgit init temp_child &&\n     +\n    -+\tGIT_TEST_PROTOCOL_VERSION=0 git -C temp_child -c protocol.version=0 \\\n    ++\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -C temp_child -c protocol.version=0 \\\n     +\t\tfetch -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n     +\n    -+\tgrep \"Ignoring server options\" err\n    ++\tgrep \"see protocol.version in\" err &&\n    ++\tgrep \"server options require protocol version 2 or later\" err\n     +'\n     +\n      test_expect_success 'upload-pack respects config using protocol v2' '\n    @@ -56,10 +58,12 @@\n      \treturn 0;\n      }\n      \n    -+static void warn_server_options_unsupported(struct transport *transport)\n    ++static void die_if_server_options(struct transport *transport)\n     +{\n    -+\twarning(_(\"Ignoring server options because protocol version does not support it\"));\n    -+\ttransport->server_options = NULL;\n    ++\tif (!transport->server_options || !transport->server_options->nr)\n    ++\t\treturn;\n    ++\tadvise(_(\"see protocol.version in 'git help config' for more details\"));\n    ++\tdie(_(\"server options require protocol version 2 or later\"));\n     +}\n     +\n      /*\n    @@ -69,7 +73,7 @@\n      \t\tbreak;\n      \tcase protocol_v1:\n      \tcase protocol_v0:\n    -+\t\twarn_server_options_unsupported(transport);\n    ++\t\tdie_if_server_options(transport);\n      \t\tget_remote_heads(&reader, &refs,\n      \t\t\t\t for_push ? REF_NORMAL : 0,\n      \t\t\t\t &data->extra_have,\n    @@ -77,7 +81,7 @@\n      \t\tbreak;\n      \tcase protocol_v1:\n      \tcase protocol_v0:\n    -+\t\twarn_server_options_unsupported(transport);\n    ++\t\tdie_if_server_options(transport);\n      \t\trefs = fetch_pack(&args, data->fd, data->conn,\n      \t\t\t\t  refs_tmp ? refs_tmp : transport->remote_refs,\n      \t\t\t\t  dest, to_fetch, nr_heads, &data->shallow,\n2:  142c25abd2 ! 2:  f59b8244eb clone: send server options when using protocol v2\n    @@ -12,7 +12,9 @@\n         \"--server-option\".\n     \n         Explain in the documentation, both for clone and for fetch, that server\n    -    handling of server options are server-specific.\n    +    handling of server options are server-specific. This is similar to\n    +    receive-pack's handling of push options - currently, they are just sent\n    +    to hooks to interpret as they see fit.\n     \n         Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n         Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n    @@ -86,7 +88,7 @@\n      --- a/t/t5702-protocol-v2.sh\n      +++ b/t/t5702-protocol-v2.sh\n     @@\n    - \tgrep \"Ignoring server options\" err\n    + \tgrep \"server options require protocol version 2 or later\" err\n      '\n      \n     +test_expect_success 'server-options are sent when cloning' '\n    @@ -103,11 +105,12 @@\n     +test_expect_success 'warn if using server-option with clone with legacy protocol' '\n     +\ttest_when_finished \"rm -rf myclone\" &&\n     +\n    -+\tGIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n    ++\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \\\n     +\t\tclone --server-option=hello --server-option=world \\\n     +\t\t\"file://$(pwd)/file_parent\" myclone 2>err &&\n     +\n    -+\tgrep \"Ignoring server options\" err\n    ++\tgrep \"see protocol.version in\" err &&\n    ++\tgrep \"server options require protocol version 2 or later\" err\n     +'\n     +\n      test_expect_success 'upload-pack respects config using protocol v2' '\n-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373684","messageId":"63049081c98b8612bd233a6f01104c6196a09946.1555014408.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"cover.1555014408.git.jonathantanmy@google.com","subject":"[PATCH v3 1/2] transport: warn if server options are unsupported","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-11T20:30:15Z","receivedAt":"2019-04-11T20:30:24Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Server options were added in commit 5e3548ef16 (\"fetch: send server\noptions when using protocol v2\", 2018-04-24), supported only for\nprotocol version 2. But if the user specifies server options, and the\nprotocol version being used doesn't support them, the server options are\nsilently ignored.\n\nTeach any transport users to die instead in this situation, just like\nhow \"push\" dies if push options are provided when the server doesn't\nsupport them.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5702-protocol-v2.sh | 19 +++++++++++++++++++\n transport.c            | 10 ++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex db4ae09f2f..7ddcf26e76 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -182,6 +182,13 @@ test_expect_success 'server-options are sent when using ls-remote' '\n \tgrep \"server-option=world\" log\n '\n \n+test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n+\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \\\n+\t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n+\n+\tgrep \"see protocol.version in\" err &&\n+\tgrep \"server options require protocol version 2 or later\" err\n+'\n \n test_expect_success 'clone with file:// using protocol v2' '\n \ttest_when_finished \"rm -f log\" &&\n@@ -251,6 +258,18 @@ test_expect_success 'server-options are sent when fetching' '\n \tgrep \"server-option=world\" 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+\tgit init temp_child &&\n+\n+\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -C temp_child -c protocol.version=0 \\\n+\t\tfetch -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n+\n+\tgrep \"see protocol.version in\" err &&\n+\tgrep \"server options require protocol version 2 or later\" 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 e078812897..77446119da 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -252,6 +252,14 @@ static int connect_setup(struct transport *transport, int for_push)\n \treturn 0;\n }\n \n+static void die_if_server_options(struct transport *transport)\n+{\n+\tif (!transport->server_options || !transport->server_options->nr)\n+\t\treturn;\n+\tadvise(_(\"see protocol.version in 'git help config' for more details\"));\n+\tdie(_(\"server options require protocol version 2 or later\"));\n+}\n+\n /*\n  * Obtains the protocol version from the transport and writes it to\n  * transport->data->version, first connecting if not already connected.\n@@ -286,6 +294,7 @@ static struct ref *handshake(struct transport *transport, int for_push,\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n+\t\tdie_if_server_options(transport);\n \t\tget_remote_heads(&reader, &refs,\n \t\t\t\t for_push ? REF_NORMAL : 0,\n \t\t\t\t &data->extra_have,\n@@ -363,6 +372,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n+\t\tdie_if_server_options(transport);\n \t\trefs = fetch_pack(&args, data->fd, data->conn,\n \t\t\t\t  refs_tmp ? refs_tmp : transport->remote_refs,\n \t\t\t\t  dest, to_fetch, nr_heads, &data->shallow,\n-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373685","messageId":"f59b8244eb3cf75ceeb21f52a6679a5fb3b96fd1.1555014408.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"cover.1555014408.git.jonathantanmy@google.com","subject":"[PATCH v3 2/2] clone: send server options when using protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-11T20:30:16Z","receivedAt":"2019-04-11T20:30:27Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Commit 5e3548ef16 (\"fetch: send server options when using protocol v2\",\n2018-04-24) taught \"fetch\" the ability to send server options when using\nprotocol v2, but not \"clone\". This ability is triggered by \"-o\" or\n\"--server-option\".\n\nTeach \"clone\" the same ability, except that because \"clone\" already\nhas \"-o\" for another parameter, teach \"clone\" only to receive\n\"--server-option\".\n\nExplain in the documentation, both for clone and for fetch, that server\nhandling of server options are server-specific. This is similar to\nreceive-pack's handling of push options - currently, they are just sent\nto hooks to interpret as they see fit.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/fetch-options.txt |  3 ++-\n Documentation/git-clone.txt     |  8 ++++++++\n builtin/clone.c                 |  6 ++++++\n t/t5702-protocol-v2.sh          | 22 ++++++++++++++++++++++\n 4 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex fa0a3151b3..91c47752ec 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -216,7 +216,8 @@ endif::git-pull[]\n --server-option=<option>::\n \tTransmit the given string to the server when communicating using\n \tprotocol version 2.  The given string must not contain a NUL or LF\n-\tcharacter.\n+\tcharacter.  The server's handling of server options, including\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 \ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 2fd12524f9..a0f14b51f2 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -131,6 +131,14 @@ objects from the source repository into a pack in the cloned repository.\n \tis specified. This flag forces progress status even if the\n \tstandard error stream is not directed to a terminal.\n \n+--server-option=<option>::\n+\tTransmit the given string to the server when communicating using\n+\tprotocol version 2.  The given string must not contain a NUL or LF\n+\tcharacter.  The server's handling of server options, including\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+\n --no-checkout::\n -n::\n \tNo checkout of HEAD is performed after the clone is complete.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 50bde99618..31a47d190a 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -66,6 +66,7 @@ static int option_dissociate;\n static int max_jobs = -1;\n static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options;\n+static struct string_list server_options = STRING_LIST_INIT_NODUP;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -137,6 +138,8 @@ static struct option builtin_clone_options[] = {\n \t\t   N_(\"separate git dir from working tree\")),\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+\t\t\tN_(\"server-specific\"), N_(\"option to transmit\")),\n \tOPT_SET_INT('4', \"ipv4\", &family, N_(\"use IPv4 addresses only\"),\n \t\t\tTRANSPORT_FAMILY_IPV4),\n \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n@@ -1136,6 +1139,9 @@ 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+\n \tif (filter_options.choice) {\n \t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n \t\texpand_list_objects_filter_spec(&filter_options,\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 7ddcf26e76..b8d5264a45 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -270,6 +270,28 @@ test_expect_success 'warn if using server-option with fetch with legacy protocol\n \tgrep \"server options require protocol version 2 or later\" err\n '\n \n+test_expect_success 'server-options are sent when cloning' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\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+\n+\tgrep \"server-option=hello\" log &&\n+\tgrep \"server-option=world\" 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+\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \\\n+\t\tclone --server-option=hello --server-option=world \\\n+\t\t\"file://$(pwd)/file_parent\" myclone 2>err &&\n+\n+\tgrep \"see protocol.version in\" err &&\n+\tgrep \"server options require protocol version 2 or later\" 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-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373710","messageId":"xmqqzhovyee7.fsf@gitster-ct.c.googlers.com","threadId":"50881","inReplyTo":"63049081c98b8612bd233a6f01104c6196a09946.1555014408.git.jonathantanmy@google.com","subject":"Re: [PATCH v3 1/2] transport: warn if server options are unsupported","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-12T05:35:28Z","receivedAt":"2019-04-12T05:35:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan <jonathantanmy@google.com> writes:\n\n> +test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n> +\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \\\n> +\t\tls-remote -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n\ntest_must_fail is not an executable but is a shell function.  Do not\nexpect VAR=VAL in front of it to work as expected.\n\n\"make test\" triggered test-lint that caught this.\n"},{"id":"373759","messageId":"cover.1555098572.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"20190405204413.93900-1-jonathantanmy@google.com","subject":"[PATCH v4 0/2] Server options when cloning","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-12T19:51:20Z","receivedAt":"2019-04-12T19:51:28Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Thanks, Junio, for spotting the test_must_fail interaction with\nenvironment variables. I've updated that, and I also noticed that the\nfirst patch's commit message title should say \"die\" instead of \"warn\",\nso here is another version.\n\nJonathan Tan (2):\n  transport: die if server options are unsupported\n  clone: send server options when using protocol v2\n\n Documentation/fetch-options.txt |  3 ++-\n Documentation/git-clone.txt     |  8 +++++++\n builtin/clone.c                 |  6 +++++\n t/t5702-protocol-v2.sh          | 41 +++++++++++++++++++++++++++++++++\n transport.c                     | 10 ++++++++\n 5 files changed, 67 insertions(+), 1 deletion(-)\n\nRange-diff against v3:\n1:  63049081c9 ! 1:  598b2de016 transport: warn if server options are unsupported\n    @@ -1,6 +1,6 @@\n     Author: Jonathan Tan <jonathantanmy@google.com>\n     \n    -    transport: warn if server options are unsupported\n    +    transport: die if server options are unsupported\n     \n         Server options were added in commit 5e3548ef16 (\"fetch: send server\n         options when using protocol v2\", 2018-04-24), supported only for\n    @@ -22,7 +22,7 @@\n      '\n      \n     +test_expect_success 'warn if using server-option with ls-remote with legacy protocol' '\n    -+\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \\\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\" master 2>err &&\n     +\n     +\tgrep \"see protocol.version in\" err &&\n    @@ -40,7 +40,7 @@\n     +\n     +\tgit init temp_child &&\n     +\n    -+\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -C temp_child -c protocol.version=0 \\\n    ++\ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -C temp_child -c protocol.version=0 \\\n     +\t\tfetch -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n     +\n     +\tgrep \"see protocol.version in\" err &&\n2:  f59b8244eb ! 2:  48bdd462c4 clone: send server options when using protocol v2\n    @@ -105,7 +105,7 @@\n     +test_expect_success 'warn if using server-option with clone with legacy protocol' '\n     +\ttest_when_finished \"rm -rf myclone\" &&\n     +\n    -+\tGIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \\\n    ++\ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n     +\t\tclone --server-option=hello --server-option=world \\\n     +\t\t\"file://$(pwd)/file_parent\" myclone 2>err &&\n     +\n-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373760","messageId":"598b2de0162e240521f04d40d41eb11843be7198.1555098572.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"cover.1555098572.git.jonathantanmy@google.com","subject":"[PATCH v4 1/2] transport: die if server options are unsupported","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-12T19:51:21Z","receivedAt":"2019-04-12T19:51:32Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Server options were added in commit 5e3548ef16 (\"fetch: send server\noptions when using protocol v2\", 2018-04-24), supported only for\nprotocol version 2. But if the user specifies server options, and the\nprotocol version being used doesn't support them, the server options are\nsilently ignored.\n\nTeach any transport users to die instead in this situation, just like\nhow \"push\" dies if push options are provided when the server doesn't\nsupport them.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5702-protocol-v2.sh | 19 +++++++++++++++++++\n transport.c            | 10 ++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex db4ae09f2f..1e8357a4c7 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -182,6 +182,13 @@ test_expect_success 'server-options are sent when using ls-remote' '\n \tgrep \"server-option=world\" 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\" master 2>err &&\n+\n+\tgrep \"see protocol.version in\" err &&\n+\tgrep \"server options require protocol version 2 or later\" err\n+'\n \n test_expect_success 'clone with file:// using protocol v2' '\n \ttest_when_finished \"rm -f log\" &&\n@@ -251,6 +258,18 @@ test_expect_success 'server-options are sent when fetching' '\n \tgrep \"server-option=world\" 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+\tgit init temp_child &&\n+\n+\ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -C temp_child -c protocol.version=0 \\\n+\t\tfetch -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n+\n+\tgrep \"see protocol.version in\" err &&\n+\tgrep \"server options require protocol version 2 or later\" 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 e078812897..77446119da 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -252,6 +252,14 @@ static int connect_setup(struct transport *transport, int for_push)\n \treturn 0;\n }\n \n+static void die_if_server_options(struct transport *transport)\n+{\n+\tif (!transport->server_options || !transport->server_options->nr)\n+\t\treturn;\n+\tadvise(_(\"see protocol.version in 'git help config' for more details\"));\n+\tdie(_(\"server options require protocol version 2 or later\"));\n+}\n+\n /*\n  * Obtains the protocol version from the transport and writes it to\n  * transport->data->version, first connecting if not already connected.\n@@ -286,6 +294,7 @@ static struct ref *handshake(struct transport *transport, int for_push,\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n+\t\tdie_if_server_options(transport);\n \t\tget_remote_heads(&reader, &refs,\n \t\t\t\t for_push ? REF_NORMAL : 0,\n \t\t\t\t &data->extra_have,\n@@ -363,6 +372,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n+\t\tdie_if_server_options(transport);\n \t\trefs = fetch_pack(&args, data->fd, data->conn,\n \t\t\t\t  refs_tmp ? refs_tmp : transport->remote_refs,\n \t\t\t\t  dest, to_fetch, nr_heads, &data->shallow,\n-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373761","messageId":"48bdd462c4e6f609827f4970ee76316e3b54b400.1555098572.git.jonathantanmy@google.com","threadId":"50881","inReplyTo":"cover.1555098572.git.jonathantanmy@google.com","subject":"[PATCH v4 2/2] clone: send server options when using protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-04-12T19:51:22Z","receivedAt":"2019-04-12T19:51:34Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Commit 5e3548ef16 (\"fetch: send server options when using protocol v2\",\n2018-04-24) taught \"fetch\" the ability to send server options when using\nprotocol v2, but not \"clone\". This ability is triggered by \"-o\" or\n\"--server-option\".\n\nTeach \"clone\" the same ability, except that because \"clone\" already\nhas \"-o\" for another parameter, teach \"clone\" only to receive\n\"--server-option\".\n\nExplain in the documentation, both for clone and for fetch, that server\nhandling of server options are server-specific. This is similar to\nreceive-pack's handling of push options - currently, they are just sent\nto hooks to interpret as they see fit.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/fetch-options.txt |  3 ++-\n Documentation/git-clone.txt     |  8 ++++++++\n builtin/clone.c                 |  6 ++++++\n t/t5702-protocol-v2.sh          | 22 ++++++++++++++++++++++\n 4 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex fa0a3151b3..91c47752ec 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -216,7 +216,8 @@ endif::git-pull[]\n --server-option=<option>::\n \tTransmit the given string to the server when communicating using\n \tprotocol version 2.  The given string must not contain a NUL or LF\n-\tcharacter.\n+\tcharacter.  The server's handling of server options, including\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 \ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 2fd12524f9..a0f14b51f2 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -131,6 +131,14 @@ objects from the source repository into a pack in the cloned repository.\n \tis specified. This flag forces progress status even if the\n \tstandard error stream is not directed to a terminal.\n \n+--server-option=<option>::\n+\tTransmit the given string to the server when communicating using\n+\tprotocol version 2.  The given string must not contain a NUL or LF\n+\tcharacter.  The server's handling of server options, including\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+\n --no-checkout::\n -n::\n \tNo checkout of HEAD is performed after the clone is complete.\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 50bde99618..31a47d190a 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -66,6 +66,7 @@ static int option_dissociate;\n static int max_jobs = -1;\n static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;\n static struct list_objects_filter_options filter_options;\n+static struct string_list server_options = STRING_LIST_INIT_NODUP;\n \n static int recurse_submodules_cb(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -137,6 +138,8 @@ static struct option builtin_clone_options[] = {\n \t\t   N_(\"separate git dir from working tree\")),\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+\t\t\tN_(\"server-specific\"), N_(\"option to transmit\")),\n \tOPT_SET_INT('4', \"ipv4\", &family, N_(\"use IPv4 addresses only\"),\n \t\t\tTRANSPORT_FAMILY_IPV4),\n \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n@@ -1136,6 +1139,9 @@ 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+\n \tif (filter_options.choice) {\n \t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n \t\texpand_list_objects_filter_spec(&filter_options,\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1e8357a4c7..2a3575d38c 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -270,6 +270,28 @@ test_expect_success 'warn if using server-option with fetch with legacy protocol\n \tgrep \"server options require protocol version 2 or later\" err\n '\n \n+test_expect_success 'server-options are sent when cloning' '\n+\ttest_when_finished \"rm -rf log myclone\" &&\n+\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+\n+\tgrep \"server-option=hello\" log &&\n+\tgrep \"server-option=world\" 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+\ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \\\n+\t\tclone --server-option=hello --server-option=world \\\n+\t\t\"file://$(pwd)/file_parent\" myclone 2>err &&\n+\n+\tgrep \"see protocol.version in\" err &&\n+\tgrep \"server options require protocol version 2 or later\" 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-- \n2.21.0.392.gf8f6787159e-goog\n\n"},{"id":"373762","messageId":"20190412201228.GA15936@szeder.dev","threadId":"50881","inReplyTo":"598b2de0162e240521f04d40d41eb11843be7198.1555098572.git.jonathantanmy@google.com","subject":"Re: [PATCH v4 1/2] transport: die if server options are unsupported","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-04-12T20:12:28Z","receivedAt":"2019-04-12T20:12:40Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Apr 12, 2019 at 12:51:21PM -0700, Jonathan Tan wrote:\n\n[Reordering the diffs...]\n\n> diff --git a/transport.c b/transport.c\n> index e078812897..77446119da 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -252,6 +252,14 @@ static int connect_setup(struct transport *transport, int for_push)\n>  \treturn 0;\n>  }\n>  \n> +static void die_if_server_options(struct transport *transport)\n> +{\n> +\tif (!transport->server_options || !transport->server_options->nr)\n> +\t\treturn;\n> +\tadvise(_(\"see protocol.version in 'git help config' for more details\"));\n> +\tdie(_(\"server options require protocol version 2 or later\"));\n\nThese two messages are translated ...\n\n\n> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\n> index db4ae09f2f..1e8357a4c7 100755\n> --- a/t/t5702-protocol-v2.sh\n> +++ b/t/t5702-protocol-v2.sh\n> @@ -182,6 +182,13 @@ test_expect_success 'server-options are sent when using ls-remote' '\n>  \tgrep \"server-option=world\" 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\" master 2>err &&\n> +\n> +\tgrep \"see protocol.version in\" err &&\n> +\tgrep \"server options require protocol version 2 or later\" err\n\n... therefore these should be 'test_i18ngrep'.  This applies to the\nother tests in this patch series as well.\n\n> +'\n>  \n>  test_expect_success 'clone with file:// using protocol v2' '\n>  \ttest_when_finished \"rm -f log\" &&\n> @@ -251,6 +258,18 @@ test_expect_success 'server-options are sent when fetching' '\n>  \tgrep \"server-option=world\" 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> +\tgit init temp_child &&\n> +\n> +\ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -C temp_child -c protocol.version=0 \\\n> +\t\tfetch -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n> +\n> +\tgrep \"see protocol.version in\" err &&\n> +\tgrep \"server options require protocol version 2 or later\" 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"},{"id":"373844","messageId":"xmqqlg0bvppc.fsf_-_@gitster-ct.c.googlers.com","threadId":"50881","inReplyTo":"20190412201228.GA15936@szeder.dev","subject":"Re* [PATCH v4 1/2] transport: die if server options are unsupported","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-15T04:48:31Z","receivedAt":"2019-04-15T04:48:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> ... therefore these should be 'test_i18ngrep'.  This applies to the\n> other tests in this patch series as well.\n> ...\n\nHmph.  That would mean the following is needed, but I suspect that\n6cdccfce (\"i18n: make GETTEXT_POISON a runtime option\", 2018-11-08)\nis somewhat broken (see the bottom for a suggested fix).\n\n t/t5702-protocol-v2.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1e8357a4c7..f8912211e0 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -186,8 +186,8 @@ test_expect_success 'warn if using server-option with ls-remote with legacy prot\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\" master 2>err &&\n \n-\tgrep \"see protocol.version in\" err &&\n-\tgrep \"server options require protocol version 2 or later\" err\n+\ttest_i18ngrep \"see protocol.version in\" err &&\n+\ttest_i18ngrep \"server options require protocol version 2 or later\" err\n '\n \n test_expect_success 'clone with file:// using protocol v2' '\n@@ -266,8 +266,8 @@ test_expect_success 'warn if using server-option with fetch with legacy protocol\n \ttest_must_fail env GIT_TEST_PROTOCOL_VERSION=0 git -C temp_child -c protocol.version=0 \\\n \t\tfetch -o hello -o world \"file://$(pwd)/file_parent\" master 2>err &&\n \n-\tgrep \"see protocol.version in\" err &&\n-\tgrep \"server options require protocol version 2 or later\" err\n+\ttest_i18ngrep \"see protocol.version in\" err &&\n+\ttest_i18ngrep \"server options require protocol version 2 or later\" err\n '\n \n test_expect_success 'upload-pack respects config using protocol v2' '\n\n\n-- >8 --\nSubject: [PATCH] gettext tests: export the restored GIT_TEST_GETTEXT_POISON\n\n6cdccfce (\"i18n: make GETTEXT_POISON a runtime option\", 2018-11-08)\nmade the gettext-poison test a runtime option (which was a good\nmove) and adjusted the test framework so that Git commands we run as\npart of the framework, as opposed to the ones that are part of the\ntest proper, are not affected by the setting.  The original value\nfor the GIT_TEST_GETTEXT_POISON environment variable is saved away\nin another variable and gets unset, and then later the saved value\nis restored to the environment variable.\n\nBut the code forgot to export the variable again, which is necessary\nto restore the \"export\" bit that was lost when the variable was unset.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/test-lib.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8665b0a9b6..078efda9da 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1394,6 +1394,7 @@ test -z \"$NO_GETTEXT\" && test_set_prereq GETTEXT\n if test -n \"$GIT_TEST_GETTEXT_POISON_ORIG\"\n then\n \tGIT_TEST_GETTEXT_POISON=$GIT_TEST_GETTEXT_POISON_ORIG\n+\texport GIT_TEST_GETTEXT_POISON\n \tunset GIT_TEST_GETTEXT_POISON_ORIG\n fi\n \n-- \n2.21.0-313-ge35b8cb8e2\n\n"},{"id":"373845","messageId":"xmqqef63vpew.fsf@gitster-ct.c.googlers.com","threadId":"50881","inReplyTo":"xmqqlg0bvppc.fsf_-_@gitster-ct.c.googlers.com","subject":"Re: Re* [PATCH v4 1/2] transport: die if server options are unsupported","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-15T04:54:47Z","receivedAt":"2019-04-15T04:54:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> SZEDER Gábor <szeder.dev@gmail.com> writes:\n>\n>> ... therefore these should be 'test_i18ngrep'.  This applies to the\n>> other tests in this patch series as well.\n>> ...\n>\n> Hmph.  That would mean the following is needed,...\n\nAnd a corresponding fix for step 2/2 would look like this.\n\n t/t5702-protocol-v2.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 324bdf5715..9b048f6c8b 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -288,8 +288,8 @@ test_expect_success 'warn if using server-option with clone with legacy protocol\n \t\tclone --server-option=hello --server-option=world \\\n \t\t\"file://$(pwd)/file_parent\" myclone 2>err &&\n \n-\tgrep \"see protocol.version in\" err &&\n-\tgrep \"server options require protocol version 2 or later\" err\n+\ttest_i18ngrep \"see protocol.version in\" err &&\n+\ttest_i18ngrep \"server options require protocol version 2 or later\" err\n '\n \n test_expect_success 'upload-pack respects config using protocol v2' '\n"},{"id":"373852","messageId":"20190415094347.GE15936@szeder.dev","threadId":"50881","inReplyTo":"xmqqlg0bvppc.fsf_-_@gitster-ct.c.googlers.com","subject":"Re: Re* [PATCH v4 1/2] transport: die if server options are unsupported","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-04-15T09:43:47Z","receivedAt":"2019-04-15T09:43:54Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Apr 15, 2019 at 01:48:31PM +0900, Junio C Hamano wrote:\n> SZEDER Gábor <szeder.dev@gmail.com> writes:\n> \n> > ... therefore these should be 'test_i18ngrep'.  This applies to the\n> > other tests in this patch series as well.\n> > ...\n> \n> Hmph.  That would mean the following is needed, but I suspect that\n> 6cdccfce (\"i18n: make GETTEXT_POISON a runtime option\", 2018-11-08)\n> is somewhat broken (see the bottom for a suggested fix).\n\nOh, wow, indeed.\n\nI still notice these [1], because I prefer to run the GETTEXT_POISION\nCI jobs in my half-baked \"scrambled\" mode [2], but haven't yet got\naround to deal with the conflicts between those patches and\n'ab/dynamic-gettext-poison', so my script prepping in-flight topics\nfor my CI builds still starts with reverting\n'ab/dynamic-gettext-poison', restoring that missing 'export'.\n\n\n[1] https://travis-ci.org/szeder/git-cooking-topics-for-travis-ci/jobs/519163854#L2724\n\n[2] https://public-inbox.org/git/20181022202241.18629-1-szeder.dev@gmail.com/\n\n> -- >8 --\n> Subject: [PATCH] gettext tests: export the restored GIT_TEST_GETTEXT_POISON\n> \n> 6cdccfce (\"i18n: make GETTEXT_POISON a runtime option\", 2018-11-08)\n> made the gettext-poison test a runtime option (which was a good\n> move) and adjusted the test framework so that Git commands we run as\n> part of the framework, as opposed to the ones that are part of the\n> test proper, are not affected by the setting.  The original value\n> for the GIT_TEST_GETTEXT_POISON environment variable is saved away\n> in another variable and gets unset, and then later the saved value\n> is restored to the environment variable.\n> \n> But the code forgot to export the variable again, which is necessary\n> to restore the \"export\" bit that was lost when the variable was unset.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  t/test-lib.sh | 1 +\n>  1 file changed, 1 insertion(+)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 8665b0a9b6..078efda9da 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -1394,6 +1394,7 @@ test -z \"$NO_GETTEXT\" && test_set_prereq GETTEXT\n>  if test -n \"$GIT_TEST_GETTEXT_POISON_ORIG\"\n>  then\n>  \tGIT_TEST_GETTEXT_POISON=$GIT_TEST_GETTEXT_POISON_ORIG\n> +\texport GIT_TEST_GETTEXT_POISON\n>  \tunset GIT_TEST_GETTEXT_POISON_ORIG\n>  fi\n\nMakes sense.\n\n"}]}