From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 20 Oct 2026 06:42:00 GMT
Subject: Re: [PATCH v3] refs/files: keep root refs out of loose ref cache
Message-ID: <atcNOMxaz2WxDIA1@pks.im>
In-Reply-To: <20261019203427.2637569-1-andy.koppe@gmail.com>

On Mon, Oct 19, 2026 at 09:34:20PM +0100, Andy Koppe wrote:
> With the files backend, root refs such as HEAD or ORIG_HEAD are only
> added to the loose ref cache if REFS_FOR_EACH_INCLUDE_ROOT_REFS is
> passed when the cache is created. If the cache has already been created
> for an earlier ref iterator, root refs are silently omitted. Conversely,
> once they have been added, any later iterator also yields them, whether
> or not they were requested. This has been the case since d0f00c1ac1
> (refs: introduce `refs_for_each_include_root_refs()`, 2024-02-23).
> 
> For example, `git for-each-ref --include-root-refs --merged=main` omits
> root refs that are merged into main, because looking up the given commit
> loads the replace refs, which populates the loose ref cache first. The
> reftable backend is not affected, as it filters root refs for each
> iterator rather than caching them.
> 
> Fix this by keeping the root refs out of the loose ref cache. Instead,
> read them into a separate, temporary ref cache whenever an iterator is
> requested with REFS_FOR_EACH_INCLUDE_ROOT_REFS, and overlay an iterator
> over that cache on the loose ref iterator. The cache is freed along with
> the iterator.
> 
> Alternatively, the root refs could always be added to the loose ref
> cache and skipped during iteration unless requested. But that would mean
> reading them in every command that uses the loose ref cache, even though
> only two call sites actually request them. Also, keeping the root ref
> cache for reuse by later iterators is not worthwhile, because those two
> call sites only request the root refs once per command.

Thanks for spelling out alternative implementations and why we didn't
use them.

> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
> ---
> Changes from v1:
> - Amend commit message for clarity and to discuss alternatives.
> - Move new create_root_ref_cache() below get_loose_ref_cache() because
>   its comment references the loose ref cache and because it makes for
>   a cleaner diff. Tighten up the comment.
> 
> Changes from v2:
> - Drop fixes to documentation of REFS_FOR_EACH_INCLUDE_ROOT_REFS and
>   --include-root-refs added in v2. I'll create a separate patch with
>   those as well as other fixes for outdated uses of "pseudoref".
> 
> The range diff shows the changes from v1, because they're of more
> interest than dropping the documentation changes I'd added in v2.

Thanks for this. By the way, I would recommend to have a look at b4,
which handles all of the bookkeeping for you.

In any case, this patch looks good to me. Thanks!

Patrick