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 12:40:35PM -0400, Jeff King wrote:
> On Mon, Jun 15, 2026 at 03:56:53PM +0200, Patrick Steinhardt wrote:
[snip]
> I'd expect the ref database config (like the ref format) to be read not
> through the regular config subsystem, but via read_repository_format()
> and friends. And while that does build on the regular config code, it
> should never enable includes at all. So includeIf.onbranch:foo.path is
> just another uninteresting config key to it.

This feels rather painful though, as we'd now have to do this for every
single backend that we know about. Also, I think not enabling includes
is an overly broad fix: there isn't any reason why "includeif.gitdir"
and all the other conditions shouldn't apply. We really only want to
disable "onbranch".

[snip]
> > The consequence is that we have recursion:
> > 
> >   1. We call `get_main_ref_store()`.
> > 
> >   2. We don't yet have a reference store, so we call `ref_store_init()`.
> > 
> >   3. We parse the configuration required for the reference store.
> > 
> >   4. We eventually end up in `include_by_branch()`.
> > 
> >   5. We have already configured the reference storage format, so we end
> >      up calling `get_main_ref_store()` again.
> 
> Ah, the culprit seems to be ref_store_init() calling into the regular
> config parser via repo_settings_get_log_all_ref_updates(). But that
> feels weird to me. Either:
> 
>   1. It is application config that should not be something we need to
>      load in order to initialize the backend. We could lazy-load it
>      later, or rely on higher level code to set the option.

I actually tried lazy-loading, but I found it to be quite painful
overall, as the above setting isn't the only one we use. The reftable
backend for example has a bunch of additional settings that it reads.

We could of course start lazy-loading all of these. But that may not
work for future backends that really _need_ to parse some configuration
at initiation time.

>   2. It is crucial to the ref backend functioning, in which case we
>      ought to be reading it alongside core.repositoryFormatVersion, etc.

I think ideally, we'd have a way to read the repository configuration
that explicitly disables parsing includes. We could for example extend
`struct config_options` to have a new "ignore_refdb" toggle then
explicitly use that in the reference backends.

I'll give that a try.

Patrick