Subject: Re: [PATCH v2 2/2] builtin/receive-pack: add option to skip connectivity check

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 3 Jun 2025 14:40:09 +0200

On Mon, Jun 02, 2025 at 10:59:53AM -0500, Justin Tobler wrote:
> On 25/06/02 05:01PM, Johannes Schindelin wrote:
> > Hi Justin,
> > 
> > On Tue, 20 May 2025, Justin Tobler wrote:
> > 
> > > diff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh
> > > index 9afea54a26..f76a22943e 100755
> > > --- a/t/t5410-receive-pack.sh
> > > +++ b/t/t5410-receive-pack.sh
> > > @@ -62,4 +62,26 @@ test_expect_success 'receive-pack missing objects fails connectivity check' '
> > >  	test_must_fail git -C remote.git cat-file -e $(git -C repo rev-parse HEAD)
> > >  '
> > >  
> > > +test_expect_success 'receive-pack missing objects bypasses connectivity check' '
> > > +	test_when_finished rm -rf repo remote.git setup.git &&
> > > +
> > > +	git init repo &&
> > > +	git -C repo commit --allow-empty -m 1 &&
> > > +	git clone --bare repo setup.git &&
> > > +	git -C repo commit --allow-empty -m 2 &&
> > > +
> > > +	# Capture git-send-pack(1) output sent to git-receive-pack(1).
> > > +	git -C repo send-pack ../setup.git --all \
> > > +		--receive-pack="tee ${SQ}$(pwd)/out${SQ} | git-receive-pack" &&
> > > +
> > > +	# Replay captured git-send-pack(1) output on new empty repository.
> > > +	git init --bare remote.git &&
> > > +	git receive-pack --skip-connectivity-check remote.git <out >actual 2>err &&
> > > +
> > > +	test_grep ! "missing necessary objects" actual &&
> > > +	test_must_be_empty err &&
> > > +	git -C remote.git cat-file -e $(git -C repo rev-parse HEAD) &&
> > > +	test_must_fail git -C remote.git rev-list $(git -C repo rev-parse HEAD)
> > > +'
> > > +
> > >  test_done
> > 
> > This test case seems to hang occasionally in the "win+Meson test" jobs on
> > GitHub (I tried to find the same failure at
> > https://gitlab.com/gitlab-org/git/-/pipelines but couldn't find any). See
> > for example
> > https://github.com/gitgitgadget/git/actions/runs/15383915635/job/43279134837#step:6:627
> > 
> > Note that this problem afflicts only the "win+Meson test" jobs; The
> > corresponding "win test" job seems not to hang.
> > 
> > Even in the Git for Windows project, where the `win+VS test` jobs are run,
> > the t5410 test passes within a dozen seconds or so, see e.g.
> > https://github.com/git-for-windows/git/actions/runs/15383945895/job/43279689086#step:5:143
> > (confusingly, the subset of tests run in the matrix jobs differs between
> > the `win+Meson test` jobs and the `win+VS test` jobs, but if you click
> > through all of the `win+Meson test` jobs, expand the `test` step,
> > patiently wait a few seconds for the log to be lazy loaded "enough" for
> > the search to work, you will notice that t5410 is not mentioned in any of
> > them, and the only one that times out after 4h37m11s is
> > https://github.com/git-for-windows/git/actions/runs/15383945895/job/43279753911,
> > likely while running 5410, too).
> > 
> > Do you have any idea why this particular test case, in conjunction with
> > Windows and Meson (and only on GitHub) acts up like this?
> 
> Thanks Johannes for the report. I'm not quite sure yet what is going on
> here, but I'll dig into this a bit and see what I can figure out. :)

I've been banging my head against this issue for a bit today. A couple
of findings:

  - The issue is specific to Git for Windows, I could only reproduce it
    when working with aa550efd0bb (fixup??? survey: add command line
    opts to select references, 2025-05-08).

  - When working on top of the above commit the bug is consistent. It
    doesn't only happen in GitHub, but also happens in GitLab CI [1].

  - That being said, I still can't reproduce it locally?! This one is
    quite puzzling to me. I have tried to get my environment as close as
    possible to the environment we have in the CI systems.

  - I have a fix, see the patch further down. But I don't understand
    that fix just yet.

I saw that all other sites where inject a custom receive-pack command
also use a wrapper script, so it's not the worst thing to do. But it
would be great to understand why this issue exists in the first place.

Patrick

[1]: https://github.com/pks-t/git/actions/runs/15416185892/job/43379399861

diff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh
index f76a22943ef..112da408d45 100755
--- a/t/t5410-receive-pack.sh
+++ b/t/t5410-receive-pack.sh
@@ -49,9 +49,13 @@ test_expect_success 'receive-pack missing objects fails connectivity check' '
 	git clone --bare repo setup.git &&
 	git -C repo commit --allow-empty -m 2 &&
 
+	write_script receive-pack-wrapper <<-EOF &&
+	tee "$(pwd)/out" | git-receive-pack "\$@"
+	EOF
+
 	# Capture git-send-pack(1) output sent to git-receive-pack(1).
 	git -C repo send-pack ../setup.git --all \
-		--receive-pack="tee ${SQ}$(pwd)/out${SQ} | git-receive-pack" &&
+		--receive-pack="${SQ}$(pwd)${SQ}/receive-pack-wrapper" &&
 
 	# Replay captured git-send-pack(1) output on new empty repository.
 	git init --bare remote.git &&
@@ -70,9 +74,13 @@ test_expect_success 'receive-pack missing objects bypasses connectivity check' '
 	git clone --bare repo setup.git &&
 	git -C repo commit --allow-empty -m 2 &&
 
+	write_script receive-pack-wrapper <<-EOF &&
+	tee "$(pwd)/out" | git-receive-pack "\$@"
+	EOF
+
 	# Capture git-send-pack(1) output sent to git-receive-pack(1).
 	git -C repo send-pack ../setup.git --all \
-		--receive-pack="tee ${SQ}$(pwd)/out${SQ} | git-receive-pack" &&
+		--receive-pack="${SQ}$(pwd)${SQ}/receive-pack-wrapper" &&
 
 	# Replay captured git-send-pack(1) output on new empty repository.
 	git init --bare remote.git &&