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

[Patrick Steinhardt <ps@pks.im>]

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]
> > > 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.

> > 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