{"thread":{"id":"66538","subject":"[PATCH] refs/files: keep root refs out of loose ref cache","startedAt":"2026-10-14T15:32:08Z","lastAt":"2026-10-21T05:36:40Z","messageCount":13,"participants":["Andy Koppe","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"555032","messageId":"20261014153208.2194727-1-andy.koppe@gmail.com","threadId":"66538","inReplyTo":null,"subject":"[PATCH] refs/files: keep root refs out of loose ref cache","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2026-10-14T15:32:08Z","receivedAt":"2026-10-14T15:32:08Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"With the files backend, root refs such as HEAD or ORIG_HEAD are only\nadded to the loose ref cache if REFS_FOR_EACH_INCLUDE_ROOT_REFS is\npassed to the iteration that creates the cache. If the cache has already\nbeen created by an earlier iteration, root refs are silently omitted.\nConversely, once they have been added, any later iteration also yields\nthem, whether or not it asked for them. This has been the case since\nd0f00c1ac17 (refs: introduce `refs_for_each_include_root_refs()`).\n\nFor example, `git for-each-ref --include-root-refs --merged=main` omits\nroot refs that are merged into main, because looking up the given commit\nloads the replace refs, which populates the loose ref cache first. The\nreftable backend is not affected.\n\nFix this by keeping the root refs out of the shared loose ref cache.\nInstead, read them into a separate ref cache whenever an iteration asks\nfor them, and overlay its iterator on the loose ref iterator. The cache\nis freed when the iteration finishes.\n\nSigned-off-by: Andy Koppe <andy.koppe@gmail.com>\n---\n refs/files-backend.c           | 38 +++++++++++++++++++++++++++-------\n t/t6302-for-each-ref-filter.sh | 12 +++++++++++\n 2 files changed, 42 insertions(+), 8 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 71628550f2..74409878b6 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -481,8 +481,21 @@ static void add_root_refs(struct files_ref_store *refs,\n \tfor_each_root_ref(refs, fill_root_ref, &data);\n }\n \n-static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n-\t\t\t\t\t     unsigned int flags)\n+/*\n+ * Create a separate ref cache containing only the root refs. These are kept\n+ * out of the loose ref cache, which may already exist and is shared by\n+ * iterations that should not include them.\n+ */\n+static struct ref_cache *create_root_ref_cache(struct files_ref_store *refs)\n+{\n+\tstruct ref_cache *cache = create_ref_cache(&refs->base, NULL);\n+\n+\tcache->root->flag &= ~REF_INCOMPLETE;\n+\tadd_root_refs(refs, get_ref_dir(cache->root));\n+\treturn cache;\n+}\n+\n+static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)\n {\n \tif (!refs->loose) {\n \t\tstruct ref_dir *dir;\n@@ -499,9 +512,6 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n \n \t\tdir = get_ref_dir(refs->loose->root);\n \n-\t\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS)\n-\t\t\tadd_root_refs(refs, dir);\n-\n \t\t/*\n \t\t * Add an incomplete entry for \"refs/\" (to be filled\n \t\t * lazily):\n@@ -1004,6 +1014,7 @@ struct files_ref_iterator {\n \tstruct ref_iterator base;\n \n \tstruct ref_iterator *iter0;\n+\tstruct ref_cache *root_refs;\n \tstruct repository *repo;\n \tunsigned int flags;\n };\n@@ -1053,6 +1064,7 @@ static void files_ref_iterator_release(struct ref_iterator *ref_iterator)\n \tstruct files_ref_iterator *iter =\n \t\t(struct files_ref_iterator *)ref_iterator;\n \tref_iterator_free(iter->iter0);\n+\tfree_ref_cache(iter->root_refs);\n }\n \n static struct ref_iterator_vtable files_ref_iterator_vtable = {\n@@ -1068,6 +1080,7 @@ static struct ref_iterator *files_ref_iterator_begin(\n {\n \tstruct files_ref_store *refs;\n \tstruct ref_iterator *loose_iter, *packed_iter, *overlay_iter;\n+\tstruct ref_cache *root_refs = NULL;\n \tstruct files_ref_iterator *iter;\n \tstruct ref_iterator *ref_iterator;\n \tunsigned int required_flags = REF_STORE_READ;\n@@ -1094,9 +1107,17 @@ static struct ref_iterator *files_ref_iterator_begin(\n \t * disk, and re-reads it if not.\n \t */\n \n-\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),\n+\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n \t\t\t\t\t      prefix, ref_store->repo, 1);\n \n+\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {\n+\t\troot_refs = create_root_ref_cache(refs);\n+\t\tloose_iter = overlay_ref_iterator_begin(\n+\t\t\t\tcache_ref_iterator_begin(root_refs, prefix,\n+\t\t\t\t\t\t\t ref_store->repo, 0),\n+\t\t\t\tloose_iter);\n+\t}\n+\n \t/*\n \t * The packed-refs file might contain broken references, for\n \t * example an old version of a reference that points at an\n@@ -1118,6 +1139,7 @@ static struct ref_iterator *files_ref_iterator_begin(\n \tref_iterator = &iter->base;\n \tbase_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable);\n \titer->iter0 = overlay_iter;\n+\titer->root_refs = root_refs;\n \titer->repo = ref_store->repo;\n \titer->flags = flags;\n \n@@ -1490,7 +1512,7 @@ static int should_pack_refs(struct files_ref_store *refs,\n \tif (limit < 16)\n \t\tlimit = 16;\n \n-\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL,\n+\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,\n \t\t\t\t\trefs->base.repo, 0);\n \twhile ((ret = ref_iterator_advance(iter)) == ITER_OK) {\n \t\tif (should_pack_ref(refs, &iter->ref, opts))\n@@ -1530,7 +1552,7 @@ static int files_optimize(struct ref_store *ref_store,\n \n \tpacked_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR, &err);\n \n-\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL,\n+\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,\n \t\t\t\t\trefs->base.repo, 0);\n \twhile ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n \t\t/*\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 7f060d97bf..a2fc257f98 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -96,6 +96,18 @@ test_expect_success '--include-root-refs omits dangling symrefs' '\n \t)\n '\n \n+test_expect_success '--include-root-refs with --merged' '\n+\tcat >expect <<-\\EOF &&\n+\tHEAD\n+\tORIG_HEAD\n+\tEOF\n+\ttest_when_finished \"git update-ref -d ORIG_HEAD\" &&\n+\tgit update-ref ORIG_HEAD main &&\n+\tgit for-each-ref --format=\"%(refname)\" --include-root-refs \\\n+\t\t--merged=main \"*HEAD\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'filtering with --points-at' '\n \tcat >expect <<-\\EOF &&\n \trefs/heads/main\n-- \n2.56.0\n\n\n"},{"id":"555098","messageId":"atBraYoY1SVwnezN@pks.im","threadId":"66538","inReplyTo":"20261014153208.2194727-1-andy.koppe@gmail.com","subject":"Re: [PATCH] refs/files: keep root refs out of loose ref cache","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-15T05:58:01Z","receivedAt":"2026-10-15T05:58:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 14, 2026 at 04:32:08PM +0100, Andy Koppe wrote:\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 71628550f2..74409878b6 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -481,8 +481,21 @@ static void add_root_refs(struct files_ref_store *refs,\n>  \tfor_each_root_ref(refs, fill_root_ref, &data);\n>  }\n>  \n> -static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n> -\t\t\t\t\t     unsigned int flags)\n> +/*\n> + * Create a separate ref cache containing only the root refs. These are kept\n> + * out of the loose ref cache, which may already exist and is shared by\n> + * iterations that should not include them.\n> + */\n\nThis raises the question whether an alternative fix could have been to\nteach the cache itself to skip over root refs in case the caller is not\ninterested in them. The answer may very well be a \"no\", but I would be\ncurious to learn why you settled on the proposed fix instead.\n\n> +static struct ref_cache *create_root_ref_cache(struct files_ref_store *refs)\n> +{\n> +\tstruct ref_cache *cache = create_ref_cache(&refs->base, NULL);\n> +\n> +\tcache->root->flag &= ~REF_INCOMPLETE;\n> +\tadd_root_refs(refs, get_ref_dir(cache->root));\n> +\treturn cache;\n> +}\n> +\n> +static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)\n>  {\n>  \tif (!refs->loose) {\n>  \t\tstruct ref_dir *dir;\n> @@ -1094,9 +1107,17 @@ static struct ref_iterator *files_ref_iterator_begin(\n>  \t * disk, and re-reads it if not.\n>  \t */\n>  \n> -\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),\n> +\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n>  \t\t\t\t\t      prefix, ref_store->repo, 1);\n>  \n> +\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {\n> +\t\troot_refs = create_root_ref_cache(refs);\n> +\t\tloose_iter = overlay_ref_iterator_begin(\n> +\t\t\t\tcache_ref_iterator_begin(root_refs, prefix,\n> +\t\t\t\t\t\t\t ref_store->repo, 0),\n> +\t\t\t\tloose_iter);\n> +\t}\n\nIt's a bit weird that we recreate the root ref \"cache\" for every single\nref iterator we create that asks for root refs. Is there a reason why\nyou have decided to not actually cache the root refs the same way as we\ncache the loose refs?\n\nThanks!\n\nPatrick\n\n"},{"id":"555175","messageId":"xmqqmrseooor.fsf@gitster.g","threadId":"66538","inReplyTo":"atBraYoY1SVwnezN@pks.im","subject":"Re: [PATCH] refs/files: keep root refs out of loose ref cache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-15T17:31:48Z","receivedAt":"2026-10-15T17:31:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> +/*\n>> + * Create a separate ref cache containing only the root refs. These are kept\n>> + * out of the loose ref cache, which may already exist and is shared by\n>> + * iterations that should not include them.\n>> + */\n>\n> This raises the question whether an alternative fix could have been to\n> teach the cache itself to skip over root refs in case the caller is not\n> interested in them. The answer may very well be a \"no\", but I would be\n> curious to learn why you settled on the proposed fix instead.\n> ...\n>> -\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),\n>> +\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n>>  \t\t\t\t\t      prefix, ref_store->repo, 1);\n>>  \n>> +\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {\n>> +\t\troot_refs = create_root_ref_cache(refs);\n>> +\t\tloose_iter = overlay_ref_iterator_begin(\n>> +\t\t\t\tcache_ref_iterator_begin(root_refs, prefix,\n>> +\t\t\t\t\t\t\t ref_store->repo, 0),\n>> +\t\t\t\tloose_iter);\n>> +\t}\n>\n> It's a bit weird that we recreate the root ref \"cache\" for every single\n> ref iterator we create that asks for root refs. Is there a reason why\n> you have decided to not actually cache the root refs the same way as we\n> cache the loose refs?\n\nCurious.\n\nSo an alternative would be to have two caches, the primary one for\nthings under refs/ hierarchy and the other one for those outside,\nand callers can choose to use only the primary one or use both of\nthem?\n\n"},{"id":"555188","messageId":"CAHWeT-Z4n=txTGaBNuGPj3jcOou91owB59gvaPvF=2_p50eFHA@mail.gmail.com","threadId":"66538","inReplyTo":"xmqqmrseooor.fsf@gitster.g","subject":"Re: [PATCH] refs/files: keep root refs out of loose ref cache","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2026-10-15T19:43:06Z","receivedAt":"2026-10-15T19:43:06Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n>>> +/*\n>>> + * Create a separate ref cache containing only the root refs. These are kept\n>>> + * out of the loose ref cache, which may already exist and is shared by\n>>> + * iterations that should not include them.\n>>> + */\n>>\n>> This raises the question whether an alternative fix could have been to\n>> teach the cache itself to skip over root refs in case the caller is not\n>> interested in them. The answer may very well be a \"no\", but I would be\n>> curious to learn why you settled on the proposed fix instead.\n>> ...\n>>> -    loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),\n>>> +    loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n>>>                                            prefix, ref_store->repo, 1);\n>>>\n>>> +    if (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {\n>>> +            root_refs = create_root_ref_cache(refs);\n>>> +            loose_iter = overlay_ref_iterator_begin(\n>>> +                            cache_ref_iterator_begin(root_refs, prefix,\n>>> +                                                     ref_store->repo, 0),\n>>> +                            loose_iter);\n>>> +    }\n>>\n>> It's a bit weird that we recreate the root ref \"cache\" for every single\n>> ref iterator we create that asks for root refs. Is there a reason why\n>> you have decided to not actually cache the root refs the same way as we\n>> cache the loose refs?\n>\n> Curious.\n>\n> So an alternative would be to have two caches, the primary one for\n> things under refs/ hierarchy and the other one for those outside,\n> and callers can choose to use only the primary one or use both of\n> them?\n\nThanks for considering this.\n\nI think it makes for a simpler change with less overhead to not\nactually cache them, as there are only two uses of the\nREFS_FOR_EACH_INCLUDE_ROOT_REFS flag: in builtin/for-each-ref.c for\n`git for-each-ref --include-root-refs`, and in builtin/refs.c for `git\nrefs migrate`. And both only invoke it once.\n\nI considered keeping the root refs in the loose ref cache but skipping\nthem during iteration if the flag is unset. But that would mean more\nimpact on that much more common case.\n\nA cleaner approach might be to drop the flag and change all call sites\nthat currently ask for all refs without setting the flag, so that they\nexplicitly ask for \"refs/\" only. That way root refs wouldn't need as\nmuch special handling. But of course that would be a much bigger and\nmore risky change.\n\nAndy\n\n"},{"id":"555214","messageId":"atHLkIADa6BlO_a-@pks.im","threadId":"66538","inReplyTo":"CAHWeT-Z4n=txTGaBNuGPj3jcOou91owB59gvaPvF=2_p50eFHA@mail.gmail.com","subject":"Re: [PATCH] refs/files: keep root refs out of loose ref cache","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-16T07:00:32Z","receivedAt":"2026-10-16T07:00:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Oct 15, 2026 at 08:43:06PM +0100, Andy Koppe wrote:\n> Junio C Hamano <gitster@pobox.com> wrote:\n> > Patrick Steinhardt <ps@pks.im> writes:\n> >>> +/*\n> >>> + * Create a separate ref cache containing only the root refs. These are kept\n> >>> + * out of the loose ref cache, which may already exist and is shared by\n> >>> + * iterations that should not include them.\n> >>> + */\n> >>\n> >> This raises the question whether an alternative fix could have been to\n> >> teach the cache itself to skip over root refs in case the caller is not\n> >> interested in them. The answer may very well be a \"no\", but I would be\n> >> curious to learn why you settled on the proposed fix instead.\n> >> ...\n> >>> -    loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),\n> >>> +    loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n> >>>                                            prefix, ref_store->repo, 1);\n> >>>\n> >>> +    if (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {\n> >>> +            root_refs = create_root_ref_cache(refs);\n> >>> +            loose_iter = overlay_ref_iterator_begin(\n> >>> +                            cache_ref_iterator_begin(root_refs, prefix,\n> >>> +                                                     ref_store->repo, 0),\n> >>> +                            loose_iter);\n> >>> +    }\n> >>\n> >> It's a bit weird that we recreate the root ref \"cache\" for every single\n> >> ref iterator we create that asks for root refs. Is there a reason why\n> >> you have decided to not actually cache the root refs the same way as we\n> >> cache the loose refs?\n> >\n> > Curious.\n> >\n> > So an alternative would be to have two caches, the primary one for\n> > things under refs/ hierarchy and the other one for those outside,\n> > and callers can choose to use only the primary one or use both of\n> > them?\n> \n> Thanks for considering this.\n> \n> I think it makes for a simpler change with less overhead to not\n> actually cache them, as there are only two uses of the\n> REFS_FOR_EACH_INCLUDE_ROOT_REFS flag: in builtin/for-each-ref.c for\n> `git for-each-ref --include-root-refs`, and in builtin/refs.c for `git\n> refs migrate`. And both only invoke it once.\n> \n> I considered keeping the root refs in the loose ref cache but skipping\n> them during iteration if the flag is unset. But that would mean more\n> impact on that much more common case.\n> \n> A cleaner approach might be to drop the flag and change all call sites\n> that currently ask for all refs without setting the flag, so that they\n> explicitly ask for \"refs/\" only. That way root refs wouldn't need as\n> much special handling. But of course that would be a much bigger and\n> more risky change.\n\nThat's fair. I guess the part that triggers me is that it's called a\n\"cache\" whereas we don't really use it as one in this instance. But\nthat's a historical wart, and I agree that the proposed solution is good\nenough.\n\nThanks!\n\nPatrick\n\n"},{"id":"555244","messageId":"xmqqqzhpit2j.fsf@gitster.g","threadId":"66538","inReplyTo":"atHLkIADa6BlO_a-@pks.im","subject":"Re: [PATCH] refs/files: keep root refs out of loose ref cache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-16T15:05:56Z","receivedAt":"2026-10-16T15:05:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Thu, Oct 15, 2026 at 08:43:06PM +0100, Andy Koppe wrote:\n>> Junio C Hamano <gitster@pobox.com> wrote:\n>> > Patrick Steinhardt <ps@pks.im> writes:\n>> > ...\n\nSo in short ...\n\n>> >> This raises the question whether an alternative fix could have been to\n>> >> teach the cache itself to skip over root refs in case the caller is not\n>> >> interested in them. The answer may very well be a \"no\", but I would be\n>> >> curious to learn why you settled on the proposed fix instead.\n\n... with this cleared in the proposed log message of v2 iteration, ...\n\n>> A cleaner approach might be to drop the flag and change all call sites\n>> that currently ask for all refs without setting the flag, so that they\n>> explicitly ask for \"refs/\" only. That way root refs wouldn't need as\n>> much special handling. But of course that would be a much bigger and\n>> more risky change.\n>\n> That's fair. I guess the part that triggers me is that it's called a\n> \"cache\" whereas we don't really use it as one in this instance. But\n> that's a historical wart, and I agree that the proposed solution is good\n> enough.\n\n... the topic will be ready to go?\n\nThanks.\n\n"},{"id":"555334","messageId":"20261017200405.2332914-1-andy.koppe@gmail.com","threadId":"66538","inReplyTo":"20261014153208.2194727-1-andy.koppe@gmail.com","subject":"[PATCH v2] refs/files: keep root refs out of loose ref cache","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2026-10-17T20:04:00Z","receivedAt":"2026-10-17T20:04:00Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"With the files backend, root refs such as HEAD or ORIG_HEAD are only\nadded to the loose ref cache if REFS_FOR_EACH_INCLUDE_ROOT_REFS is\npassed when the cache is created. If the cache has already been created\nfor an earlier ref iterator, root refs are silently omitted. Conversely,\nonce they have been added, any later iterator also yields them, whether\nor not they were requested. This has been the case since d0f00c1ac1\n(refs: introduce `refs_for_each_include_root_refs()`, 2024-02-23).\n\nFor example, `git for-each-ref --include-root-refs --merged=main` omits\nroot refs that are merged into main, because looking up the given commit\nloads the replace refs, which populates the loose ref cache first. The\nreftable backend is not affected, as it filters root refs for each\niterator rather than caching them.\n\nFix this by keeping the root refs out of the loose ref cache. Instead,\nread them into a separate, temporary ref cache whenever an iterator is\nrequested with REFS_FOR_EACH_INCLUDE_ROOT_REFS, and overlay an iterator\nover that cache on the loose ref iterator. The cache is freed along with\nthe iterator.\n\nAlternatively, the root refs could always be added to the loose ref\ncache and skipped during iteration unless requested. But that would mean\nreading them in every command that uses the loose ref cache, even though\nonly two call sites actually request them. Also, keeping the root ref\ncache for reuse by later iterators is not worthwhile, because those two\ncall sites only request the root refs once per command.\n\nCorrect the documentation, help text and flag comment for\n`--include-root-refs`, which wrongly claim that it lists pseudorefs.\n\nSigned-off-by: Andy Koppe <andy.koppe@gmail.com>\n---\nChanges since v1:\n- Move create_root_ref_cache() below get_loose_ref_cache() for a smaller\n  diff, and tweak its comment.\n- Correct the documentation, help text and refs.h comment for\n  `--include-root-refs`, which claimed that it lists pseudorefs.\n- Consider alternative approaches in the log message.\n\n Documentation/for-each-ref-options.adoc |  3 +-\n builtin/for-each-ref.c                  |  2 +-\n refs.h                                  |  4 +--\n refs/files-backend.c                    | 37 +++++++++++++++++++------\n t/t6302-for-each-ref-filter.sh          | 12 ++++++++\n 5 files changed, 46 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/for-each-ref-options.adoc b/Documentation/for-each-ref-options.adoc\nindex 54e2fa95c2..b93ffbd74d 100644\n--- a/Documentation/for-each-ref-options.adoc\n+++ b/Documentation/for-each-ref-options.adoc\n@@ -73,7 +73,8 @@ TAB %(refname)`.\n \tusing the same rules as _<pattern>_ above.\n \n `--include-root-refs`::\n-\tList root refs (`HEAD` and pseudorefs) apart from regular refs.\n+\tList root refs such as `HEAD` and `ORIG_HEAD` in addition to regular\n+\trefs. (The pseudorefs `FETCH_HEAD` and `MERGE_HEAD` are not included.)\n \n `--start-after=<marker>`::\n     Allows paginating the output by skipping references up to and including the\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 4a2fc421db..a12e46cdbe 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -48,7 +48,7 @@ int for_each_ref_core(int argc, const char **argv, const char *prefix, struct re\n \t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only refs which don't contain the commit\")),\n \t\tOPT_BOOL(0, \"ignore-case\", &icase, N_(\"sorting and filtering are case insensitive\")),\n \t\tOPT_BOOL(0, \"stdin\", &from_stdin, N_(\"read reference patterns from stdin\")),\n-\t\tOPT_BOOL(0, \"include-root-refs\", &include_root_refs, N_(\"also include HEAD ref and pseudorefs\")),\n+\t\tOPT_BOOL(0, \"include-root-refs\", &include_root_refs, N_(\"also include root refs like HEAD\")),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/refs.h b/refs.h\nindex 9979446d15..5319e0b570 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -444,8 +444,8 @@ enum refs_for_each_flag {\n \tREFS_FOR_EACH_OMIT_DANGLING_SYMREFS = (1 << 2),\n \n \t/*\n-\t * Include root refs i.e. HEAD and pseudorefs along with the regular\n-\t * refs.\n+\t * Include root refs such as HEAD and ORIG_HEAD in addition to the\n+\t * regular refs. Pseudorefs are not root refs and thus not included.\n \t */\n \tREFS_FOR_EACH_INCLUDE_ROOT_REFS = (1 << 3),\n };\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 71628550f2..9d1e527114 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -481,8 +481,7 @@ static void add_root_refs(struct files_ref_store *refs,\n \tfor_each_root_ref(refs, fill_root_ref, &data);\n }\n \n-static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n-\t\t\t\t\t     unsigned int flags)\n+static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)\n {\n \tif (!refs->loose) {\n \t\tstruct ref_dir *dir;\n@@ -499,9 +498,6 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n \n \t\tdir = get_ref_dir(refs->loose->root);\n \n-\t\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS)\n-\t\t\tadd_root_refs(refs, dir);\n-\n \t\t/*\n \t\t * Add an incomplete entry for \"refs/\" (to be filled\n \t\t * lazily):\n@@ -511,6 +507,19 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n \treturn refs->loose;\n }\n \n+/*\n+ * Root refs are kept out of the loose ref cache because ref iterators must\n+ * only include them when requested with REFS_FOR_EACH_INCLUDE_ROOT_REFS.\n+ */\n+static struct ref_cache *create_root_ref_cache(struct files_ref_store *refs)\n+{\n+\tstruct ref_cache *cache = create_ref_cache(&refs->base, NULL);\n+\n+\tcache->root->flag &= ~REF_INCOMPLETE;\n+\tadd_root_refs(refs, get_ref_dir(cache->root));\n+\treturn cache;\n+}\n+\n static int read_ref_internal(struct ref_store *ref_store, const char *refname,\n \t\t\t     struct object_id *oid, struct strbuf *referent,\n \t\t\t     unsigned int *type, int *failure_errno, int skip_packed_refs)\n@@ -1004,6 +1013,7 @@ struct files_ref_iterator {\n \tstruct ref_iterator base;\n \n \tstruct ref_iterator *iter0;\n+\tstruct ref_cache *root_refs;\n \tstruct repository *repo;\n \tunsigned int flags;\n };\n@@ -1053,6 +1063,7 @@ static void files_ref_iterator_release(struct ref_iterator *ref_iterator)\n \tstruct files_ref_iterator *iter =\n \t\t(struct files_ref_iterator *)ref_iterator;\n \tref_iterator_free(iter->iter0);\n+\tfree_ref_cache(iter->root_refs);\n }\n \n static struct ref_iterator_vtable files_ref_iterator_vtable = {\n@@ -1068,6 +1079,7 @@ static struct ref_iterator *files_ref_iterator_begin(\n {\n \tstruct files_ref_store *refs;\n \tstruct ref_iterator *loose_iter, *packed_iter, *overlay_iter;\n+\tstruct ref_cache *root_refs = NULL;\n \tstruct files_ref_iterator *iter;\n \tstruct ref_iterator *ref_iterator;\n \tunsigned int required_flags = REF_STORE_READ;\n@@ -1094,9 +1106,17 @@ static struct ref_iterator *files_ref_iterator_begin(\n \t * disk, and re-reads it if not.\n \t */\n \n-\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),\n+\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n \t\t\t\t\t      prefix, ref_store->repo, 1);\n \n+\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {\n+\t\troot_refs = create_root_ref_cache(refs);\n+\t\tloose_iter = overlay_ref_iterator_begin(\n+\t\t\t\tcache_ref_iterator_begin(root_refs, prefix,\n+\t\t\t\t\t\t\t ref_store->repo, 0),\n+\t\t\t\tloose_iter);\n+\t}\n+\n \t/*\n \t * The packed-refs file might contain broken references, for\n \t * example an old version of a reference that points at an\n@@ -1118,6 +1138,7 @@ static struct ref_iterator *files_ref_iterator_begin(\n \tref_iterator = &iter->base;\n \tbase_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable);\n \titer->iter0 = overlay_iter;\n+\titer->root_refs = root_refs;\n \titer->repo = ref_store->repo;\n \titer->flags = flags;\n \n@@ -1490,7 +1511,7 @@ static int should_pack_refs(struct files_ref_store *refs,\n \tif (limit < 16)\n \t\tlimit = 16;\n \n-\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL,\n+\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,\n \t\t\t\t\trefs->base.repo, 0);\n \twhile ((ret = ref_iterator_advance(iter)) == ITER_OK) {\n \t\tif (should_pack_ref(refs, &iter->ref, opts))\n@@ -1530,7 +1551,7 @@ static int files_optimize(struct ref_store *ref_store,\n \n \tpacked_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR, &err);\n \n-\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL,\n+\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,\n \t\t\t\t\trefs->base.repo, 0);\n \twhile ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n \t\t/*\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 7f060d97bf..a2fc257f98 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -96,6 +96,18 @@ test_expect_success '--include-root-refs omits dangling symrefs' '\n \t)\n '\n \n+test_expect_success '--include-root-refs with --merged' '\n+\tcat >expect <<-\\EOF &&\n+\tHEAD\n+\tORIG_HEAD\n+\tEOF\n+\ttest_when_finished \"git update-ref -d ORIG_HEAD\" &&\n+\tgit update-ref ORIG_HEAD main &&\n+\tgit for-each-ref --format=\"%(refname)\" --include-root-refs \\\n+\t\t--merged=main \"*HEAD\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'filtering with --points-at' '\n \tcat >expect <<-\\EOF &&\n \trefs/heads/main\n-- \n2.56.0\n\n\n"},{"id":"555378","messageId":"atWxNvZn3CYTBuFp@pks.im","threadId":"66538","inReplyTo":"20261017200405.2332914-1-andy.koppe@gmail.com","subject":"Re: [PATCH v2] refs/files: keep root refs out of loose ref cache","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-19T05:57:10Z","receivedAt":"2026-10-19T05:57:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Oct 17, 2026 at 09:04:00PM +0100, Andy Koppe wrote:\n> Changes since v1:\n> - Move create_root_ref_cache() below get_loose_ref_cache() for a smaller\n>   diff, and tweak its comment.\n> - Correct the documentation, help text and refs.h comment for\n>   `--include-root-refs`, which claimed that it lists pseudorefs.\n> - Consider alternative approaches in the log message.\n\nIt would help to also get a range-diff relative to v1 so that a reviewer\ncan see what exxactly has changed between the two versions.\n\n> diff --git a/Documentation/for-each-ref-options.adoc b/Documentation/for-each-ref-options.adoc\n> index 54e2fa95c2..b93ffbd74d 100644\n> --- a/Documentation/for-each-ref-options.adoc\n> +++ b/Documentation/for-each-ref-options.adoc\n> @@ -73,7 +73,8 @@ TAB %(refname)`.\n>  \tusing the same rules as _<pattern>_ above.\n>  \n>  `--include-root-refs`::\n> -\tList root refs (`HEAD` and pseudorefs) apart from regular refs.\n> +\tList root refs such as `HEAD` and `ORIG_HEAD` in addition to regular\n> +\trefs. (The pseudorefs `FETCH_HEAD` and `MERGE_HEAD` are not included.)\n>  \n>  `--start-after=<marker>`::\n>      Allows paginating the output by skipping references up to and including the\n\nThese changes make sense, but they are unrelated to the fix. They should\nthus go into a separate commit.\n\nPatrick \n\n"},{"id":"555461","messageId":"20261019203427.2637569-1-andy.koppe@gmail.com","threadId":"66538","inReplyTo":"20261014153208.2194727-1-andy.koppe@gmail.com","subject":"[PATCH v3] refs/files: keep root refs out of loose ref cache","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2026-10-19T20:34:20Z","receivedAt":"2026-10-19T20:34:20Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"With the files backend, root refs such as HEAD or ORIG_HEAD are only\nadded to the loose ref cache if REFS_FOR_EACH_INCLUDE_ROOT_REFS is\npassed when the cache is created. If the cache has already been created\nfor an earlier ref iterator, root refs are silently omitted. Conversely,\nonce they have been added, any later iterator also yields them, whether\nor not they were requested. This has been the case since d0f00c1ac1\n(refs: introduce `refs_for_each_include_root_refs()`, 2024-02-23).\n\nFor example, `git for-each-ref --include-root-refs --merged=main` omits\nroot refs that are merged into main, because looking up the given commit\nloads the replace refs, which populates the loose ref cache first. The\nreftable backend is not affected, as it filters root refs for each\niterator rather than caching them.\n\nFix this by keeping the root refs out of the loose ref cache. Instead,\nread them into a separate, temporary ref cache whenever an iterator is\nrequested with REFS_FOR_EACH_INCLUDE_ROOT_REFS, and overlay an iterator\nover that cache on the loose ref iterator. The cache is freed along with\nthe iterator.\n\nAlternatively, the root refs could always be added to the loose ref\ncache and skipped during iteration unless requested. But that would mean\nreading them in every command that uses the loose ref cache, even though\nonly two call sites actually request them. Also, keeping the root ref\ncache for reuse by later iterators is not worthwhile, because those two\ncall sites only request the root refs once per command.\n\nSigned-off-by: Andy Koppe <andy.koppe@gmail.com>\n---\nChanges from v1:\n- Amend commit message for clarity and to discuss alternatives.\n- Move new create_root_ref_cache() below get_loose_ref_cache() because\n  its comment references the loose ref cache and because it makes for\n  a cleaner diff. Tighten up the comment.\n\nChanges from v2:\n- Drop fixes to documentation of REFS_FOR_EACH_INCLUDE_ROOT_REFS and\n  --include-root-refs added in v2. I'll create a separate patch with\n  those as well as other fixes for outdated uses of \"pseudoref\".\n\nThe range diff shows the changes from v1, because they're of more\ninterest than dropping the documentation changes I'd added in v2.\n\n refs/files-backend.c           | 37 ++++++++++++++++++++++++++--------\n t/t6302-for-each-ref-filter.sh | 12 +++++++++++\n 2 files changed, 41 insertions(+), 8 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 71628550f2..9d1e527114 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -481,8 +481,7 @@ static void add_root_refs(struct files_ref_store *refs,\n \tfor_each_root_ref(refs, fill_root_ref, &data);\n }\n \n-static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n-\t\t\t\t\t     unsigned int flags)\n+static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)\n {\n \tif (!refs->loose) {\n \t\tstruct ref_dir *dir;\n@@ -499,9 +498,6 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n \n \t\tdir = get_ref_dir(refs->loose->root);\n \n-\t\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS)\n-\t\t\tadd_root_refs(refs, dir);\n-\n \t\t/*\n \t\t * Add an incomplete entry for \"refs/\" (to be filled\n \t\t * lazily):\n@@ -511,6 +507,19 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n \treturn refs->loose;\n }\n \n+/*\n+ * Root refs are kept out of the loose ref cache because ref iterators must\n+ * only include them when requested with REFS_FOR_EACH_INCLUDE_ROOT_REFS.\n+ */\n+static struct ref_cache *create_root_ref_cache(struct files_ref_store *refs)\n+{\n+\tstruct ref_cache *cache = create_ref_cache(&refs->base, NULL);\n+\n+\tcache->root->flag &= ~REF_INCOMPLETE;\n+\tadd_root_refs(refs, get_ref_dir(cache->root));\n+\treturn cache;\n+}\n+\n static int read_ref_internal(struct ref_store *ref_store, const char *refname,\n \t\t\t     struct object_id *oid, struct strbuf *referent,\n \t\t\t     unsigned int *type, int *failure_errno, int skip_packed_refs)\n@@ -1004,6 +1013,7 @@ struct files_ref_iterator {\n \tstruct ref_iterator base;\n \n \tstruct ref_iterator *iter0;\n+\tstruct ref_cache *root_refs;\n \tstruct repository *repo;\n \tunsigned int flags;\n };\n@@ -1053,6 +1063,7 @@ static void files_ref_iterator_release(struct ref_iterator *ref_iterator)\n \tstruct files_ref_iterator *iter =\n \t\t(struct files_ref_iterator *)ref_iterator;\n \tref_iterator_free(iter->iter0);\n+\tfree_ref_cache(iter->root_refs);\n }\n \n static struct ref_iterator_vtable files_ref_iterator_vtable = {\n@@ -1068,6 +1079,7 @@ static struct ref_iterator *files_ref_iterator_begin(\n {\n \tstruct files_ref_store *refs;\n \tstruct ref_iterator *loose_iter, *packed_iter, *overlay_iter;\n+\tstruct ref_cache *root_refs = NULL;\n \tstruct files_ref_iterator *iter;\n \tstruct ref_iterator *ref_iterator;\n \tunsigned int required_flags = REF_STORE_READ;\n@@ -1094,9 +1106,17 @@ static struct ref_iterator *files_ref_iterator_begin(\n \t * disk, and re-reads it if not.\n \t */\n \n-\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),\n+\tloose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),\n \t\t\t\t\t      prefix, ref_store->repo, 1);\n \n+\tif (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {\n+\t\troot_refs = create_root_ref_cache(refs);\n+\t\tloose_iter = overlay_ref_iterator_begin(\n+\t\t\t\tcache_ref_iterator_begin(root_refs, prefix,\n+\t\t\t\t\t\t\t ref_store->repo, 0),\n+\t\t\t\tloose_iter);\n+\t}\n+\n \t/*\n \t * The packed-refs file might contain broken references, for\n \t * example an old version of a reference that points at an\n@@ -1118,6 +1138,7 @@ static struct ref_iterator *files_ref_iterator_begin(\n \tref_iterator = &iter->base;\n \tbase_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable);\n \titer->iter0 = overlay_iter;\n+\titer->root_refs = root_refs;\n \titer->repo = ref_store->repo;\n \titer->flags = flags;\n \n@@ -1490,7 +1511,7 @@ static int should_pack_refs(struct files_ref_store *refs,\n \tif (limit < 16)\n \t\tlimit = 16;\n \n-\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL,\n+\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,\n \t\t\t\t\trefs->base.repo, 0);\n \twhile ((ret = ref_iterator_advance(iter)) == ITER_OK) {\n \t\tif (should_pack_ref(refs, &iter->ref, opts))\n@@ -1530,7 +1551,7 @@ static int files_optimize(struct ref_store *ref_store,\n \n \tpacked_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR, &err);\n \n-\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL,\n+\titer = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,\n \t\t\t\t\trefs->base.repo, 0);\n \twhile ((ok = ref_iterator_advance(iter)) == ITER_OK) {\n \t\t/*\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 7f060d97bf..a2fc257f98 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -96,6 +96,18 @@ test_expect_success '--include-root-refs omits dangling symrefs' '\n \t)\n '\n \n+test_expect_success '--include-root-refs with --merged' '\n+\tcat >expect <<-\\EOF &&\n+\tHEAD\n+\tORIG_HEAD\n+\tEOF\n+\ttest_when_finished \"git update-ref -d ORIG_HEAD\" &&\n+\tgit update-ref ORIG_HEAD main &&\n+\tgit for-each-ref --format=\"%(refname)\" --include-root-refs \\\n+\t\t--merged=main \"*HEAD\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'filtering with --points-at' '\n \tcat >expect <<-\\EOF &&\n \trefs/heads/main\n\nRange-diff against v1:\n1:  5db169a16e ! 1:  49f2b11aad refs/files: keep root refs out of loose ref cache\n    @@ Commit message\n     \n         With the files backend, root refs such as HEAD or ORIG_HEAD are only\n         added to the loose ref cache if REFS_FOR_EACH_INCLUDE_ROOT_REFS is\n    -    passed to the iteration that creates the cache. If the cache has already\n    -    been created by an earlier iteration, root refs are silently omitted.\n    -    Conversely, once they have been added, any later iteration also yields\n    -    them, whether or not it asked for them. This has been the case since\n    -    d0f00c1ac17 (refs: introduce `refs_for_each_include_root_refs()`).\n    +    passed when the cache is created. If the cache has already been created\n    +    for an earlier ref iterator, root refs are silently omitted. Conversely,\n    +    once they have been added, any later iterator also yields them, whether\n    +    or not they were requested. This has been the case since d0f00c1ac1\n    +    (refs: introduce `refs_for_each_include_root_refs()`, 2024-02-23).\n     \n         For example, `git for-each-ref --include-root-refs --merged=main` omits\n         root refs that are merged into main, because looking up the given commit\n         loads the replace refs, which populates the loose ref cache first. The\n    -    reftable backend is not affected.\n    +    reftable backend is not affected, as it filters root refs for each\n    +    iterator rather than caching them.\n     \n    -    Fix this by keeping the root refs out of the shared loose ref cache.\n    -    Instead, read them into a separate ref cache whenever an iteration asks\n    -    for them, and overlay its iterator on the loose ref iterator. The cache\n    -    is freed when the iteration finishes.\n    +    Fix this by keeping the root refs out of the loose ref cache. Instead,\n    +    read them into a separate, temporary ref cache whenever an iterator is\n    +    requested with REFS_FOR_EACH_INCLUDE_ROOT_REFS, and overlay an iterator\n    +    over that cache on the loose ref iterator. The cache is freed along with\n    +    the iterator.\n    +\n    +    Alternatively, the root refs could always be added to the loose ref\n    +    cache and skipped during iteration unless requested. But that would mean\n    +    reading them in every command that uses the loose ref cache, even though\n    +    only two call sites actually request them. Also, keeping the root ref\n    +    cache for reuse by later iterators is not worthwhile, because those two\n    +    call sites only request the root refs once per command.\n     \n         Signed-off-by: Andy Koppe <andy.koppe@gmail.com>\n     \n    @@ refs/files-backend.c: static void add_root_refs(struct files_ref_store *refs,\n      \n     -static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n     -\t\t\t\t\t     unsigned int flags)\n    -+/*\n    -+ * Create a separate ref cache containing only the root refs. These are kept\n    -+ * out of the loose ref cache, which may already exist and is shared by\n    -+ * iterations that should not include them.\n    -+ */\n    -+static struct ref_cache *create_root_ref_cache(struct files_ref_store *refs)\n    -+{\n    -+\tstruct ref_cache *cache = create_ref_cache(&refs->base, NULL);\n    -+\n    -+\tcache->root->flag &= ~REF_INCOMPLETE;\n    -+\tadd_root_refs(refs, get_ref_dir(cache->root));\n    -+\treturn cache;\n    -+}\n    -+\n     +static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)\n      {\n      \tif (!refs->loose) {\n    @@ refs/files-backend.c: static struct ref_cache *get_loose_ref_cache(struct files_\n      \t\t/*\n      \t\t * Add an incomplete entry for \"refs/\" (to be filled\n      \t\t * lazily):\n    +@@ refs/files-backend.c: static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,\n    + \treturn refs->loose;\n    + }\n    + \n    ++/*\n    ++ * Root refs are kept out of the loose ref cache because ref iterators must\n    ++ * only include them when requested with REFS_FOR_EACH_INCLUDE_ROOT_REFS.\n    ++ */\n    ++static struct ref_cache *create_root_ref_cache(struct files_ref_store *refs)\n    ++{\n    ++\tstruct ref_cache *cache = create_ref_cache(&refs->base, NULL);\n    ++\n    ++\tcache->root->flag &= ~REF_INCOMPLETE;\n    ++\tadd_root_refs(refs, get_ref_dir(cache->root));\n    ++\treturn cache;\n    ++}\n    ++\n    + static int read_ref_internal(struct ref_store *ref_store, const char *refname,\n    + \t\t\t     struct object_id *oid, struct strbuf *referent,\n    + \t\t\t     unsigned int *type, int *failure_errno, int skip_packed_refs)\n     @@ refs/files-backend.c: struct files_ref_iterator {\n      \tstruct ref_iterator base;\n      \n-- \n2.56.0\n\n\n"},{"id":"555471","messageId":"CAHWeT-azx6pHvUGy=-Jp+gjbErsJvJ3StTms4twLvoM50+a=zQ@mail.gmail.com","threadId":"66538","inReplyTo":"atWxNvZn3CYTBuFp@pks.im","subject":"Re: [PATCH v2] refs/files: keep root refs out of loose ref cache","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2026-10-19T21:30:01Z","receivedAt":"2026-10-19T21:30:01Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"On Mon, 19 Oct 2026 at 06:57, Patrick Steinhardt wrote:\n> On Sat, Oct 17, 2026 at 09:04:00PM +0100, Andy Koppe wrote:\n> > Changes since v1:\n> > - Move create_root_ref_cache() below get_loose_ref_cache() for a smaller\n> >   diff, and tweak its comment.\n> > - Correct the documentation, help text and refs.h comment for\n> >   `--include-root-refs`, which claimed that it lists pseudorefs.\n> > - Consider alternative approaches in the log message.\n>\n> It would help to also get a range-diff relative to v1 so that a reviewer\n> can see what exxactly has changed between the two versions.\n\nDone with v3, relative to v1.\n\n> > diff --git a/Documentation/for-each-ref-options.adoc b/Documentation/for-each-ref-options.adoc\n> > index 54e2fa95c2..b93ffbd74d 100644\n> > --- a/Documentation/for-each-ref-options.adoc\n> > +++ b/Documentation/for-each-ref-options.adoc\n> > @@ -73,7 +73,8 @@ TAB %(refname)`.\n> >       using the same rules as _<pattern>_ above.\n> >\n> >  `--include-root-refs`::\n> > -     List root refs (`HEAD` and pseudorefs) apart from regular refs.\n> > +     List root refs such as `HEAD` and `ORIG_HEAD` in addition to regular\n> > +     refs. (The pseudorefs `FETCH_HEAD` and `MERGE_HEAD` are not included.)\n> >\n> >  `--start-after=<marker>`::\n> >      Allows paginating the output by skipping references up to and including the\n>\n> These changes make sense, but they are unrelated to the fix. They should\n> thus go into a separate commit.\n\nDropped from v3. I'll put them into a separate patch, along with other\nfixes for outdated uses of \"pseudoref\".\n\nAndy\n\n"},{"id":"555501","messageId":"atcNOMxaz2WxDIA1@pks.im","threadId":"66538","inReplyTo":"20261019203427.2637569-1-andy.koppe@gmail.com","subject":"Re: [PATCH v3] refs/files: keep root refs out of loose ref cache","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-20T06:42:00Z","receivedAt":"2026-10-20T06:42:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 19, 2026 at 09:34:20PM +0100, Andy Koppe wrote:\n> With the files backend, root refs such as HEAD or ORIG_HEAD are only\n> added to the loose ref cache if REFS_FOR_EACH_INCLUDE_ROOT_REFS is\n> passed when the cache is created. If the cache has already been created\n> for an earlier ref iterator, root refs are silently omitted. Conversely,\n> once they have been added, any later iterator also yields them, whether\n> or not they were requested. This has been the case since d0f00c1ac1\n> (refs: introduce `refs_for_each_include_root_refs()`, 2024-02-23).\n> \n> For example, `git for-each-ref --include-root-refs --merged=main` omits\n> root refs that are merged into main, because looking up the given commit\n> loads the replace refs, which populates the loose ref cache first. The\n> reftable backend is not affected, as it filters root refs for each\n> iterator rather than caching them.\n> \n> Fix this by keeping the root refs out of the loose ref cache. Instead,\n> read them into a separate, temporary ref cache whenever an iterator is\n> requested with REFS_FOR_EACH_INCLUDE_ROOT_REFS, and overlay an iterator\n> over that cache on the loose ref iterator. The cache is freed along with\n> the iterator.\n> \n> Alternatively, the root refs could always be added to the loose ref\n> cache and skipped during iteration unless requested. But that would mean\n> reading them in every command that uses the loose ref cache, even though\n> only two call sites actually request them. Also, keeping the root ref\n> cache for reuse by later iterators is not worthwhile, because those two\n> call sites only request the root refs once per command.\n\nThanks for spelling out alternative implementations and why we didn't\nuse them.\n\n> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>\n> ---\n> Changes from v1:\n> - Amend commit message for clarity and to discuss alternatives.\n> - Move new create_root_ref_cache() below get_loose_ref_cache() because\n>   its comment references the loose ref cache and because it makes for\n>   a cleaner diff. Tighten up the comment.\n> \n> Changes from v2:\n> - Drop fixes to documentation of REFS_FOR_EACH_INCLUDE_ROOT_REFS and\n>   --include-root-refs added in v2. I'll create a separate patch with\n>   those as well as other fixes for outdated uses of \"pseudoref\".\n> \n> The range diff shows the changes from v1, because they're of more\n> interest than dropping the documentation changes I'd added in v2.\n\nThanks for this. By the way, I would recommend to have a look at b4,\nwhich handles all of the bookkeeping for you.\n\nIn any case, this patch looks good to me. Thanks!\n\nPatrick\n\n"},{"id":"555642","messageId":"xmqqik2vss86.fsf@gitster.g","threadId":"66538","inReplyTo":"atcNOMxaz2WxDIA1@pks.im","subject":"Re: [PATCH v3] refs/files: keep root refs out of loose ref cache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-21T02:27:37Z","receivedAt":"2026-10-21T02:27:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> Changes from v2:\n>> - Drop fixes to documentation of REFS_FOR_EACH_INCLUDE_ROOT_REFS and\n>>   --include-root-refs added in v2. I'll create a separate patch with\n>>   those as well as other fixes for outdated uses of \"pseudoref\".\n>> \n>> The range diff shows the changes from v1, because they're of more\n>> interest than dropping the documentation changes I'd added in v2.\n>\n> Thanks for this. By the way, I would recommend to have a look at b4,\n> which handles all of the bookkeeping for you.\n\nPerhaps it cuts both ways.  Can you tell b4 to do the flexible thing\nthe author did in this case?\n\n> In any case, this patch looks good to me. Thanks!\n\nThanks for writing and reviewing.  Let me mark the topic for 'next'.\n\n"},{"id":"555650","messageId":"athPaILS_RqqMUap@pks.im","threadId":"66538","inReplyTo":"xmqqik2vss86.fsf@gitster.g","subject":"Re: [PATCH v3] refs/files: keep root refs out of loose ref cache","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-21T05:36:40Z","receivedAt":"2026-10-21T05:36:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Oct 20, 2026 at 07:27:37PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> >> Changes from v2:\n> >> - Drop fixes to documentation of REFS_FOR_EACH_INCLUDE_ROOT_REFS and\n> >>   --include-root-refs added in v2. I'll create a separate patch with\n> >>   those as well as other fixes for outdated uses of \"pseudoref\".\n> >> \n> >> The range diff shows the changes from v1, because they're of more\n> >> interest than dropping the documentation changes I'd added in v2.\n> >\n> > Thanks for this. By the way, I would recommend to have a look at b4,\n> > which handles all of the bookkeeping for you.\n> \n> Perhaps it cuts both ways.  Can you tell b4 to do the flexible thing\n> the author did in this case?\n\nNo, it's more restricted in this case. But mistakes like this wouldn't\nhappen anymore if one were to use it right from the start, so it would\nnot even have been necessary to have the flexibility :)\n\nPatrick\n\n"}]}