The skipping negotiator pushes entries onto the priority queue without ensuring that the commit is parsed, resulting in the entry ending up in the wrong position due to a lack of commit time. Fix this by parsing the commit whenever we push an entry onto the priority queue.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- This was noticed by Aevar in [1]. With this fix, 163 "have" lines are produced instead of the 14002 reported in [1].
I have included a test to demonstrate the issue, but I'm not sure if it's worth including it in the source tree.
[1] https://public-inbox.org/git/87o9ciisg6.fsf@evledraar.gmail.com/ --- negotiator/skipping.c | 2 +- t/t5552-skipping-fetch-negotiator.sh | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-)
Show changes to 2 files +23 −1
negotiator/skipping.c, t/t5552-skipping-fetch-negotiator.sh
diff --git a/negotiator/skipping.c b/negotiator/skipping.c index dffbc76c49..9ce0555840 100644 --- a/negotiator/skipping.c +++ b/negotiator/skipping.c @@ -64,6 +64,7 @@ static struct entry *rev_list_push(struct data *data, struct commit *commit, int entry = xcalloc(1, sizeof(*entry)); entry->commit = commit; + parse_commit(commit); prio_queue_put(&data->rev_list, entry); if (!(mark & COMMON)) @@ -177,7 +178,6 @@ static const struct object_id *get_rev(struct data *data) if (!(commit->object.flags & COMMON) && !entry->ttl) to_send = commit; - parse_commit(commit); for (p = commit->parents; p; p = p->next) parent_pushed |= push_parent(data, entry, p->item); diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh index 30857b84a8..f2f938e54e 100755 --- a/t/t5552-skipping-fetch-negotiator.sh +++ b/t/t5552-skipping-fetch-negotiator.sh @@ -83,6 +83,28 @@ test_expect_success 'unknown fetch.negotiationAlgorithm values error out' ' test_i18ngrep ! "unknown fetch negotiation algorithm" err ' +test_expect_success 'works in absence of tags' ' + rm -rf server client trace && + git init server && + test_commit -C server to_fetch && + + git init client && + for i in $(test_seq 11) + do + test_commit -C client c$i + done && + git -C client tag middle c6 && + for i in $(test_seq 11) + do + git -C client tag -d c$i + done && + + test_config -C client fetch.negotiationalgorithm skipping && + trace_fetch client "$(pwd)/server" && + have_sent HEAD HEAD~2 HEAD~5 HEAD~10 && + have_not_sent HEAD~1 HEAD~3 HEAD~4 HEAD~6 HEAD~7 HEAD~8 HEAD~9 +' + test_expect_success 'when two skips collide, favor the larger one' ' rm -rf server client trace && git init server &&
-- 2.19.0.605.g01d371f741-goog
