From 7cc6ed2d06d6631726af9c14b5258d723c513327 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Tue, 1 May 2018 17:31:29 -0700 Subject: 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. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- t/t5701-git-serve.sh | 14 ++++++++++++++ upload-pack.c | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index 72d7bc5628..1b4b13cc2b 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -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 /dev/null 2>err && + grep "unexpected line: .this-is-not-a-command." err +' + test_done diff --git a/upload-pack.c b/upload-pack.c index 87b4d32a6e..c4456bb88a 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -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); } } -- cgit v1.3 From 5459268751941a71b32816902698376b7932cec5 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 3 May 2018 16:46:55 -0700 Subject: upload-pack: read config when serving protocol v2 The 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. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- t/t5702-protocol-v2.sh | 14 ++++++++++++++ upload-pack.c | 2 ++ 2 files changed, 16 insertions(+) diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 56f7c3c326..abb15cd6de 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -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 diff --git a/upload-pack.c b/upload-pack.c index c4456bb88a..113edd32d4 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -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; -- cgit v1.3 From ba95710a3bdcb2a80495b1d93a0e482dd69905e1 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 3 May 2018 16:46:56 -0700 Subject: {fetch,upload}-pack: support filter in protocol v2 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 Signed-off-by: Junio C Hamano --- 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(-) diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt index 136179d7d8..38d24fd2bc 100644 --- a/Documentation/technical/protocol-v2.txt +++ b/Documentation/technical/protocol-v2.txt @@ -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 + 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. diff --git a/fetch-pack.c b/fetch-pack.c index f93723fec4..3ed40aa464 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -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); diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index abb15cd6de..25bf046b36 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -215,6 +215,104 @@ test_expect_success 'upload-pack respects config using protocol v2' ' test_path_is_file server/.git/hookout ' +test_expect_success 'setup filter tests' ' + rm -rf server client && + git init server && + + # 1 commit to create a file, and 1 commit to modify it + test_commit -C server message1 a.txt && + test_commit -C server message2 a.txt && + git -C server config protocol.version 2 && + git -C server config uploadpack.allowfilter 1 && + git -C server config uploadpack.allowanysha1inwant 1 && + git -C server config protocol.version 2 +' + +test_expect_success 'partial clone' ' + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 \ + clone --filter=blob:none "file://$(pwd)/server" client && + grep "version 2" trace && + + # Ensure that the old version of the file is missing + git -C client rev-list master --quiet --objects --missing=print \ + >observed.oids && + grep "$(git -C server rev-parse message1:a.txt)" observed.oids && + + # Ensure that client passes fsck + git -C client fsck +' + +test_expect_success 'dynamically fetch missing object' ' + rm "$(pwd)/trace" && + GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \ + cat-file -p $(git -C server rev-parse message1:a.txt) && + grep "version 2" trace +' + +test_expect_success 'partial fetch' ' + rm -rf client "$(pwd)/trace" && + git init client && + SERVER="file://$(pwd)/server" && + test_config -C client extensions.partialClone "$SERVER" && + + GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \ + fetch --filter=blob:none "$SERVER" master:refs/heads/other && + grep "version 2" trace && + + # Ensure that the old version of the file is missing + git -C client rev-list other --quiet --objects --missing=print \ + >observed.oids && + grep "$(git -C server rev-parse message1:a.txt)" observed.oids && + + # Ensure that client passes fsck + git -C client fsck +' + +test_expect_success 'do not advertise filter if not configured to do so' ' + SERVER="file://$(pwd)/server" && + + rm "$(pwd)/trace" && + git -C server config uploadpack.allowfilter 1 && + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 \ + ls-remote "$SERVER" && + grep "fetch=.*filter" trace && + + rm "$(pwd)/trace" && + git -C server config uploadpack.allowfilter 0 && + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 \ + ls-remote "$SERVER" && + grep "fetch=" trace >fetch_capabilities && + ! grep filter fetch_capabilities +' + +test_expect_success 'partial clone warns if filter is not advertised' ' + rm -rf client && + git -C server config uploadpack.allowfilter 0 && + git -c protocol.version=2 \ + clone --filter=blob:none "file://$(pwd)/server" client 2>err && + test_i18ngrep "filtering not recognized by server, ignoring" err +' + +test_expect_success 'even with handcrafted request, filter does not work if not advertised' ' + git -C server config uploadpack.allowfilter 0 && + + # Custom request that tries to filter even though it is not advertised. + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + want $(git -C server rev-parse master) + filter blob:none + 0000 + EOF + + test_must_fail git -C server serve --stateless-rpc /dev/null 2>err && + grep "unexpected line: .filter blob:none." err && + + # Exercise to ensure that if advertised, filter works + git -C server config uploadpack.allowfilter 1 && + git -C server serve --stateless-rpc /dev/null +' + # Test protocol v2 with 'http://' transport # . "$TEST_DIRECTORY"/lib-httpd.sh diff --git a/upload-pack.c b/upload-pack.c index 113edd32d4..82c16cae32 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -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; } -- cgit v1.3