# [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension 8 messages from 2005-10-23 to 2005-10-26. Participants: Johannes Schindelin, Alex Riesen, Junio C Hamano. Thread: https://gitlist.dev/t/2197 ## Johannes Schindelin, 2005-10-23 01:40 Subject: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510230339090.21239%40wbgn013.biozentrum.uni-wuerzburg.de ``` This patch concludes the series, which makes git-fetch-pack/git-upload-pack negotiate a potentially better set of common revs. It should make a difference when fetching from a repository with a few branches. Signed-off-by: Johannes Schindelin --- connect.c | 5 ++++- fetch-pack.c | 50 +++++++++++++++++++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 14 deletions(-) applies-to: 6b4b7d9acf60aa99d961b599f37d0c824be79e27 9adb6b3971e7daa79221d7dbe05b66327b266b86 diff --git a/connect.c b/connect.c index b171c5d..57e25a3 100644 --- a/connect.c +++ b/connect.c @@ -59,8 +59,11 @@ int get_ack(int fd, unsigned char *resul if (!strcmp(line, "NAK")) return 0; if (!strncmp(line, "ACK ", 3)) { - if (!get_sha1_hex(line+4, result_sha1)) + if (!get_sha1_hex(line+4, result_sha1)) { + if (strstr(line+45, "continue")) + return 2; return 1; + } } die("git-fetch_pack: expected ACK/NAK, got '%s'", line); } diff --git a/fetch-pack.c b/fetch-pack.c index 3a903c4..57602b9 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -125,7 +125,7 @@ static int find_common(int fd[2], unsign struct ref *refs) { int fetching; - int count = 0, flushes = 0, retval; + int count = 0, flushes = 0, multi_ack = 0, retval; const unsigned char *sha1; for_each_ref(rev_list_append_sha1); @@ -156,20 +156,22 @@ static int find_common(int fd[2], unsign continue; } - packet_write(fd[1], "want %s\n", sha1_to_hex(remote)); + packet_write(fd[1], "want %s multi_ack\n", sha1_to_hex(remote)); fetching++; } packet_flush(fd[1]); if (!fetching) return 1; - flushes = 1; + flushes = 0; retval = -1; while ((sha1 = get_rev())) { packet_write(fd[1], "have %s\n", sha1_to_hex(sha1)); if (verbose) fprintf(stderr, "have %s\n", sha1_to_hex(sha1)); if (!(31 & ++count)) { + int ack; + packet_flush(fd[1]); flushes++; @@ -179,26 +181,48 @@ static int find_common(int fd[2], unsign */ if (count == 32) continue; - if (get_ack(fd[0], result_sha1)) { - flushes = 0; - retval = 0; - if (verbose) - fprintf(stderr, "got ack\n"); - break; - } + + do { + ack = get_ack(fd[0], result_sha1); + if (verbose && ack) + fprintf(stderr, "got ack %d %s\n", ack, + sha1_to_hex(result_sha1)); + if (ack == 1) { + if (!multi_ack) + flushes = 0; + retval = 0; + goto done; + } else if (ack == 2) { + multi_ack = 1; + mark_common((struct commit *) + lookup_object(result_sha1)); + retval = 0; + } + } while(ack); flushes--; } } +done: + if (multi_ack) { + packet_flush(fd[1]); + flushes++; + } packet_write(fd[1], "done\n"); if (verbose) fprintf(stderr, "done\n"); + if (retval != 0) + flushes++; while (flushes) { - flushes--; if (get_ack(fd[0], result_sha1)) { if (verbose) - fprintf(stderr, "got ack\n"); - return 0; + fprintf(stderr, "got ack %s\n", + sha1_to_hex(result_sha1)); + if (!multi_ack) + return 0; + retval = 0; + continue; } + flushes--; } return retval; } --- 0.99.8.GIT ``` ## Alex Riesen, 2005-10-25 20:47 Subject: Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: <20051025204754.GA8030@steel.home> URL: https://gitlist.dev/e/20051025204754.GA8030%40steel.home In-Reply-To: ``` Johannes Schindelin, Sun, Oct 23, 2005 03:40:13 +0200: > This patch concludes the series, which makes > git-fetch-pack/git-upload-pack negotiate a potentially better set of > common revs. It should make a difference when fetching from a repository > with a few branches. This broke git-pull for me (the local one): /d/e/f.git$ git-pull fatal: bad pack file fatal: git-unpack-objects died with error code 128 Fetch failure: /a/b/c/.git > applies-to: 6b4b7d9acf60aa99d961b599f37d0c824be79e27 > 9adb6b3971e7daa79221d7dbe05b66327b266b86 ... > diff --git a/fetch-pack.c b/fetch-pack.c > index 3a903c4..57602b9 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c Reverting just fetch-pack.c part of the patch helps. ``` ## Johannes Schindelin, 2005-10-25 21:02 Subject: Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510252300290.15756%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20051025204754.GA8030@steel.home> ``` Hi, On Tue, 25 Oct 2005, Alex Riesen wrote: > Johannes Schindelin, Sun, Oct 23, 2005 03:40:13 +0200: > > This patch concludes the series, which makes > > git-fetch-pack/git-upload-pack negotiate a potentially better set of > > common revs. It should make a difference when fetching from a repository > > with a few branches. > > This broke git-pull for me (the local one): > > /d/e/f.git$ git-pull > fatal: bad pack file > fatal: git-unpack-objects died with error code 128 > Fetch failure: /a/b/c/.git > > > applies-to: 6b4b7d9acf60aa99d961b599f37d0c824be79e27 > > 9adb6b3971e7daa79221d7dbe05b66327b266b86 > ... > > diff --git a/fetch-pack.c b/fetch-pack.c > > index 3a903c4..57602b9 100644 > > --- a/fetch-pack.c > > +++ b/fetch-pack.c > > Reverting just fetch-pack.c part of the patch helps. 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. Ciao, Dscho ``` ## Junio C Hamano, 2005-10-25 21:04 Subject: Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: <7vvezlcnpi.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vvezlcnpi.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20051025204754.GA8030@steel.home> ``` Alex Riesen writes: > Johannes Schindelin, Sun, Oct 23, 2005 03:40:13 +0200: >> This patch concludes the series, which makes >> git-fetch-pack/git-upload-pack negotiate a potentially better set of >> common revs. It should make a difference when fetching from a repository >> with a few branches. > > This broke git-pull for me (the local one): Is this the same problem I fixed with this commit, which sits at the tip on the "master" branch? commit 7efc8e43508b415e2540dbcb79521bde16c51e0c tree 6234aa4f7095054a137e030030f914dc6633f809 parent 40a10462498bdd23d4e49f02867b8be50eb78704 author Junio C Hamano 1130061738 -0700 committer Junio C Hamano 1130192018 -0700 upload-pack: fix thinko in common-commit finder code. The code to check if we have the object the other side has was bogus (my fault). Signed-off-by: Junio C Hamano ``` ## Johannes Schindelin, 2005-10-25 21:26 Subject: Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510252323110.18248%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vvezlcnpi.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Tue, 25 Oct 2005, Junio C Hamano wrote: > Alex Riesen writes: > > > Johannes Schindelin, Sun, Oct 23, 2005 03:40:13 +0200: > >> This patch concludes the series, which makes > >> git-fetch-pack/git-upload-pack negotiate a potentially better set of > >> common revs. It should make a difference when fetching from a repository > >> with a few branches. > > > > This broke git-pull for me (the local one): > > Is this the same problem I fixed with this commit, which sits at > the tip on the "master" branch? > > commit 7efc8e43508b415e2540dbcb79521bde16c51e0c > tree 6234aa4f7095054a137e030030f914dc6633f809 > parent 40a10462498bdd23d4e49f02867b8be50eb78704 > author Junio C Hamano 1130061738 -0700 > committer Junio C Hamano 1130192018 -0700 > > upload-pack: fix thinko in common-commit finder code. > > The code to check if we have the object the other side has was bogus > (my fault). > > Signed-off-by: Junio C Hamano It looks to me like it is not. This looks more like upload-pack speaks multi_ack, but fetch-pack does not, since it gets the "got ack .. continue" too late. I'll send out a patch implementing a first version of my "nasty" fetch-pack tests in a few minutes. This test showed me my error. Ciao, Dscho ``` ## Alex Riesen, 2005-10-26 06:46 Subject: Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: <81b0412b0510252346t3806892dx71f9c0dc1efe4073@mail.gmail.com> URL: https://gitlist.dev/e/81b0412b0510252346t3806892dx71f9c0dc1efe4073%40mail.gmail.com In-Reply-To: ``` On 10/25/05, Johannes Schindelin wrote: > > > This patch concludes the series, which makes > > > git-fetch-pack/git-upload-pack negotiate a potentially better set of > > > common revs. It should make a difference when fetching from a repository > > > with a few branches. > > > > This broke git-pull for me (the local one): > > > > /d/e/f.git$ git-pull > > fatal: bad pack file > > fatal: git-unpack-objects died with error code 128 > > Fetch failure: /a/b/c/.git > > > > > applies-to: 6b4b7d9acf60aa99d961b599f37d0c824be79e27 > > > 9adb6b3971e7daa79221d7dbe05b66327b266b86 > > ... > > > diff --git a/fetch-pack.c b/fetch-pack.c > > > index 3a903c4..57602b9 100644 > > > --- a/fetch-pack.c > > > +++ b/fetch-pack.c > > > > Reverting just fetch-pack.c part of the patch helps. > > 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? ``` ## Johannes Schindelin, 2005-10-26 08:41 Subject: Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510261041100.7424%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <81b0412b0510252346t3806892dx71f9c0dc1efe4073@mail.gmail.com> ``` Hi, On Wed, 26 Oct 2005, Alex Riesen wrote: > > 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. Ciao, Dscho ``` ## Alex Riesen, 2005-10-26 09:16 Subject: Re: [PATCH 4/4] git-fetch-pack: Implement client part of the multi_ack extension Message-ID: <81b0412b0510260216s7571dba6w97163927a335818d@mail.gmail.com> URL: https://gitlist.dev/e/81b0412b0510260216s7571dba6w97163927a335818d%40mail.gmail.com In-Reply-To: ``` On 10/26/05, Johannes Schindelin wrote: > > > 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. > Will try. Which patch is it? ```