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

[Ben Knoble <ben.knoble@gmail.com>]

> Le 31 août 2026 à 18:06, Patrick Steinhardt <ps@pks.im> a écrit :
> 
> 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.

Interesting, yeah. I can’t say I’m too motivated to look into this further, personally, but the config system seems fairly complex…

Maybe I’ll take a tour of it one day though, depending on the next itch I scratch ;)

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

Thanks!