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

Re: [PATCH] connect: also update offset for features without values

From
Jeff King <peff@peff.net>
Date
Sep 23, 2021, 21:38 UTC
Message-ID
<YUzzwCwlR9AwSeOD@coredump.intra.peff.net>
In-Reply-To
<xmqq4kabyoo3.fsf@gitster.g>
On Thu, Sep 23, 2021 at 02:20:28PM -0700, Junio C Hamano wrote:
Show 32 quoted lines
> > +test_expect_success 'bogus symref in v0 capabilities' '
> > +	test_commit foo &&
> > +	oid=$(git rev-parse HEAD) &&
> > +	{
> > +		printf "%s HEAD\0symref object-format=%s\n" "$oid" "$GIT_DEFAULT_HASH" |
> > +			test-tool pkt-line pack-raw-stdin &&
> > +		printf "0000"
> > +	} >input &&
> > +	git ls-remote --upload-pack="cat input ;:" . >actual &&
> > +	printf "%s\tHEAD\n" "$oid" >expect &&
> > +	test_cmp expect actual
> > +'
> > +
> >  test_done
> >
> > base-commit: 186eaaae567db501179c0af0bf89b34cbea02c26
> 
> I've been seeing an occasional and not-reliably-reproducible test
> failure from t5704 in 'seen' these days---since this is the only
> commit that touches t5704, I am suspecting if there is something
> racy about it, but I am coming up empty after staring at it for a
> few minutes.
> 
> Building 87446480 (connect: also update offset for features without
> values, 2021-09-18), which is an application of the patch directly on
> top of v2.33.0, and doing
> 
>     $ cd t
>     $ while sh t5704-*.sh; do :; done
> 
> I can get it fail in a dozen iterations or so when the box is
> loaded, so it does seem timing dependent.

I think the problem is that our fake upload-pack exits immediately, so ls-remote gets SIGPIPE. In a v0 conversation, ls-remote expects to say "0000" to indicate that it's not interested in fetching anything (in v2, it doesn't bother, since fetching would be a separate request that it just declines to make).

This seems to fix it:
diff --git a/t/t5704-protocol-violations.sh b/t/t5704-protocol-violations.sh
index 34538cebf0..0983c2b507 100755
--- a/t/t5704-protocol-violations.sh
+++ b/t/t5704-protocol-violations.sh
@@ -40,7 +40,7 @@ test_expect_success 'bogus symref in v0 capabilities' '
 			test-tool pkt-line pack-raw-stdin &&
 		printf "0000"
 	} >input &&
-	git ls-remote --upload-pack="cat input ;:" . >actual &&
+	git ls-remote --upload-pack="cat input; read junk;:" . >actual &&
 	printf "%s\tHEAD\n" "$oid" >expect &&
 	test_cmp expect actual
 '

-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 12 of 17 in “connect: also update offset for features without values”
  1. connect: also update offset for features without valuesAndrzej Hunt via GitGitGadget, Sep 18, 2021
  2. Taylor BlauSep 18, 2021
  3. Jeff KingSep 18, 2021
  4. Taylor BlauSep 18, 2021
  5. Eric SunshineSep 19, 2021
  6. Jeff KingSep 19, 2021
  7. Eric SunshineSep 19, 2021
  8. Junio C HamanoSep 19, 2021
  9. Andrzej HuntSep 26, 2021
  10. brian m. carlsonSep 18, 2021
  11. Junio C HamanoSep 23, 2021
  12. Jeff KingSep 23, 2021
  13. Junio C HamanoSep 23, 2021
  14. Jeff KingSep 23, 2021
  15. Andrzej HuntSep 26, 2021
  16. connect: also update offset for features without valuesAndrzej Hunt via GitGitGadget, Sep 26, 2021
  17. Jeff KingSep 27, 2021

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.