Supporting partial clones in protocol v2
cover Supporting partial clones in protocol v2
This patch set is built on "next". (I had to build it on "next" because bw/protocol-v2 is built on v2.16, which does not have support for partial clones.)
Partial clones and protocol v2 were built independently, so here is a patch to support partial clones in protocol v2.
One thing I am a little unhappy about is the fact that the upload-pack config has to be read in multiple places, but perhaps there is no choice about that.
Jonathan Tan (2):
upload-pack: fix error message typo {fetch,upload}-pack: support filter in protocol v2
Documentation/technical/protocol-v2.txt | 9 +++ fetch-pack.c | 23 +++++- t/t5701-git-serve.sh | 15 ++++ t/t5702-protocol-v2.sh | 97 +++++++++++++++++++++++++ upload-pack.c | 14 +++- 5 files changed, 151 insertions(+), 7 deletions(-)
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH 1/2 upload-pack: fix error message typo
Fix a typo in an error message.
Also, this line was introduced in 3145ea957d2c ("upload-pack: introduce fetch server command", 2018-03-15), which did not contain a test for the case which causes this error to be printed, so introduce a test.
--- t/t5701-git-serve.sh | 15 +++++++++++++++ upload-pack.c | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-)
t/t5701-git-serve.sh +15 -0
@@ -173,4 +173,19 @@ test_expect_success 'symrefs parameter' ' test_cmp actual expect ' +test_expect_success 'unexpected lines are not allowed in fetch request' ' + git init server && + + # Custom request that tries to filter even though it is not advertised. + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + this-is-not-a-command + 0000 + EOF + + test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err && + grep "unexpected line: .this-is-not-a-command." err +' + test_done
upload-pack.c +1 -1
@@ -1252,7 +1252,7 @@ static void process_args(struct packet_reader *request, }/* ignore unknown lines maybe? */ -die ("unexpect line: '%s'" , arg); +die ("unexpected line: '%s'" , arg); } }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH 1/2] upload-pack: fix error message typo
t/t5701-git-serve.sh test_expect_success 'symrefs parameter' ' · line 179
Quoted 4 lines · On Tue, 1 May 2018 15:22:20 -0700 Jonathan Tan <[email protected]>…
t/t5701-git-serve.sh +4
-0+test_expect_success 'unexpected lines are not allowed in fetch request' ' + git init server && + + # Custom request that tries to filter even though it is not advertised.
Oops...I saw this copy-and-paste error right after I sent the e-mails. I'll remove this in the next reroll (if there is one).
Quoted 10 lines
t/t5701-git-serve.sh +10
-0+ test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + this-is-not-a-command + 0000 + EOF + + test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err && + grep "unexpected line: .this-is-not-a-command." err +'
PATCH 2/2 {fetch,upload}-pack: support filter in protocol v2
In reply to Jonathan TanThe fetch-pack/upload-pack protocol v2 was developed independently of the filter parameter (used in partial fetches), thus it did not include support for it. Add support for the filter parameter.
Like in the legacy protocol, the server advertises and supports "filter" only if uploadpack.allowfilter is configured.
Like in the legacy protocol, the client continues with a warning if "--filter" is specified, but the server does not advertise it.
--- Documentation/technical/protocol-v2.txt | 9 +++ fetch-pack.c | 23 +++++- t/t5702-protocol-v2.sh | 97 +++++++++++++++++++++++++ upload-pack.c | 12 ++- 4 files changed, 135 insertions(+), 6 deletions(-)
Documentation/technical/protocol-v2.txt +9 -0
@@ -290,6 +290,15 @@ included in the clients request as well as the potential addition of the Cannot be used with "deepen", but can be used with "deepen-since". +If the 'filter' feature is advertised, the following argument can be +included in the client's request: + + filter <filter-spec> + Request that various objects from the packfile be omitted + using one of several filtering techniques. These are intended + for use with partial clone and partial fetch operations. See + `rev-list` for possible "filter-spec" values. + The response of `fetch` is broken into a number of sections separated by delimiter packets (0001), with each section beginning with its section header.
fetch-pack.c +19 -4
@@ -1191,14 +1191,29 @@ static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,else if (is_repository_shallow () || args->deepen)die (_ ("Server does not support shallow requests" )); +/* Add filter */ +if (server_supports_feature ("fetch" ,"filter" ,0 ) && + args->filter_options.choice) { +print_verbose (args,_ ("Server supports filter" )); +packet_buf_write (&req_buf,"filter %s" , + args->filter_options.filter_spec); + }else if (args->filter_options.choice) { +warning ("filtering not recognized by server, ignoring" ); + } +/* add wants */ add_wants (wants, &req_buf); -/* Add all of the common commits we've found in previous rounds */ -add_common (&req_buf, common); +if (args->no_dependents) { +packet_buf_write (&req_buf,"done" ); + ret =1 ; + }else { +/* Add all of the common commits we've found in previous rounds */ +add_common (&req_buf, common); -/* Add initial haves */ - ret =add_haves (&req_buf, haves_to_send, in_vain); +/* Add initial haves */ + ret =add_haves (&req_buf, haves_to_send, in_vain); + }/* Send request */ packet_buf_flush (&req_buf);
t/t5702-protocol-v2.sh +97 -0
103 lines not on this page · raw message
upload-pack.c +10 -2
@@ -1205,6 +1205,7 @@ static void process_args(struct packet_reader *request, {while (packet_reader_read (request) !=PACKET_READ_FLUSH ) {const char *arg = request->line; +const char *p;/* process want */ if (parse_want (arg))
@@ -1251,6 +1252,11 @@ static void process_args(struct packet_reader *request,continue ; } +if (allow_filter &&skip_prefix (arg,"filter " , &p)) { +parse_list_objects_filter (&filter_options, p); +continue ; + } +/* ignore unknown lines maybe? */ die ("unexpected line: '%s'" , arg); }
@@ -1428,7 +1434,9 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,int upload_pack_advertise (struct repository *r,struct strbuf *value) { -if (value) -strbuf_addstr (value,"shallow" ); +git_config (upload_pack_config,NULL ); +if (value) { +strbuf_addf (value,"%sshallow" , allow_filter ?"filter " :"" ); + }return 1 ; }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH 2/2] {fetch,upload}-pack: support filter in protocol v2
Quoted 20 lines · Jonathan Tan
The fetch-pack/upload-pack protocol v2 was developed independently of the filter parameter (used in partial fetches), thus it did not include support for it. Add support for the filter parameter.
Like in the legacy protocol, the server advertises and supports "filter" only if uploadpack.allowfilter is configured.
Like in the legacy protocol, the client continues with a warning if "--filter" is specified, but the server does not advertise it.
Signed-off-by Jonathan Tan <[email protected]>--- Documentation/technical/protocol-v2.txt | 9 +++ fetch-pack.c | 23 +++++- t/t5702-protocol-v2.sh | 97 +++++++++++++++++++++++++ upload-pack.c | 12 ++- 4 files changed, 135 insertions(+), 6 deletions(-)
upload-pack.c int upload_pack_v2(struct repository *r, struct argv_array *keys, · line 1440
Quoted 9 lines
upload-pack.c +4
-2@@ -1428,7 +1434,9 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,int upload_pack_advertise (struct repository *r,struct strbuf *value) { -if (value) -strbuf_addstr (value,"shallow" ); +git_config (upload_pack_config,NULL ); +if (value) { +strbuf_addf (value,"%sshallow" , allow_filter ?"filter " :"" ); + }
This is a bit difficult to read and there is no reason why we would need to read the entire upload_pack_config to determine if we need to filter things (we will need to read the config if cmd "fetch" is requested though). Instead it may be better to do the following:
if (value) {
strbuf_addstr(value, "shallow");
if (repo_config_get(r, "uplaodpack.filter"))
strbuf_addstr(value, " filter");
}This way its easier to read and you only are reading the required value from the config.
Quoted 5 lines
upload-pack.c +0
-0return 1 ; }Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Signature · 1 line
Brandon Williams
v2 cover Supporting partial clones in protocol v2
In reply to Jonathan TanQuoted 5 lines
This is a bit difficult to read and there is no reason why we would need to read the entire
upload_pack_configto determine if we need to filter things (we will need to read the config if cmd "fetch" is requested though). Instead it may be better to do the following:
Quoted 8 lines
if (value) { strbuf_addstr(value, "shallow"); if (repo_config_get(r, "uplaodpack.filter")) strbuf_addstr(value, " filter"); }This way its easier to read and you only are reading the required value from the config.
Thanks, Brandon. I went ahead and used repo_config_get_bool(), and indeed it works.
Removing the call to git_config() from there exposed another issue in that configs were not read if upload-pack was used with protocol v2, so I inserted a patch in the middle addressing that. While writing that patch, I noticed that uploadpack.packobjectshook couldn't take filenames with spaces, which I think is due to prepare_shell_cmd() in run-command.c not quoting properly. Adding single quotes around "%s" worked, but made other tests fail. Instead of continuing down that rabbit hole, I just made the uploadpack.packobjectshook not have any spaces, just like in t5544.
Jonathan Tan (3):
upload-pack: fix error message typo
upload-pack: read config when serving protocol v2 {fetch,upload}-pack: support filter in protocol v2
Documentation/technical/protocol-v2.txt | 9 ++ fetch-pack.c | 23 ++++- t/t5701-git-serve.sh | 14 +++ t/t5702-protocol-v2.sh | 112 ++++++++++++++++++++++++ upload-pack.c | 19 +++- 5 files changed, 171 insertions(+), 6 deletions(-)
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH v2 1/3 upload-pack: fix error message typo
Fix a typo in an error message.
Also, this line was introduced in 3145ea957d2c ("upload-pack: introduce fetch server command", 2018-03-15), which did not contain a test for the case which causes this error to be printed, so introduce a test.
--- t/t5701-git-serve.sh | 14 ++++++++++++++ upload-pack.c | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-)
t/t5701-git-serve.sh +14 -0
@@ -173,4 +173,18 @@ test_expect_success 'symrefs parameter' ' test_cmp actual expect ' +test_expect_success 'unexpected lines are not allowed in fetch request' ' + git init server && + + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + this-is-not-a-command + 0000 + EOF + + test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err && + grep "unexpected line: .this-is-not-a-command." err +' + test_done
upload-pack.c +1 -1
@@ -1252,7 +1252,7 @@ static void process_args(struct packet_reader *request, }/* ignore unknown lines maybe? */ -die ("unexpect line: '%s'" , arg); +die ("unexpected line: '%s'" , arg); } }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH v2 1/3] upload-pack: fix error message typo
Quoted 7 lines · Jonathan Tan
Fix a typo in an error message.
Also, this line was introduced in
3145ea957d2c("upload-pack: introduce fetch server command", 2018-03-15), which did not contain a test for the case which causes this error to be printed, so introduce a test.Signed-off-by Jonathan Tan <[email protected]>
This is Reviewed-by: Stefan Beller <[email protected]>
Quoted 14 lines
--- t/t5701-git-serve.sh | 14 ++++++++++++++ upload-pack.c | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-)t/t5701-git-serve.sh +1
-0@@ -173,4 +173,18 @@ test_expect_success 'symrefs parameter' ' test_cmp actual expect ' +test_expect_success 'unexpected lines are not allowed in fetch request' '
t/t5701-git-serve.sh test_expect_success 'symrefs parameter' ' · line 186
Quoted 10 lines
t/t5701-git-serve.sh +10
-0+ git init server && + + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + this-is-not-a-command + 0000 + EOF + + test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&
Minor nit:
Why do we pipe stdout to /dev/null ?
Re: [PATCH v2 1/3] upload-pack: fix error message typo
Quoted 4 lines · On Thu, 3 May 2018 11:58:59 -0700 Stefan Beller <[email protected]> wrote:
Quoted 1 line
+
test_must_failgit -C server serve --stateless-rpc <in >/dev/null 2>err &&Minor nit:
Why do we pipe stdout to/dev/null?
Usually there's a binary packfile produced, but not in this case. I'll remove it.
Re: [PATCH v2 1/3] upload-pack: fix error message typo
Quoted 10 lines · Jonathan Tan
Quoted 4 lines · On Thu, 3 May 2018 11:58:59 -0700 Stefan Beller <[email protected]> wrote:
Quoted 1 line
+
test_must_failgit -C server serve --stateless-rpc <in >/dev/null 2>err &&Minor nit:
Why do we pipe stdout to/dev/null?Usually there's a binary packfile produced, but not in this case. I'll remove it.
Hmm, when somebody breaks "git server serve", we probably would not want to see the binary spewed to the output while debugging it. So I'd probably keep the redirection---it may be an improvement to use ">out" and then checking it is empty after the expected failure.
Re: [PATCH v2 1/3] upload-pack: fix error message typo
Quoted 4 lines · On Fri, 04 May 2018 11:24:39 +0900 Junio C Hamano <[email protected]> wrote:
Hmm, when somebody breaks "git server serve", we probably would not want to see the binary spewed to the output while debugging it. So I'd probably keep the redirection---it may be an improvement to use ">out" and then checking it is empty after the expected failure.
That's a good point - I've readded the redirection in my local copy. I'll send out the new version if needed.
I checked the other patches and patch 3 has a similar situation but still has the >/dev/null because I forgot to remove it when I removed it in patch 1, so nothing needs to be changed in patch 3.
PATCH v2 2/3 upload-pack: read config when serving protocol v2
In reply to Jonathan TanThe upload-pack code paths never call git_config() with upload_pack_config() when protocol v2 is used, causing options like uploadpack.packobjectshook to not take effect. Ensure that this function is called.
--- t/t5702-protocol-v2.sh | 14 ++++++++++++++ upload-pack.c | 2 ++ 2 files changed, 16 insertions(+)
t/t5702-protocol-v2.sh +14 -0
@@ -201,6 +201,20 @@ test_expect_success 'ref advertisment is filtered during fetch using protocol v2 ! grep "refs/tags/three" log ' +test_expect_success 'upload-pack respects config using protocol v2' ' + git init server && + write_script server/.git/hook <<-\EOF && + touch hookout + "$@" + EOF + test_commit -C server one && + + test_config_global uploadpack.packobjectshook ./hook && + test ! -f server/.git/hookout && + GIT_TRACE=/tmp/y git -c protocol.version=2 clone "file://$(pwd)/server" client && + test -f server/.git/hookout +' + # Test protocol v2 with 'http://' transport # . "$TEST_DIRECTORY"/lib-httpd.sh
upload-pack.c +2 -0
@@ -1376,6 +1376,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,enum fetch_state state =FETCH_PROCESS_ARGS ;struct upload_pack_data data; +git_config (upload_pack_config,NULL ); +upload_pack_data_init (&data); use_sideband =LARGE_PACKET_MAX ;
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH v2 2/3] upload-pack: read config when serving protocol v2
Quoted 19 lines · Jonathan Tan
The upload-pack code paths never call
git_config()withupload_pack_config()when protocol v2 is used, causing options like uploadpack.packobjectshook to not take effect. Ensure that this function is called.Signed-off-by Jonathan Tan <[email protected]>--- t/t5702-protocol-v2.sh | 14 ++++++++++++++ upload-pack.c | 2 ++ 2 files changed, 16 insertions(+)t/t5702-protocol-v2.sh +0
-0@@ -201,6 +201,20 @@ test_expect_success 'ref advertisment is filtered during fetch using protocol v2 ! grep "refs/tags/three" log '
t/t5702-protocol-v2.sh test_expect_success 'ref advertisment is filtered during fetch using protocol v2 · line 213
Quoted 10 lines
t/t5702-protocol-v2.sh +10
-0+test_expect_success 'upload-pack respects config using protocol v2' ' + git init server && + write_script server/.git/hook <<-\EOF && + touch hookout + "$@" + EOF + test_commit -C server one && + + test_config_global uploadpack.packobjectshook ./hook && + test ! -f server/.git/hookout &&
test_path_is_missing ?
t/t5702-protocol-v2.sh test_expect_success 'ref advertisment is filtered during fetch using protocol v2 · line 214
Quoted 1 line
t/t5702-protocol-v2.sh +1
-0+ GIT_TRACE=/tmp/y git -c protocol.version=2 clone "file://$(pwd)/server" client &&
Why do we redirect GIT_TRACE outside the test suite? do we read that back or want to read it out of the hook?
Is it possible to redirect to /$(pwd)/trace or such?
t/t5702-protocol-v2.sh test_expect_success 'ref advertisment is filtered during fetch using protocol v2 · line 215
Quoted 1 line
t/t5702-protocol-v2.sh +1
-0+ test -f server/.git/hookout
test_path_is_file ?
Quoted 21 lines
t/t5702-protocol-v2.sh +2
-0+' + # Test protocol v2 with 'http://' transport # . "$TEST_DIRECTORY"/lib-httpd.shupload-pack.c +2
-1@@ -1376,6 +1376,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,enum fetch_state state =FETCH_PROCESS_ARGS ;struct upload_pack_data data; +git_config (upload_pack_config,NULL ); +upload_pack_data_init (&data); use_sideband =LARGE_PACKET_MAX ; --2.17.0.441.gb46fe60e1d-goog
Re: [PATCH v2 2/3] upload-pack: read config when serving protocol v2
Quoted 1 line · On Thu, 3 May 2018 12:08:16 -0700 Stefan Beller <[email protected]> wrote:
test_path_is_missing?
Thanks for the pointer. Done.
Quoted 6 lines
Quoted 1 line
+ GIT_TRACE=/tmp/y git -c protocol.version=2 clone "file://$(pwd)/server" client &&
Why do we redirect
GIT_TRACEoutside the test suite? do we read that back or want to read it out of the hook?Is it possible to redirect to /$(pwd)/trace or such?
Good catch - that was a leftover from debugging. I've just removed it.
Quoted 3 lines
Quoted 1 line
+ test -f server/.git/hookout
test_path_is_file?
Done.
PATCH v2 3/3 {fetch,upload}-pack: support filter in protocol v2
In reply to Jonathan TanThe fetch-pack/upload-pack protocol v2 was developed independently of the filter parameter (used in partial fetches), thus it did not include support for it. Add support for the filter parameter.
Like in the legacy protocol, the server advertises and supports "filter" only if uploadpack.allowfilter is configured.
Like in the legacy protocol, the client continues with a warning if "--filter" is specified, but the server does not advertise it.
--- Documentation/technical/protocol-v2.txt | 9 +++ fetch-pack.c | 23 +++++- t/t5702-protocol-v2.sh | 98 +++++++++++++++++++++++++ upload-pack.c | 15 +++- 4 files changed, 140 insertions(+), 5 deletions(-)
Documentation/technical/protocol-v2.txt +9 -0
@@ -290,6 +290,15 @@ included in the clients request as well as the potential addition of the Cannot be used with "deepen", but can be used with "deepen-since". +If the 'filter' feature is advertised, the following argument can be +included in the client's request: + + filter <filter-spec> + Request that various objects from the packfile be omitted + using one of several filtering techniques. These are intended + for use with partial clone and partial fetch operations. See + `rev-list` for possible "filter-spec" values. + The response of `fetch` is broken into a number of sections separated by delimiter packets (0001), with each section beginning with its section header.
fetch-pack.c +19 -4
@@ -1191,14 +1191,29 @@ static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,else if (is_repository_shallow () || args->deepen)die (_ ("Server does not support shallow requests" )); +/* Add filter */ +if (server_supports_feature ("fetch" ,"filter" ,0 ) && + args->filter_options.choice) { +print_verbose (args,_ ("Server supports filter" )); +packet_buf_write (&req_buf,"filter %s" , + args->filter_options.filter_spec); + }else if (args->filter_options.choice) { +warning ("filtering not recognized by server, ignoring" ); + } +/* add wants */ add_wants (wants, &req_buf); -/* Add all of the common commits we've found in previous rounds */ -add_common (&req_buf, common); +if (args->no_dependents) { +packet_buf_write (&req_buf,"done" ); + ret =1 ; + }else { +/* Add all of the common commits we've found in previous rounds */ +add_common (&req_buf, common); -/* Add initial haves */ - ret =add_haves (&req_buf, haves_to_send, in_vain); +/* Add initial haves */ + ret =add_haves (&req_buf, haves_to_send, in_vain); + }/* Send request */ packet_buf_flush (&req_buf);
t/t5702-protocol-v2.sh +98 -0
104 lines not on this page · raw message
upload-pack.c +14 -1
@@ -1205,6 +1205,7 @@ static void process_args(struct packet_reader *request, {while (packet_reader_read (request) !=PACKET_READ_FLUSH ) {const char *arg = request->line; +const char *p;/* process want */ if (parse_want (arg))
@@ -1251,6 +1252,11 @@ static void process_args(struct packet_reader *request,continue ; } +if (allow_filter &&skip_prefix (arg,"filter " , &p)) { +parse_list_objects_filter (&filter_options, p); +continue ; + } +/* ignore unknown lines maybe? */ die ("unexpected line: '%s'" , arg); }
@@ -1430,7 +1436,14 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,int upload_pack_advertise (struct repository *r,struct strbuf *value) { -if (value) +if (value) { +int allow_filter_value;strbuf_addstr (value,"shallow" ); +if (!repo_config_get_bool (the_repository, +"uploadpack.allowfilter" , + &allow_filter_value) && + allow_filter_value) +strbuf_addstr (value," filter" ); + }return 1 ; }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Re: [PATCH v2 3/3] {fetch,upload}-pack: support filter in protocol v2
Hi Jonathan,
Quoted 11 lines · Jonathan Tan
The fetch-pack/upload-pack protocol v2 was developed independently of the filter parameter (used in partial fetches), thus it did not include support for it. Add support for the filter parameter.
Like in the legacy protocol, the server advertises and supports "filter" only if uploadpack.allowfilter is configured.
Like in the legacy protocol, the client continues with a warning if "--filter" is specified, but the server does not advertise it.
Signed-off-by Jonathan Tan <[email protected]>
The whole series looks good,
Thanks,
Stefan
v3 cover Supporting partial clones in protocol v2
In reply to Jonathan TanChanges from v2: followed all Stefan's comments.
Jonathan Tan (3):
upload-pack: fix error message typo
upload-pack: read config when serving protocol v2 {fetch,upload}-pack: support filter in protocol v2
Documentation/technical/protocol-v2.txt | 9 ++ fetch-pack.c | 23 ++++- t/t5701-git-serve.sh | 14 +++ t/t5702-protocol-v2.sh | 112 ++++++++++++++++++++++++ upload-pack.c | 19 +++- 5 files changed, 171 insertions(+), 6 deletions(-)
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH v3 1/3 upload-pack: fix error message typo
Fix a typo in an error message.
Also, this line was introduced in 3145ea957d2c ("upload-pack: introduce fetch server command", 2018-03-15), which did not contain a test for the case which causes this error to be printed, so introduce a test.
--- t/t5701-git-serve.sh | 14 ++++++++++++++ upload-pack.c | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-)
t/t5701-git-serve.sh +14 -0
@@ -173,4 +173,18 @@ test_expect_success 'symrefs parameter' ' test_cmp actual expect ' +test_expect_success 'unexpected lines are not allowed in fetch request' ' + git init server && + + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + this-is-not-a-command + 0000 + EOF + + test_must_fail git -C server serve --stateless-rpc <in 2>err && + grep "unexpected line: .this-is-not-a-command." err +' + test_done
upload-pack.c +1 -1
@@ -1252,7 +1252,7 @@ static void process_args(struct packet_reader *request, }/* ignore unknown lines maybe? */ -die ("unexpect line: '%s'" , arg); +die ("unexpected line: '%s'" , arg); } }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH v3 2/3 upload-pack: read config when serving protocol v2
In reply to Jonathan TanThe upload-pack code paths never call git_config() with upload_pack_config() when protocol v2 is used, causing options like uploadpack.packobjectshook to not take effect. Ensure that this function is called.
--- t/t5702-protocol-v2.sh | 14 ++++++++++++++ upload-pack.c | 2 ++ 2 files changed, 16 insertions(+)
t/t5702-protocol-v2.sh +14 -0
@@ -201,6 +201,20 @@ test_expect_success 'ref advertisment is filtered during fetch using protocol v2 ! grep "refs/tags/three" log ' +test_expect_success 'upload-pack respects config using protocol v2' ' + git init server && + write_script server/.git/hook <<-\EOF && + touch hookout + "$@" + EOF + test_commit -C server one && + + test_config_global uploadpack.packobjectshook ./hook && + test_path_is_missing server/.git/hookout && + git -c protocol.version=2 clone "file://$(pwd)/server" client && + test_path_is_file server/.git/hookout +' + # Test protocol v2 with 'http://' transport # . "$TEST_DIRECTORY"/lib-httpd.sh
upload-pack.c +2 -0
@@ -1376,6 +1376,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,enum fetch_state state =FETCH_PROCESS_ARGS ;struct upload_pack_data data; +git_config (upload_pack_config,NULL ); +upload_pack_data_init (&data); use_sideband =LARGE_PACKET_MAX ;
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
PATCH v3 3/3 {fetch,upload}-pack: support filter in protocol v2
In reply to Jonathan TanThe fetch-pack/upload-pack protocol v2 was developed independently of the filter parameter (used in partial fetches), thus it did not include support for it. Add support for the filter parameter.
Like in the legacy protocol, the server advertises and supports "filter" only if uploadpack.allowfilter is configured.
Like in the legacy protocol, the client continues with a warning if "--filter" is specified, but the server does not advertise it.
--- Documentation/technical/protocol-v2.txt | 9 +++ fetch-pack.c | 23 +++++- t/t5702-protocol-v2.sh | 98 +++++++++++++++++++++++++ upload-pack.c | 15 +++- 4 files changed, 140 insertions(+), 5 deletions(-)
Documentation/technical/protocol-v2.txt +9 -0
@@ -290,6 +290,15 @@ included in the clients request as well as the potential addition of the Cannot be used with "deepen", but can be used with "deepen-since". +If the 'filter' feature is advertised, the following argument can be +included in the client's request: + + filter <filter-spec> + Request that various objects from the packfile be omitted + using one of several filtering techniques. These are intended + for use with partial clone and partial fetch operations. See + `rev-list` for possible "filter-spec" values. + The response of `fetch` is broken into a number of sections separated by delimiter packets (0001), with each section beginning with its section header.
fetch-pack.c +19 -4
@@ -1191,14 +1191,29 @@ static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,else if (is_repository_shallow () || args->deepen)die (_ ("Server does not support shallow requests" )); +/* Add filter */ +if (server_supports_feature ("fetch" ,"filter" ,0 ) && + args->filter_options.choice) { +print_verbose (args,_ ("Server supports filter" )); +packet_buf_write (&req_buf,"filter %s" , + args->filter_options.filter_spec); + }else if (args->filter_options.choice) { +warning ("filtering not recognized by server, ignoring" ); + } +/* add wants */ add_wants (wants, &req_buf); -/* Add all of the common commits we've found in previous rounds */ -add_common (&req_buf, common); +if (args->no_dependents) { +packet_buf_write (&req_buf,"done" ); + ret =1 ; + }else { +/* Add all of the common commits we've found in previous rounds */ +add_common (&req_buf, common); -/* Add initial haves */ - ret =add_haves (&req_buf, haves_to_send, in_vain); +/* Add initial haves */ + ret =add_haves (&req_buf, haves_to_send, in_vain); + }/* Send request */ packet_buf_flush (&req_buf);
t/t5702-protocol-v2.sh +98 -0
104 lines not on this page · raw message
upload-pack.c +14 -1
@@ -1205,6 +1205,7 @@ static void process_args(struct packet_reader *request, {while (packet_reader_read (request) !=PACKET_READ_FLUSH ) {const char *arg = request->line; +const char *p;/* process want */ if (parse_want (arg))
@@ -1251,6 +1252,11 @@ static void process_args(struct packet_reader *request,continue ; } +if (allow_filter &&skip_prefix (arg,"filter " , &p)) { +parse_list_objects_filter (&filter_options, p); +continue ; + } +/* ignore unknown lines maybe? */ die ("unexpected line: '%s'" , arg); }
@@ -1430,7 +1436,14 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,int upload_pack_advertise (struct repository *r,struct strbuf *value) { -if (value) +if (value) { +int allow_filter_value;strbuf_addstr (value,"shallow" ); +if (!repo_config_get_bool (the_repository, +"uploadpack.allowfilter" , + &allow_filter_value) && + allow_filter_value) +strbuf_addstr (value," filter" ); + }return 1 ; }
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog