{"thread":{"id":"48395","subject":"[PATCH 0/2] Supporting partial clones in protocol v2","startedAt":"2018-05-01T22:22:31Z","lastAt":"2018-05-04T16:10:43Z","messageCount":20,"participants":["Jonathan Tan","Brandon Williams","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"346345","messageId":"cover.1525213052.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":null,"subject":"[PATCH 0/2] Supporting partial clones in protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-01T22:22:19Z","receivedAt":"2018-05-01T22:22:31Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"This patch set is built on \"next\". (I had to build it on \"next\" because\nbw/protocol-v2 is built on v2.16, which does not have support for\npartial clones.)\n\nPartial clones and protocol v2 were built independently, so here is a\npatch to support partial clones in protocol v2.\n\nOne thing I am a little unhappy about is the fact that the upload-pack\nconfig has to be read in multiple places, but perhaps there is no choice\nabout that.\n\nJonathan Tan (2):\n  upload-pack: fix error message typo\n  {fetch,upload}-pack: support filter in protocol v2\n\n Documentation/technical/protocol-v2.txt |  9 +++\n fetch-pack.c                            | 23 +++++-\n t/t5701-git-serve.sh                    | 15 ++++\n t/t5702-protocol-v2.sh                  | 97 +++++++++++++++++++++++++\n upload-pack.c                           | 14 +++-\n 5 files changed, 151 insertions(+), 7 deletions(-)\n\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346346","messageId":"15e1fa5e6167932d878aa08e81d541c403935172.1525213052.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525213052.git.jonathantanmy@google.com","subject":"[PATCH 1/2] upload-pack: fix error message typo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-01T22:22:20Z","receivedAt":"2018-05-01T22:22:34Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Fix a typo in an error message.\n\nAlso, this line was introduced in 3145ea957d2c (\"upload-pack: introduce\nfetch server command\", 2018-03-15), which did not contain a test for the\ncase which causes this error to be printed, so introduce a test.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5701-git-serve.sh | 15 +++++++++++++++\n upload-pack.c        |  2 +-\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\nindex 72d7bc562..abbe5b19e 100755\n--- a/t/t5701-git-serve.sh\n+++ b/t/t5701-git-serve.sh\n@@ -173,4 +173,19 @@ test_expect_success 'symrefs parameter' '\n \ttest_cmp actual expect\n '\n \n+test_expect_success 'unexpected lines are not allowed in fetch request' '\n+\tgit init server &&\n+\n+\t# Custom request that tries to filter even though it is not advertised.\n+\ttest-pkt-line pack >in <<-EOF &&\n+\tcommand=fetch\n+\t0001\n+\tthis-is-not-a-command\n+\t0000\n+\tEOF\n+\n+\ttest_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n+\tgrep \"unexpected line: .this-is-not-a-command.\" err\n+'\n+\n test_done\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 87b4d32a6..c4456bb88 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1252,7 +1252,7 @@ static void process_args(struct packet_reader *request,\n \t\t}\n \n \t\t/* ignore unknown lines maybe? */\n-\t\tdie(\"unexpect line: '%s'\", arg);\n+\t\tdie(\"unexpected line: '%s'\", arg);\n \t}\n }\n \n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346347","messageId":"0486dfdfe614f908b322552b02ebbb3f1f6cd6fc.1525213052.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525213052.git.jonathantanmy@google.com","subject":"[PATCH 2/2] {fetch,upload}-pack: support filter in protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-01T22:22:21Z","receivedAt":"2018-05-01T22:22:37Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"The fetch-pack/upload-pack protocol v2 was developed independently of\nthe filter parameter (used in partial fetches), thus it did not include\nsupport for it. Add support for the filter parameter.\n\nLike in the legacy protocol, the server advertises and supports \"filter\"\nonly if uploadpack.allowfilter is configured.\n\nLike in the legacy protocol, the client continues with a warning if\n\"--filter\" is specified, but the server does not advertise it.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Documentation/technical/protocol-v2.txt |  9 +++\n fetch-pack.c                            | 23 +++++-\n t/t5702-protocol-v2.sh                  | 97 +++++++++++++++++++++++++\n upload-pack.c                           | 12 ++-\n 4 files changed, 135 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 136179d7d..38d24fd2b 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -290,6 +290,15 @@ included in the clients request as well as the potential addition of the\n \tCannot be used with \"deepen\", but can be used with\n \t\"deepen-since\".\n \n+If the 'filter' feature is advertised, the following argument can be\n+included in the client's request:\n+\n+    filter <filter-spec>\n+\tRequest that various objects from the packfile be omitted\n+\tusing one of several filtering techniques. These are intended\n+\tfor use with partial clone and partial fetch operations. See\n+\t`rev-list` for possible \"filter-spec\" values.\n+\n The response of `fetch` is broken into a number of sections separated by\n delimiter packets (0001), with each section beginning with its section\n header.\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex f93723fec..3ed40aa46 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1191,14 +1191,29 @@ static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,\n \telse if (is_repository_shallow() || args->deepen)\n \t\tdie(_(\"Server does not support shallow requests\"));\n \n+\t/* Add filter */\n+\tif (server_supports_feature(\"fetch\", \"filter\", 0) &&\n+\t    args->filter_options.choice) {\n+\t\tprint_verbose(args, _(\"Server supports filter\"));\n+\t\tpacket_buf_write(&req_buf, \"filter %s\",\n+\t\t\t\t args->filter_options.filter_spec);\n+\t} else if (args->filter_options.choice) {\n+\t\twarning(\"filtering not recognized by server, ignoring\");\n+\t}\n+\n \t/* add wants */\n \tadd_wants(wants, &req_buf);\n \n-\t/* Add all of the common commits we've found in previous rounds */\n-\tadd_common(&req_buf, common);\n+\tif (args->no_dependents) {\n+\t\tpacket_buf_write(&req_buf, \"done\");\n+\t\tret = 1;\n+\t} else {\n+\t\t/* Add all of the common commits we've found in previous rounds */\n+\t\tadd_common(&req_buf, common);\n \n-\t/* Add initial haves */\n-\tret = add_haves(&req_buf, haves_to_send, in_vain);\n+\t\t/* Add initial haves */\n+\t\tret = add_haves(&req_buf, haves_to_send, in_vain);\n+\t}\n \n \t/* Send request */\n \tpacket_buf_flush(&req_buf);\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 56f7c3c32..834ccc6f2 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -201,6 +201,103 @@ test_expect_success 'ref advertisment is filtered during fetch using protocol v2\n \t! grep \"refs/tags/three\" log\n '\n \n+test_expect_success 'setup filter tests' '\n+\tgit init server &&\n+\n+\t# 1 commit to create a file, and 1 commit to modify it\n+\ttest_commit -C server message1 a.txt &&\n+\ttest_commit -C server message2 a.txt &&\n+\tgit -C server config protocol.version 2 &&\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tgit -C server config uploadpack.allowanysha1inwant 1 &&\n+\tgit -C server config protocol.version 2\n+'\n+\n+test_expect_success 'partial clone' '\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tclone --filter=blob:none \"file://$(pwd)/server\" client &&\n+\tgrep \"version 2\" trace &&\n+\n+\t# Ensure that the old version of the file is missing\n+\tgit -C client rev-list master --quiet --objects --missing=print \\\n+\t\t>observed.oids &&\n+\tgrep \"$(git -C server rev-parse message1:a.txt)\" observed.oids &&\n+\n+\t# Ensure that client passes fsck\n+\tgit -C client fsck\n+'\n+\n+test_expect_success 'dynamically fetch missing object' '\n+\trm \"$(pwd)/trace\" &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n+\t\tcat-file -p $(git -C server rev-parse message1:a.txt) &&\n+\tgrep \"version 2\" trace\n+'\n+\n+test_expect_success 'partial fetch' '\n+\trm -rf client \"$(pwd)/trace\" &&\n+\tgit init client &&\n+\tSERVER=\"file://$(pwd)/server\" &&\n+\ttest_config -C client extensions.partialClone \"$SERVER\" &&\n+\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n+\t\tfetch --filter=blob:none \"$SERVER\" master:refs/heads/other &&\n+\tgrep \"version 2\" trace &&\n+\n+\t# Ensure that the old version of the file is missing\n+\tgit -C client rev-list other --quiet --objects --missing=print \\\n+\t\t>observed.oids &&\n+\tgrep \"$(git -C server rev-parse message1:a.txt)\" observed.oids &&\n+\n+\t# Ensure that client passes fsck\n+\tgit -C client fsck\n+'\n+\n+test_expect_success 'do not advertise filter if not configured to do so' '\n+\tSERVER=\"file://$(pwd)/server\" &&\n+\n+\trm \"$(pwd)/trace\" &&\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tls-remote \"$SERVER\" &&\n+\tgrep \"fetch=.*filter\" trace &&\n+\n+\trm \"$(pwd)/trace\" &&\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tls-remote \"$SERVER\" &&\n+\tgrep \"fetch=\" trace >fetch_capabilities &&\n+\t! grep filter fetch_capabilities\n+'\n+\n+test_expect_success 'partial clone warns if filter is not advertised' '\n+\trm -rf client &&\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\tgit -c protocol.version=2 \\\n+\t\tclone --filter=blob:none \"file://$(pwd)/server\" client 2>err &&\n+\ttest_i18ngrep \"filtering not recognized by server, ignoring\" err\n+'\n+\n+test_expect_success 'even with handcrafted request, filter does not work if not advertised' '\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\n+\t# Custom request that tries to filter even though it is not advertised.\n+\ttest-pkt-line pack >in <<-EOF &&\n+\tcommand=fetch\n+\t0001\n+\twant $(git -C server rev-parse master)\n+\tfilter blob:none\n+\t0000\n+\tEOF\n+\n+\ttest_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n+\tgrep \"unexpected line: .filter blob:none.\" err &&\n+\n+\t# Exercise to ensure that if advertised, filter works\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tgit -C server serve --stateless-rpc <in >/dev/null\n+'\n+\n # Test protocol v2 with 'http://' transport\n #\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c4456bb88..3872fa518 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1205,6 +1205,7 @@ static void process_args(struct packet_reader *request,\n {\n \twhile (packet_reader_read(request) != PACKET_READ_FLUSH) {\n \t\tconst char *arg = request->line;\n+\t\tconst char *p;\n \n \t\t/* process want */\n \t\tif (parse_want(arg))\n@@ -1251,6 +1252,11 @@ static void process_args(struct packet_reader *request,\n \t\t\tcontinue;\n \t\t}\n \n+\t\tif (allow_filter && skip_prefix(arg, \"filter \", &p)) {\n+\t\t\tparse_list_objects_filter(&filter_options, p);\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\t/* ignore unknown lines maybe? */\n \t\tdie(\"unexpected line: '%s'\", arg);\n \t}\n@@ -1428,7 +1434,9 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n int upload_pack_advertise(struct repository *r,\n \t\t\t  struct strbuf *value)\n {\n-\tif (value)\n-\t\tstrbuf_addstr(value, \"shallow\");\n+\tgit_config(upload_pack_config, NULL);\n+\tif (value) {\n+\t\tstrbuf_addf(value, \"%sshallow\", allow_filter ? \"filter \" : \"\");\n+\t}\n \treturn 1;\n }\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346348","messageId":"20180501152502.eecab4844934aef6d3f553b0@google.com","threadId":"48395","inReplyTo":"15e1fa5e6167932d878aa08e81d541c403935172.1525213052.git.jonathantanmy@google.com","subject":"Re: [PATCH 1/2] upload-pack: fix error message typo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-01T22:25:02Z","receivedAt":"2018-05-01T22:25:07Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue,  1 May 2018 15:22:20 -0700\nJonathan Tan <jonathantanmy@google.com> wrote:\n\n> +test_expect_success 'unexpected lines are not allowed in fetch request' '\n> +\tgit init server &&\n> +\n> +\t# Custom request that tries to filter even though it is not advertised.\n\nOops...I saw this copy-and-paste error right after I sent the e-mails.\nI'll remove this in the next reroll (if there is one).\n\n> +\ttest-pkt-line pack >in <<-EOF &&\n> +\tcommand=fetch\n> +\t0001\n> +\tthis-is-not-a-command\n> +\t0000\n> +\tEOF\n> +\n> +\ttest_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n> +\tgrep \"unexpected line: .this-is-not-a-command.\" err\n> +'\n"},{"id":"346349","messageId":"20180501223619.GA122314@google.com","threadId":"48395","inReplyTo":"0486dfdfe614f908b322552b02ebbb3f1f6cd6fc.1525213052.git.jonathantanmy@google.com","subject":"Re: [PATCH 2/2] {fetch,upload}-pack: support filter in protocol v2","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-05-01T22:36:19Z","receivedAt":"2018-05-01T22:36:27Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/01, Jonathan Tan wrote:\n> The fetch-pack/upload-pack protocol v2 was developed independently of\n> the filter parameter (used in partial fetches), thus it did not include\n> support for it. Add support for the filter parameter.\n> \n> Like in the legacy protocol, the server advertises and supports \"filter\"\n> only if uploadpack.allowfilter is configured.\n> \n> Like in the legacy protocol, the client continues with a warning if\n> \"--filter\" is specified, but the server does not advertise it.\n> \n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> ---\n>  Documentation/technical/protocol-v2.txt |  9 +++\n>  fetch-pack.c                            | 23 +++++-\n>  t/t5702-protocol-v2.sh                  | 97 +++++++++++++++++++++++++\n>  upload-pack.c                           | 12 ++-\n>  4 files changed, 135 insertions(+), 6 deletions(-)\n> \n\n> @@ -1428,7 +1434,9 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n>  int upload_pack_advertise(struct repository *r,\n>  \t\t\t  struct strbuf *value)\n>  {\n> -\tif (value)\n> -\t\tstrbuf_addstr(value, \"shallow\");\n> +\tgit_config(upload_pack_config, NULL);\n> +\tif (value) {\n> +\t\tstrbuf_addf(value, \"%sshallow\", allow_filter ? \"filter \" : \"\");\n> +\t}\n\nThis is a bit difficult to read and there is no reason why we would need\nto read the entire upload_pack_config to determine if we need to filter\nthings (we will need to read the config if cmd \"fetch\" is requested\nthough).  Instead it may be better to do the following:\n\n  if (value) {\n    strbuf_addstr(value, \"shallow\");\n    if (repo_config_get(r, \"uplaodpack.filter\"))\n      strbuf_addstr(value, \" filter\");\n  }\n\nThis way its easier to read and you only are reading the required value\nfrom the config.\n\n>  \treturn 1;\n>  }\n> -- \n> 2.17.0.441.gb46fe60e1d-goog\n> \n\n-- \nBrandon Williams\n"},{"id":"346405","messageId":"cover.1525220786.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525213052.git.jonathantanmy@google.com","subject":"[PATCH v2 0/3] Supporting partial clones in protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-02T00:31:28Z","receivedAt":"2018-05-02T00:31:41Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> This is a bit difficult to read and there is no reason why we would need\n> to read the entire upload_pack_config to determine if we need to filter\n> things (we will need to read the config if cmd \"fetch\" is requested\n> though).  Instead it may be better to do the following:\n> \n>   if (value) {\n>     strbuf_addstr(value, \"shallow\");\n>     if (repo_config_get(r, \"uplaodpack.filter\"))\n>       strbuf_addstr(value, \" filter\");\n>   }\n> \n> This way its easier to read and you only are reading the required value\n> from the config.\n\nThanks, Brandon. I went ahead and used repo_config_get_bool(), and\nindeed it works.\n\nRemoving the call to git_config() from there exposed another issue in\nthat configs were not read if upload-pack was used with protocol v2, so\nI inserted a patch in the middle addressing that. While writing that\npatch, I noticed that uploadpack.packobjectshook couldn't take filenames\nwith spaces, which I think is due to prepare_shell_cmd() in\nrun-command.c not quoting properly. Adding single quotes around \"%s\"\nworked, but made other tests fail. Instead of continuing down that\nrabbit hole, I just made the uploadpack.packobjectshook not have any\nspaces, just like in t5544.\n\nJonathan Tan (3):\n  upload-pack: fix error message typo\n  upload-pack: read config when serving protocol v2\n  {fetch,upload}-pack: support filter in protocol v2\n\n Documentation/technical/protocol-v2.txt |   9 ++\n fetch-pack.c                            |  23 ++++-\n t/t5701-git-serve.sh                    |  14 +++\n t/t5702-protocol-v2.sh                  | 112 ++++++++++++++++++++++++\n upload-pack.c                           |  19 +++-\n 5 files changed, 171 insertions(+), 6 deletions(-)\n\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346406","messageId":"177c6eb7058adc1996f0595ddc3cc848c91b578f.1525220786.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525220786.git.jonathantanmy@google.com","subject":"[PATCH v2 1/3] upload-pack: fix error message typo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-02T00:31:29Z","receivedAt":"2018-05-02T00:31:44Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Fix a typo in an error message.\n\nAlso, this line was introduced in 3145ea957d2c (\"upload-pack: introduce\nfetch server command\", 2018-03-15), which did not contain a test for the\ncase which causes this error to be printed, so introduce a test.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5701-git-serve.sh | 14 ++++++++++++++\n upload-pack.c        |  2 +-\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\nindex 72d7bc562..1b4b13cc2 100755\n--- a/t/t5701-git-serve.sh\n+++ b/t/t5701-git-serve.sh\n@@ -173,4 +173,18 @@ test_expect_success 'symrefs parameter' '\n \ttest_cmp actual expect\n '\n \n+test_expect_success 'unexpected lines are not allowed in fetch request' '\n+\tgit init server &&\n+\n+\ttest-pkt-line pack >in <<-EOF &&\n+\tcommand=fetch\n+\t0001\n+\tthis-is-not-a-command\n+\t0000\n+\tEOF\n+\n+\ttest_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n+\tgrep \"unexpected line: .this-is-not-a-command.\" err\n+'\n+\n test_done\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 87b4d32a6..c4456bb88 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1252,7 +1252,7 @@ static void process_args(struct packet_reader *request,\n \t\t}\n \n \t\t/* ignore unknown lines maybe? */\n-\t\tdie(\"unexpect line: '%s'\", arg);\n+\t\tdie(\"unexpected line: '%s'\", arg);\n \t}\n }\n \n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346407","messageId":"6e85aaa555f6789156626330327085207b2d2dff.1525220786.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525220786.git.jonathantanmy@google.com","subject":"[PATCH v2 2/3] upload-pack: read config when serving protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-02T00:31:30Z","receivedAt":"2018-05-02T00:31:46Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"The upload-pack code paths never call git_config() with\nupload_pack_config() when protocol v2 is used, causing options like\nuploadpack.packobjectshook to not take effect. Ensure that this function\nis called.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5702-protocol-v2.sh | 14 ++++++++++++++\n upload-pack.c          |  2 ++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 56f7c3c32..0ead99993 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -201,6 +201,20 @@ test_expect_success 'ref advertisment is filtered during fetch using protocol v2\n \t! grep \"refs/tags/three\" 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+\t\ttouch hookout\n+\t\t\"$@\"\n+\tEOF\n+\ttest_commit -C server one &&\n+\n+\ttest_config_global uploadpack.packobjectshook ./hook &&\n+\ttest ! -f server/.git/hookout &&\n+\tGIT_TRACE=/tmp/y git -c protocol.version=2 clone \"file://$(pwd)/server\" client &&\n+\ttest -f server/.git/hookout\n+'\n+\n # Test protocol v2 with 'http://' transport\n #\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c4456bb88..113edd32d 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1376,6 +1376,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n \tenum fetch_state state = FETCH_PROCESS_ARGS;\n \tstruct upload_pack_data data;\n \n+\tgit_config(upload_pack_config, NULL);\n+\n \tupload_pack_data_init(&data);\n \tuse_sideband = LARGE_PACKET_MAX;\n \n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346408","messageId":"38aae093ffc23f886ab4aaeff72eae5e7c794df0.1525220786.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525220786.git.jonathantanmy@google.com","subject":"[PATCH v2 3/3] {fetch,upload}-pack: support filter in protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-02T00:31:31Z","receivedAt":"2018-05-02T00:31:49Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"The fetch-pack/upload-pack protocol v2 was developed independently of\nthe filter parameter (used in partial fetches), thus it did not include\nsupport for it. Add support for the filter parameter.\n\nLike in the legacy protocol, the server advertises and supports \"filter\"\nonly if uploadpack.allowfilter is configured.\n\nLike in the legacy protocol, the client continues with a warning if\n\"--filter\" is specified, but the server does not advertise it.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Documentation/technical/protocol-v2.txt |  9 +++\n fetch-pack.c                            | 23 +++++-\n t/t5702-protocol-v2.sh                  | 98 +++++++++++++++++++++++++\n upload-pack.c                           | 15 +++-\n 4 files changed, 140 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 136179d7d..38d24fd2b 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -290,6 +290,15 @@ included in the clients request as well as the potential addition of the\n \tCannot be used with \"deepen\", but can be used with\n \t\"deepen-since\".\n \n+If the 'filter' feature is advertised, the following argument can be\n+included in the client's request:\n+\n+    filter <filter-spec>\n+\tRequest that various objects from the packfile be omitted\n+\tusing one of several filtering techniques. These are intended\n+\tfor use with partial clone and partial fetch operations. See\n+\t`rev-list` for possible \"filter-spec\" values.\n+\n The response of `fetch` is broken into a number of sections separated by\n delimiter packets (0001), with each section beginning with its section\n header.\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex f93723fec..3ed40aa46 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1191,14 +1191,29 @@ static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,\n \telse if (is_repository_shallow() || args->deepen)\n \t\tdie(_(\"Server does not support shallow requests\"));\n \n+\t/* Add filter */\n+\tif (server_supports_feature(\"fetch\", \"filter\", 0) &&\n+\t    args->filter_options.choice) {\n+\t\tprint_verbose(args, _(\"Server supports filter\"));\n+\t\tpacket_buf_write(&req_buf, \"filter %s\",\n+\t\t\t\t args->filter_options.filter_spec);\n+\t} else if (args->filter_options.choice) {\n+\t\twarning(\"filtering not recognized by server, ignoring\");\n+\t}\n+\n \t/* add wants */\n \tadd_wants(wants, &req_buf);\n \n-\t/* Add all of the common commits we've found in previous rounds */\n-\tadd_common(&req_buf, common);\n+\tif (args->no_dependents) {\n+\t\tpacket_buf_write(&req_buf, \"done\");\n+\t\tret = 1;\n+\t} else {\n+\t\t/* Add all of the common commits we've found in previous rounds */\n+\t\tadd_common(&req_buf, common);\n \n-\t/* Add initial haves */\n-\tret = add_haves(&req_buf, haves_to_send, in_vain);\n+\t\t/* Add initial haves */\n+\t\tret = add_haves(&req_buf, haves_to_send, in_vain);\n+\t}\n \n \t/* Send request */\n \tpacket_buf_flush(&req_buf);\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 0ead99993..858d3bb2d 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -215,6 +215,104 @@ test_expect_success 'upload-pack respects config using protocol v2' '\n \ttest -f server/.git/hookout\n '\n \n+test_expect_success 'setup filter tests' '\n+\trm -rf server client &&\n+\tgit init server &&\n+\n+\t# 1 commit to create a file, and 1 commit to modify it\n+\ttest_commit -C server message1 a.txt &&\n+\ttest_commit -C server message2 a.txt &&\n+\tgit -C server config protocol.version 2 &&\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tgit -C server config uploadpack.allowanysha1inwant 1 &&\n+\tgit -C server config protocol.version 2\n+'\n+\n+test_expect_success 'partial clone' '\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tclone --filter=blob:none \"file://$(pwd)/server\" client &&\n+\tgrep \"version 2\" trace &&\n+\n+\t# Ensure that the old version of the file is missing\n+\tgit -C client rev-list master --quiet --objects --missing=print \\\n+\t\t>observed.oids &&\n+\tgrep \"$(git -C server rev-parse message1:a.txt)\" observed.oids &&\n+\n+\t# Ensure that client passes fsck\n+\tgit -C client fsck\n+'\n+\n+test_expect_success 'dynamically fetch missing object' '\n+\trm \"$(pwd)/trace\" &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n+\t\tcat-file -p $(git -C server rev-parse message1:a.txt) &&\n+\tgrep \"version 2\" trace\n+'\n+\n+test_expect_success 'partial fetch' '\n+\trm -rf client \"$(pwd)/trace\" &&\n+\tgit init client &&\n+\tSERVER=\"file://$(pwd)/server\" &&\n+\ttest_config -C client extensions.partialClone \"$SERVER\" &&\n+\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n+\t\tfetch --filter=blob:none \"$SERVER\" master:refs/heads/other &&\n+\tgrep \"version 2\" trace &&\n+\n+\t# Ensure that the old version of the file is missing\n+\tgit -C client rev-list other --quiet --objects --missing=print \\\n+\t\t>observed.oids &&\n+\tgrep \"$(git -C server rev-parse message1:a.txt)\" observed.oids &&\n+\n+\t# Ensure that client passes fsck\n+\tgit -C client fsck\n+'\n+\n+test_expect_success 'do not advertise filter if not configured to do so' '\n+\tSERVER=\"file://$(pwd)/server\" &&\n+\n+\trm \"$(pwd)/trace\" &&\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tls-remote \"$SERVER\" &&\n+\tgrep \"fetch=.*filter\" trace &&\n+\n+\trm \"$(pwd)/trace\" &&\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tls-remote \"$SERVER\" &&\n+\tgrep \"fetch=\" trace >fetch_capabilities &&\n+\t! grep filter fetch_capabilities\n+'\n+\n+test_expect_success 'partial clone warns if filter is not advertised' '\n+\trm -rf client &&\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\tgit -c protocol.version=2 \\\n+\t\tclone --filter=blob:none \"file://$(pwd)/server\" client 2>err &&\n+\ttest_i18ngrep \"filtering not recognized by server, ignoring\" err\n+'\n+\n+test_expect_success 'even with handcrafted request, filter does not work if not advertised' '\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\n+\t# Custom request that tries to filter even though it is not advertised.\n+\ttest-pkt-line pack >in <<-EOF &&\n+\tcommand=fetch\n+\t0001\n+\twant $(git -C server rev-parse master)\n+\tfilter blob:none\n+\t0000\n+\tEOF\n+\n+\ttest_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n+\tgrep \"unexpected line: .filter blob:none.\" err &&\n+\n+\t# Exercise to ensure that if advertised, filter works\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tgit -C server serve --stateless-rpc <in >/dev/null\n+'\n+\n # Test protocol v2 with 'http://' transport\n #\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 113edd32d..82c16cae3 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1205,6 +1205,7 @@ static void process_args(struct packet_reader *request,\n {\n \twhile (packet_reader_read(request) != PACKET_READ_FLUSH) {\n \t\tconst char *arg = request->line;\n+\t\tconst char *p;\n \n \t\t/* process want */\n \t\tif (parse_want(arg))\n@@ -1251,6 +1252,11 @@ static void process_args(struct packet_reader *request,\n \t\t\tcontinue;\n \t\t}\n \n+\t\tif (allow_filter && skip_prefix(arg, \"filter \", &p)) {\n+\t\t\tparse_list_objects_filter(&filter_options, p);\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\t/* ignore unknown lines maybe? */\n \t\tdie(\"unexpected line: '%s'\", arg);\n \t}\n@@ -1430,7 +1436,14 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n int upload_pack_advertise(struct repository *r,\n \t\t\t  struct strbuf *value)\n {\n-\tif (value)\n+\tif (value) {\n+\t\tint allow_filter_value;\n \t\tstrbuf_addstr(value, \"shallow\");\n+\t\tif (!repo_config_get_bool(the_repository,\n+\t\t\t\t\t \"uploadpack.allowfilter\",\n+\t\t\t\t\t &allow_filter_value) &&\n+\t\t    allow_filter_value)\n+\t\t\tstrbuf_addstr(value, \" filter\");\n+\t}\n \treturn 1;\n }\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346569","messageId":"CAGZ79kbTwqzKgZjN7GSXn-NpKX0kkDVYbXmdz6CC6TrDWbqyQg@mail.gmail.com","threadId":"48395","inReplyTo":"177c6eb7058adc1996f0595ddc3cc848c91b578f.1525220786.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 1/3] upload-pack: fix error message typo","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T18:58:59Z","receivedAt":"2018-05-03T19:00:30Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 1, 2018 at 5:31 PM, Jonathan Tan <jonathantanmy@google.com> wrote:\n> Fix a typo in an error message.\n>\n> Also, this line was introduced in 3145ea957d2c (\"upload-pack: introduce\n> fetch server command\", 2018-03-15), which did not contain a test for the\n> case which causes this error to be printed, so introduce a test.\n>\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n\nThis is Reviewed-by: Stefan Beller <sbeller@google.com>\n\n> ---\n>  t/t5701-git-serve.sh | 14 ++++++++++++++\n>  upload-pack.c        |  2 +-\n>  2 files changed, 15 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\n> index 72d7bc562..1b4b13cc2 100755\n> --- a/t/t5701-git-serve.sh\n> +++ b/t/t5701-git-serve.sh\n> @@ -173,4 +173,18 @@ test_expect_success 'symrefs parameter' '\n>         test_cmp actual expect\n>  '\n>\n> +test_expect_success 'unexpected lines are not allowed in fetch request' '\n> +       git init server &&\n> +\n> +       test-pkt-line pack >in <<-EOF &&\n> +       command=fetch\n> +       0001\n> +       this-is-not-a-command\n> +       0000\n> +       EOF\n> +\n> +       test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n\nMinor nit:\nWhy do we pipe stdout to /dev/null ?\n"},{"id":"346570","messageId":"CAGZ79kb4gHO=5aYMbT=AhZrOexkmkRakJRPRZd_GMrQJTF3WwQ@mail.gmail.com","threadId":"48395","inReplyTo":"6e85aaa555f6789156626330327085207b2d2dff.1525220786.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 2/3] upload-pack: read config when serving protocol v2","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T19:08:16Z","receivedAt":"2018-05-03T19:08:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 1, 2018 at 5:31 PM, Jonathan Tan <jonathantanmy@google.com> wrote:\n> The upload-pack code paths never call git_config() with\n> upload_pack_config() when protocol v2 is used, causing options like\n> uploadpack.packobjectshook to not take effect. Ensure that this function\n> is called.\n>\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> ---\n>  t/t5702-protocol-v2.sh | 14 ++++++++++++++\n>  upload-pack.c          |  2 ++\n>  2 files changed, 16 insertions(+)\n>\n> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\n> index 56f7c3c32..0ead99993 100755\n> --- a/t/t5702-protocol-v2.sh\n> +++ b/t/t5702-protocol-v2.sh\n> @@ -201,6 +201,20 @@ test_expect_success 'ref advertisment is filtered during fetch using protocol v2\n>         ! grep \"refs/tags/three\" log\n>  '\n>\n> +test_expect_success 'upload-pack respects config using protocol v2' '\n> +       git init server &&\n> +       write_script server/.git/hook <<-\\EOF &&\n> +               touch hookout\n> +               \"$@\"\n> +       EOF\n> +       test_commit -C server one &&\n> +\n> +       test_config_global uploadpack.packobjectshook ./hook &&\n> +       test ! -f server/.git/hookout &&\n\ntest_path_is_missing ?\n\n\n> +       GIT_TRACE=/tmp/y git -c protocol.version=2 clone \"file://$(pwd)/server\" client &&\n\nWhy do we redirect GIT_TRACE outside the test suite? do we read that\nback or want to read it out of the hook?\n\nIs it possible to redirect to  /$(pwd)/trace or such?\n\n> +       test -f server/.git/hookout\n\ntest_path_is_file ?\n\n\n> +'\n> +\n>  # Test protocol v2 with 'http://' transport\n>  #\n>  . \"$TEST_DIRECTORY\"/lib-httpd.sh\n> diff --git a/upload-pack.c b/upload-pack.c\n> index c4456bb88..113edd32d 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -1376,6 +1376,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n>         enum fetch_state state = FETCH_PROCESS_ARGS;\n>         struct upload_pack_data data;\n>\n> +       git_config(upload_pack_config, NULL);\n> +\n>         upload_pack_data_init(&data);\n>         use_sideband = LARGE_PACKET_MAX;\n>\n> --\n> 2.17.0.441.gb46fe60e1d-goog\n>\n"},{"id":"346573","messageId":"CAGZ79kaHHNVW+Mj=RkQsWmc1ta1GPRLQaZR7aEDe6PAr_7wD+Q@mail.gmail.com","threadId":"48395","inReplyTo":"38aae093ffc23f886ab4aaeff72eae5e7c794df0.1525220786.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 3/3] {fetch,upload}-pack: support filter in protocol v2","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T20:15:48Z","receivedAt":"2018-05-03T20:15:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi Jonathan,\n\nOn Tue, May 1, 2018 at 5:31 PM, Jonathan Tan <jonathantanmy@google.com> wrote:\n> The fetch-pack/upload-pack protocol v2 was developed independently of\n> the filter parameter (used in partial fetches), thus it did not include\n> support for it. Add support for the filter parameter.\n>\n> Like in the legacy protocol, the server advertises and supports \"filter\"\n> only if uploadpack.allowfilter is configured.\n>\n> Like in the legacy protocol, the client continues with a warning if\n> \"--filter\" is specified, but the server does not advertise it.\n>\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n\nThe whole series looks good,\n\nThanks,\nStefan\n"},{"id":"346603","messageId":"20180503164155.6112764a927d50e23600dc49@google.com","threadId":"48395","inReplyTo":"CAGZ79kbTwqzKgZjN7GSXn-NpKX0kkDVYbXmdz6CC6TrDWbqyQg@mail.gmail.com","subject":"Re: [PATCH v2 1/3] upload-pack: fix error message typo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T23:41:55Z","receivedAt":"2018-05-03T23:42:01Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 3 May 2018 11:58:59 -0700\nStefan Beller <sbeller@google.com> wrote:\n\n> > +       test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n> \n> Minor nit:\n> Why do we pipe stdout to /dev/null ?\n\nUsually there's a binary packfile produced, but not in this case. I'll\nremove it.\n"},{"id":"346604","messageId":"20180503164509.83dd9c113ddb849584863c08@google.com","threadId":"48395","inReplyTo":"CAGZ79kb4gHO=5aYMbT=AhZrOexkmkRakJRPRZd_GMrQJTF3WwQ@mail.gmail.com","subject":"Re: [PATCH v2 2/3] upload-pack: read config when serving protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T23:45:09Z","receivedAt":"2018-05-03T23:45:15Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 3 May 2018 12:08:16 -0700\nStefan Beller <sbeller@google.com> wrote:\n\n> test_path_is_missing ?\n\nThanks for the pointer. Done.\n\n> > +       GIT_TRACE=/tmp/y git -c protocol.version=2 clone \"file://$(pwd)/server\" client &&\n> \n> Why do we redirect GIT_TRACE outside the test suite? do we read that\n> back or want to read it out of the hook?\n> \n> Is it possible to redirect to  /$(pwd)/trace or such?\n\nGood catch - that was a leftover from debugging. I've just removed it.\n\n> > +       test -f server/.git/hookout\n> \n> test_path_is_file ?\n\nDone.\n"},{"id":"346605","messageId":"cover.1525391172.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525213052.git.jonathantanmy@google.com","subject":"[PATCH v3 0/3] Supporting partial clones in protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T23:46:53Z","receivedAt":"2018-05-03T23:47:13Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Changes from v2: followed all Stefan's comments.\n\nJonathan Tan (3):\n  upload-pack: fix error message typo\n  upload-pack: read config when serving protocol v2\n  {fetch,upload}-pack: support filter in protocol v2\n\n Documentation/technical/protocol-v2.txt |   9 ++\n fetch-pack.c                            |  23 ++++-\n t/t5701-git-serve.sh                    |  14 +++\n t/t5702-protocol-v2.sh                  | 112 ++++++++++++++++++++++++\n upload-pack.c                           |  19 +++-\n 5 files changed, 171 insertions(+), 6 deletions(-)\n\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346606","messageId":"182d1575765b611111a24a38c1397e2a99a2adab.1525391172.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525391172.git.jonathantanmy@google.com","subject":"[PATCH v3 1/3] upload-pack: fix error message typo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T23:46:54Z","receivedAt":"2018-05-03T23:47:16Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Fix a typo in an error message.\n\nAlso, this line was introduced in 3145ea957d2c (\"upload-pack: introduce\nfetch server command\", 2018-03-15), which did not contain a test for the\ncase which causes this error to be printed, so introduce a test.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5701-git-serve.sh | 14 ++++++++++++++\n upload-pack.c        |  2 +-\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh\nindex 72d7bc562..6439f7792 100755\n--- a/t/t5701-git-serve.sh\n+++ b/t/t5701-git-serve.sh\n@@ -173,4 +173,18 @@ test_expect_success 'symrefs parameter' '\n \ttest_cmp actual expect\n '\n \n+test_expect_success 'unexpected lines are not allowed in fetch request' '\n+\tgit init server &&\n+\n+\ttest-pkt-line pack >in <<-EOF &&\n+\tcommand=fetch\n+\t0001\n+\tthis-is-not-a-command\n+\t0000\n+\tEOF\n+\n+\ttest_must_fail git -C server serve --stateless-rpc <in 2>err &&\n+\tgrep \"unexpected line: .this-is-not-a-command.\" err\n+'\n+\n test_done\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 87b4d32a6..c4456bb88 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1252,7 +1252,7 @@ static void process_args(struct packet_reader *request,\n \t\t}\n \n \t\t/* ignore unknown lines maybe? */\n-\t\tdie(\"unexpect line: '%s'\", arg);\n+\t\tdie(\"unexpected line: '%s'\", arg);\n \t}\n }\n \n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346607","messageId":"73642e0e75dc7543e1057116d57670dc7a6e34c0.1525391172.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525391172.git.jonathantanmy@google.com","subject":"[PATCH v3 2/3] upload-pack: read config when serving protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T23:46:55Z","receivedAt":"2018-05-03T23:47:19Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"The upload-pack code paths never call git_config() with\nupload_pack_config() when protocol v2 is used, causing options like\nuploadpack.packobjectshook to not take effect. Ensure that this function\nis called.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5702-protocol-v2.sh | 14 ++++++++++++++\n upload-pack.c          |  2 ++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 56f7c3c32..abb15cd6d 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -201,6 +201,20 @@ test_expect_success 'ref advertisment is filtered during fetch using protocol v2\n \t! grep \"refs/tags/three\" 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+\t\ttouch hookout\n+\t\t\"$@\"\n+\tEOF\n+\ttest_commit -C server one &&\n+\n+\ttest_config_global uploadpack.packobjectshook ./hook &&\n+\ttest_path_is_missing server/.git/hookout &&\n+\tgit -c protocol.version=2 clone \"file://$(pwd)/server\" client &&\n+\ttest_path_is_file server/.git/hookout\n+'\n+\n # Test protocol v2 with 'http://' transport\n #\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c4456bb88..113edd32d 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1376,6 +1376,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n \tenum fetch_state state = FETCH_PROCESS_ARGS;\n \tstruct upload_pack_data data;\n \n+\tgit_config(upload_pack_config, NULL);\n+\n \tupload_pack_data_init(&data);\n \tuse_sideband = LARGE_PACKET_MAX;\n \n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346608","messageId":"b1eca0f60a61c6a35ee4e7a6903aa056c0aaf710.1525391172.git.jonathantanmy@google.com","threadId":"48395","inReplyTo":"cover.1525391172.git.jonathantanmy@google.com","subject":"[PATCH v3 3/3] {fetch,upload}-pack: support filter in protocol v2","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T23:46:56Z","receivedAt":"2018-05-03T23:47:23Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"The fetch-pack/upload-pack protocol v2 was developed independently of\nthe filter parameter (used in partial fetches), thus it did not include\nsupport for it. Add support for the filter parameter.\n\nLike in the legacy protocol, the server advertises and supports \"filter\"\nonly if uploadpack.allowfilter is configured.\n\nLike in the legacy protocol, the client continues with a warning if\n\"--filter\" is specified, but the server does not advertise it.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Documentation/technical/protocol-v2.txt |  9 +++\n fetch-pack.c                            | 23 +++++-\n t/t5702-protocol-v2.sh                  | 98 +++++++++++++++++++++++++\n upload-pack.c                           | 15 +++-\n 4 files changed, 140 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 136179d7d..38d24fd2b 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -290,6 +290,15 @@ included in the clients request as well as the potential addition of the\n \tCannot be used with \"deepen\", but can be used with\n \t\"deepen-since\".\n \n+If the 'filter' feature is advertised, the following argument can be\n+included in the client's request:\n+\n+    filter <filter-spec>\n+\tRequest that various objects from the packfile be omitted\n+\tusing one of several filtering techniques. These are intended\n+\tfor use with partial clone and partial fetch operations. See\n+\t`rev-list` for possible \"filter-spec\" values.\n+\n The response of `fetch` is broken into a number of sections separated by\n delimiter packets (0001), with each section beginning with its section\n header.\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex f93723fec..3ed40aa46 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1191,14 +1191,29 @@ static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,\n \telse if (is_repository_shallow() || args->deepen)\n \t\tdie(_(\"Server does not support shallow requests\"));\n \n+\t/* Add filter */\n+\tif (server_supports_feature(\"fetch\", \"filter\", 0) &&\n+\t    args->filter_options.choice) {\n+\t\tprint_verbose(args, _(\"Server supports filter\"));\n+\t\tpacket_buf_write(&req_buf, \"filter %s\",\n+\t\t\t\t args->filter_options.filter_spec);\n+\t} else if (args->filter_options.choice) {\n+\t\twarning(\"filtering not recognized by server, ignoring\");\n+\t}\n+\n \t/* add wants */\n \tadd_wants(wants, &req_buf);\n \n-\t/* Add all of the common commits we've found in previous rounds */\n-\tadd_common(&req_buf, common);\n+\tif (args->no_dependents) {\n+\t\tpacket_buf_write(&req_buf, \"done\");\n+\t\tret = 1;\n+\t} else {\n+\t\t/* Add all of the common commits we've found in previous rounds */\n+\t\tadd_common(&req_buf, common);\n \n-\t/* Add initial haves */\n-\tret = add_haves(&req_buf, haves_to_send, in_vain);\n+\t\t/* Add initial haves */\n+\t\tret = add_haves(&req_buf, haves_to_send, in_vain);\n+\t}\n \n \t/* Send request */\n \tpacket_buf_flush(&req_buf);\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex abb15cd6d..25bf046b3 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -215,6 +215,104 @@ test_expect_success 'upload-pack respects config using protocol v2' '\n \ttest_path_is_file server/.git/hookout\n '\n \n+test_expect_success 'setup filter tests' '\n+\trm -rf server client &&\n+\tgit init server &&\n+\n+\t# 1 commit to create a file, and 1 commit to modify it\n+\ttest_commit -C server message1 a.txt &&\n+\ttest_commit -C server message2 a.txt &&\n+\tgit -C server config protocol.version 2 &&\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tgit -C server config uploadpack.allowanysha1inwant 1 &&\n+\tgit -C server config protocol.version 2\n+'\n+\n+test_expect_success 'partial clone' '\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tclone --filter=blob:none \"file://$(pwd)/server\" client &&\n+\tgrep \"version 2\" trace &&\n+\n+\t# Ensure that the old version of the file is missing\n+\tgit -C client rev-list master --quiet --objects --missing=print \\\n+\t\t>observed.oids &&\n+\tgrep \"$(git -C server rev-parse message1:a.txt)\" observed.oids &&\n+\n+\t# Ensure that client passes fsck\n+\tgit -C client fsck\n+'\n+\n+test_expect_success 'dynamically fetch missing object' '\n+\trm \"$(pwd)/trace\" &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n+\t\tcat-file -p $(git -C server rev-parse message1:a.txt) &&\n+\tgrep \"version 2\" trace\n+'\n+\n+test_expect_success 'partial fetch' '\n+\trm -rf client \"$(pwd)/trace\" &&\n+\tgit init client &&\n+\tSERVER=\"file://$(pwd)/server\" &&\n+\ttest_config -C client extensions.partialClone \"$SERVER\" &&\n+\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n+\t\tfetch --filter=blob:none \"$SERVER\" master:refs/heads/other &&\n+\tgrep \"version 2\" trace &&\n+\n+\t# Ensure that the old version of the file is missing\n+\tgit -C client rev-list other --quiet --objects --missing=print \\\n+\t\t>observed.oids &&\n+\tgrep \"$(git -C server rev-parse message1:a.txt)\" observed.oids &&\n+\n+\t# Ensure that client passes fsck\n+\tgit -C client fsck\n+'\n+\n+test_expect_success 'do not advertise filter if not configured to do so' '\n+\tSERVER=\"file://$(pwd)/server\" &&\n+\n+\trm \"$(pwd)/trace\" &&\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tls-remote \"$SERVER\" &&\n+\tgrep \"fetch=.*filter\" trace &&\n+\n+\trm \"$(pwd)/trace\" &&\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 \\\n+\t\tls-remote \"$SERVER\" &&\n+\tgrep \"fetch=\" trace >fetch_capabilities &&\n+\t! grep filter fetch_capabilities\n+'\n+\n+test_expect_success 'partial clone warns if filter is not advertised' '\n+\trm -rf client &&\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\tgit -c protocol.version=2 \\\n+\t\tclone --filter=blob:none \"file://$(pwd)/server\" client 2>err &&\n+\ttest_i18ngrep \"filtering not recognized by server, ignoring\" err\n+'\n+\n+test_expect_success 'even with handcrafted request, filter does not work if not advertised' '\n+\tgit -C server config uploadpack.allowfilter 0 &&\n+\n+\t# Custom request that tries to filter even though it is not advertised.\n+\ttest-pkt-line pack >in <<-EOF &&\n+\tcommand=fetch\n+\t0001\n+\twant $(git -C server rev-parse master)\n+\tfilter blob:none\n+\t0000\n+\tEOF\n+\n+\ttest_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n+\tgrep \"unexpected line: .filter blob:none.\" err &&\n+\n+\t# Exercise to ensure that if advertised, filter works\n+\tgit -C server config uploadpack.allowfilter 1 &&\n+\tgit -C server serve --stateless-rpc <in >/dev/null\n+'\n+\n # Test protocol v2 with 'http://' transport\n #\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 113edd32d..82c16cae3 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1205,6 +1205,7 @@ static void process_args(struct packet_reader *request,\n {\n \twhile (packet_reader_read(request) != PACKET_READ_FLUSH) {\n \t\tconst char *arg = request->line;\n+\t\tconst char *p;\n \n \t\t/* process want */\n \t\tif (parse_want(arg))\n@@ -1251,6 +1252,11 @@ static void process_args(struct packet_reader *request,\n \t\t\tcontinue;\n \t\t}\n \n+\t\tif (allow_filter && skip_prefix(arg, \"filter \", &p)) {\n+\t\t\tparse_list_objects_filter(&filter_options, p);\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\t/* ignore unknown lines maybe? */\n \t\tdie(\"unexpected line: '%s'\", arg);\n \t}\n@@ -1430,7 +1436,14 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n int upload_pack_advertise(struct repository *r,\n \t\t\t  struct strbuf *value)\n {\n-\tif (value)\n+\tif (value) {\n+\t\tint allow_filter_value;\n \t\tstrbuf_addstr(value, \"shallow\");\n+\t\tif (!repo_config_get_bool(the_repository,\n+\t\t\t\t\t \"uploadpack.allowfilter\",\n+\t\t\t\t\t &allow_filter_value) &&\n+\t\t    allow_filter_value)\n+\t\t\tstrbuf_addstr(value, \" filter\");\n+\t}\n \treturn 1;\n }\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346612","messageId":"xmqqr2mstaxk.fsf@gitster-ct.c.googlers.com","threadId":"48395","inReplyTo":"20180503164155.6112764a927d50e23600dc49@google.com","subject":"Re: [PATCH v2 1/3] upload-pack: fix error message typo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-04T02:24:39Z","receivedAt":"2018-05-04T02:24:45Z","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> On Thu, 3 May 2018 11:58:59 -0700\n> Stefan Beller <sbeller@google.com> wrote:\n>\n>> > +       test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&\n>> \n>> Minor nit:\n>> Why do we pipe stdout to /dev/null ?\n>\n> Usually there's a binary packfile produced, but not in this case. I'll\n> remove it.\n\nHmm, when somebody breaks \"git server serve\", we probably would not\nwant to see the binary spewed to the output while debugging it.  So\nI'd probably keep the redirection---it may be an improvement to use\n\">out\" and then checking it is empty after the expected failure.\n"},{"id":"346678","messageId":"20180504091037.ff633aaa412e925d4493ab3f@google.com","threadId":"48395","inReplyTo":"xmqqr2mstaxk.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/3] upload-pack: fix error message typo","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-04T16:10:37Z","receivedAt":"2018-05-04T16:10:43Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 04 May 2018 11:24:39 +0900\nJunio C Hamano <gitster@pobox.com> wrote:\n\n> Hmm, when somebody breaks \"git server serve\", we probably would not\n> want to see the binary spewed to the output while debugging it.  So\n> I'd probably keep the redirection---it may be an improvement to use\n> \">out\" and then checking it is empty after the expected failure.\n\nThat's a good point - I've readded the redirection in my local copy.\nI'll send out the new version if needed.\n\nI checked the other patches and patch 3 has a similar situation but\nstill has the >/dev/null because I forgot to remove it when I removed it\nin patch 1, so nothing needs to be changed in patch 3.\n"}]}