From: Mark Jamsek <mark@jamsek.com>
Subject: Re: fix gotd rejecting multiple have lines
To: Omar Polo <op@omarpolo.com>
Cc: Stefan Sperling <stsp@stsp.name>, gameoftrees@openbsd.org
Date: Wed, 18 Jan 2023 22:54:45 +1100

On 23-01-18 12:44PM, Omar Polo wrote:
> On 2023/01/18 12:14:23 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > While testing 'git pull' against got.gameoftrees.org I came across
> > this error:
> > 
> >   fetch-pack: protocol error: bad band #69
> >   fatal: protocol error: bad pack header
> >   gotsh: unexpected 'have' packet
> > 
> > This occurs if there is a deep enough branch history to trigger
> > git into sending lots of have-lines until the server sends an ack.
> > Because this communication happens asynchronously we might already
> > be expecting a 'done' packet while more have-lines keep arriving.
> > 
> > Fix this problem and add a test to hopefully trigger the issue reliably.
> > Because this problem depends on timing it might not be 100% reproducible.
> > But the test kept failing reliably on my laptop at least.
> 
> run the regress a few times against an unpatched gotd and failed
> reilably for me too.
> 
> > This might also fix the spurious 'unexpected flush packet' errors seen with
> > 'got fetch' while already up-to-date, but I have not yet confirmed this.
> > 
> > ok?
> 
> ok op@
> 
> > [...]
> > +	got tree -R -r $testroot/repo-clone2 > $testroot/stdout
> > +	cat > $testroot/stdout.expected <<EOF
> > +alpha
> > +beta
> > +gamma/
> > +gamma/delta
> 
> These two shouldn't be there:
> 
> > +psi/
> > +psi/new
> 
> otherwise the test always fails for me.

That's really strange, if I remove those two lines, the tests fails for
me:

----8<------------
test_fetch_more_history --- /tmp/gotd-test-fetch_more_history-E5zZyhdG/stdout.expected  Wed Jan 18 22:52:12 2023
+++ /tmp/gotd-test-fetch_more_history-E5zZyhdG/stdout   Wed Jan 18 22:52:12 2023
@@ -2,3 +2,5 @@
 beta
 gamma/
 gamma/delta
+psi/
+psi/new
test failed; leaving test data in /tmp/gotd-test-fetch_more_history-E5zZyhdG
*** Error 1 in target 'test_repo_write' (ignored)
------------>8----

but running the test as stsp had it, the test passes!

> > +EOF
> > [...]
> > +}
> 
> style nit: double empty line
> 
> > +
> > +
> >  test_parseargs "$@"
> >  run_test test_send_basic
> > +run_test test_fetch_more_history
> 
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68