Web lists-archives.com

[PATCH] negotiator/skipping: parse commit before queueing




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@xxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxx/
---
 negotiator/skipping.c                |  2 +-
 t/t5552-skipping-fetch-negotiator.sh | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

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