Re: [PATCH v5 3/3] core: convert build-time USE_NSEC into runtime core.useNanosec

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 31, 2026, 14:47 UTC
Message-ID: <apWUGfzQxx7vArpo@pks.im>
In-Reply-To: <CALnO6CCNwXC1_PUCTWEU-HXBk+W+sBGqn7Sr8D=ZHW3Mxcu20g@mail.gmail.com>

On Mon, Aug 31, 2026 at 08:57:49AM -0400, D. Ben Knoble wrote:

> On Mon, Aug 31, 2026 at 5:27 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Sun, Aug 30, 2026 at 08:27:13PM -0400, D. Ben Knoble wrote:
> > > On Sun, Aug 30, 2026 at 5:15 PM Junio C Hamano <gitster@pobox.com> wrote:

[snip]

Show 34 quoted lines

> > > I would happily prove that at least none of our existing tests fail
> > > with core.useNanosec=true, but I'm not really sure how to shove
> > > configuration into every test invocation of git. Even if we could, I'm
> > > not sure we necessarily want to add another CI job for that (though
> > > that's a separate matter).
> > >
> > > In particular, (among others) I have not received any concrete comments
> > for
> > >
> > > > Comments welcome: I haven't touched any tests; I saw a bunch of hits
> > for
> > > > "git grep racy t" but wasn't sure how to fit this particular change in,
> > > > especially since it won't be equally valid on all systems? Advice
> > > > welcome.
> > >
> > > so if there's at least a way to exercise this path on all the tests on
> > > my system (which should support it), that would probably be a good
> > > thing.
> >
> > Yeah, I simply don't have a good answer here. It's messy, and I'm not a
> > fan of the current direction of `repo_config_values()` because nobody
> > has yet stepped up to untangle it from `the_repository`. I gave it a
> > quick shot at one point in time, but the result was messy at best
> > because of how we populate it via `repo_config(git_default_config)`.
> >
> 
> I took a quick look (being unfamiliar), and yeah, it does seem pretty
> tangled. I suppose one way to go about it would be to have repo_config()
> forward the repository argument through configset_iter to the config_fn_t
> callback? I'm a bit surprised (leaving aside how pervasive the_repository
> is otherwise) to see it doesn't already do that :)
> 
> Is that the approach you took? Or, where else did you feel hung up about
> the resulting code? Just wondering.

Yeah, that's what I did. I don't quite remember what was awkward about it though. It might've been that callers have to be aware whether a repo is initialized, and whether it has all info to be able to read its own configuration? Or I was trying to make it auto-lazy-load or something like that, but because our config subsystem is so fragile that led to lots of weird edge cases.

Sometimes I really wonder whether that whole caching layer is even worth it. We already store the configuration as part of the configset, so caching the parsed values probably does not buy us a lot. For some very central aspects like the bareness of a repository or the location of the worktree it probably even makes sense, but for everything else... I dunno. By now I feel like it would make more sense there to find localized solutions specific to subsystems instead of having that one big global struct that has weird semantics.

Show 9 quoted lines

> > In any case, if we see that your changes interact badly with some edge
> > cases that we don't currently have on our radar then we can still
> > refactor the series and move the value into `struct repo_settings`
> > instead, as that structure works alright with different repositories.
> 
> This sounds reasonable to me. If nothing else, this series might become
> good motivation to untangle repo_config_values…
> 
> Sounds to me like we might be ready for 'next'?

Works for me.

Patrick