From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 07 Nov 2025 06:04:45 GMT
Subject: Re: v2.52.0-rc0 test failure on cygwin
Message-ID: <aQ2L_a3q7MAUJI-L@pks.im>
In-Reply-To: <a8a03a31-8e06-4b72-b847-b59548156e60@ramsayjones.plus.com>

On Thu, Nov 06, 2025 at 08:28:35PM +0000, Ramsay Jones wrote:
> On 06/11/2025 10:53 am, Patrick Steinhardt wrote:
> > I wonder whether the issue is surfaced because we use the shell to
> > truncate the file. If you instead use `file-tool truncate 0` for example
> > then I cannot reproduce the flake anymore:
> > 
> > diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> > index 3ea5d51532..1058f83993 100755
> > --- a/t/t0610-reftable-basics.sh
> > +++ b/t/t0610-reftable-basics.sh
> > @@ -207,7 +207,7 @@ test_expect_success 'ref transaction: corrupted tables cause failure' '
> >  		test_commit file1 &&
> >  		for f in .git/reftable/*.ref
> >  		do
> > -			: >"$f" || return 1
> > +			test-tool truncate "$f" 0 || return 1
> >  		done &&
> >  		test_must_fail git update-ref refs/heads/main HEAD
> >  	)
> > 
> > But this may very well just be due to timing again -- spawning the
> > process will be slower than using shell redirection to trim the file.
> 
> I tried this patch tonight, letting:
> 
>     $ ./t0610-reftable-basics.sh --run=29 --stress-limit=10
> 
> finish, which it did without failure. So that's 32 * 10 successful runs.
> 
> (I had expected 16 * 10 yesterday, ie 2 * cores * 10, but this laptop
> has 8 cores 16 threads, so 'getconf _NPROCESSORS_ONLN' returns 16 not 8).

Nice :)

> > All of this is quite curious. I don't really have any better idea than
> > to use something like the above patch. It's ugly, doubly so because I
> > don't understand either the root cause nor why the patch properly fixes
> > it. So I'd be grateful if anyone were to enlighten me :)
> 
> Me too! :)
> 
> > I have verified that the flake already exists in Git 2.51, so at least
> > it's not a regression in the current release cycle.
> OK, that's good to know.
> 
> Despite the mystery, I think a patch based on the above would be
> the best solution for now. (Assuming nobody has a better idea).

Please feel free to take it and turn it into a proper patch. My main
goal was to verify that this is not a regression and that nothing new
broke in the reftable backend. I'm happy to let you take over from here,
as I'm a bit short on time otherwise.

Thanks!

Patrick