[RFC 3/4] fetch-pack: support new server endpoint
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 <jonathantanmy@google.com>
---
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
