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

Re: [PATCH 6/9] fetch: ask server to advertise HEAD for config-less fetch

From
Taylor Blau <me@ttaylorr.com>
Date
Mar 12, 2025, 21:43 UTC
Message-ID
<Z9H//JHtYTGqHI3n@nand.local>
In-Reply-To
<20250309030847.GF2334191@coredump.intra.peff.net>
On Sat, Mar 08, 2025 at 10:08:47PM -0500, Jeff King wrote:
Show 18 quoted lines
> If we're not given any refspecs (either on the command line or via
> config) and we have no branch merge config, then we fetch the remote
> HEAD into our local FETCH_HEAD. In that case we do not send any
> ref-prefix option to the server at all, and we see the full
> advertisement.
>
> But this is sub-optimal. We only care about HEAD, so we can just ask
> for that, and ignore all of the other refs.
>
> The new test demonstrates a case where we see fewer refs (in this case
> only one less, but in theory we could be ignoring millions of them).
>
> This also removes the only case where we care about seeing some refs
> from the other side, but don't add anything to the ref_prefixes list.
> Cleaning this up means one less maintenance burden. Before this patch,
> any code which wanted to add to the list had to make sure the list was
> not empty, since an empty list meant "ask for everything". Now it really
> means "we are not interested in any refs".

Yes. The optimization is nice on its own, but I think this is the real benefit to this patch IMHO.

> This should let us optimize a few more cases in subsequent patches.
;-).
Show 8 quoted lines
> Note that we'll add "HEAD" to the list of prefixes, and later code for
> updating "refs/remotes/<remote>/HEAD" may likewise do so. In theory this
> could cause duplicates in the list, but in practice these can't both
> trigger. We hit our new case only if there are no refspecs, and the
> "<remote>/HEAD" feature is enabled only when we are fetching from a
> remote with configured refspecs. We could be defensive with a flag, but
> it didn't seem worth it to me (the absolute worse case is a useless
> redundant ref-prefix line sent to the server).

Yeah, I think that we already do this in some instances that you and I talked about off-list, but I can't remember exactly what I did to provoke it. In either case, the server responds correctly, so I don't think it's so urgent to deal with ATM.

Show 16 quoted lines
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 95fd0018b9..f142756441 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1766,6 +1766,14 @@ static int do_fetch(struct transport *transport,
>  					    branch->merge[i]->src);
>  			}
>  		}
> +
> +		/*
> +		 * If there are no refs specified to fetch, then we just
> +		 * fetch HEAD; mention that to narrow the advertisement.
> +		 */
> +		if (!transport_ls_refs_options.ref_prefixes.nr)
> +			strvec_push(&transport_ls_refs_options.ref_prefixes,
> +				    "HEAD");

Makes sense. If we know that we just want to fetch HEAD into FETCH_HEAD and we haven't already limited the advertisement, we are free to do so now.

Show 12 quoted lines
>  	if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
> index cea8f92a3d..2f0a52a72d 100755
> --- a/t/t5702-protocol-v2.sh
> +++ b/t/t5702-protocol-v2.sh
> @@ -679,6 +679,21 @@ test_expect_success 'default refspec is used to filter ref when fetching' '
>  	grep "ref-prefix refs/tags/" log
>  '
>
> +test_expect_success 'set up parent for prefix tests' '
> +	git init prefix-parent &&
> +	git -C prefix-parent commit --allow-empty -m foo &&
Any reason to use a bona-fide "commit" here instead of "test_commit"?
Not a big deal either way, of course, I'm just curious.
Show 10 quoted lines
> +	git -C prefix-parent branch unrelated-branch
> +'
> +
> +test_expect_success 'empty refspec filters refs when fetching' '
> +	git init configless-child &&
> +
> +	test_when_finished "rm -f log" &&
> +	GIT_TRACE_PACKET="$(pwd)/log" \
> +		git -C configless-child fetch ../prefix-parent &&
> +	test_grep ! unrelated-branch log
Very clean, nice.

Thanks, Taylor

