git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Patrick Steinhardt <ps@pks.im>
Date
Jun 3, 2025, 12:40 UTC
Message-ID
<aD7tKfXD7YxprSZh@pks.im>
In-Reply-To
<gw6j5enpzcit2zquafoaiujreoa4kbv3n6feq6yeqylcfynqim@s53ctnlg7tmm>
On Mon, Jun 02, 2025 at 10:59:53AM -0500, Justin Tobler wrote:
Show 63 quoted lines
> 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 &&
Previous: Junio C HamanoNext: Johannes Schindelin
Message 22 of 25 in “builtin/receive-pack: introduce option to skip connectivity checks”
  1. 0/2 builtin/receive-pack: introduce option to skip connectivity checksJustin Tobler, May 7, 2025
  2. 1/2 t5412: test receive-pack connectivity checkJustin Tobler, May 7, 2025
  3. Patrick SteinhardtMay 7, 2025
  4. Justin ToblerMay 19, 2025
  5. 2/2 builtin/receive-pack: add option to skip connectivity checkJustin Tobler, May 7, 2025
  6. Patrick SteinhardtMay 7, 2025
  7. Junio C HamanoMay 7, 2025
  8. 0/2 builtin/receive-pack: introduce option to skip connectivity checksJustin Tobler, May 20, 2025
  9. 1/2 t5410: test receive-pack connectivity checkJustin Tobler, May 20, 2025
  10. Karthik NayakMay 20, 2025
  11. 2/2 builtin/receive-pack: add option to skip connectivity checkJustin Tobler, May 20, 2025
  12. Patrick SteinhardtMay 20, 2025
  13. Justin ToblerMay 20, 2025
  14. Karthik NayakMay 20, 2025
  15. 0/2 builtin/receive-pack: introduce option to skip connectivity checksJustin Tobler, May 20, 2025
  16. 1/2 t5410: test receive-pack connectivity checkJustin Tobler, May 20, 2025
  17. 2/2 builtin/receive-pack: add option to skip connectivity checkJustin Tobler, May 20, 2025
  18. Johannes SchindelinJun 2, 2025
  19. Justin ToblerJun 2, 2025
  20. Johannes SchindelinJun 2, 2025
  21. Junio C HamanoJun 2, 2025
  22. Patrick SteinhardtJun 3, 2025
  23. Johannes SchindelinJun 5, 2025
  24. Patrick SteinhardtJun 5, 2025
  25. Karthik NayakMay 22, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.