{"thread":{"id":"48891","subject":"[PATCH] negotiator/skipping: skip commits during fetch","startedAt":"2018-07-16T18:44:08Z","lastAt":"2018-09-27T22:47:04Z","messageCount":17,"participants":["Jonathan Tan","Junio C Hamano","Johannes Schindelin","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"352646","messageId":"20180716184401.168576-1-jonathantanmy@google.com","threadId":"48891","inReplyTo":null,"subject":"[PATCH] negotiator/skipping: skip commits during fetch","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-07-16T18:44:01Z","receivedAt":"2018-07-16T18:44:08Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Introduce a new negotiation algorithm used during fetch that skips\ncommits in an effort to find common ancestors faster. The skips grow\nsimilarly to the Fibonacci sequence as the commit walk proceeds further\naway from the tips. The skips may cause unnecessary commits to be\nincluded in the packfile, but the negotiation step typically ends more\nquickly.\n\nUsage of this algorithm is guarded behind the configuration flag\nfetch.negotiationAlgorithm.\n\nSigned-off-by: Jonathan Tan \n---\nThis is on jt/fetch-pack-negotiator, but also applies cleanly on\njt/fetch-nego-tip.\n\nA previous version of this had rough backtracking after a \"match\" is\nfound, but that backtracking implementation was complicated and inexact,\nso I've removed it.\n\nAt $DAY_JOB we have been using this patch with some success, so I'm\nsending it to this mailing list.\n---\n Documentation/config.txt | 9 +\n Makefile | 1 +\n fetch-negotiator.c | 8 +-\n fetch-negotiator.h | 3 +-\n fetch-pack.c | 7 +-\n negotiator/skipping.c | 250 +++++++++++++++++++++++++++\n negotiator/skipping.h | 8 +\n t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++\n 8 files changed, 461 insertions(+), 4 deletions(-)\n create mode 100644 negotiator/skipping.c\n create mode 100644 negotiator/skipping.h\n create mode 100755 t/t5552-skipping-fetch-negotiator.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ab641bf5a9..53443fb9db 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1491,6 +1491,15 @@ fetch.output::\n \t`full` and `compact`. Default value is `full`. See section\n \tOUTPUT in linkgit:git-fetch[1] for detail.\n \n+fetch.negotiationAlgorithm::\n+\tControl how information about the commits in the local repository is\n+\tsent when negotiating the contents of the packfile to be sent by the\n+\tserver. Set to \"skipping\" to use an algorithm that skips commits in an\n+\teffort to converge faster, but may result in a larger-than-necessary\n+\tpackfile; any other value instructs Git to use the default algorithm\n+\tthat never skips commits (unless the server has acknowledged it or one\n+\tof its descendants).\n+\n format.attach::\n \tEnable multipart/mixed attachments as the default for\n \t'format-patch'. The value can also be a double quoted string\ndiff --git a/Makefile b/Makefile\nindex 96f84d1dcf..617475622b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -893,6 +893,7 @@ LIB_OBJS += merge-recursive.o\n LIB_OBJS += mergesort.o\n LIB_OBJS += name-hash.o\n LIB_OBJS += negotiator/default.o\n+LIB_OBJS += negotiator/skipping.o\n LIB_OBJS += notes.o\n LIB_OBJS += notes-cache.o\n LIB_OBJS += notes-merge.o\ndiff --git a/fetch-negotiator.c b/fetch-negotiator.c\nindex 2675d120fe..5d283049f4 100644\n--- a/fetch-negotiator.c\n+++ b/fetch-negotiator.c\n@@ -1,8 +1,14 @@\n #include \"git-compat-util.h\"\n #include \"fetch-negotiator.h\"\n #include \"negotiator/default.h\"\n+#include \"negotiator/skipping.h\"\n \n-void fetch_negotiator_init(struct fetch_negotiator *negotiator)\n+void fetch_negotiator_init(struct fetch_negotiator *negotiator,\n+\t\t\t const char *algorithm)\n {\n+\tif (algorithm && !strcmp(algorithm, \"skipping\")) {\n+\t\tskipping_negotiator_init(negotiator);\n+\t\treturn;\n+\t}\n \tdefault_negotiator_init(negotiator);\n }\ndiff --git a/fetch-negotiator.h b/fetch-negotiator.h\nindex b1290aa9c6..ddb44a22dc 100644\n--- a/fetch-negotiator.h\n+++ b/fetch-negotiator.h\n@@ -52,6 +52,7 @@ struct fetch_negotiator {\n \tvoid *data;\n };\n \n-void fetch_negotiator_init(struct fetch_negotiator *negotiator);\n+void fetch_negotiator_init(struct fetch_negotiator *negotiator,\n+\t\t\t const char *algorithm);\n \n #endif\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 1e50d90082..50773fdde3 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -33,6 +33,7 @@ static int agent_supported;\n static int server_supports_filtering;\n static struct lock_file shallow_lock;\n static const char *alternate_shallow_file;\n+static char *negotiation_algorithm;\n \n /* Remember to update object flag allocation in object.h */\n #define COMPLETE\t(1U << 0)\n@@ -905,7 +906,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \tconst char *agent_feature;\n \tint agent_len;\n \tstruct fetch_negotiator negotiator;\n-\tfetch_negotiator_init(&negotiator);\n+\tfetch_negotiator_init(&negotiator, negotiation_algorithm);\n \n \tsort_ref_list(&ref, ref_compare_name);\n \tQSORT(sought, nr_sought, cmp_ref_by_name);\n@@ -1287,7 +1288,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,\n \tint in_vain = 0;\n \tint haves_to_send = INITIAL_FLUSH;\n \tstruct fetch_negotiator negotiator;\n-\tfetch_negotiator_init(&negotiator);\n+\tfetch_negotiator_init(&negotiator, negotiation_algorithm);\n \tpacket_reader_init(&reader, fd[0], NULL, 0,\n \t\t\t PACKET_READ_CHOMP_NEWLINE);\n \n@@ -1366,6 +1367,8 @@ static void fetch_pack_config(void)\n \tgit_config_get_bool(\"repack.usedeltabaseoffset\", &prefer_ofs_delta);\n \tgit_config_get_bool(\"fetch.fsckobjects\", &fetch_fsck_objects);\n \tgit_config_get_bool(\"transfer.fsckobjects\", &transfer_fsck_objects);\n+\tgit_config_get_string(\"fetch.negotiationalgorithm\",\n+\t\t\t &negotiation_algorithm);\n \n \tgit_config(git_default_config, NULL);\n }\ndiff --git a/negotiator/skipping.c b/negotiator/skipping.c\nnew file mode 100644\nindex 0000000000..74d25e8825\n--- /dev/null\n+++ b/negotiator/skipping.c\n@@ -0,0 +1,250 @@\n+#include \"cache.h\"\n+#include \"skipping.h\"\n+#include \"../commit.h\"\n+#include \"../fetch-negotiator.h\"\n+#include \"../prio-queue.h\"\n+#include \"../refs.h\"\n+#include \"../tag.h\"\n+\n+/* Remember to update object flag allocation in object.h */\n+/*\n+ * Both us and the server know that both parties have this object.\n+ */\n+#define COMMON\t\t(1U << 2)\n+/*\n+ * The server has told us that it has this object. We still need to tell the\n+ * server that we have this object (or one of its descendants), but since we are\n+ * going to do that, we do not need to tell the server about its ancestors.\n+ */\n+#define ADVERTISED\t(1U << 3)\n+/*\n+ * This commit has entered the priority queue.\n+ */\n+#define SEEN\t\t(1U << 4)\n+/*\n+ * This commit has left the priority queue.\n+ */\n+#define POPPED\t\t(1U << 5)\n+\n+static int marked;\n+\n+/*\n+ * An entry in the priority queue.\n+ */\n+struct entry {\n+\tstruct commit *commit;\n+\n+\t/*\n+\t * Used only if commit is not COMMON.\n+\t */\n+\tuint16_t original_ttl;\n+\tuint16_t ttl;\n+};\n+\n+struct data {\n+\tstruct prio_queue rev_list;\n+\n+\t/*\n+\t * The number of non-COMMON commits in rev_list.\n+\t */\n+\tint non_common_revs;\n+};\n+\n+static int compare(const void *a_, const void *b_, void *unused)\n+{\n+\tconst struct entry *a = a_;\n+\tconst struct entry *b = b_;\n+\treturn compare_commits_by_commit_date(a->commit, b->commit, NULL);\n+}\n+\n+static struct entry *rev_list_push(struct data *data, struct commit *commit, int mark)\n+{\n+\tstruct entry *entry;\n+\tcommit->object.flags |= mark | SEEN;\n+\n+\tentry = xcalloc(1, sizeof(*entry));\n+\tentry->commit = commit;\n+\tprio_queue_put(&data->rev_list, entry);\n+\n+\tif (!(mark & COMMON))\n+\t\tdata->non_common_revs++;\n+\treturn entry;\n+}\n+\n+static int clear_marks(const char *refname, const struct object_id *oid,\n+\t\t int flag, void *cb_data)\n+{\n+\tstruct object *o = deref_tag(parse_object(oid), refname, 0);\n+\n+\tif (o && o->type == OBJ_COMMIT)\n+\t\tclear_commit_marks((struct commit *)o,\n+\t\t\t\t COMMON | ADVERTISED | SEEN | POPPED);\n+\treturn 0;\n+}\n+\n+/*\n+ * Mark this SEEN commit and all its SEEN ancestors as COMMON.\n+ */\n+static void mark_common(struct data *data, struct commit *c)\n+{\n+\tstruct commit_list *p;\n+\n+\tif (c->object.flags & COMMON)\n+\t\treturn;\n+\tc->object.flags |= COMMON;\n+\tif (!(c->object.flags & POPPED))\n+\t\tdata->non_common_revs--;\n+\n+\tif (!c->object.parsed)\n+\t\treturn;\n+\tfor (p = c->parents; p; p = p->next) {\n+\t\tif (p->item->object.flags & SEEN)\n+\t\t\tmark_common(data, p->item);\n+\t}\n+}\n+\n+/*\n+ * Ensure that the priority queue has an entry for to_push, and ensure that the\n+ * entry has the correct flags and ttl.\n+ *\n+ * This function returns 1 if an entry was found or created, and 0 otherwise\n+ * (because the entry for this commit had already been popped).\n+ */\n+static int push_parent(struct data *data, struct entry *entry,\n+\t\t struct commit *to_push)\n+{\n+\tstruct entry *parent_entry;\n+\n+\tif (to_push->object.flags & SEEN) {\n+\t\tint i;\n+\t\tif (to_push->object.flags & POPPED)\n+\t\t\t/*\n+\t\t\t * The entry for this commit has already been popped,\n+\t\t\t * due to clock skew. Pretend that this parent does not\n+\t\t\t * exist.\n+\t\t\t */\n+\t\t\treturn 0;\n+\t\t/*\n+\t\t * Find the existing entry and use it.\n+\t\t */\n+\t\tfor (i = 0; i < data->rev_list.nr; i++) {\n+\t\t\tparent_entry = data->rev_list.array[i].data;\n+\t\t\tif (parent_entry->commit == to_push)\n+\t\t\t\tgoto parent_found;\n+\t\t}\n+\t\tBUG(\"missing parent in priority queue\");\n+parent_found:\n+\t\t;\n+\t} else {\n+\t\tparent_entry = rev_list_push(data, to_push, 0);\n+\t}\n+\n+\tif (entry->commit->object.flags & (COMMON | ADVERTISED)) {\n+\t\tmark_common(data, to_push);\n+\t} else {\n+\t\tuint16_t new_original_ttl = entry->ttl\n+\t\t\t? entry->original_ttl : entry->original_ttl * 3 / 2 + 1;\n+\t\tuint16_t new_ttl = entry->ttl\n+\t\t\t? entry->ttl - 1 : new_original_ttl;\n+\t\tif (parent_entry->original_ttl < new_original_ttl) {\n+\t\t\tparent_entry->original_ttl = new_original_ttl;\n+\t\t\tparent_entry->ttl = new_ttl;\n+\t\t}\n+\t}\n+\n+\treturn 1;\n+}\n+\n+static const struct object_id *get_rev(struct data *data)\n+{\n+\tstruct commit *to_send = NULL;\n+\n+\twhile (to_send == NULL) {\n+\t\tstruct entry *entry;\n+\t\tstruct commit *commit;\n+\t\tstruct commit_list *p;\n+\t\tint parent_pushed = 0;\n+\n+\t\tif (data->rev_list.nr == 0 || data->non_common_revs == 0)\n+\t\t\treturn NULL;\n+\n+\t\tentry = prio_queue_get(&data->rev_list);\n+\t\tcommit = entry->commit;\n+\t\tcommit->object.flags |= POPPED;\n+\t\tif (!(commit->object.flags & COMMON))\n+\t\t\tdata->non_common_revs--;\n+\n+\t\tif (!(commit->object.flags & COMMON) && !entry->ttl)\n+\t\t\tto_send = commit;\n+\n+\t\tparse_commit(commit);\n+\t\tfor (p = commit->parents; p; p = p->next)\n+\t\t\tparent_pushed |= push_parent(data, entry, p->item);\n+\n+\t\tif (!(commit->object.flags & COMMON) && !parent_pushed)\n+\t\t\t/*\n+\t\t\t * This commit has no parents, or all of its parents\n+\t\t\t * have already been popped (due to clock skew), so send\n+\t\t\t * it anyway.\n+\t\t\t */\n+\t\t\tto_send = commit;\n+\n+\t\tfree(entry);\n+\t}\n+\n+\treturn &to_send->object.oid;\n+}\n+\n+static void known_common(struct fetch_negotiator *n, struct commit *c)\n+{\n+\tif (c->object.flags & SEEN)\n+\t\treturn;\n+\trev_list_push(n->data, c, ADVERTISED);\n+}\n+\n+static void add_tip(struct fetch_negotiator *n, struct commit *c)\n+{\n+\tn->known_common = NULL;\n+\tif (c->object.flags & SEEN)\n+\t\treturn;\n+\trev_list_push(n->data, c, 0);\n+}\n+\n+static const struct object_id *next(struct fetch_negotiator *n)\n+{\n+\tn->known_common = NULL;\n+\tn->add_tip = NULL;\n+\treturn get_rev(n->data);\n+}\n+\n+static int ack(struct fetch_negotiator *n, struct commit *c)\n+{\n+\tint known_to_be_common = !!(c->object.flags & COMMON);\n+\tif (!(c->object.flags & SEEN))\n+\t\tdie(\"received ack for commit %s not sent as 'have'\\n\",\n+\t\t oid_to_hex(&c->object.oid));\n+\tmark_common(n->data, c);\n+\treturn known_to_be_common;\n+}\n+\n+static void release(struct fetch_negotiator *n)\n+{\n+\tclear_prio_queue(&((struct data *)n->data)->rev_list);\n+\tFREE_AND_NULL(n->data);\n+}\n+\n+void skipping_negotiator_init(struct fetch_negotiator *negotiator)\n+{\n+\tstruct data *data;\n+\tnegotiator->known_common = known_common;\n+\tnegotiator->add_tip = add_tip;\n+\tnegotiator->next = next;\n+\tnegotiator->ack = ack;\n+\tnegotiator->release = release;\n+\tnegotiator->data = data = xcalloc(1, sizeof(*data));\n+\tdata->rev_list.compare = compare;\n+\n+\tif (marked)\n+\t\tfor_each_ref(clear_marks, NULL);\n+\tmarked = 1;\n+}\ndiff --git a/negotiator/skipping.h b/negotiator/skipping.h\nnew file mode 100644\nindex 0000000000..d7dfa6c6e4\n--- /dev/null\n+++ b/negotiator/skipping.h\n@@ -0,0 +1,8 @@\n+#ifndef NEGOTIATOR_SKIPPING_H\n+#define NEGOTIATOR_SKIPPING_H\n+\n+struct fetch_negotiator;\n+\n+void skipping_negotiator_init(struct fetch_negotiator *negotiator);\n+\n+#endif\ndiff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh\nnew file mode 100755\nindex 0000000000..8c417f435f\n--- /dev/null\n+++ b/t/t5552-skipping-fetch-negotiator.sh\n@@ -0,0 +1,179 @@\n+#!/bin/sh\n+\n+test_description='test skipping fetch negotiator'\n+. ./test-lib.sh\n+\n+have_sent () {\n+\twhile test \"$#\" -ne 0\n+\tdo\n+\t\tgrep \"fetch> have $(git -C client rev-parse $1)\" trace\n+\t\tif test $? -ne 0\n+\t\tthen\n+\t\t\techo \"No have $(git -C client rev-parse $1) ($1)\"\n+\t\t\treturn 1\n+\t\tfi\n+\t\tshift\n+\tdone\n+}\t\n+\n+have_not_sent () {\n+\twhile test \"$#\" -ne 0\n+\tdo\n+\t\tgrep \"fetch> have $(git -C client rev-parse $1)\" trace\n+\t\tif test $? -eq 0\n+\t\tthen\n+\t\t\treturn 1\n+\t\tfi\n+\t\tshift\n+\tdone\n+}\n+\n+test_expect_success 'commits with no parents are sent regardless of skip distance' '\n+\tgit init server &&\n+\ttest_commit -C server to_fetch &&\n+\n+\tgit init client &&\n+\tfor i in $(seq 7)\n+\tdo\n+\t\ttest_commit -C client c$i\n+\tdone &&\n+\n+\t# We send: \"c7\" (skip 1) \"c5\" (skip 2) \"c2\" (skip 4). After that, since\n+\t# \"c1\" has no parent, it is still sent as \"have\" even though it would\n+\t# normally be skipped.\n+\ttest_config -C client fetch.negotiationalgorithm skipping &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" &&\n+\thave_sent c7 c5 c2 c1 &&\n+\thave_not_sent c6 c4 c3\n+'\n+\n+test_expect_success 'when two skips collide, favor the larger one' '\n+\trm -rf server client trace &&\n+\tgit init server &&\n+\ttest_commit -C server to_fetch &&\n+\n+\tgit init client &&\n+\tfor i in $(seq 11)\n+\tdo\n+\t\ttest_commit -C client c$i\n+\tdone &&\n+\tgit -C client checkout c5 &&\n+\ttest_commit -C client c5side &&\n+\n+\t# Before reaching c5, we send \"c5side\" (skip 1) and \"c11\" (skip 1) \"c9\"\n+\t# (skip 2) \"c6\" (skip 4). The larger skip (skip 4) takes precedence, so\n+\t# the next \"have\" sent will be \"c1\" (from \"c6\" skip 4) and not \"c4\"\n+\t# (from \"c5side\" skip 1).\n+\ttest_config -C client fetch.negotiationalgorithm skipping &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" &&\n+\thave_sent c5side c11 c9 c6 c1 &&\n+\thave_not_sent c10 c8 c7 c5 c4 c3 c2\n+'\n+\n+test_expect_success 'use ref advertisement to filter out commits' '\n+\trm -rf server client trace &&\n+\tgit init server &&\n+\ttest_commit -C server c1 &&\n+\ttest_commit -C server c2 &&\n+\ttest_commit -C server c3 &&\n+\tgit -C server tag -d c1 c2 c3 &&\n+\n+\tgit clone server client &&\n+\ttest_commit -C client c4 &&\n+\ttest_commit -C client c5 &&\n+\tgit -C client checkout c4^^ &&\n+\ttest_commit -C client c2side &&\n+\n+\tgit -C server checkout --orphan anotherbranch &&\n+\ttest_commit -C server to_fetch &&\n+\n+\t# The server advertising \"c3\" (as \"refs/heads/master\") means that we do\n+\t# not need to send any ancestors of \"c3\", but we still need to send \"c3\"\n+\t# itself.\n+\ttest_config -C client fetch.negotiationalgorithm skipping &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch origin to_fetch &&\n+\thave_sent c5 c4^ c2side &&\n+\thave_not_sent c4 c4^^ c4^^^\n+'\n+\n+test_expect_success 'handle clock skew' '\n+\trm -rf server client trace &&\n+\tgit init server &&\n+\ttest_commit -C server to_fetch &&\n+\n+\tgit init client &&\n+\n+\t# 2 regular commits\n+\ttest_tick=2000000000 &&\n+\ttest_commit -C client c1 &&\n+\ttest_commit -C client c2 &&\n+\n+\t# 4 old commits\n+\ttest_tick=1000000000 &&\n+\tgit -C client checkout c1 &&\n+\ttest_commit -C client old1 &&\n+\ttest_commit -C client old2 &&\n+\ttest_commit -C client old3 &&\n+\ttest_commit -C client old4 &&\n+\n+\t# \"c2\" and \"c1\" are popped first, then \"old4\" to \"old1\". \"old1\" would\n+\t# normally be skipped, but is treated as a commit without a parent here\n+\t# and sent, because (due to clock skew) its only parent has already been\n+\t# popped off the priority queue.\n+\ttest_config -C client fetch.negotiationalgorithm skipping &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" &&\n+\thave_sent c2 c1 old4 old2 old1 &&\n+\thave_not_sent old3\n+'\n+\n+test_expect_success 'do not send \"have\" with ancestors of commits that server ACKed' '\n+\trm -rf server client trace &&\n+\tgit init server &&\n+\ttest_commit -C server to_fetch &&\n+\n+\tgit init client &&\n+\tfor i in $(seq 8)\n+\tdo\n+\t\tgit -C client checkout --orphan b$i &&\n+\t\ttest_commit -C client b$i.c0\n+\tdone &&\n+\tfor j in $(seq 19)\n+\tdo\n+\t\tfor i in $(seq 8)\n+\t\tdo\n+\t\t\tgit -C client checkout b$i &&\n+\t\t\ttest_commit -C client b$i.c$j\n+\t\tdone\n+\tdone &&\n+\n+\t# Copy this branch over to the server and add a commit on it so that it\n+\t# is reachable but not advertised.\n+\tgit -C server fetch --no-tags \"$(pwd)/client\" b1:refs/heads/b1 &&\n+\tgit -C server checkout b1 &&\n+\ttest_commit -C server commit-on-b1 &&\n+\n+\ttest_config -C client fetch.negotiationalgorithm skipping &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" to_fetch &&\n+\tgrep \" fetch\" trace &&\n+\n+\t# fetch-pack sends 2 requests each containing 16 \"have\" lines before\n+\t# processing the first response. In these 2 requests, 4 commits from\n+\t# each branch are sent. Just check the first branch.\n+\thave_sent b1.c19 b1.c17 b1.c14 b1.c9 &&\n+\thave_not_sent b1.c18 b1.c16 b1.c15 b1.c13 b1.c12 b1.c11 b1.c10 &&\n+\n+\t# While fetch-pack is processing the first response, it should read that\n+\t# the server ACKs b1.c19 and b1.c17.\n+\tgrep \"fetch< ACK $(git -C client rev-parse b1.c19) common\" trace &&\n+\tgrep \"fetch< ACK $(git -C client rev-parse b1.c17) common\" trace &&\n+\n+\t# fetch-pack should thus not send any more commits in the b1 branch, but\n+\t# should still send the others (in this test, just check b2).\n+\tfor i in $(seq 0 8)\n+\tdo\n+\t\thave_not_sent b1.c$i\n+\tdone &&\n+\thave_sent b2.c1 b2.c0\n+'\n+\n+test_done\n-- \n2.18.0.203.gfac676dfb9-goog\n\n"},{"id":"352711","messageId":"xmqqsh4irdhk.fsf@gitster-ct.c.googlers.com","threadId":"48891","inReplyTo":"20180716184401.168576-1-jonathantanmy@google.com","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T23:02:15Z","receivedAt":"2018-07-16T23:02:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> Introduce a new negotiation algorithm used during fetch that skips\n> commits in an effort to find common ancestors faster. The skips grow\n> similarly to the Fibonacci sequence as the commit walk proceeds further\n> away from the tips. The skips may cause unnecessary commits to be\n> included in the packfile, but the negotiation step typically ends more\n> quickly.\n>\n> Usage of this algorithm is guarded behind the configuration flag\n> fetch.negotiationAlgorithm.\n>\n> Signed-off-by: Jonathan Tan \n> ---\n> This is on jt/fetch-pack-negotiator, but also applies cleanly on\n> jt/fetch-nego-tip.\n\nSounds sensible.\n\nUnfortunately, this one is among many others that get hurt by\nneedless semantic conflicts caused by reusing the old function name\nand changing the function signature to pass the_repository thru some\ncodepaths, without adding transition macros. I am running out of\ntime today as I need some post-office-move clean-ups before getting\norganized enough, so I expect I won't be able to clean it up and\npush it out on 'pu' by the end of day today.\n\nThanks.\n\n"},{"id":"353632","messageId":"nycvar.QRO.7.76.6.1807261233300.71@tvgsbejvaqbjf.bet","threadId":"48891","inReplyTo":"20180716184401.168576-1-jonathantanmy@google.com","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-26T10:36:24Z","receivedAt":"2018-07-26T10:36:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Mon, 16 Jul 2018, Jonathan Tan wrote:\n\n> t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++\n\nThis test seems to be failing consistently in the recent `pu` builds:\n\nhttps://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs\n\nCould you have a look, please?\n\nCiao,\nDscho\n\nP.S.: For your convenience, I will paste the last part of the output with\n`-i -v -x` here:\n\n-- snipsnap --\n2018-07-26T08:18:39.7864833Z expecting success: \n2018-07-26T08:18:39.7868553Z \trm -rf server client trace &&\n2018-07-26T08:18:39.7869403Z \tgit init server &&\n2018-07-26T08:18:39.7869606Z \ttest_commit -C server to_fetch &&\n2018-07-26T08:18:39.7870066Z \n2018-07-26T08:18:39.7870281Z \tgit init client &&\n2018-07-26T08:18:39.7870403Z \n2018-07-26T08:18:39.7870579Z \t# 2 regular commits\n2018-07-26T08:18:39.7870779Z \ttest_tick=2000000000 &&\n2018-07-26T08:18:39.7870943Z \ttest_commit -C client c1 &&\n2018-07-26T08:18:39.7871103Z \ttest_commit -C client c2 &&\n2018-07-26T08:18:39.7871228Z \n2018-07-26T08:18:39.7871419Z \t# 4 old commits\n2018-07-26T08:18:39.7871575Z \ttest_tick=1000000000 &&\n2018-07-26T08:18:39.7871734Z \tgit -C client checkout c1 &&\n2018-07-26T08:18:39.7871916Z \ttest_commit -C client old1 &&\n2018-07-26T08:18:39.7872081Z \ttest_commit -C client old2 &&\n2018-07-26T08:18:39.7872396Z \ttest_commit -C client old3 &&\n2018-07-26T08:18:39.7872598Z \ttest_commit -C client old4 &&\n2018-07-26T08:18:39.7872743Z \n2018-07-26T08:18:39.7872918Z \t# \"c2\" and \"c1\" are popped first, then \"old4\" to \"old1\". \"old1\" would\n2018-07-26T08:18:39.7873114Z \t# normally be skipped, but is treated as a commit without a parent here\n2018-07-26T08:18:39.7873329Z \t# and sent, because (due to clock skew) its only parent has already been\n2018-07-26T08:18:39.7873524Z \t# popped off the priority queue.\n2018-07-26T08:18:39.7873700Z \ttest_config -C client fetch.negotiationalgorithm skipping &&\n2018-07-26T08:18:39.7873908Z \tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" &&\n2018-07-26T08:18:39.7874091Z \thave_sent c2 c1 old4 old2 old1 &&\n2018-07-26T08:18:39.7874262Z \thave_not_sent old3\n2018-07-26T08:18:39.7874383Z \n2018-07-26T08:18:39.8353323Z ++ rm -rf server client trace\n2018-07-26T08:18:40.3404166Z ++ git init server\n2018-07-26T08:18:40.3756394Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server/.git/\n2018-07-26T08:18:40.3769512Z ++ test_commit -C server to_fetch\n2018-07-26T08:18:40.3776271Z ++ notick=\n2018-07-26T08:18:40.3777103Z ++ signoff=\n2018-07-26T08:18:40.3777282Z ++ indir=\n2018-07-26T08:18:40.3777465Z ++ test 3 '!=' 0\n2018-07-26T08:18:40.3777648Z ++ case \"$1\" in\n2018-07-26T08:18:40.3777801Z ++ indir=server\n2018-07-26T08:18:40.3777948Z ++ shift\n2018-07-26T08:18:40.3778093Z ++ shift\n2018-07-26T08:18:40.3778493Z ++ test 1 '!=' 0\n2018-07-26T08:18:40.3778921Z ++ case \"$1\" in\n2018-07-26T08:18:40.3779072Z ++ break\n2018-07-26T08:18:40.3779241Z ++ indir=server/\n2018-07-26T08:18:40.3779431Z ++ file=to_fetch.t\n2018-07-26T08:18:40.3779603Z ++ echo to_fetch\n2018-07-26T08:18:40.3779923Z ++ git -C server/ add to_fetch.t\n2018-07-26T08:18:40.4072248Z ++ test -z ''\n2018-07-26T08:18:40.4072727Z ++ test_tick\n2018-07-26T08:18:40.4072948Z ++ test -z set\n2018-07-26T08:18:40.4073113Z ++ test_tick=1112913673\n2018-07-26T08:18:40.4073758Z ++ GIT_COMMITTER_DATE='1112913673 -0700'\n2018-07-26T08:18:40.4074001Z ++ GIT_AUTHOR_DATE='1112913673 -0700'\n2018-07-26T08:18:40.4074178Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n2018-07-26T08:18:40.4074357Z ++ git -C server/ commit -m to_fetch\n2018-07-26T08:18:40.4485364Z [master (root-commit) ff85695] to_fetch\n2018-07-26T08:18:40.4485997Z Author: A U Thor \n2018-07-26T08:18:40.4486201Z 1 file changed, 1 insertion(+)\n2018-07-26T08:18:40.4486414Z create mode 100644 to_fetch.t\n2018-07-26T08:18:40.4499970Z ++ git -C server/ tag to_fetch\n2018-07-26T08:18:40.4809208Z ++ git init client\n2018-07-26T08:18:40.5139949Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/client/.git/\n2018-07-26T08:18:40.5158270Z ++ test_tick=2000000000\n2018-07-26T08:18:40.5158466Z ++ test_commit -C client c1\n2018-07-26T08:18:40.5159077Z ++ notick=\n2018-07-26T08:18:40.5159492Z ++ signoff=\n2018-07-26T08:18:40.5159697Z ++ indir=\n2018-07-26T08:18:40.5159855Z ++ test 3 '!=' 0\n2018-07-26T08:18:40.5160010Z ++ case \"$1\" in\n2018-07-26T08:18:40.5160209Z ++ indir=client\n2018-07-26T08:18:40.5160362Z ++ shift\n2018-07-26T08:18:40.5160507Z ++ shift\n2018-07-26T08:18:40.5160657Z ++ test 1 '!=' 0\n2018-07-26T08:18:40.5160831Z ++ case \"$1\" in\n2018-07-26T08:18:40.5161289Z ++ break\n2018-07-26T08:18:40.5161582Z ++ indir=client/\n2018-07-26T08:18:40.5161764Z ++ file=c1.t\n2018-07-26T08:18:40.5161916Z ++ echo c1\n2018-07-26T08:18:40.5162231Z ++ git -C client/ add c1.t\n2018-07-26T08:18:40.5456318Z ++ test -z ''\n2018-07-26T08:18:40.5460548Z ++ test_tick\n2018-07-26T08:18:40.5461417Z ++ test -z set\n2018-07-26T08:18:40.5463657Z ++ test_tick=2000000060\n2018-07-26T08:18:40.5464369Z ++ GIT_COMMITTER_DATE='2000000060 -0700'\n2018-07-26T08:18:40.5464617Z ++ GIT_AUTHOR_DATE='2000000060 -0700'\n2018-07-26T08:18:40.5464805Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n2018-07-26T08:18:40.5464988Z ++ git -C client/ commit -m c1\n2018-07-26T08:18:40.5857440Z [master (root-commit) dc824fa] c1\n2018-07-26T08:18:40.5858031Z Author: A U Thor \n2018-07-26T08:18:40.5858251Z 1 file changed, 1 insertion(+)\n2018-07-26T08:18:40.5858451Z create mode 100644 c1.t\n2018-07-26T08:18:40.5872839Z ++ git -C client/ tag c1\n2018-07-26T08:18:40.6174770Z ++ test_commit -C client c2\n2018-07-26T08:18:40.6175120Z ++ notick=\n2018-07-26T08:18:40.6175398Z ++ signoff=\n2018-07-26T08:18:40.6175583Z ++ indir=\n2018-07-26T08:18:40.6175737Z ++ test 3 '!=' 0\n2018-07-26T08:18:40.6175925Z ++ case \"$1\" in\n2018-07-26T08:18:40.6176079Z ++ indir=client\n2018-07-26T08:18:40.6176246Z ++ shift\n2018-07-26T08:18:40.6176415Z ++ shift\n2018-07-26T08:18:40.6176569Z ++ test 1 '!=' 0\n2018-07-26T08:18:40.6176738Z ++ case \"$1\" in\n2018-07-26T08:18:40.6176905Z ++ break\n2018-07-26T08:18:40.6177052Z ++ indir=client/\n2018-07-26T08:18:40.6177200Z ++ file=c2.t\n2018-07-26T08:18:40.6177369Z ++ echo c2\n2018-07-26T08:18:40.6177525Z ++ git -C client/ add c2.t\n2018-07-26T08:18:40.6474943Z ++ test -z ''\n2018-07-26T08:18:40.6479175Z ++ test_tick\n2018-07-26T08:18:40.6479861Z ++ test -z set\n2018-07-26T08:18:40.6482344Z ++ test_tick=2000000120\n2018-07-26T08:18:40.6483064Z ++ GIT_COMMITTER_DATE='2000000120 -0700'\n2018-07-26T08:18:40.6483243Z ++ GIT_AUTHOR_DATE='2000000120 -0700'\n2018-07-26T08:18:40.6483412Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n2018-07-26T08:18:40.6483597Z ++ git -C client/ commit -m c2\n2018-07-26T08:18:40.6883597Z [master 9ab4692] c2\n2018-07-26T08:18:40.6884552Z Author: A U Thor \n2018-07-26T08:18:40.6884902Z 1 file changed, 1 insertion(+)\n2018-07-26T08:18:40.6885078Z create mode 100644 c2.t\n2018-07-26T08:18:40.6898418Z ++ git -C client/ tag c2\n2018-07-26T08:18:40.7214970Z ++ test_tick=1000000000\n2018-07-26T08:18:40.7215737Z ++ git -C client checkout c1\n2018-07-26T08:18:40.7537971Z Note: checking out 'c1'.\n2018-07-26T08:18:40.7538294Z \n2018-07-26T08:18:40.7538485Z You are in 'detached HEAD' state. You can look around, make experimental\n2018-07-26T08:18:40.7538901Z changes and commit them, and you can discard any commits you make in this\n2018-07-26T08:18:40.7539153Z state without impacting any branches by performing another checkout.\n2018-07-26T08:18:40.7539288Z \n2018-07-26T08:18:40.7539455Z If you want to create a new branch to retain commits you create, you may\n2018-07-26T08:18:40.7539646Z do so (now or later) by using -b with the checkout command again. Example:\n2018-07-26T08:18:40.7539799Z \n2018-07-26T08:18:40.7539979Z git checkout -b \n2018-07-26T08:18:40.7540099Z \n2018-07-26T08:18:40.7540264Z HEAD is now at dc824fa c1\n2018-07-26T08:18:40.7552832Z ++ test_commit -C client old1\n2018-07-26T08:18:40.7559118Z ++ notick=\n2018-07-26T08:18:40.7559789Z ++ signoff=\n2018-07-26T08:18:40.7559966Z ++ indir=\n2018-07-26T08:18:40.7560066Z ++ test 3 '!=' 0\n2018-07-26T08:18:40.7565193Z ++ case \"$1\" in\n2018-07-26T08:18:40.7565286Z ++ indir=client\n2018-07-26T08:18:40.7565373Z ++ shift\n2018-07-26T08:18:40.7565456Z ++ shift\n2018-07-26T08:18:40.7566662Z ++ test 1 '!=' 0\n2018-07-26T08:18:40.7566796Z ++ case \"$1\" in\n2018-07-26T08:18:40.7566879Z ++ break\n2018-07-26T08:18:40.7566961Z ++ indir=client/\n2018-07-26T08:18:40.7567066Z ++ file=old1.t\n2018-07-26T08:18:40.7567150Z ++ echo old1\n2018-07-26T08:18:40.7567238Z ++ git -C client/ add old1.t\n2018-07-26T08:18:40.7962371Z ++ test -z ''\n2018-07-26T08:18:40.7962668Z ++ test_tick\n2018-07-26T08:18:40.7963247Z ++ test -z set\n2018-07-26T08:18:40.7963453Z ++ test_tick=1000000060\n2018-07-26T08:18:40.7963649Z ++ GIT_COMMITTER_DATE='1000000060 -0700'\n2018-07-26T08:18:40.7963832Z ++ GIT_AUTHOR_DATE='1000000060 -0700'\n2018-07-26T08:18:40.7964000Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n2018-07-26T08:18:40.7964164Z ++ git -C client/ commit -m old1\n2018-07-26T08:18:40.8356576Z [detached HEAD e9a2c09] old1\n2018-07-26T08:18:40.8357378Z Author: A U Thor \n2018-07-26T08:18:40.8357554Z 1 file changed, 1 insertion(+)\n2018-07-26T08:18:40.8357652Z create mode 100644 old1.t\n2018-07-26T08:18:40.8372002Z ++ git -C client/ tag old1\n2018-07-26T08:18:40.8671359Z ++ test_commit -C client old2\n2018-07-26T08:18:40.8676749Z ++ notick=\n2018-07-26T08:18:40.8677398Z ++ signoff=\n2018-07-26T08:18:40.8679262Z ++ indir=\n2018-07-26T08:18:40.8679957Z ++ test 3 '!=' 0\n2018-07-26T08:18:40.8680150Z ++ case \"$1\" in\n2018-07-26T08:18:40.8680305Z ++ indir=client\n2018-07-26T08:18:40.8680447Z ++ shift\n2018-07-26T08:18:40.8680587Z ++ shift\n2018-07-26T08:18:40.8680798Z ++ test 1 '!=' 0\n2018-07-26T08:18:40.8680949Z ++ case \"$1\" in\n2018-07-26T08:18:40.8681143Z ++ break\n2018-07-26T08:18:40.8681311Z ++ indir=client/\n2018-07-26T08:18:40.8681458Z ++ file=old2.t\n2018-07-26T08:18:40.8681604Z ++ echo old2\n2018-07-26T08:18:40.8681789Z ++ git -C client/ add old2.t\n2018-07-26T08:18:40.8990053Z ++ test -z ''\n2018-07-26T08:18:40.8990392Z ++ test_tick\n2018-07-26T08:18:40.8990953Z ++ test -z set\n2018-07-26T08:18:40.8991246Z ++ test_tick=1000000120\n2018-07-26T08:18:40.8991421Z ++ GIT_COMMITTER_DATE='1000000120 -0700'\n2018-07-26T08:18:40.8991585Z ++ GIT_AUTHOR_DATE='1000000120 -0700'\n2018-07-26T08:18:40.8991771Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n2018-07-26T08:18:40.8991936Z ++ git -C client/ commit -m old2\n2018-07-26T08:18:40.9402744Z [detached HEAD 41bd8dc] old2\n2018-07-26T08:18:40.9403286Z Author: A U Thor \n2018-07-26T08:18:40.9403510Z 1 file changed, 1 insertion(+)\n2018-07-26T08:18:40.9403678Z create mode 100644 old2.t\n2018-07-26T08:18:40.9418122Z ++ git -C client/ tag old2\n2018-07-26T08:18:40.9736807Z ++ test_commit -C client old3\n2018-07-26T08:18:40.9737058Z ++ notick=\n2018-07-26T08:18:40.9737152Z ++ signoff=\n2018-07-26T08:18:40.9737238Z ++ indir=\n2018-07-26T08:18:40.9737327Z ++ test 3 '!=' 0\n2018-07-26T08:18:40.9737600Z ++ case \"$1\" in\n2018-07-26T08:18:40.9737690Z ++ indir=client\n2018-07-26T08:18:40.9737776Z ++ shift\n2018-07-26T08:18:40.9737860Z ++ shift\n2018-07-26T08:18:40.9737964Z ++ test 1 '!=' 0\n2018-07-26T08:18:40.9738054Z ++ case \"$1\" in\n2018-07-26T08:18:40.9738140Z ++ break\n2018-07-26T08:18:40.9738248Z ++ indir=client/\n2018-07-26T08:18:40.9738338Z ++ file=old3.t\n2018-07-26T08:18:40.9738445Z ++ echo old3\n2018-07-26T08:18:40.9738540Z ++ git -C client/ add old3.t\n2018-07-26T08:18:41.0035565Z ++ test -z ''\n2018-07-26T08:18:41.0036056Z ++ test_tick\n2018-07-26T08:18:41.0036299Z ++ test -z set\n2018-07-26T08:18:41.0036467Z ++ test_tick=1000000180\n2018-07-26T08:18:41.0036638Z ++ GIT_COMMITTER_DATE='1000000180 -0700'\n2018-07-26T08:18:41.0037189Z ++ GIT_AUTHOR_DATE='1000000180 -0700'\n2018-07-26T08:18:41.0037403Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n2018-07-26T08:18:41.0037574Z ++ git -C client/ commit -m old3\n2018-07-26T08:18:41.0429238Z [detached HEAD 4ff0db5] old3\n2018-07-26T08:18:41.0429619Z Author: A U Thor \n2018-07-26T08:18:41.0429799Z 1 file changed, 1 insertion(+)\n2018-07-26T08:18:41.0429965Z create mode 100644 old3.t\n2018-07-26T08:18:41.0443795Z ++ git -C client/ tag old3\n2018-07-26T08:18:41.0752553Z ++ test_commit -C client old4\n2018-07-26T08:18:41.0752824Z ++ notick=\n2018-07-26T08:18:41.0752936Z ++ signoff=\n2018-07-26T08:18:41.0753905Z ++ indir=\n2018-07-26T08:18:41.0754193Z ++ test 3 '!=' 0\n2018-07-26T08:18:41.0754374Z ++ case \"$1\" in\n2018-07-26T08:18:41.0754531Z ++ indir=client\n2018-07-26T08:18:41.0754682Z ++ shift\n2018-07-26T08:18:41.0754828Z ++ shift\n2018-07-26T08:18:41.0755007Z ++ test 1 '!=' 0\n2018-07-26T08:18:41.0755162Z ++ case \"$1\" in\n2018-07-26T08:18:41.0755340Z ++ break\n2018-07-26T08:18:41.0755491Z ++ indir=client/\n2018-07-26T08:18:41.0755664Z ++ file=old4.t\n2018-07-26T08:18:41.0755814Z ++ echo old4\n2018-07-26T08:18:41.0755971Z ++ git -C client/ add old4.t\n2018-07-26T08:18:41.1064316Z ++ test -z ''\n2018-07-26T08:18:41.1064885Z ++ test_tick\n2018-07-26T08:18:41.1065169Z ++ test -z set\n2018-07-26T08:18:41.1065432Z ++ test_tick=1000000240\n2018-07-26T08:18:41.1065637Z ++ GIT_COMMITTER_DATE='1000000240 -0700'\n2018-07-26T08:18:41.1065820Z ++ GIT_AUTHOR_DATE='1000000240 -0700'\n2018-07-26T08:18:41.1066008Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n2018-07-26T08:18:41.1066212Z ++ git -C client/ commit -m old4\n2018-07-26T08:18:41.1488203Z [detached HEAD caef059] old4\n2018-07-26T08:18:41.1488670Z Author: A U Thor \n2018-07-26T08:18:41.1489153Z 1 file changed, 1 insertion(+)\n2018-07-26T08:18:41.1489370Z create mode 100644 old4.t\n2018-07-26T08:18:41.1502887Z ++ git -C client/ tag old4\n2018-07-26T08:18:41.1824546Z ++ test_config -C client fetch.negotiationalgorithm skipping\n2018-07-26T08:18:41.1825670Z ++ config_dir=\n2018-07-26T08:18:41.1826072Z ++ test -C = -C\n2018-07-26T08:18:41.1826299Z ++ shift\n2018-07-26T08:18:41.1826528Z ++ config_dir=client\n2018-07-26T08:18:41.1826809Z ++ shift\n2018-07-26T08:18:41.1827079Z ++ test_when_finished 'test_unconfig -C '\\''client'\\'' '\\''fetch.negotiationalgorithm'\\'''\n2018-07-26T08:18:41.1827289Z ++ test 0 = 0\n2018-07-26T08:18:41.1827561Z ++ test_cleanup='{ test_unconfig -C '\\''client'\\'' '\\''fetch.negotiationalgorithm'\\''\n2018-07-26T08:18:41.1827751Z \t\t} && (exit \"$eval_ret\"); eval_ret=$?; :'\n2018-07-26T08:18:41.1827930Z ++ git -C client config fetch.negotiationalgorithm skipping\n2018-07-26T08:18:41.2196451Z +++ pwd\n2018-07-26T08:18:41.2196831Z +++ builtin pwd -W\n2018-07-26T08:18:41.2274040Z +++ pwd\n2018-07-26T08:18:41.2274458Z +++ builtin pwd -W\n2018-07-26T08:18:41.2285081Z ++ GIT_TRACE_PACKET='D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/trace'\n2018-07-26T08:18:41.2285515Z ++ git -C client fetch 'D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server'\n2018-07-26T08:18:41.3054360Z warning: no common commits\n2018-07-26T08:18:41.3264762Z From D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server\n2018-07-26T08:18:41.3265204Z * branch HEAD -> FETCH_HEAD\n2018-07-26T08:18:41.3362819Z ++ have_sent c2 c1 old4 old2 old1\n2018-07-26T08:18:41.3370525Z ++ test 5 -ne 0\n2018-07-26T08:18:41.3423124Z +++ git -C client rev-parse c2\n2018-07-26T08:18:41.3756643Z ++ grep 'fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e' trace\n2018-07-26T08:18:41.3878403Z packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e\n2018-07-26T08:18:41.3883984Z ++ test 0 -ne 0\n2018-07-26T08:18:41.3884900Z ++ shift\n2018-07-26T08:18:41.3885199Z ++ test 4 -ne 0\n2018-07-26T08:18:41.3938298Z +++ git -C client rev-parse c1\n2018-07-26T08:18:41.4243782Z ++ grep 'fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe' trace\n2018-07-26T08:18:41.4375402Z packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe\n2018-07-26T08:18:41.4383803Z ++ test 0 -ne 0\n2018-07-26T08:18:41.4384733Z ++ shift\n2018-07-26T08:18:41.4385018Z ++ test 3 -ne 0\n2018-07-26T08:18:41.4436622Z +++ git -C client rev-parse old4\n2018-07-26T08:18:41.4749084Z ++ grep 'fetch> have caef059de69917b9119176a11b88afcef769331d' trace\n2018-07-26T08:18:41.4888266Z ++ test 1 -ne 0\n2018-07-26T08:18:41.4941092Z +++ git -C client rev-parse old4\n2018-07-26T08:18:41.5253206Z ++ echo 'No have caef059de69917b9119176a11b88afcef769331d (old4)'\n2018-07-26T08:18:41.5253602Z ++ return 1\n2018-07-26T08:18:41.5254746Z error: last command exited with $?=1\n2018-07-26T08:18:41.5254865Z No have caef059de69917b9119176a11b88afcef769331d (old4)\n2018-07-26T08:18:41.5260970Z not ok 4 - handle clock skew\n2018-07-26T08:18:41.5441990Z #\t\n2018-07-26T08:18:41.5442184Z #\t\trm -rf server client trace &&\n2018-07-26T08:18:41.5442422Z #\t\tgit init server &&\n2018-07-26T08:18:41.5448007Z #\t\ttest_commit -C server to_fetch &&\n2018-07-26T08:18:41.5448223Z #\t\n2018-07-26T08:18:41.5448357Z #\t\tgit init client &&\n2018-07-26T08:18:41.5448466Z #\t\n2018-07-26T08:18:41.5448661Z #\t\t# 2 regular commits\n2018-07-26T08:18:41.5448810Z #\t\ttest_tick=2000000000 &&\n2018-07-26T08:18:41.5449231Z #\t\ttest_commit -C client c1 &&\n2018-07-26T08:18:41.5449393Z #\t\ttest_commit -C client c2 &&\n2018-07-26T08:18:41.5449509Z #\t\n2018-07-26T08:18:41.5449679Z #\t\t# 4 old commits\n2018-07-26T08:18:41.5449859Z #\t\ttest_tick=1000000000 &&\n2018-07-26T08:18:41.5450017Z #\t\tgit -C client checkout c1 &&\n2018-07-26T08:18:41.5450220Z #\t\ttest_commit -C client old1 &&\n2018-07-26T08:18:41.5450343Z #\t\ttest_commit -C client old2 &&\n2018-07-26T08:18:41.5450449Z #\t\ttest_commit -C client old3 &&\n2018-07-26T08:18:41.5450667Z #\t\ttest_commit -C client old4 &&\n2018-07-26T08:18:41.5450821Z #\t\n2018-07-26T08:18:41.5450954Z #\t\t# \"c2\" and \"c1\" are popped first, then \"old4\" to \"old1\". \"old1\" would\n2018-07-26T08:18:41.5451133Z #\t\t# normally be skipped, but is treated as a commit without a parent here\n2018-07-26T08:18:41.5451392Z #\t\t# and sent, because (due to clock skew) its only parent has already been\n2018-07-26T08:18:41.5451547Z #\t\t# popped off the priority queue.\n2018-07-26T08:18:41.5451675Z #\t\ttest_config -C client fetch.negotiationalgorithm skipping &&\n2018-07-26T08:18:41.5451829Z #\t\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" &&\n2018-07-26T08:18:41.5451961Z #\t\thave_sent c2 c1 old4 old2 old1 &&\n2018-07-26T08:18:41.5452091Z #\t\thave_not_sent old3\n2018-07-26T08:18:41.5452186Z #\t\n"},{"id":"353644","messageId":"nycvar.QRO.7.76.6.1807261613420.71@tvgsbejvaqbjf.bet","threadId":"48891","inReplyTo":"nycvar.QRO.7.76.6.1807261233300.71@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-26T14:14:17Z","receivedAt":"2018-07-26T14:14:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Thu, 26 Jul 2018, Johannes Schindelin wrote:\n\n> On Mon, 16 Jul 2018, Jonathan Tan wrote:\n> \n> > t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++\n> \n> This test seems to be failing consistently in the recent `pu` builds:\n> \n> https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs\n\nIt now also causes `next` builds to fail:\n\nhttps://git-for-windows.visualstudio.com/git/_build/results?buildId=14345&view=logs\n\nPlease have a look,\nDscho\n\n> Could you have a look, please?\n> \n> Ciao,\n> Dscho\n> \n> P.S.: For your convenience, I will paste the last part of the output with\n> `-i -v -x` here:\n> \n> -- snipsnap --\n> 2018-07-26T08:18:39.7864833Z expecting success: \n> 2018-07-26T08:18:39.7868553Z \trm -rf server client trace &&\n> 2018-07-26T08:18:39.7869403Z \tgit init server &&\n> 2018-07-26T08:18:39.7869606Z \ttest_commit -C server to_fetch &&\n> 2018-07-26T08:18:39.7870066Z \n> 2018-07-26T08:18:39.7870281Z \tgit init client &&\n> 2018-07-26T08:18:39.7870403Z \n> 2018-07-26T08:18:39.7870579Z \t# 2 regular commits\n> 2018-07-26T08:18:39.7870779Z \ttest_tick=2000000000 &&\n> 2018-07-26T08:18:39.7870943Z \ttest_commit -C client c1 &&\n> 2018-07-26T08:18:39.7871103Z \ttest_commit -C client c2 &&\n> 2018-07-26T08:18:39.7871228Z \n> 2018-07-26T08:18:39.7871419Z \t# 4 old commits\n> 2018-07-26T08:18:39.7871575Z \ttest_tick=1000000000 &&\n> 2018-07-26T08:18:39.7871734Z \tgit -C client checkout c1 &&\n> 2018-07-26T08:18:39.7871916Z \ttest_commit -C client old1 &&\n> 2018-07-26T08:18:39.7872081Z \ttest_commit -C client old2 &&\n> 2018-07-26T08:18:39.7872396Z \ttest_commit -C client old3 &&\n> 2018-07-26T08:18:39.7872598Z \ttest_commit -C client old4 &&\n> 2018-07-26T08:18:39.7872743Z \n> 2018-07-26T08:18:39.7872918Z \t# \"c2\" and \"c1\" are popped first, then \"old4\" to \"old1\". \"old1\" would\n> 2018-07-26T08:18:39.7873114Z \t# normally be skipped, but is treated as a commit without a parent here\n> 2018-07-26T08:18:39.7873329Z \t# and sent, because (due to clock skew) its only parent has already been\n> 2018-07-26T08:18:39.7873524Z \t# popped off the priority queue.\n> 2018-07-26T08:18:39.7873700Z \ttest_config -C client fetch.negotiationalgorithm skipping &&\n> 2018-07-26T08:18:39.7873908Z \tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" &&\n> 2018-07-26T08:18:39.7874091Z \thave_sent c2 c1 old4 old2 old1 &&\n> 2018-07-26T08:18:39.7874262Z \thave_not_sent old3\n> 2018-07-26T08:18:39.7874383Z \n> 2018-07-26T08:18:39.8353323Z ++ rm -rf server client trace\n> 2018-07-26T08:18:40.3404166Z ++ git init server\n> 2018-07-26T08:18:40.3756394Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server/.git/\n> 2018-07-26T08:18:40.3769512Z ++ test_commit -C server to_fetch\n> 2018-07-26T08:18:40.3776271Z ++ notick=\n> 2018-07-26T08:18:40.3777103Z ++ signoff=\n> 2018-07-26T08:18:40.3777282Z ++ indir=\n> 2018-07-26T08:18:40.3777465Z ++ test 3 '!=' 0\n> 2018-07-26T08:18:40.3777648Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.3777801Z ++ indir=server\n> 2018-07-26T08:18:40.3777948Z ++ shift\n> 2018-07-26T08:18:40.3778093Z ++ shift\n> 2018-07-26T08:18:40.3778493Z ++ test 1 '!=' 0\n> 2018-07-26T08:18:40.3778921Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.3779072Z ++ break\n> 2018-07-26T08:18:40.3779241Z ++ indir=server/\n> 2018-07-26T08:18:40.3779431Z ++ file=to_fetch.t\n> 2018-07-26T08:18:40.3779603Z ++ echo to_fetch\n> 2018-07-26T08:18:40.3779923Z ++ git -C server/ add to_fetch.t\n> 2018-07-26T08:18:40.4072248Z ++ test -z ''\n> 2018-07-26T08:18:40.4072727Z ++ test_tick\n> 2018-07-26T08:18:40.4072948Z ++ test -z set\n> 2018-07-26T08:18:40.4073113Z ++ test_tick=1112913673\n> 2018-07-26T08:18:40.4073758Z ++ GIT_COMMITTER_DATE='1112913673 -0700'\n> 2018-07-26T08:18:40.4074001Z ++ GIT_AUTHOR_DATE='1112913673 -0700'\n> 2018-07-26T08:18:40.4074178Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> 2018-07-26T08:18:40.4074357Z ++ git -C server/ commit -m to_fetch\n> 2018-07-26T08:18:40.4485364Z [master (root-commit) ff85695] to_fetch\n> 2018-07-26T08:18:40.4485997Z Author: A U Thor \n> 2018-07-26T08:18:40.4486201Z 1 file changed, 1 insertion(+)\n> 2018-07-26T08:18:40.4486414Z create mode 100644 to_fetch.t\n> 2018-07-26T08:18:40.4499970Z ++ git -C server/ tag to_fetch\n> 2018-07-26T08:18:40.4809208Z ++ git init client\n> 2018-07-26T08:18:40.5139949Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/client/.git/\n> 2018-07-26T08:18:40.5158270Z ++ test_tick=2000000000\n> 2018-07-26T08:18:40.5158466Z ++ test_commit -C client c1\n> 2018-07-26T08:18:40.5159077Z ++ notick=\n> 2018-07-26T08:18:40.5159492Z ++ signoff=\n> 2018-07-26T08:18:40.5159697Z ++ indir=\n> 2018-07-26T08:18:40.5159855Z ++ test 3 '!=' 0\n> 2018-07-26T08:18:40.5160010Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.5160209Z ++ indir=client\n> 2018-07-26T08:18:40.5160362Z ++ shift\n> 2018-07-26T08:18:40.5160507Z ++ shift\n> 2018-07-26T08:18:40.5160657Z ++ test 1 '!=' 0\n> 2018-07-26T08:18:40.5160831Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.5161289Z ++ break\n> 2018-07-26T08:18:40.5161582Z ++ indir=client/\n> 2018-07-26T08:18:40.5161764Z ++ file=c1.t\n> 2018-07-26T08:18:40.5161916Z ++ echo c1\n> 2018-07-26T08:18:40.5162231Z ++ git -C client/ add c1.t\n> 2018-07-26T08:18:40.5456318Z ++ test -z ''\n> 2018-07-26T08:18:40.5460548Z ++ test_tick\n> 2018-07-26T08:18:40.5461417Z ++ test -z set\n> 2018-07-26T08:18:40.5463657Z ++ test_tick=2000000060\n> 2018-07-26T08:18:40.5464369Z ++ GIT_COMMITTER_DATE='2000000060 -0700'\n> 2018-07-26T08:18:40.5464617Z ++ GIT_AUTHOR_DATE='2000000060 -0700'\n> 2018-07-26T08:18:40.5464805Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> 2018-07-26T08:18:40.5464988Z ++ git -C client/ commit -m c1\n> 2018-07-26T08:18:40.5857440Z [master (root-commit) dc824fa] c1\n> 2018-07-26T08:18:40.5858031Z Author: A U Thor \n> 2018-07-26T08:18:40.5858251Z 1 file changed, 1 insertion(+)\n> 2018-07-26T08:18:40.5858451Z create mode 100644 c1.t\n> 2018-07-26T08:18:40.5872839Z ++ git -C client/ tag c1\n> 2018-07-26T08:18:40.6174770Z ++ test_commit -C client c2\n> 2018-07-26T08:18:40.6175120Z ++ notick=\n> 2018-07-26T08:18:40.6175398Z ++ signoff=\n> 2018-07-26T08:18:40.6175583Z ++ indir=\n> 2018-07-26T08:18:40.6175737Z ++ test 3 '!=' 0\n> 2018-07-26T08:18:40.6175925Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.6176079Z ++ indir=client\n> 2018-07-26T08:18:40.6176246Z ++ shift\n> 2018-07-26T08:18:40.6176415Z ++ shift\n> 2018-07-26T08:18:40.6176569Z ++ test 1 '!=' 0\n> 2018-07-26T08:18:40.6176738Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.6176905Z ++ break\n> 2018-07-26T08:18:40.6177052Z ++ indir=client/\n> 2018-07-26T08:18:40.6177200Z ++ file=c2.t\n> 2018-07-26T08:18:40.6177369Z ++ echo c2\n> 2018-07-26T08:18:40.6177525Z ++ git -C client/ add c2.t\n> 2018-07-26T08:18:40.6474943Z ++ test -z ''\n> 2018-07-26T08:18:40.6479175Z ++ test_tick\n> 2018-07-26T08:18:40.6479861Z ++ test -z set\n> 2018-07-26T08:18:40.6482344Z ++ test_tick=2000000120\n> 2018-07-26T08:18:40.6483064Z ++ GIT_COMMITTER_DATE='2000000120 -0700'\n> 2018-07-26T08:18:40.6483243Z ++ GIT_AUTHOR_DATE='2000000120 -0700'\n> 2018-07-26T08:18:40.6483412Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> 2018-07-26T08:18:40.6483597Z ++ git -C client/ commit -m c2\n> 2018-07-26T08:18:40.6883597Z [master 9ab4692] c2\n> 2018-07-26T08:18:40.6884552Z Author: A U Thor \n> 2018-07-26T08:18:40.6884902Z 1 file changed, 1 insertion(+)\n> 2018-07-26T08:18:40.6885078Z create mode 100644 c2.t\n> 2018-07-26T08:18:40.6898418Z ++ git -C client/ tag c2\n> 2018-07-26T08:18:40.7214970Z ++ test_tick=1000000000\n> 2018-07-26T08:18:40.7215737Z ++ git -C client checkout c1\n> 2018-07-26T08:18:40.7537971Z Note: checking out 'c1'.\n> 2018-07-26T08:18:40.7538294Z \n> 2018-07-26T08:18:40.7538485Z You are in 'detached HEAD' state. You can look around, make experimental\n> 2018-07-26T08:18:40.7538901Z changes and commit them, and you can discard any commits you make in this\n> 2018-07-26T08:18:40.7539153Z state without impacting any branches by performing another checkout.\n> 2018-07-26T08:18:40.7539288Z \n> 2018-07-26T08:18:40.7539455Z If you want to create a new branch to retain commits you create, you may\n> 2018-07-26T08:18:40.7539646Z do so (now or later) by using -b with the checkout command again. Example:\n> 2018-07-26T08:18:40.7539799Z \n> 2018-07-26T08:18:40.7539979Z git checkout -b \n> 2018-07-26T08:18:40.7540099Z \n> 2018-07-26T08:18:40.7540264Z HEAD is now at dc824fa c1\n> 2018-07-26T08:18:40.7552832Z ++ test_commit -C client old1\n> 2018-07-26T08:18:40.7559118Z ++ notick=\n> 2018-07-26T08:18:40.7559789Z ++ signoff=\n> 2018-07-26T08:18:40.7559966Z ++ indir=\n> 2018-07-26T08:18:40.7560066Z ++ test 3 '!=' 0\n> 2018-07-26T08:18:40.7565193Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.7565286Z ++ indir=client\n> 2018-07-26T08:18:40.7565373Z ++ shift\n> 2018-07-26T08:18:40.7565456Z ++ shift\n> 2018-07-26T08:18:40.7566662Z ++ test 1 '!=' 0\n> 2018-07-26T08:18:40.7566796Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.7566879Z ++ break\n> 2018-07-26T08:18:40.7566961Z ++ indir=client/\n> 2018-07-26T08:18:40.7567066Z ++ file=old1.t\n> 2018-07-26T08:18:40.7567150Z ++ echo old1\n> 2018-07-26T08:18:40.7567238Z ++ git -C client/ add old1.t\n> 2018-07-26T08:18:40.7962371Z ++ test -z ''\n> 2018-07-26T08:18:40.7962668Z ++ test_tick\n> 2018-07-26T08:18:40.7963247Z ++ test -z set\n> 2018-07-26T08:18:40.7963453Z ++ test_tick=1000000060\n> 2018-07-26T08:18:40.7963649Z ++ GIT_COMMITTER_DATE='1000000060 -0700'\n> 2018-07-26T08:18:40.7963832Z ++ GIT_AUTHOR_DATE='1000000060 -0700'\n> 2018-07-26T08:18:40.7964000Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> 2018-07-26T08:18:40.7964164Z ++ git -C client/ commit -m old1\n> 2018-07-26T08:18:40.8356576Z [detached HEAD e9a2c09] old1\n> 2018-07-26T08:18:40.8357378Z Author: A U Thor \n> 2018-07-26T08:18:40.8357554Z 1 file changed, 1 insertion(+)\n> 2018-07-26T08:18:40.8357652Z create mode 100644 old1.t\n> 2018-07-26T08:18:40.8372002Z ++ git -C client/ tag old1\n> 2018-07-26T08:18:40.8671359Z ++ test_commit -C client old2\n> 2018-07-26T08:18:40.8676749Z ++ notick=\n> 2018-07-26T08:18:40.8677398Z ++ signoff=\n> 2018-07-26T08:18:40.8679262Z ++ indir=\n> 2018-07-26T08:18:40.8679957Z ++ test 3 '!=' 0\n> 2018-07-26T08:18:40.8680150Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.8680305Z ++ indir=client\n> 2018-07-26T08:18:40.8680447Z ++ shift\n> 2018-07-26T08:18:40.8680587Z ++ shift\n> 2018-07-26T08:18:40.8680798Z ++ test 1 '!=' 0\n> 2018-07-26T08:18:40.8680949Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.8681143Z ++ break\n> 2018-07-26T08:18:40.8681311Z ++ indir=client/\n> 2018-07-26T08:18:40.8681458Z ++ file=old2.t\n> 2018-07-26T08:18:40.8681604Z ++ echo old2\n> 2018-07-26T08:18:40.8681789Z ++ git -C client/ add old2.t\n> 2018-07-26T08:18:40.8990053Z ++ test -z ''\n> 2018-07-26T08:18:40.8990392Z ++ test_tick\n> 2018-07-26T08:18:40.8990953Z ++ test -z set\n> 2018-07-26T08:18:40.8991246Z ++ test_tick=1000000120\n> 2018-07-26T08:18:40.8991421Z ++ GIT_COMMITTER_DATE='1000000120 -0700'\n> 2018-07-26T08:18:40.8991585Z ++ GIT_AUTHOR_DATE='1000000120 -0700'\n> 2018-07-26T08:18:40.8991771Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> 2018-07-26T08:18:40.8991936Z ++ git -C client/ commit -m old2\n> 2018-07-26T08:18:40.9402744Z [detached HEAD 41bd8dc] old2\n> 2018-07-26T08:18:40.9403286Z Author: A U Thor \n> 2018-07-26T08:18:40.9403510Z 1 file changed, 1 insertion(+)\n> 2018-07-26T08:18:40.9403678Z create mode 100644 old2.t\n> 2018-07-26T08:18:40.9418122Z ++ git -C client/ tag old2\n> 2018-07-26T08:18:40.9736807Z ++ test_commit -C client old3\n> 2018-07-26T08:18:40.9737058Z ++ notick=\n> 2018-07-26T08:18:40.9737152Z ++ signoff=\n> 2018-07-26T08:18:40.9737238Z ++ indir=\n> 2018-07-26T08:18:40.9737327Z ++ test 3 '!=' 0\n> 2018-07-26T08:18:40.9737600Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.9737690Z ++ indir=client\n> 2018-07-26T08:18:40.9737776Z ++ shift\n> 2018-07-26T08:18:40.9737860Z ++ shift\n> 2018-07-26T08:18:40.9737964Z ++ test 1 '!=' 0\n> 2018-07-26T08:18:40.9738054Z ++ case \"$1\" in\n> 2018-07-26T08:18:40.9738140Z ++ break\n> 2018-07-26T08:18:40.9738248Z ++ indir=client/\n> 2018-07-26T08:18:40.9738338Z ++ file=old3.t\n> 2018-07-26T08:18:40.9738445Z ++ echo old3\n> 2018-07-26T08:18:40.9738540Z ++ git -C client/ add old3.t\n> 2018-07-26T08:18:41.0035565Z ++ test -z ''\n> 2018-07-26T08:18:41.0036056Z ++ test_tick\n> 2018-07-26T08:18:41.0036299Z ++ test -z set\n> 2018-07-26T08:18:41.0036467Z ++ test_tick=1000000180\n> 2018-07-26T08:18:41.0036638Z ++ GIT_COMMITTER_DATE='1000000180 -0700'\n> 2018-07-26T08:18:41.0037189Z ++ GIT_AUTHOR_DATE='1000000180 -0700'\n> 2018-07-26T08:18:41.0037403Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> 2018-07-26T08:18:41.0037574Z ++ git -C client/ commit -m old3\n> 2018-07-26T08:18:41.0429238Z [detached HEAD 4ff0db5] old3\n> 2018-07-26T08:18:41.0429619Z Author: A U Thor \n> 2018-07-26T08:18:41.0429799Z 1 file changed, 1 insertion(+)\n> 2018-07-26T08:18:41.0429965Z create mode 100644 old3.t\n> 2018-07-26T08:18:41.0443795Z ++ git -C client/ tag old3\n> 2018-07-26T08:18:41.0752553Z ++ test_commit -C client old4\n> 2018-07-26T08:18:41.0752824Z ++ notick=\n> 2018-07-26T08:18:41.0752936Z ++ signoff=\n> 2018-07-26T08:18:41.0753905Z ++ indir=\n> 2018-07-26T08:18:41.0754193Z ++ test 3 '!=' 0\n> 2018-07-26T08:18:41.0754374Z ++ case \"$1\" in\n> 2018-07-26T08:18:41.0754531Z ++ indir=client\n> 2018-07-26T08:18:41.0754682Z ++ shift\n> 2018-07-26T08:18:41.0754828Z ++ shift\n> 2018-07-26T08:18:41.0755007Z ++ test 1 '!=' 0\n> 2018-07-26T08:18:41.0755162Z ++ case \"$1\" in\n> 2018-07-26T08:18:41.0755340Z ++ break\n> 2018-07-26T08:18:41.0755491Z ++ indir=client/\n> 2018-07-26T08:18:41.0755664Z ++ file=old4.t\n> 2018-07-26T08:18:41.0755814Z ++ echo old4\n> 2018-07-26T08:18:41.0755971Z ++ git -C client/ add old4.t\n> 2018-07-26T08:18:41.1064316Z ++ test -z ''\n> 2018-07-26T08:18:41.1064885Z ++ test_tick\n> 2018-07-26T08:18:41.1065169Z ++ test -z set\n> 2018-07-26T08:18:41.1065432Z ++ test_tick=1000000240\n> 2018-07-26T08:18:41.1065637Z ++ GIT_COMMITTER_DATE='1000000240 -0700'\n> 2018-07-26T08:18:41.1065820Z ++ GIT_AUTHOR_DATE='1000000240 -0700'\n> 2018-07-26T08:18:41.1066008Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> 2018-07-26T08:18:41.1066212Z ++ git -C client/ commit -m old4\n> 2018-07-26T08:18:41.1488203Z [detached HEAD caef059] old4\n> 2018-07-26T08:18:41.1488670Z Author: A U Thor \n> 2018-07-26T08:18:41.1489153Z 1 file changed, 1 insertion(+)\n> 2018-07-26T08:18:41.1489370Z create mode 100644 old4.t\n> 2018-07-26T08:18:41.1502887Z ++ git -C client/ tag old4\n> 2018-07-26T08:18:41.1824546Z ++ test_config -C client fetch.negotiationalgorithm skipping\n> 2018-07-26T08:18:41.1825670Z ++ config_dir=\n> 2018-07-26T08:18:41.1826072Z ++ test -C = -C\n> 2018-07-26T08:18:41.1826299Z ++ shift\n> 2018-07-26T08:18:41.1826528Z ++ config_dir=client\n> 2018-07-26T08:18:41.1826809Z ++ shift\n> 2018-07-26T08:18:41.1827079Z ++ test_when_finished 'test_unconfig -C '\\''client'\\'' '\\''fetch.negotiationalgorithm'\\'''\n> 2018-07-26T08:18:41.1827289Z ++ test 0 = 0\n> 2018-07-26T08:18:41.1827561Z ++ test_cleanup='{ test_unconfig -C '\\''client'\\'' '\\''fetch.negotiationalgorithm'\\''\n> 2018-07-26T08:18:41.1827751Z \t\t} && (exit \"$eval_ret\"); eval_ret=$?; :'\n> 2018-07-26T08:18:41.1827930Z ++ git -C client config fetch.negotiationalgorithm skipping\n> 2018-07-26T08:18:41.2196451Z +++ pwd\n> 2018-07-26T08:18:41.2196831Z +++ builtin pwd -W\n> 2018-07-26T08:18:41.2274040Z +++ pwd\n> 2018-07-26T08:18:41.2274458Z +++ builtin pwd -W\n> 2018-07-26T08:18:41.2285081Z ++ GIT_TRACE_PACKET='D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/trace'\n> 2018-07-26T08:18:41.2285515Z ++ git -C client fetch 'D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server'\n> 2018-07-26T08:18:41.3054360Z warning: no common commits\n> 2018-07-26T08:18:41.3264762Z From D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server\n> 2018-07-26T08:18:41.3265204Z * branch HEAD -> FETCH_HEAD\n> 2018-07-26T08:18:41.3362819Z ++ have_sent c2 c1 old4 old2 old1\n> 2018-07-26T08:18:41.3370525Z ++ test 5 -ne 0\n> 2018-07-26T08:18:41.3423124Z +++ git -C client rev-parse c2\n> 2018-07-26T08:18:41.3756643Z ++ grep 'fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e' trace\n> 2018-07-26T08:18:41.3878403Z packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e\n> 2018-07-26T08:18:41.3883984Z ++ test 0 -ne 0\n> 2018-07-26T08:18:41.3884900Z ++ shift\n> 2018-07-26T08:18:41.3885199Z ++ test 4 -ne 0\n> 2018-07-26T08:18:41.3938298Z +++ git -C client rev-parse c1\n> 2018-07-26T08:18:41.4243782Z ++ grep 'fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe' trace\n> 2018-07-26T08:18:41.4375402Z packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe\n> 2018-07-26T08:18:41.4383803Z ++ test 0 -ne 0\n> 2018-07-26T08:18:41.4384733Z ++ shift\n> 2018-07-26T08:18:41.4385018Z ++ test 3 -ne 0\n> 2018-07-26T08:18:41.4436622Z +++ git -C client rev-parse old4\n> 2018-07-26T08:18:41.4749084Z ++ grep 'fetch> have caef059de69917b9119176a11b88afcef769331d' trace\n> 2018-07-26T08:18:41.4888266Z ++ test 1 -ne 0\n> 2018-07-26T08:18:41.4941092Z +++ git -C client rev-parse old4\n> 2018-07-26T08:18:41.5253206Z ++ echo 'No have caef059de69917b9119176a11b88afcef769331d (old4)'\n> 2018-07-26T08:18:41.5253602Z ++ return 1\n> 2018-07-26T08:18:41.5254746Z error: last command exited with $?=1\n> 2018-07-26T08:18:41.5254865Z No have caef059de69917b9119176a11b88afcef769331d (old4)\n> 2018-07-26T08:18:41.5260970Z not ok 4 - handle clock skew\n> 2018-07-26T08:18:41.5441990Z #\t\n> 2018-07-26T08:18:41.5442184Z #\t\trm -rf server client trace &&\n> 2018-07-26T08:18:41.5442422Z #\t\tgit init server &&\n> 2018-07-26T08:18:41.5448007Z #\t\ttest_commit -C server to_fetch &&\n> 2018-07-26T08:18:41.5448223Z #\t\n> 2018-07-26T08:18:41.5448357Z #\t\tgit init client &&\n> 2018-07-26T08:18:41.5448466Z #\t\n> 2018-07-26T08:18:41.5448661Z #\t\t# 2 regular commits\n> 2018-07-26T08:18:41.5448810Z #\t\ttest_tick=2000000000 &&\n> 2018-07-26T08:18:41.5449231Z #\t\ttest_commit -C client c1 &&\n> 2018-07-26T08:18:41.5449393Z #\t\ttest_commit -C client c2 &&\n> 2018-07-26T08:18:41.5449509Z #\t\n> 2018-07-26T08:18:41.5449679Z #\t\t# 4 old commits\n> 2018-07-26T08:18:41.5449859Z #\t\ttest_tick=1000000000 &&\n> 2018-07-26T08:18:41.5450017Z #\t\tgit -C client checkout c1 &&\n> 2018-07-26T08:18:41.5450220Z #\t\ttest_commit -C client old1 &&\n> 2018-07-26T08:18:41.5450343Z #\t\ttest_commit -C client old2 &&\n> 2018-07-26T08:18:41.5450449Z #\t\ttest_commit -C client old3 &&\n> 2018-07-26T08:18:41.5450667Z #\t\ttest_commit -C client old4 &&\n> 2018-07-26T08:18:41.5450821Z #\t\n> 2018-07-26T08:18:41.5450954Z #\t\t# \"c2\" and \"c1\" are popped first, then \"old4\" to \"old1\". \"old1\" would\n> 2018-07-26T08:18:41.5451133Z #\t\t# normally be skipped, but is treated as a commit without a parent here\n> 2018-07-26T08:18:41.5451392Z #\t\t# and sent, because (due to clock skew) its only parent has already been\n> 2018-07-26T08:18:41.5451547Z #\t\t# popped off the priority queue.\n> 2018-07-26T08:18:41.5451675Z #\t\ttest_config -C client fetch.negotiationalgorithm skipping &&\n> 2018-07-26T08:18:41.5451829Z #\t\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client fetch \"$(pwd)/server\" &&\n> 2018-07-26T08:18:41.5451961Z #\t\thave_sent c2 c1 old4 old2 old1 &&\n> 2018-07-26T08:18:41.5452091Z #\t\thave_not_sent old3\n> 2018-07-26T08:18:41.5452186Z #\t\n> \n"},{"id":"353672","messageId":"20180726191609.176343-1-jonathantanmy@google.com","threadId":"48891","inReplyTo":"nycvar.QRO.7.76.6.1807261233300.71@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-07-26T19:16:09Z","receivedAt":"2018-07-26T19:16:17Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> Hi Jonathan,\n> \n> On Mon, 16 Jul 2018, Jonathan Tan wrote:\n> \n> > t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++\n> \n> This test seems to be failing consistently in the recent `pu` builds:\n> \n> https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs\n> \n> Could you have a look, please?\n\nHmm...on my Linux computer, this test passes on both pu (as of the time\nof writing) and 838143aa5c (\"Merge branch 'ab/newhash-is-sha256' into\npu\", 2018-07-25) (pu at the time of that build, according to the website\nyou linked above). If you could rerun that test with additional code,\ncould you add a \"cat trace\" and show me what the client sends? When I do\nthat, the relevant parts are:\n\n packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e\n packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe\n packet: fetch> have caef059de69917b9119176a11b88afcef769331d\n packet: fetch> have 41bd8dc092ee110ba80e350a346ec507ab2e42a0\n packet: fetch> have e9a2c092a8e911567a377c881a7f6031e7f892ea\n packet: fetch> done\n\nwhich is exactly as I (and the test) expect.\n\nTwo possible reasons for the discrepancy that I can think of offhand are\nthat (1) my computer generates different commits from your test system,\nand (2) the priority queue pops commits in a different order. For (1),\nthat's not possible because the SHA-1s are the same (as can be seen by\ncomparing your link and the \"have\" lines I quoted above), and for (2),\nthe code seems OK:\n\n static int compare(const void *a_, const void *b_, void *unused)\n {\n \tconst struct entry *a = a_;\n \tconst struct entry *b = b_;\n \treturn compare_commits_by_commit_date(a->commit, b->commit, NULL);\n }\n\nLet me know if you can observe the output of \"cat trace\" or if you have\nany other ideas.\n"},{"id":"353721","messageId":"nycvar.QRO.7.76.6.1807271737470.10478@tvgsbejvaqbjf.bet","threadId":"48891","inReplyTo":"20180726191609.176343-1-jonathantanmy@google.com","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-27T15:48:57Z","receivedAt":"2018-07-27T15:49:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Thu, 26 Jul 2018, Jonathan Tan wrote:\n\n> > On Mon, 16 Jul 2018, Jonathan Tan wrote:\n> > \n> > > t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++\n> > \n> > This test seems to be failing consistently in the recent `pu` builds:\n> > \n> > https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs\n> > \n> > Could you have a look, please?\n> \n> Hmm...on my Linux computer, this test passes on both pu (as of the time\n> of writing) and 838143aa5c (\"Merge branch 'ab/newhash-is-sha256' into\n> pu\", 2018-07-25) (pu at the time of that build, according to the website\n> you linked above). If you could rerun that test with additional code,\n> could you add a \"cat trace\" and show me what the client sends?\n\nI can give you something even better: a playground. Just open a PR at\nhttps://github.com/gitgitgadget/git (all of the branches on gitster/git ar\nmirrored, including yours, I am sure, so you can target that branch\nspecifically).\n\nOnce you open a Pull Request, it will automatically build and run the test\nsuite on Windows, macOS and Linux. You will see it in the \"checks\" section\non the bottom. Example for my range-diff series:\n\nhttps://git-for-windows.visualstudio.com/git/_build/results?buildId=14279\n\nFor a quicker turnaround, you could add a commit that forces the `all`\ntarget in `t/Makefile` to run only your test.\n\n> When I do that, the relevant parts are:\n> \n> packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e\n> packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe\n> packet: fetch> have caef059de69917b9119176a11b88afcef769331d\n> packet: fetch> have 41bd8dc092ee110ba80e350a346ec507ab2e42a0\n> packet: fetch> have e9a2c092a8e911567a377c881a7f6031e7f892ea\n> packet: fetch> done\n> \n> which is exactly as I (and the test) expect.\n> \n> Two possible reasons for the discrepancy that I can think of offhand are\n> that (1) my computer generates different commits from your test system,\n> and (2) the priority queue pops commits in a different order. For (1),\n> that's not possible because the SHA-1s are the same (as can be seen by\n> comparing your link and the \"have\" lines I quoted above), and for (2),\n> the code seems OK:\n> \n> static int compare(const void *a_, const void *b_, void *unused)\n> {\n> \tconst struct entry *a = a_;\n> \tconst struct entry *b = b_;\n> \treturn compare_commits_by_commit_date(a->commit, b->commit, NULL);\n> }\n> \n> Let me know if you can observe the output of \"cat trace\" or if you have\n> any other ideas.\n\nLike I said, you can use those \"CI\" builds, I think that would be more\neffective than if you waited for me to react, I am quite overwhelmed these\ndays.\n\nCiao,\nDscho\n"},{"id":"354039","messageId":"87r2jjtpmb.fsf@evledraar.gmail.com","threadId":"48891","inReplyTo":"20180716184401.168576-1-jonathantanmy@google.com","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-07-31T15:02:36Z","receivedAt":"2018-07-31T15:02:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jul 16 2018, Jonathan Tan wrote:\n\nDidn't catch this until this was in next, sorry.\n\nRe-arranged the diff a bit:\n\n> -void fetch_negotiator_init(struct fetch_negotiator *negotiator)\n> +void fetch_negotiator_init(struct fetch_negotiator *negotiator,\n> +\t\t\t const char *algorithm)\n> {\n> +\tif (algorithm && !strcmp(algorithm, \"skipping\")) {\n> +\t\tskipping_negotiator_init(negotiator);\n> +\t\treturn;\n> +\t}\n> \tdefault_negotiator_init(negotiator);\n> }\n\nOkey, I understand that's how it works now, but....\n\n> +fetch.negotiationAlgorithm::\n> +\tControl how information about the commits in the local repository is\n> +\tsent when negotiating the contents of the packfile to be sent by the\n> +\tserver. Set to \"skipping\" to use an algorithm that skips commits in an\n> +\teffort to converge faster, but may result in a larger-than-necessary\n> +\tpackfile; any other value instructs Git to use the default algorithm\n> +\tthat never skips commits (unless the server has acknowledged it or one\n> +\tof its descendants).\n> +\n\n...let's instead document that there's just the values \"skipping\" and\n\"default\", and say \"default\" is provided by default, and perhaps change\nthe code to warn about anything that isn't those two.\n\nThen we're not painting ourselves into a corner by needing to break a\npromise in the docs (\"any other value instructs Git to use the default\")\nif we add a new one of these, and aren't silently falling back on the\ndefault if we add new-fancy-algo the user's version doesn't know about.\n\nAlso, switching gears entirely, I'm very excited about this whole thing\nbecause it allows me to address something I've been meaning to get to\nfor a while.\n\nAt work I sometimes want to see what commits I've made to all our git\nrepos, for remembering what I was doing last February or whatever (this\nis for filling in quarterly reports).\n\nSo I have this script that basically does this:\n\n for repo in $(get-list-of-all-the-things)\n do\n git config \"remote.$repo.url\" git@git-server.example.com:$repo.git\n git config \"remote.$repo.fetch\" \"+HEAD:$repo/HEAD\"\n git config \"remote.$repo.tagOpt\" \"--no-tags\"\n done &&\n git fetch --all\n\nI.e. for every repo like git/git I'll fetch its upstream HEAD as the\nbranch git/git/HEAD. Then I can do stuff like:\n\n git shortlog --author=Ævar --since=2018-02-01 --until=2018-03-01\n\nNow, running that \"git fetch --all\" takes ages, and I know why. It's\nbecause the in the negotiation for \"git fetch some/small-repo\" I'm\nemitting hundreds of thousands of \"have\" lines for SHA1s found in other\nunrelated repos, only to get a NAK for all of them.\n\nOne way to fix that with this facility would be to have some way to pass\nin arguments, similar to what we have for merge drivers, so I can say\n\"just emit 'have' lines for stuff found in this branch\". The most\npathological cases are when I'm fetching a remote that has one commit,\nand I'm desperately trying to find something in common by asking if the\nremote has hundreds of K of commits it has no chance of having.\n\nOr there may be some smarter way to do this, what do you think?\n"},{"id":"354096","messageId":"20180731180248.42787-1-jonathantanmy@google.com","threadId":"48891","inReplyTo":"87r2jjtpmb.fsf@evledraar.gmail.com","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-07-31T18:02:48Z","receivedAt":"2018-07-31T18:02:55Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> > +fetch.negotiationAlgorithm::\n> > +\tControl how information about the commits in the local repository is\n> > +\tsent when negotiating the contents of the packfile to be sent by the\n> > +\tserver. Set to \"skipping\" to use an algorithm that skips commits in an\n> > +\teffort to converge faster, but may result in a larger-than-necessary\n> > +\tpackfile; any other value instructs Git to use the default algorithm\n> > +\tthat never skips commits (unless the server has acknowledged it or one\n> > +\tof its descendants).\n> > +\n> \n> ...let's instead document that there's just the values \"skipping\" and\n> \"default\", and say \"default\" is provided by default, and perhaps change\n> the code to warn about anything that isn't those two.\n> \n> Then we're not painting ourselves into a corner by needing to break a\n> promise in the docs (\"any other value instructs Git to use the default\")\n> if we add a new one of these, and aren't silently falling back on the\n> default if we add new-fancy-algo the user's version doesn't know about.\n\nMy intention was to allow future versions of Git to introduce more\nalgorithms, but have older versions of Git still work even if a\nrepository is configured to use a newer algorithm. But your suggestion\nis reasonable too.\n\n> Now, running that \"git fetch --all\" takes ages, and I know why. It's\n> because the in the negotiation for \"git fetch some/small-repo\" I'm\n> emitting hundreds of thousands of \"have\" lines for SHA1s found in other\n> unrelated repos, only to get a NAK for all of them.\n> \n> One way to fix that with this facility would be to have some way to pass\n> in arguments, similar to what we have for merge drivers, so I can say\n> \"just emit 'have' lines for stuff found in this branch\". The most\n> pathological cases are when I'm fetching a remote that has one commit,\n> and I'm desperately trying to find something in common by asking if the\n> remote has hundreds of K of commits it has no chance of having.\n> \n> Or there may be some smarter way to do this, what do you think?\n\nWell, there is already a commit in \"next\" that does this :-)\n\n3390e42adb (\"fetch-pack: support negotiation tip whitelist\", 2018-07-03)\n"},{"id":"354158","messageId":"20180801151835.9182-1-avarab@gmail.com","threadId":"48891","inReplyTo":"20180731180248.42787-1-jonathantanmy@google.com","subject":"[PATCH 0/2] negotiator: improve recent behavior + docs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-01T15:18:33Z","receivedAt":"2018-08-01T15:18:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 31 2018, Jonathan Tan wrote:\n\n>> > +fetch.negotiationAlgorithm::\n>> > + Control how information about the commits in the local repository is\n>> > + sent when negotiating the contents of the packfile to be sent by the\n>> > + server. Set to \"skipping\" to use an algorithm that skips commits in an\n>> > + effort to converge faster, but may result in a larger-than-necessary\n>> > + packfile; any other value instructs Git to use the default algorithm\n>> > + that never skips commits (unless the server has acknowledged it or one\n>> > + of its descendants).\n>> > +\n>> \n>> ...let's instead document that there's just the values \"skipping\" and\n>> \"default\", and say \"default\" is provided by default, and perhaps change\n>> the code to warn about anything that isn't those two.\n>> \n>> Then we're not painting ourselves into a corner by needing to break a\n>> promise in the docs (\"any other value instructs Git to use the default\")\n>> if we add a new one of these, and aren't silently falling back on the\n>> default if we add new-fancy-algo the user's version doesn't know about.\n>\n> My intention was to allow future versions of Git to introduce more\n> algorithms, but have older versions of Git still work even if a\n> repository is configured to use a newer algorithm. But your suggestion\n> is reasonable too.\n\nI think 01/02 in this patch series implements something that's better\n& more future-proof.\n\n>> Now, running that \"git fetch --all\" takes ages, and I know why. It's\n>> because the in the negotiation for \"git fetch some/small-repo\" I'm\n>> emitting hundreds of thousands of \"have\" lines for SHA1s found in other\n>> unrelated repos, only to get a NAK for all of them.\n>> \n>> One way to fix that with this facility would be to have some way to pass\n>> in arguments, similar to what we have for merge drivers, so I can say\n>> \"just emit 'have' lines for stuff found in this branch\". The most\n>> pathological cases are when I'm fetching a remote that has one commit,\n>> and I'm desperately trying to find something in common by asking if the\n>> remote has hundreds of K of commits it has no chance of having.\n>> \n>> Or there may be some smarter way to do this, what do you think?\n>\n> Well, there is already a commit in \"next\" that does this :-)\n>\n> 3390e42adb (\"fetch-pack: support negotiation tip whitelist\", 2018-07-03)\n\nThat's awesome. This is exactly what I wanted, this patch series also\nfixes another small issue in 02/02; which is that the docs for the two\nreally should cross-link to make these discoverable from one another.\n\nAnother thing I noticed, which I have not fixed, is that there's no\nway to say \"I don't want to you to say that anything is in\ncommon\". Currently I'm hacking around in my script by doing:\n\n parallel --eta --verbose --progress '\n if git rev-parse {}/HEAD 2>/dev/null\n then\n git fetch --negotiation-tip={}/HEAD {}\n else\n git fetch --negotiation-tip=ref-i-have-with-one-commit {}\n fi\n ' ::: $(git remote)\n\nI.e. the way I'm doing this is I add all the remotes first, then I\nfetch them all in parallel, but because the first time around I don't\nhave anything for that remote (and they don't share any commits) I\nneed to fake it up and pretend to be fetching from a repo that has\njust one commit.\n\nIt would be better if I could somehow say that I don't mind that the\nref doesn't exist, but currently you either error out with this, or\nignore the glob, depending on the mode.\n\nSo I want this, but can't think of a less shitty UI than:\n\n git fetch --negotiation-tip=$REF --negotiation-tip-error-handling=missing-ref-means-no-want\n\nOr something equally atrocious, do you have any better ideas?\n\nÆvar Arnfjörð Bjarmason (2):\n negotiator: unknown fetch.negotiationAlgorithm should error out\n fetch doc: cross-link two new negotiation options\n\n Documentation/config.txt | 5 ++++-\n Documentation/fetch-options.txt | 3 +++\n fetch-negotiator.c | 12 +++++++++---\n t/t5552-skipping-fetch-negotiator.sh | 23 +++++++++++++++++++++++\n 4 files changed, 39 insertions(+), 4 deletions(-)\n\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"354159","messageId":"20180801151835.9182-2-avarab@gmail.com","threadId":"48891","inReplyTo":"20180731180248.42787-1-jonathantanmy@google.com","subject":"[PATCH 1/2] negotiator: unknown fetch.negotiationAlgorithm should error out","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-01T15:18:34Z","receivedAt":"2018-08-01T15:18:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the handling of fetch.negotiationAlgorithm= to error out\non unknown strings, i.e. everything except \"default\" or \"skipping\".\n\nThis changes the behavior added in 42cc7485a2 (\"negotiator/skipping:\nskip commits during fetch\", 2018-07-16) which would ignore all unknown\nvalues and silently fall back to the \"default\" value.\n\nFor a feature like this it's much better to produce an error than\nproceed. We don't want users to debug some amazingly slow fetch that\nshould benefit from \"skipping\", only to find that they'd forgotten to\ndeploy the new git version on that particular machine.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/config.txt | 3 ++-\n fetch-negotiator.c | 12 +++++++++---\n t/t5552-skipping-fetch-negotiator.sh | 23 +++++++++++++++++++++++\n 3 files changed, 34 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 63365dcf3d..84f73d7458 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1536,9 +1536,10 @@ fetch.negotiationAlgorithm::\n \tsent when negotiating the contents of the packfile to be sent by the\n \tserver. Set to \"skipping\" to use an algorithm that skips commits in an\n \teffort to converge faster, but may result in a larger-than-necessary\n-\tpackfile; any other value instructs Git to use the default algorithm\n+\tpackfile; The default is \"default\" which instructs Git to use the default algorithm\n \tthat never skips commits (unless the server has acknowledged it or one\n \tof its descendants).\n+\tUnknown values will cause 'git fetch' to error out.\n \n format.attach::\n \tEnable multipart/mixed attachments as the default for\ndiff --git a/fetch-negotiator.c b/fetch-negotiator.c\nindex 5d283049f4..d6d685cba0 100644\n--- a/fetch-negotiator.c\n+++ b/fetch-negotiator.c\n@@ -6,9 +6,15 @@\n void fetch_negotiator_init(struct fetch_negotiator *negotiator,\n \t\t\t const char *algorithm)\n {\n-\tif (algorithm && !strcmp(algorithm, \"skipping\")) {\n-\t\tskipping_negotiator_init(negotiator);\n-\t\treturn;\n+\tif (algorithm) {\n+\t\tif (!strcmp(algorithm, \"skipping\")) {\n+\t\t\tskipping_negotiator_init(negotiator);\n+\t\t\treturn;\n+\t\t} else if (!strcmp(algorithm, \"default\")) {\n+\t\t\t/* Fall through to default initialization */\n+\t\t} else {\n+\t\t\tdie(\"unknown fetch negotiation algorithm '%s'\", algorithm);\n+\t\t}\n \t}\n \tdefault_negotiator_init(negotiator);\n }\ndiff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh\nindex 0a8e0e42ed..3b60bd44e0 100755\n--- a/t/t5552-skipping-fetch-negotiator.sh\n+++ b/t/t5552-skipping-fetch-negotiator.sh\n@@ -47,6 +47,29 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc\n \thave_not_sent c6 c4 c3\n '\n \n+test_expect_success 'unknown fetch.negotiationAlgorithm values error out' '\n+\trm -rf server client trace &&\n+\tgit init server &&\n+\ttest_commit -C server to_fetch &&\n+\n+\tgit init client &&\n+\ttest_commit -C client on_client &&\n+\tgit -C client checkout on_client &&\n+\n+\ttest_config -C client fetch.negotiationAlgorithm invalid &&\n+\ttest_must_fail git -C client fetch \"$(pwd)/server\" 2>err &&\n+\ttest_i18ngrep \"unknown fetch negotiation algorithm\" err &&\n+\n+\t# Explicit \"default\" value\n+\ttest_config -C client fetch.negotiationAlgorithm default &&\n+\tgit -C client -c fetch.negotiationAlgorithm=default fetch \"$(pwd)/server\" &&\n+\n+\t# Implementation detail: If there is nothing to fetch, we will not error out\n+\ttest_config -C client fetch.negotiationAlgorithm invalid &&\n+\tgit -C client fetch \"$(pwd)/server\" 2>err &&\n+\ttest_i18ngrep ! \"unknown fetch negotiation algorithm\" err\n+'\n+\n test_expect_success 'when two skips collide, favor the larger one' '\n \trm -rf server client trace &&\n \tgit init server &&\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"354160","messageId":"20180801151835.9182-3-avarab@gmail.com","threadId":"48891","inReplyTo":"20180731180248.42787-1-jonathantanmy@google.com","subject":"[PATCH 2/2] fetch doc: cross-link two new negotiation options","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-01T15:18:35Z","receivedAt":"2018-08-01T15:18:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Users interested in the fetch.negotiationAlgorithm variable added in\n42cc7485a2 (\"negotiator/skipping: skip commits during fetch\",\n2018-07-16) are probably interested in the related --negotiation-tip\noption added in 3390e42adb (\"fetch-pack: support negotiation tip\nwhitelist\", 2018-07-02).\n\nChange the documentation for those two to reference one another to\npoint readers in the right direction.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/config.txt | 2 ++\n Documentation/fetch-options.txt | 3 +++\n 2 files changed, 5 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 84f73d7458..dc55ff17e0 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1540,6 +1540,8 @@ fetch.negotiationAlgorithm::\n \tthat never skips commits (unless the server has acknowledged it or one\n \tof its descendants).\n \tUnknown values will cause 'git fetch' to error out.\n++\n+See also the `--negotiation-tip` option for linkgit:git-fetch[1].\n \n format.attach::\n \tEnable multipart/mixed attachments as the default for\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex 2d09f87b4b..8bc36af4b1 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -57,6 +57,9 @@ commits reachable from any of the given commits.\n The argument to this option may be a glob on ref names, a ref, or the (possibly\n abbreviated) SHA-1 of a commit. Specifying a glob is equivalent to specifying\n this option multiple times, one for each matching ref name.\n++\n+See also the `fetch.negotiationAlgorithm` configuration variable\n+documented in linkgit:git-config[1].\n \n ifndef::git-pull[]\n --dry-run::\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"354188","messageId":"20180801202545.204151-1-jonathantanmy@google.com","threadId":"48891","inReplyTo":"20180801151835.9182-1-avarab@gmail.com","subject":"Re: [PATCH 0/2] negotiator: improve recent behavior + docs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-08-01T20:25:45Z","receivedAt":"2018-08-01T20:25:51Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> I think 01/02 in this patch series implements something that's better\n> & more future-proof.\n\nThanks. Both patches are:\nReviewed-by: Jonathan Tan \n\nA small note:\n\n> -\tpackfile; any other value instructs Git to use the default algorithm\n> +\tpackfile; The default is \"default\" which instructs Git to use the default algorithm\n\nI think we generally don't capitalize words after semicolons.\n\nThanks for noticing that the check of fetch.negotiationAlgorithm only\nhappens when a negotiation actually occurs - before your patches, it\ndidn't really matter because we tolerated anything, but now we do. I\nthink this is fine - as far as I know, Git commands generally only read\nthe configs relevant to them, and if fetch.negotiationAlgorithm is not\nrelevant in a certain situation, we don't need to read it.\n\n> That's awesome. This is exactly what I wanted, this patch series also\n> fixes another small issue in 02/02; which is that the docs for the two\n> really should cross-link to make these discoverable from one another.\n\nThat's a good idea; thanks for doing it.\n\n> I.e. the way I'm doing this is I add all the remotes first, then I\n> fetch them all in parallel, but because the first time around I don't\n> have anything for that remote (and they don't share any commits) I\n> need to fake it up and pretend to be fetching from a repo that has\n> just one commit.\n> \n> It would be better if I could somehow say that I don't mind that the\n> ref doesn't exist, but currently you either error out with this, or\n> ignore the glob, depending on the mode.\n> \n> So I want this, but can't think of a less shitty UI than:\n> \n> git fetch --negotiation-tip=$REF --negotiation-tip-error-handling=missing-ref-means-no-want\n> \n> Or something equally atrocious, do you have any better ideas?\n\nIf you wanted to do this, it seems better to me to just declare a \"null\"\nnegotiation algorithm that does not perform any negotiation at all.\n"},{"id":"354195","messageId":"87in4tu6wb.fsf@evledraar.gmail.com","threadId":"48891","inReplyTo":"20180801202545.204151-1-jonathantanmy@google.com","subject":"Re: [PATCH 0/2] negotiator: improve recent behavior + docs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-01T21:13:56Z","receivedAt":"2018-08-01T21:14:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Aug 01 2018, Jonathan Tan wrote:\n\n>> I think 01/02 in this patch series implements something that's better\n>> & more future-proof.\n>\n> Thanks. Both patches are:\n> Reviewed-by: Jonathan Tan \n>\n> A small note:\n>\n>> -\tpackfile; any other value instructs Git to use the default algorithm\n>> +\tpackfile; The default is \"default\" which instructs Git to use the default algorithm\n>\n> I think we generally don't capitalize words after semicolons.\n\nYeah I think that's right. Will fix (or if there's no other comments\nperhaps Junio will munge it...) :)\n\n> Thanks for noticing that the check of fetch.negotiationAlgorithm only\n> happens when a negotiation actually occurs - before your patches, it\n> didn't really matter because we tolerated anything, but now we do. I\n> think this is fine - as far as I know, Git commands generally only read\n> the configs relevant to them, and if fetch.negotiationAlgorithm is not\n> relevant in a certain situation, we don't need to read it.\n\nYeah I think that's OK.\n\n>> That's awesome. This is exactly what I wanted, this patch series also\n>> fixes another small issue in 02/02; which is that the docs for the two\n>> really should cross-link to make these discoverable from one another.\n>\n> That's a good idea; thanks for doing it.\n>\n>> I.e. the way I'm doing this is I add all the remotes first, then I\n>> fetch them all in parallel, but because the first time around I don't\n>> have anything for that remote (and they don't share any commits) I\n>> need to fake it up and pretend to be fetching from a repo that has\n>> just one commit.\n>>\n>> It would be better if I could somehow say that I don't mind that the\n>> ref doesn't exist, but currently you either error out with this, or\n>> ignore the glob, depending on the mode.\n>>\n>> So I want this, but can't think of a less shitty UI than:\n>>\n>> git fetch --negotiation-tip=$REF --negotiation-tip-error-handling=missing-ref-means-no-want\n>>\n>> Or something equally atrocious, do you have any better ideas?\n>\n> If you wanted to do this, it seems better to me to just declare a \"null\"\n> negotiation algorithm that does not perform any negotiation at all.\n\nI think such an algorithm is a good idea in general, especially for\ntesting, and yeah, maybe that's the best way out of this, i.e. to do:\n\n if git rev-parse {}/HEAD 2>/dev/null\n then\n git fetch --negotiation-tip={}/HEAD {}\n else\n git -c fetch.negotiationAlgorithm=null fetch {}\n fi\n\nWould such an algorithm be added by overriding default.c's add_tip\nfunction to never add anything by calling default_negotiator_init()\nfollowed by null_negotiator_init(), which would only override add_tip?\n(yay C OO)\n\nIf so from fetch-pack.c it looks like there may be the limitation on the\ninterface that the negotiator can't exit early (in\nfetch-pack.c:mark_tips). But I've just skimmed this, so maybe I've\nmissed something.\n\nAlso, looks like because of the current interface =null and\n--negotiation-tip=* would (somewhat confusingly) do a \"real\" negotiation\nif done that way, since it'll bypass the API and insert tips for it to\nnegotiate, but it looks like overriding next() will get around that.\n"},{"id":"354382","messageId":"nycvar.QRO.7.76.6.1808031506461.71@tvgsbejvaqbjf.bet","threadId":"48891","inReplyTo":"nycvar.QRO.7.76.6.1807271737470.10478@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] negotiator/skipping: skip commits during fetch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-03T13:07:25Z","receivedAt":"2018-08-03T13:07:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jonathan,\n\nOn Fri, 27 Jul 2018, Johannes Schindelin wrote:\n\n> On Thu, 26 Jul 2018, Jonathan Tan wrote:\n> \n> > > On Mon, 16 Jul 2018, Jonathan Tan wrote:\n> > > \n> > > > t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++\n> > > \n> > > This test seems to be failing consistently in the recent `pu` builds:\n> > > \n> > > https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs\n> > > \n> > > Could you have a look, please?\n> > \n> > Hmm...on my Linux computer, this test passes on both pu (as of the time\n> > of writing) and 838143aa5c (\"Merge branch 'ab/newhash-is-sha256' into\n> > pu\", 2018-07-25) (pu at the time of that build, according to the website\n> > you linked above). If you could rerun that test with additional code,\n> > could you add a \"cat trace\" and show me what the client sends?\n> \n> I can give you something even better: a playground. Just open a PR at\n> https://github.com/gitgitgadget/git (all of the branches on gitster/git ar\n> mirrored, including yours, I am sure, so you can target that branch\n> specifically).\n> \n> Once you open a Pull Request, it will automatically build and run the test\n> suite on Windows, macOS and Linux. You will see it in the \"checks\" section\n> on the bottom. Example for my range-diff series:\n> \n> https://git-for-windows.visualstudio.com/git/_build/results?buildId=14279\n> \n> For a quicker turnaround, you could add a commit that forces the `all`\n> target in `t/Makefile` to run only your test.\n> \n> > When I do that, the relevant parts are:\n> > \n> > packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e\n> > packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe\n> > packet: fetch> have caef059de69917b9119176a11b88afcef769331d\n> > packet: fetch> have 41bd8dc092ee110ba80e350a346ec507ab2e42a0\n> > packet: fetch> have e9a2c092a8e911567a377c881a7f6031e7f892ea\n> > packet: fetch> done\n> > \n> > which is exactly as I (and the test) expect.\n> > \n> > Two possible reasons for the discrepancy that I can think of offhand are\n> > that (1) my computer generates different commits from your test system,\n> > and (2) the priority queue pops commits in a different order. For (1),\n> > that's not possible because the SHA-1s are the same (as can be seen by\n> > comparing your link and the \"have\" lines I quoted above), and for (2),\n> > the code seems OK:\n> > \n> > static int compare(const void *a_, const void *b_, void *unused)\n> > {\n> > \tconst struct entry *a = a_;\n> > \tconst struct entry *b = b_;\n> > \treturn compare_commits_by_commit_date(a->commit, b->commit, NULL);\n> > }\n> > \n> > Let me know if you can observe the output of \"cat trace\" or if you have\n> > any other ideas.\n> \n> Like I said, you can use those \"CI\" builds, I think that would be more\n> effective than if you waited for me to react, I am quite overwhelmed these\n> days.\n\nHopefully you have a chance to do so. I got the impression that it is\nactually more of a flakey test than a consistent test failure:\n\nhttps://git-for-windows.visualstudio.com/git/_build/results?buildId=15015&view=logs\n\nCiao,\nDscho\n"},{"id":"359107","messageId":"20180927194125.8380-1-jonathantanmy@google.com","threadId":"48891","inReplyTo":"87in4tu6wb.fsf@evledraar.gmail.com","subject":"Re: [PATCH 0/2] negotiator: improve recent behavior + docs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-09-27T19:41:25Z","receivedAt":"2018-09-27T19:41:38Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> > If you wanted to do this, it seems better to me to just declare a \"null\"\n> > negotiation algorithm that does not perform any negotiation at all.\n> \n> I think such an algorithm is a good idea in general, especially for\n> testing, and yeah, maybe that's the best way out of this, i.e. to do:\n> \n> if git rev-parse {}/HEAD 2>/dev/null\n> then\n> git fetch --negotiation-tip={}/HEAD {}\n> else\n> git -c fetch.negotiationAlgorithm=null fetch {}\n> fi\n> \n> Would such an algorithm be added by overriding default.c's add_tip\n> function to never add anything by calling default_negotiator_init()\n> followed by null_negotiator_init(), which would only override add_tip?\n> (yay C OO)\n> \n> If so from fetch-pack.c it looks like there may be the limitation on the\n> interface that the negotiator can't exit early (in\n> fetch-pack.c:mark_tips). But I've just skimmed this, so maybe I've\n> missed something.\n\n(I was reminded to reply to this offlist - sorry for the late reply.)\n\nI think too many things need to be replaced (known_common, add_tip, and\nack all need to do nothing), so it's best to start from scratch. That\nway, we also don't need to deal with the subtleties of C OO :-)\n\n> Also, looks like because of the current interface =null and\n> --negotiation-tip=* would (somewhat confusingly) do a \"real\" negotiation\n> if done that way, since it'll bypass the API and insert tips for it to\n> negotiate, but it looks like overriding next() will get around that.\n\nIf you do it as I suggest (in particular, add_tip doing nothing) then\nthere is the opposite problem that it won't be easy to inform the user\nthat --negotiation-tip does nothing in this case. Maybe there needs to\nbe an \"accepts_tips\" field in struct fetch_negotiator that, if false,\nmeans that custom tips (or any tips) are not accepted, allowing the\ncaller of the negotiator to print a warning message in this case.\n"},{"id":"359117","messageId":"87o9ciisg6.fsf@evledraar.gmail.com","threadId":"48891","inReplyTo":"20180927194125.8380-1-jonathantanmy@google.com","subject":"Re: [PATCH 0/2] negotiator: improve recent behavior + docs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-27T20:41:29Z","receivedAt":"2018-09-27T20:41:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Sep 27 2018, Jonathan Tan wrote:\n\n>> > If you wanted to do this, it seems better to me to just declare a \"null\"\n>> > negotiation algorithm that does not perform any negotiation at all.\n>>\n>> I think such an algorithm is a good idea in general, especially for\n>> testing, and yeah, maybe that's the best way out of this, i.e. to do:\n>>\n>> if git rev-parse {}/HEAD 2>/dev/null\n>> then\n>> git fetch --negotiation-tip={}/HEAD {}\n>> else\n>> git -c fetch.negotiationAlgorithm=null fetch {}\n>> fi\n>>\n>> Would such an algorithm be added by overriding default.c's add_tip\n>> function to never add anything by calling default_negotiator_init()\n>> followed by null_negotiator_init(), which would only override add_tip?\n>> (yay C OO)\n>>\n>> If so from fetch-pack.c it looks like there may be the limitation on the\n>> interface that the negotiator can't exit early (in\n>> fetch-pack.c:mark_tips). But I've just skimmed this, so maybe I've\n>> missed something.\n>\n> (I was reminded to reply to this offlist - sorry for the late reply.)\n>\n> I think too many things need to be replaced (known_common, add_tip, and\n> ack all need to do nothing), so it's best to start from scratch. That\n> way, we also don't need to deal with the subtleties of C OO :-)\n>\n>> Also, looks like because of the current interface =null and\n>> --negotiation-tip=* would (somewhat confusingly) do a \"real\" negotiation\n>> if done that way, since it'll bypass the API and insert tips for it to\n>> negotiate, but it looks like overriding next() will get around that.\n>\n> If you do it as I suggest (in particular, add_tip doing nothing) then\n> there is the opposite problem that it won't be easy to inform the user\n> that --negotiation-tip does nothing in this case. Maybe there needs to\n> be an \"accepts_tips\" field in struct fetch_negotiator that, if false,\n> means that custom tips (or any tips) are not accepted, allowing the\n> caller of the negotiator to print a warning message in this case.\n\nThanks, yeah it seems the interface would need to be tweaked for such a\n\"null\" negotiator.\n\nSome more general questions (which I can turn into docs once I\nunderstand this). If I run this, as a testcase for two random repos\nwhere I \"fetch\" an unrelated one and use the first ever commit to\ngit.git as an alias for this \"null\" negotiatior, i.e. \"just present this\none commit\":\n\n (\n rm -rf /tmp/git &&\n git clone https://github.com/git/git.git /tmp/git &&\n cd /tmp/git &&\n git remote add gitlab-shell https://github.com/cr-marcstevens/sha1collisiondetection &&\n GIT_TRACE_PACKET=/tmp/git/packet.trace git fetch --negotiation-tip=$(git log --reverse|head -n 1|cut -d ' ' -f2) gitlab-shell &&\n grep -c \"fetch-pack> have\" /tmp/git/packet.trace\n )\n\nI get:\n\n warning: Ignoring --negotiation-tip because the protocol does not support it.\n\nAnd the grep -c shows we tried to present 55170 commits in \"have\" lines\nto the server. Now, change that to SSH and all is well:\n\n (\n rm -rf /tmp/git &&\n git clone git@github.com:git/git.git /tmp/git &&\n cd /tmp/git &&\n git remote add gitlab-shell git@github.com:cr-marcstevens/sha1collisiondetection &&\n GIT_TRACE_PACKET=/tmp/git/packet.trace git fetch --negotiation-tip=$(git log --reverse|head -n 1|cut -d ' ' -f2) gitlab-shell &&\n grep -c \"fetch-pack> have\" /tmp/git/packet.trace\n )\n\nI don't understand this limitation. With the SSH version we skip\nstraight to saying we \"want\" with just the 1 \"have\" line of\n\"e83c5163316f89bfbde7d9ab23ca2e25604af290\".\n\nWhy aren't we doing the same over http? I don't get how protocol support\nis needed, it's us who decide to send over the \"have\" lines. Some\nvariant of this does work over \"skipping\":\n\n (\n rm -rf /tmp/git &&\n git clone https://github.com/git/git.git /tmp/git &&\n cd /tmp/git &&\n git remote add gitlab-shell https://github.com/cr-marcstevens/sha1collisiondetection &&\n GIT_TRACE_PACKET=/tmp/git/packet.trace git -c fetch.negotiationAlgorithm=skipping fetch gitlab-shell &&\n grep -c \"fetch-pack> have\" /tmp/git/packet.trace\n )\n\nThere we send 14002 \"have\" lines, which seems expected, but then with\nthe same thing over SSH we don't send any:\n\n (\n rm -rf /tmp/git &&\n git clone git@github.com:git/git.git /tmp/git &&\n cd /tmp/git &&\n git remote add gitlab-shell git@github.com:cr-marcstevens/sha1collisiondetection &&\n GIT_TRACE_PACKET=/tmp/git/packet.trace git -c fetch.negotiationAlgorithm=skipping fetch gitlab-shell &&\n grep -c \"fetch-pack> have\" /tmp/git/packet.trace\n )\n\nSo that seems like another bug, and as an aside, a \"skipping\"\nimplementation that sends ~1/4 of the commits in the repo seems way less\naggressive than it should be. I was expecting something that would\ngradually \"ramp up\" from the tips. Where say starting at master/next/pu\nwe present every 100th commit as a \"have\" until the 1000th commit, then\nevery 1000 commits until 10k and quickly after that step up the size\nrapidly.\n"},{"id":"359158","messageId":"20180927224647.111690-1-jonathantanmy@google.com","threadId":"48891","inReplyTo":"87o9ciisg6.fsf@evledraar.gmail.com","subject":"Re: [PATCH 0/2] negotiator: improve recent behavior + docs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-09-27T22:46:47Z","receivedAt":"2018-09-27T22:47:04Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> I get:\n> \n> warning: Ignoring --negotiation-tip because the protocol does not support it.\n\nWhen I implemented --negotiation-tip, I only implemented it for\nprotocols that support connect or stateless-connect, because\nimplementing it fully would have required expanding the protocol helper\nfunctionality. For reference, the commit is 3390e42adb (\"fetch-pack:\nsupport negotiation tip whitelist\", 2018-07-03).\n\nSo HTTPS wouldn't work unless you were using protocol v2.\n\n> So that seems like another bug, and as an aside, a \"skipping\"\n> implementation that sends ~1/4 of the commits in the repo seems way less\n> aggressive than it should be. I was expecting something that would\n> gradually \"ramp up\" from the tips. Where say starting at master/next/pu\n> we present every 100th commit as a \"have\" until the 1000th commit, then\n> every 1000 commits until 10k and quickly after that step up the size\n> rapidly.\n\nI reproduced using your commands, and yes, there is a bug - I'll take a\nlook.\n"}]}