From: Ben Knoble Date: Tue, 01 Sep 2026 00:35:20 GMT Subject: Re: [PATCH v5 3/3] core: convert build-time USE_NSEC into runtime core.useNanosec Message-ID: In-Reply-To: > Le 31 août 2026 à 18:06, Patrick Steinhardt 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 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 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!