From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 06 Nov 2025 10:53:58 GMT
Subject: Re: v2.52.0-rc0 test failure on cygwin
Message-ID: <aQx-RnNX28BPU2cS@pks.im>
In-Reply-To: <f22c95ad-43c8-41de-8315-e707224e830b@ramsayjones.plus.com>

On Tue, Nov 04, 2025 at 11:49:46PM +0000, Ramsay Jones wrote:
> Just a quick heads up: the rc0 build on cygwin has a flaky test, thus:
>   
>   $ tail test-out-2-52-rc0 
>   Test Summary Report
>   -------------------
>   t0610-reftable-basics.sh                         (Wstat: 256 (exited 1) Tests: 90 Failed: 1)
>     Failed test:  29
>     Non-zero exit status: 1
>   Files=1024, Tests=32232, 2703 wallclock secs (23.38 usr 60.53 sys + 7886.88 cusr 10419.88 csys = 18390.67 CPU)
>   Result: FAIL
>   make[1]: *** [Makefile:78: prove] Error 1
>   make[1]: Leaving directory '/home/ramsay/git/t'
>   make: *** [Makefile:3327: test] Error 2
>   $ 
>  
> Initially, while investigating the failure, I was running the test by hand and it
> didn't fail ... So, I tried a stess test, like so: 

Interesting. My first hunch is that the root cause is auto-maintenance.
git-maintenance(1) spawns `git pack-refs --auto`, and that process will
open the stack so that it can verify whether it needs to be packed or
not. And Windows being Windows, the file being open may mean that it
cannot be written by another process at the same point in time.

In any case, I was able to reproduce the issue. But disabling auto
maintenance with the following patch does not fix the flake.

diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 3ea5d51532..52bbf4fe57 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -204,6 +204,7 @@ test_expect_success 'ref transaction: corrupted tables cause failure' '
 	git init repo &&
 	(
 		cd repo &&
+		git config set maintenance.auto false &&
 		test_commit file1 &&
 		for f in .git/reftable/*.ref
 		do

And I guess that makes sense? I'd assume that Cygwin already knows to
open files with POSIX semantics, so it should be possible to write to
the file even if it was held open by another Git process.

[snip]
> So, not really an answer, but I have noted several times over the years
> that cygwin seems to delay setting some file attributes until after the
> process has exited ... [yeah, I don't see how either! ;) ].

What? That's horrible if true. How doesn't this cause more issues?

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.

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 :)

> I noted the above last night and, unfortunately, I haven't had any
> time to look into this tonight. (hopefully tomorrow).
> 
> [I haven't tried bisecting because, well ... flaky test! ;) ]

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.

Thanks!

Patrick