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

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jun 18, 2026 at 09:15:00AM -0500, Justin Tobler wrote:
> 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.

We could, but I'm not sure what that would really buy us. It would
basically be one more bit of state that we have to track going forward,
and thus one more source of inconsistencies.

Patrick