This is a patch to demonstrate that fetch-pack only requires a small amount of changes to support the new server endpoint, and that things generally work in various situations, including both stateless RPC and non-stateless RPC (as can be seen from the tests).
Some minor issues remain: - Names of refs should be treated as strings (when interfacing with the new endpoint is requested) instead of being parsed into struct ref unconditionally. - Capability management could probably be done better instead of checking for "--new-way" everywhere. - I'm not sure how to test the upload-pack code path that sends "ACK ready". Signed-off-by: Jonathan Tan <[email protected]> --- Makefile | 1 + builtin/fetch-pack.c | 10 +- fetch-pack.c | 75 +++++++++++---- fetch-pack.h | 1 + t/helper/.gitignore | 1 + t/helper/test-un-pkt.c | 40 ++++++++ t/t9999-mytests.sh | 242 +++++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 349 insertions(+), 21 deletions(-) create mode 100644 t/helper/test-un-pkt.c create mode 100644 t/t9999-mytests.sh diff --git a/Makefile b/Makefile index 0d3813772..0b4510d3f 100644 --- a/Makefile +++ b/Makefile @@ -641,6 +641,7 @@ TEST_PROGRAMS_NEED_X += test-string-list TEST_PROGRAMS_NEED_X += test-submodule-config TEST_PROGRAMS_NEED_X += test-subprocess TEST_PROGRAMS_NEED_X += test-svn-fe +TEST_PROGRAMS_NEED_X += test-un-pkt TEST_PROGRAMS_NEED_X += test-urlmatch-normalization TEST_PROGRAMS_NEED_X += test-wildmatch diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 2a1c1c213..4bd83c4ff 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -143,6 +143,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) args.update_shallow = 1; continue; } + if (!strcmp("--new-way", arg)) { + args.new_way = 1; + continue; + } usage(fetch_pack_usage); } if (deepen_not.nr) @@ -193,7 +197,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) if (!conn) return args.diag_url ? 0 : 1; } - get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow); + if (!args.new_way) + get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow); ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought, &shallow, pack_lockfile_ptr); @@ -219,7 +224,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) * remote no-such-ref' would silently succeed without issuing * an error. */ - ret |= report_unmatched_refs(sought, nr_sought); + if (!args.new_way) + ret |= report_unmatched_refs(sought, nr_sought); while (ref) { printf("%s %s\n", diff --git a/fetch-pack.c b/fetch-pack.c index 74771a283..4cbdada7b 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -251,7 +251,7 @@ static void consume_shallow_list(struct fetch_pack_args *args, int fd) } } -static enum ack_type get_ack(int fd, unsigned char *result_sha1) +static enum ack_type get_ack(int fd, unsigned char *result_sha1, struct ref **ref, struct sha1_array *shallow) { int len; char *line = packet_read_line(fd, &len); @@ -261,6 +261,23 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1) die(_("git fetch-pack: expected ACK/NAK, got EOF")); if (!strcmp(line, "NAK")) return NAK; + if (ref && skip_prefix(line, "wanted ", &arg)) { + struct object_id oid; + if (!get_sha1_hex(arg, oid.hash) && arg[40] == ' ' && arg[41]) { + struct ref *new_ref = alloc_ref(arg + 41); + oidcpy(&new_ref->old_oid, &oid); + new_ref->next = *ref; + *ref = new_ref; + return get_ack(fd, result_sha1, ref, shallow); + } + } + if (shallow && skip_prefix(line, "shallow ", &arg)) { + struct object_id oid; + if (!get_sha1_hex(arg, oid.hash)) { + sha1_array_append(shallow, oid.hash); + return get_ack(fd, result_sha1, ref, shallow); + } + } if (skip_prefix(line, "ACK ", &arg)) { if (!get_sha1_hex(arg, result_sha1)) { arg += 40; @@ -370,7 +387,7 @@ static char *get_wants(const struct fetch_pack_args *args, struct ref *refs) static int find_common(struct fetch_pack_args *args, int fd[2], unsigned char *result_sha1, - char *wants) + char *wants, struct ref **ref, struct sha1_array *shallow) { int count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval; const unsigned char *sha1; @@ -475,7 +492,7 @@ static int find_common(struct fetch_pack_args *args, consume_shallow_list(args, fd[0]); do { - ack = get_ack(fd[0], result_sha1); + ack = get_ack(fd[0], result_sha1, NULL, NULL); if (ack) print_verbose(args, _("got %s %d %s"), "ack", ack, sha1_to_hex(result_sha1)); @@ -544,7 +561,7 @@ static int find_common(struct fetch_pack_args *args, if (!got_ready || !no_done) consume_shallow_list(args, fd[0]); while (flushes || multi_ack) { - int ack = get_ack(fd[0], result_sha1); + int ack = get_ack(fd[0], result_sha1, ref, shallow); if (ack) { print_verbose(args, _("got %s (%d) %s"), "ack", ack, sha1_to_hex(result_sha1)); @@ -878,22 +895,22 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, struct shallow_info *si, char **pack_lockfile) { - struct ref *ref = copy_ref_list(orig_ref); + struct ref *ref; unsigned char sha1[20]; const char *agent_feature; int agent_len; + int find_common_result; - sort_ref_list(&ref, ref_compare_name); - QSORT(sought, nr_sought, cmp_ref_by_name); - - if ((args->depth > 0 || is_repository_shallow()) && !server_supports("shallow")) - die(_("Server does not support shallow clients")); + if (!args->new_way) { + if ((args->depth > 0 || is_repository_shallow()) && !server_supports("shallow")) + die(_("Server does not support shallow clients")); + } if (args->depth > 0 || args->deepen_since || args->deepen_not) args->deepen = 1; - if (server_supports("multi_ack_detailed")) { + if (server_supports("multi_ack_detailed") || args->new_way) { print_verbose(args, _("Server supports multi_ack_detailed")); multi_ack = 2; - if (server_supports("no-done")) { + if (server_supports("no-done") || args->new_way) { print_verbose(args, _("Server supports no-done")); if (args->stateless_rpc) no_done = 1; @@ -936,22 +953,42 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, print_verbose(args, _("Server version is %.*s"), agent_len, agent_feature); } - if (server_supports("deepen-since")) + if (server_supports("deepen-since") || args->new_way) deepen_since_ok = 1; else if (args->deepen_since) die(_("Server does not support --shallow-since")); - if (server_supports("deepen-not")) + if (server_supports("deepen-not") || args->new_way) deepen_not_ok = 1; else if (args->deepen_not) die(_("Server does not support --shallow-exclude")); if (!server_supports("deepen-relative") && args->deepen_relative) die(_("Server does not support --deepen")); - if (everything_local(args, &ref, sought, nr_sought)) { - packet_flush(fd[1]); - goto all_done; + if (args->new_way) { + struct strbuf wants = STRBUF_INIT; + int i; + struct sha1_array shallow = SHA1_ARRAY_INIT; + + ref = NULL; + multi_ack = 2; + use_sideband = 2; + + packet_buf_write(&wants, "fetch-refs\n"); + for (i = 0; i < nr_sought; i++) + packet_buf_write(&wants, "want %s\n", sought[i]->name); + find_common_result = find_common(args, fd, sha1, strbuf_detach(&wants, NULL), &ref, &shallow); + prepare_shallow_info(si, &shallow); + } else { + ref = copy_ref_list(orig_ref); + sort_ref_list(&ref, ref_compare_name); + QSORT(sought, nr_sought, cmp_ref_by_name); + if (everything_local(args, &ref, sought, nr_sought)) { + packet_flush(fd[1]); + goto all_done; + } + find_common_result = find_common(args, fd, sha1, get_wants(args, ref), NULL, NULL); } - if (find_common(args, fd, sha1, get_wants(args, ref)) < 0) + if (find_common_result < 0) if (!args->keep_pack) /* When cloning, it is not unusual to have * no common commit. @@ -1128,7 +1165,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args, if (nr_sought) nr_sought = remove_duplicates_in_refs(sought, nr_sought); - if (!ref) { + if (!args->new_way && !ref) { packet_flush(fd[1]); die(_("no matching remote head")); } diff --git a/fetch-pack.h b/fetch-pack.h index a2d46e6e7..ab7a80696 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -29,6 +29,7 @@ struct fetch_pack_args { unsigned cloning:1; unsigned update_shallow:1; unsigned deepen:1; + unsigned new_way:1; }; /* diff --git a/t/helper/.gitignore b/t/helper/.gitignore index d6e8b3679..cce5069cb 100644 --- a/t/helper/.gitignore +++ b/t/helper/.gitignore @@ -29,5 +29,6 @@ /test-submodule-config /test-subprocess /test-svn-fe +/test-un-pkt /test-urlmatch-normalization /test-wildmatch diff --git a/t/helper/test-un-pkt.c b/t/helper/test-un-pkt.c new file mode 100644 index 000000000..6dcf87980 --- /dev/null +++ b/t/helper/test-un-pkt.c @@ -0,0 +1,40 @@ +/* + * This program takes payloads in pkt-line format (one or more pkt-lines + * followed by a flush pkt) and runs the given command once per payload. + */ +#include "cache.h" +#include "pkt-line.h" +#include "run-command.h" + +int cmd_main(int argc, const char **argv) +{ + struct child_process *cmd = NULL; + char buffer[LARGE_PACKET_MAX]; + int size; + + while ((size = packet_read(0, NULL, NULL, buffer, sizeof(buffer), + PACKET_READ_GENTLE_ON_EOF)) >= 0) { + if (size > 0) { + if (!cmd) { + cmd = xmalloc(sizeof(*cmd)); + child_process_init(cmd); + cmd->argv = argv + 1; + cmd->git_cmd = 1; + cmd->in = -1; + cmd->out = 0; + if (start_command(cmd)) + die("could not start command"); + } + write_in_full(cmd->in, buffer, size); + } else if (cmd) { + close(cmd->in); + if (finish_command(cmd)) + die("could not finish command"); + free(cmd); + cmd = NULL; + if (size < 0) + break; + } + } + return 0; +} diff --git a/t/t9999-mytests.sh b/t/t9999-mytests.sh new file mode 100644 index 000000000..9bb4e85e3 --- /dev/null +++ b/t/t9999-mytests.sh @@ -0,0 +1,242 @@ +#!/bin/sh + +test_description='my tests' + +. ./test-lib.sh + +test_expect_success 'fetch-pack --new-way basic' ' + rm -rf server && + git init server && + ( + cd server && + test_commit 0 && + + git checkout -b one + test_commit 1 && + git checkout master && + + git checkout -b two + test_commit 2 && + git checkout master && + + git checkout -b dont_fetch_this + test_commit 3 && + git checkout master + ) && + git fetch-pack --new-way --exec=git-server-endpoint server refs/heads/one refs/heads/two\* >actual && + + grep "$(printf "%s refs/heads/one" $(git -C server rev-parse --verify one))" actual && + grep "$(printf "%s refs/heads/two" $(git -C server rev-parse --verify two))" actual && + ! grep dont_fetch_this actual +' + +test_expect_success 'fetch-pack --new-way hideRefs' ' + rm -rf server && + git init server && + test_config -C server transfer.hideRefs refs/heads/b2 && + ( + cd server && + test_commit 0 && + + git checkout -b b1 + test_commit 1 && + git checkout master && + + git checkout -b b2 + test_commit 2 && + git checkout master + ) && + git fetch-pack --new-way --exec=git-server-endpoint server refs/heads/b\* >actual && + + grep "$(printf "%s refs/heads/b1" $(git -C server rev-parse --verify 1))" actual && + ! grep refs/heads/b2 actual +' + +test_expect_success 'fetch-pack --new-way long negotiation' ' + rm -rf server client && + git init server && + test_commit -C server 0 && + + git clone server client && + + test_commit -C server 1 && + + git -C client checkout -b sidebranch && + for i in $(seq 2 32) + do + test_commit -C client $i + done && + + git -C client fetch-pack --new-way --exec=git-server-endpoint ../server refs/heads/master >actual && + + grep "$(printf "%s refs/heads/master" $(git -C server rev-parse --verify 1))" actual +' + +test_expect_success 'fetch-pack --new-way with shallow client' ' + rm -rf server && + git init server && + ( + cd server && + test_commit 0 && + test_commit 1 + ) && + rm -rf client && + git clone --depth=1 "file://$(pwd)/server" client && + ( + cd server && + git checkout -b two 0 && + test_commit 2 + ) && + + # check that the shallow clone does not include this parent commit + test_must_fail git -C client cat-file -e $(git -C server rev-parse 0) && + + git -C client fetch-pack --new-way --exec=git-server-endpoint ../server refs/heads/two >actual && + # 0 is the parent of 2, so it must be included now + git -C client cat-file -e $(git -C server rev-parse 0) +' + +test_expect_success 'fetch-pack --new-way --depth' ' + rm -rf server && + git init server && + ( + cd server && + test_commit 0 + ) && + rm -rf client && + git clone server client && + ( + cd server && + test_commit 1 && + test_commit 2 + ) && + + git -C client fetch-pack --new-way --exec=git-server-endpoint --depth=1 ../server refs/heads/master >actual && + git -C client cat-file -e $(git -C server rev-parse 2) && + test_must_fail git -C client cat-file -e $(git -C server rev-parse 1) +' + +test_expect_success 'fetch-pack --new-way --shallow-since' ' + rm -rf server && + git init server && + ( + cd server && + test_commit 0 + ) && + rm -rf client && + git clone server client && + ( + cd server && + test_commit 1 && + test_commit 2 + ) && + DATE=$(git -C server log --format="format:%at" --no-walk 2) && + + git -C client fetch-pack --new-way --exec=git-server-endpoint --shallow-since=$DATE ../server refs/heads/master >actual && + git -C client cat-file -e $(git -C server rev-parse 2) && + test_must_fail git -C client cat-file -e $(git -C server rev-parse 1) +' + +test_expect_success 'fetch-pack --new-way --shallow-exclude' ' + rm -rf server && + git init server && + ( + cd server && + test_commit 0 + ) && + rm -rf client && + git clone server client && + ( + cd server && + test_commit 1 && + test_commit 2 + ) && + + git -C client fetch-pack --new-way --exec=git-server-endpoint --shallow-exclude=1 ../server refs/heads/master >actual && + git -C client cat-file -e $(git -C server rev-parse 2) && + test_must_fail git -C client cat-file -e $(git -C server rev-parse 1) +' + +test_expect_success PIPE 'fetch-pack --new-way --stateless-rpc' ' + rm -rf server && + git init server && + ( + cd server && + test_commit 0 && + + git checkout -b one + test_commit 1 && + git checkout master && + + git checkout -b two + test_commit 2 && + git checkout master && + + git checkout -b dont_fetch_this + test_commit 3 && + git checkout master + ) && + rm -rf client && + git init client && + + mkfifo se-out && + + git -C client fetch-pack --new-way --stateless-rpc ../server refs/heads/one refs/heads/two\* <se-out | test-un-pkt server-endpoint server --stateless-rpc >se-out && + + git -C client cat-file -e $(git -C server rev-parse 1) && + git -C client cat-file -e $(git -C server rev-parse 2) && + test_must_fail git -C client cat-file -e $(git -C server rev-parse 3) +' + +test_expect_success 'fetch-pack --new-way --stateless-rpc long negotiation' ' + rm -rf server client && + git init server && + test_commit -C server 0 && + + git clone server client && + + test_commit -C server 1 && + + git -C client checkout -b sidebranch && + for i in $(seq 2 32) + do + test_commit -C client $i + done && + + rm -f se-out && + mkfifo se-out && + + git -C client fetch-pack --new-way --stateless-rpc ../server refs/heads/master <se-out | test-un-pkt server-endpoint server --stateless-rpc >se-out && + + git -C client cat-file -e $(git -C server rev-parse 1) +' + +test_expect_success 'fetch-pack --new-way --stateless-rpc namespaces' ' + rm -rf server && + git init server && + ( + cd server && + test_commit 0 && + + git checkout -b mybranch + test_commit 1 && + git checkout master && + + git checkout -b mybranch_ns + test_commit 2 && + git checkout master && + git update-ref refs/namespaces/ns/refs/heads/mybranch mybranch_ns + ) && + rm -rf client && + git init client && + + rm -f se-out && + mkfifo se-out && + + git -C client fetch-pack --new-way --stateless-rpc ../server refs/heads/mybranch <se-out | GIT_NAMESPACE=ns test-un-pkt server-endpoint server --stateless-rpc >se-out && + + git -C client cat-file -e $(git -C server rev-parse 2) && + test_must_fail git -C client cat-file -e $(git -C server rev-parse 1) +' + +test_done -- 2.12.2.715.g7642488e1d-goog
