threads / patch / 2221

patchfetch/upload: Fix corner case with few revs

Subject: [PATCH] fetch/upload: Fix corner case with few revs

## tl;dr

4 messages between Oct 25, 2005 and Oct 26, 2005. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Johannes Schindelin· Oct 25, 2005, 15:34 UTC · lore

When git-fetch-pack did not have enough revs to send, it did not realize that the server actually speaks multi_ack. The server would now continue sending ack´s, but the client would try to unpack objects. Oops.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
	I have a sizable collection of brown paper bags by now.
 fetch-pack.c  |   13 +++++++++----
 upload-pack.c |   15 +++++++++++----
 2 files changed, 20 insertions(+), 8 deletions(-)

applies-to: f4786932e8753bdd07e44829a97a47749b329ee8 9a0ea94256236f1d038b16eb834fdfa5987f308c

Show changes to 2 files +20 −8

fetch-pack.c, upload-pack.c

diff --git a/fetch-pack.c b/fetch-pack.c
index 7015dc5..b02a24a 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -224,12 +224,17 @@ done:
 	if (retval != 0)
 		flushes++;
 	while (flushes) {
-		if (get_ack(fd[0], result_sha1)) {
+		int ack = get_ack(fd[0], result_sha1);
+		if (ack) {
 			if (verbose)
-				fprintf(stderr, "got ack %s\n",
+				fprintf(stderr, "got ack (%d) %s\n", ack,
 					sha1_to_hex(result_sha1));
-			if (!multi_ack)
-				return 0;
+			if (!multi_ack) {
+				if (ack == 2)
+					multi_ack = 1;
+				else
+					return 0;
+			}
 			retval = 0;
 			continue;
 		}
diff --git a/upload-pack.c b/upload-pack.c
index 25a343e..1dbde5f 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -116,7 +116,7 @@ static int get_common_commits(void)
 {
 	static char line[1000];
 	unsigned char sha1[20];
-	int len;
+	int len, last_sent_was_nak = 0;
 
 	track_object_refs = 0;
 	save_commit_buffer = 0;
@@ -126,23 +126,30 @@ static int get_common_commits(void)
 		reset_timeout();
 
 		if (!len) {
-			if (multi_ack || nr_has == 0)
+			if (multi_ack || nr_has == 0) {
 				packet_write(1, "NAK\n");
+				last_sent_was_nak = 1;
+			}
 			continue;
 		}
 		len = strip(line, len);
 		if (!strncmp(line, "have ", 5)) {
 			if (got_sha1(line+5, sha1) &&
-					(multi_ack || nr_has == 1))
+					(multi_ack || nr_has == 1)) {
 				packet_write(1, "ACK %s%s\n",
 					sha1_to_hex(sha1),
 					multi_ack && nr_has < MAX_HAS ?
 					" continue" : "");
+				last_sent_was_nak = 0;
+			}
 			continue;
 		}
 		if (!strcmp(line, "done")) {
-			if (nr_has > 0)
+			if (nr_has > 0) {
+				if (multi_ack && !last_sent_was_nak)
+					packet_write(1, "NAK\n");
 				return 0;
+			}
 			packet_write(1, "NAK\n");
 			return -1;
 		}
---
0.99.8.GIT
Junio C Hamano· Oct 25, 2005, 17:39 UTC · re: Johannes Schindelin · lore

Re: [PATCH] fetch/upload: Fix corner case with few revs

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> When git-fetch-pack did not have enough revs to send, it did not realize 
> that the server actually speaks multi_ack. The server would now continue 
> sending ack', but the client would try to unpack objects. Oops.

I've already pushed your initial set out to "master", but I suspect we may be better of if I recall them and let it simmer a bit longer in the proposed updates branch, and defer them post 0.99.9. What do you think?

Johannes Schindelin· Oct 25, 2005, 20:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] fetch/upload: Fix corner case with few revs

Hi,
On Tue, 25 Oct 2005, Junio C Hamano wrote:
> I've already pushed your initial set out to "master", but I
> suspect we may be better of if I recall them and let it simmer a
> bit longer in the proposed updates branch, and defer them post
> 0.99.9.  What do you think?
Yes, please. Sorry for the problems.

Ciao, Dscho

Alex Riesen· Oct 26, 2005, 18:34 UTC · re: Johannes Schindelin · lore

Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension

Johannes Schindelin, Wed, Oct 26, 2005 10:41:47 +0200:
Show 5 quoted lines
> > > Could you please try the patch I sent with the subject "[PATCH]
> > > fetch/upload: Fix corner case with few revs"? Your output looks exactly
> > > like what I fixed with that patch.
> > I couldn't at the moment. Do you still need a test?
> If you have time and can test it, yes, please.
Johannes Schindelin, Tue, Oct 25, 2005 17:34:07 +0200:
> When git-fetch-pack did not have enough revs to send, it did not realize 
> that the server actually speaks multi_ack. The server would now continue 
> sending ack´s, but the client would try to unpack objects. Oops.
This patch fixed it.

← back to recent threads