On Sun, Jun 21, 2026 at 05:12:11PM -0400, Jeff King wrote:

On Fri, Jun 19, 2026 at 08:25:42AM +0200, Patrick Steinhardt wrote:

quoted

On Thu, Jun 18, 2026 at 12:40:35PM -0400, Jeff King wrote:

quoted

On Mon, Jun 15, 2026 at 03:56:53PM +0200, Patrick Steinhardt wrote:

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.

Yes, obviously there's some true chicken-and-egg issues if there are
config keys that are needed to initialize the backend. But I think there
are many that are not needed immediately (e.g., because they relate only
to writes, not reads) but still block loading.

For example, try this:

  git init
  git config core.logallrefupdates false
  git config includeIf.onbranch:main.path alt-config
  git config -f .git/alt-config core.logallrefupdates true
  git commit --allow-empty -qm foo

  echo "git-config => $(git config core.logallrefupdates)"
  echo "reflog => $(git reflog show)"

git-config will report the value as true, but git-commit will not
respect it. But this used to work! Back when onbranch was added, we'd
create the reflog. Bisecting turns up eafb126456 (environment: stop
storing "core.logAllRefUpdates" globally, 2024-09-12), which makes
sense. That commit pushed the config read down into the ref
initialization function, which created the chicken-and-egg.

Now the config shown above is a bit silly, and I don't expect anybody to
do it in real life. But what worries me is two-fold:

  1. There are some magic variables that just won't work with onbranch
     includes, but the user doesn't necessarily know what they are.

  2. We try to cache the results of config reads. Is it possible for an
     "early" request like this to cache a state that skipped the
     onbranch include, and then we use that state to look up other
     unrelated variables? Or could we see a partially completed state in
     the cache when we lookup a ref variable?

     I'm not sure. The actual backend lookups use the uncached
     repo_config() interface (and in your series here, explicitly
     disables the use of refs during that read). But the
     core.logallrefupdates lookup uses the cached version, and I think
     there are others (some of which happen deep under the hood
     through library calls, like calc_shared_perm()).

I tried to construct a few cases that might tickle this behavior, but
couldn't come up with one. But I have a nagging feeling that we are
mostly getting lucky on some of the ordering, and a seemingly unrelated
change could have bad effects.

Sorry, I know that's kind of vague and hand-wavy.

I'm not sure I have a specific recommendation for a direction. It just
feels like we're piling up hacks to avoid infinite recursion without a
clear model of what config is read when. I guess if I could suggest
anything, it would be that ref backends initialize themselves to do
reads while loading as little config as possible, and then perhaps load
additional config through the non-caching repo_config() path.

Yeah, I thought more about this issue over the weekend and kind of got
to the same conclusion. Sure, the current version where we explicitly
handle the exclusion of "onbranch" conditions is at least less awkward.
But I have to agree that it's still not the right fix, as it doesn't
really solve the root issue.

Taking a step back: all the values that we currently parse are only
relevant when writing new refs. So in theory it should be possible to
lazy-load all of them on the first write. This should be rather easy to
do for the "files" backend. But for the "reftable" backend this will
result in a large refactoring because we require the configuration when
constructing the reftable stack.

That's kind of misdesigned though: the reftable stack shouldn't really
care about write options when being constructed. What it needs to know
about is the expected hash ID, and any optional stuff like the onreload
callback. The write options should then be passed by the caller when we
actually perform a write.

I'll iterate a bit on this idea and will see where I get. I really
shouldn't have opened this can of worms.

Thanks!

Patrick