Re: [PATCH v2 7/8] refs: fix recursing `get_main_ref_store()` with "onbranch" config

[Justin Tobler <jltobler@gmail.com>]

On 26/06/18 07:59AM, Patrick Steinhardt wrote:
> On Wed, Jun 17, 2026 at 01:41:40PM -0500, Justin Tobler wrote:
> > Is this really the best signal to indicate that a repository ref store
> > has not been initialized? Temporarily setting the storage format to
> > REF_STORAGE_FORMAT_UNKNOWN feels rather awkward and suggests to me that
> > `include_by_branch()` probably shouldn't be using it to begin with if
> > its not reliable.
> 
> True, but we don't really have a better signal to the best of my
> knowledge. Ideally, we'd be able to use the existence `r->refs_private`
> as signal. But that doesn't really work as the reference database is
> lazily constructed, and the recursion happens in the exact function that
> would construct it in the first place. And there indeed are cases where
> reading the configuration is the first caller of `get_main_ref_store()`.

Ok, my first thought was also whether we could use the existence of the
ref store as a signal, but I guess that won't work here.

> My first internal iteration tried to make this non-lazily constructed so
> that we can use it as a proper signal. But that led to a bunch of
> problems where we now parsed configuration way earlier than we currently
> do, and that in turn led to all kinds of errors. I was able to fix all
> of those errors except one: we expect `git config set` to work in a
> misconfigured repository so that the user can fix the misconfig without
> having to manually edit the Git configuration files. But when
> constructing the refdb eagerly we will die early in such cases.
> 
> We could again work around that issue, but that unfortunately evolved
> into a proper mess that I eventually discarded as unworkable. I think
> this is an inherent design flaw: constructing the refdb requires us to
> be able to parse the configuration, but constructing the configuration
> may require us to construct the refdb. So this awkwardness is built into
> Git's design, unfortunately.
> 
> So I'd really love to have a better signal, as I fully agree that the
> above workaround is nothing more but a hack. But I'm just not sure what
> that signal would be. And this version here does exactly what we want:
> we honor "onbranch" conditionals in all cases, except when constructing
> the main reference store. Even if it's ugly.

Could we embed an `initialized` boolean in `struct ref_store` that gets
set when the ref store is properly initialized and use that as a signal
instead? I'm not sure how complex introducing this would be though.

-Justin