Previous: Jeff KingNext: Jeff King
Message 42 of 61 in “Tags are no longer fetched when fetching specific commit”
  1. Igor TodorovskiJan 30, 2025
  2. Taylor BlauFeb 13, 2025
  3. Bence FerdinandyFeb 14, 2025
  4. Junio C HamanoFeb 14, 2025
  5. Jeff KingFeb 21, 2025
  6. fetch: fix following tags when fetching specific OIDTaylor Blau, Mar 7, 2025
  7. Taylor BlauMar 7, 2025
  8. Junio C HamanoMar 8, 2025
  9. Bence FerdinandyMar 8, 2025
  10. 0/9 fetch: further ref-prefix cleanups and optimizationsJeff King, Mar 9, 2025
  11. 1/9 t5702: fix typo in test nameJeff King, Mar 9, 2025
  12. Taylor BlauMar 12, 2025
  13. Jeff KingMar 13, 2025
  14. 2/9 t5516: prefer "oid" to "sha1" in some test titlesJeff King, Mar 9, 2025
  15. 3/9 t5516: drop NEEDSWORK about v2 reachability behaviorJeff King, Mar 9, 2025
  16. Taylor BlauMar 12, 2025
  17. 4/9 t5516: beef up exact-oid ref prefixes testJeff King, Mar 9, 2025
  18. 5/9 refspec_ref_prefixes(): clean up refspec_item logicJeff King, Mar 9, 2025
  19. Taylor BlauMar 12, 2025
  20. Jeff KingMar 13, 2025
  21. Junio C HamanoMar 13, 2025
  22. 0/4 refspec: treat 'fetch' as a Boolean valueTaylor Blau, Mar 17, 2025
  23. 1/4 refspec: treat 'fetch' as a Boolean valueTaylor Blau, Mar 17, 2025
  24. Jeff KingMar 18, 2025
  25. Jeff KingMar 18, 2025
  26. Taylor BlauMar 18, 2025
  27. 2/4 refspec: replace `refspec_init()` with fetch/push variantsTaylor Blau, Mar 17, 2025
  28. 3/4 refspec: remove refspec_item_init_or_die()Taylor Blau, Mar 17, 2025
  29. 4/4 refspec: replace `refspec_item_init()` with fetch/push variantsTaylor Blau, Mar 17, 2025
  30. Junio C HamanoMar 17, 2025
  31. Taylor BlauMar 18, 2025
  32. 0/4 refspec: treat 'fetch' as a Boolean valueTaylor Blau, Mar 18, 2025
  33. 1/4 refspec: treat 'fetch' as a Boolean valueTaylor Blau, Mar 18, 2025
  34. 2/4 refspec: replace `refspec_init()` with fetch/push variantsTaylor Blau, Mar 18, 2025
  35. 3/4 refspec: remove refspec_item_init_or_die()Taylor Blau, Mar 18, 2025
  36. 4/4 refspec: replace `refspec_item_init()` with fetch/push variantsTaylor Blau, Mar 18, 2025
  37. Elijah NewrenMar 19, 2025
  38. Taylor BlauMar 17, 2025
  39. Junio C HamanoMar 17, 2025
  40. Taylor BlauMar 18, 2025
  41. 6/9 fetch: ask server to advertise HEAD for config-less fetchJeff King, Mar 9, 2025
  42. Taylor BlauMar 12, 2025
  43. Jeff KingMar 13, 2025
  44. Junio C HamanoMar 13, 2025
  45. Taylor BlauMar 17, 2025
  46. 7/9 fetch: stop protecting additions to ref-prefix listJeff King, Mar 9, 2025
  47. Taylor BlauMar 12, 2025
  48. 8/9 fetch: avoid ls-refs only to ask for HEAD symref updateJeff King, Mar 9, 2025
  49. Junio C HamanoMar 13, 2025
  50. Jeff KingMar 17, 2025
  51. Junio C HamanoMar 17, 2025
  52. 0/2 limiting followRemoteHEAD being usedJeff King, Mar 18, 2025
  53. 1/2 fetch: only respect followRemoteHEAD with configured refspecsJeff King, Mar 18, 2025
  54. Taylor BlauMar 18, 2025
  55. 2/2 fetch: don't ask for remote HEAD if followRemoteHEAD is "never"Jeff King, Mar 18, 2025
  56. Junio C HamanoMar 18, 2025
  57. Taylor BlauMar 18, 2025
  58. 9/9 fetch: use ref prefix list to skip ls-refsJeff King, Mar 9, 2025
  59. Taylor BlauMar 12, 2025
  60. Taylor BlauMar 12, 2025
  61. Jeff KingMar 13, 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.