All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	gitster@pobox.com, jrnieder@gmail.com
Subject: [PATCH v2 3/3] fetch-pack: in protocol v2, reset in_vain upon ACK
Date: Mon, 27 Apr 2020 17:01:10 -0700	[thread overview]
Message-ID: <e89646cb9ab1164d866ba9747575cab9199e3f62.1588031728.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1588031728.git.jonathantanmy@google.com>

In the function process_acks() in fetch-pack.c, the variable
received_ack is meant to track that an ACK was received, but it was
never set. This results in negotiation terminating prematurely through
the in_vain counter, when the counter should have been reset upon every
ACK.

Therefore, reset the in_vain counter upon every ACK.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 fetch-pack.c          |  1 +
 t/t5500-fetch-pack.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/fetch-pack.c b/fetch-pack.c
index 76691dc6c0..edbfc2af1c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1307,6 +1307,7 @@ static enum common_found process_acks(struct fetch_negotiator *negotiator,
 
 		if (skip_prefix(reader->line, "ACK ", &arg)) {
 			struct object_id oid;
+			received_ack = 1;
 			if (!get_oid_hex(arg, &oid)) {
 				struct commit *commit;
 				oidset_insert(common, &oid);
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 961cd6beec..52dd1a688c 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -403,6 +403,36 @@ test_expect_success 'in_vain not triggered before first ACK' '
 	test_i18ngrep "Total 3 " trace
 '
 
+test_expect_success 'in_vain resetted upon ACK' '
+	rm -rf myserver myclient trace &&
+	git init myserver &&
+
+	# Linked list of commits on master. The first is common; the rest are
+	# not.
+	test_commit -C myserver first_master_commit &&
+	git clone "file://$(pwd)/myserver" myclient &&
+	test_commit_bulk -C myclient 255 &&
+
+	# Another linked list of commits on anotherbranch with no connection to
+	# master. The first is common; the rest are not.
+	git -C myserver checkout --orphan anotherbranch &&
+	test_commit -C myserver first_anotherbranch_commit &&
+	git -C myclient fetch origin anotherbranch:refs/heads/anotherbranch &&
+	git -C myclient checkout anotherbranch &&
+	test_commit_bulk -C myclient 255 &&
+
+	# The new commit that the client wants to fetch.
+	git -C myserver checkout master &&
+	test_commit -C myserver to_fetch &&
+
+	# The client will send (as "have"s) all 256 commits in anotherbranch
+	# first. The 256th commit is common between the client and the server,
+	# and should reset in_vain. This allows negotiation to continue until
+	# the client reports that first_anotherbranch_commit is common.
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C myclient fetch --progress origin master &&
+	test_i18ngrep "Total 3 " trace
+'
+
 test_expect_success 'fetch in shallow repo unreachable shallow objects' '
 	(
 		git clone --bare --branch B --single-branch "file://$(pwd)/." no-reflog &&
-- 
2.26.2.303.gf8c07b1a785-goog


  parent reply	other threads:[~2020-04-28  0:01 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-25  0:56 [PATCH 0/2] Protocol v2 in_vain fixes Jonathan Tan
2020-04-25  0:56 ` [PATCH 1/2] fetch-pack: in protocol v2, in_vain only after ACK Jonathan Tan
2020-04-25  5:08   ` Junio C Hamano
2020-04-26  0:28   ` Jonathan Nieder
2020-04-27 17:27     ` Jonathan Tan
2020-04-27 22:16       ` Junio C Hamano
2020-04-25  0:56 ` [PATCH 2/2] fetch-pack: in protocol v2, reset in_vain upon ACK Jonathan Tan
2020-04-26  1:10   ` Jonathan Nieder
2020-04-27 17:28     ` Jonathan Tan
2020-04-28  0:01 ` [PATCH v2 0/3] Protocol v2 in_vain fixes Jonathan Tan
2020-04-28  0:01   ` [PATCH v2 1/3] fetch-pack: return enum from process_acks() Jonathan Tan
2020-04-28  0:53     ` Jonathan Nieder
2020-04-28 16:54       ` Junio C Hamano
2020-04-28 18:00         ` Michal Suchánek
2020-04-28 19:17           ` Junio C Hamano
2020-04-28  0:01   ` [PATCH v2 2/3] fetch-pack: in protocol v2, in_vain only after ACK Jonathan Tan
2020-04-28  0:54     ` Jonathan Nieder
2020-05-06 21:08     ` Johannes Schindelin
2020-05-06 22:07       ` [PATCH] t5500: count objects through stderr, not trace Jonathan Tan
2020-05-06 22:28         ` Junio C Hamano
2020-05-06 22:40           ` Junio C Hamano
2020-05-07 14:35         ` Johannes Schindelin
2020-10-13 14:45         ` Johannes Schindelin
2020-04-28  0:01   ` Jonathan Tan [this message]
2020-04-28  0:55     ` [PATCH v2 3/3] fetch-pack: in protocol v2, reset in_vain upon ACK Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e89646cb9ab1164d866ba9747575cab9199e3f62.1588031728.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.