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

On Thu, Aug 13, 2026 at 05:40:31PM -0400, D. Ben Knoble wrote:

On Tue, Aug 11, 2026 at 12:26 PM Ben Knoble [off-list ref] wrote:

quoted

quoted

Le 10 août 2026 à 08:44, Patrick Steinhardt [off-list ref] a écrit :
On Mon, Aug 10, 2026 at 08:27:51AM -0400, D. Ben Knoble wrote:
[snip]

quoted

Back down to being on-par with original code. So that's good. The next
version will include some variant that reads a struct member instead
of going through repo_config_get_bool().

But which? Reading the private_ member is obviously wrong; I suppose
I'm supposed to use repo_config_values() there. Or, rework the series
to put this member in repo_settings. I think I originally assumed that
struct is for things that are settings that aren't configured by
git-config, but… now I'm not sure. Looking at prepare_repo_settings()
shows lots of repo_cfg_*() calls. So I think I see how to adapt to
using repo_settings,

Patrick, Junio, and Tian had a brief discussion in
[ref] about the split creating confusion. I don't
really want to wait for it to settle to land this change, but we might
want to work together on identifying the best path forward for
core.useNanosec :)

I don't suppose it really matters to me which struct I put the member
in. As I said, v2 will definitely fix the hot path lookup here. Just a
matter of input on which struct we want to use this time, I guess.

I think `repo_config_values()` is the modern variant that we're slowly
migrating stuff into. But that struct only works with `the_repository`,
so the question is whether we ever use "core.useNsec" for a different
repository. My hunch would be yes, for example when recusing into
submodules, but I'm not sure.

Patrick

Thanks. I’m working on control-flow analysis to see what kinds of repo values end up there. Of course I’ll also run the test suite and so on with the repo_config_values change. But the analysis will take some time.

Ok, CI run: https://github.com/benknoble/git/actions/runs/31701945211.
This demonstrates that nothing our test suite does across the many CI
configurations ends up where with a non-the_repository-repository
(ahem).

I have been working on control-flow analysis by hand in my Git time
this week. It's of the form "Z calls Y calls X …" until we can see
what the repository that's (eventually) fed to repo_config_values()
here in is_racy_stat() is. My notes are one node per line, which
indentation showing callee relationships. Some lines are pointers to
other nodes to avoid duplicating work.

With that in mind, filtering out the pointer nodes, I've analyzed 214
nodes in the graph. If I'm lucky, I'm approaching the halfway mark,
but I somewhat doubt it.

But since CI shows things work… I'd rather not continue the analysis
if we're satisfied for now. (Esp. since that will give me more Git
time back for reviewing ;) It being outside-of-work time, I only have
so much of it.)

A few other related things:
- Some of the edges of the graph appear to be public libgit.a
interfaces. That means we can't guarantee that only the_repository is
used.
- On a related note, I don't know how large the current "must only use
the_repository" (e.g., via repo_config_values()) surface area is right
now. Based on the partial analysis I mentioned above, this feels like
it's introducing (or at least contributing to) a rather large surface
area. So, this change might make it more critical to resolve the
limitation mentioned in the other thread. OTOH, I don't think this
change is likely to represent the only pervasive the_repository-only
limitation, and I'm afraid it will never land if it must be
the_repository clean (unless repo_settings is the_repository clean and
we decide that's an acceptable place for this member).

So, idk. If we're happy with the CI run + use of repo_config_values()
overall, I can send a v2 shortly (in next 24h), I think.

Thoughts? Strong opinions?

No strong opinions from my side, other than that we should stop
converting everything to `repo_config_values()` until we have a plan for
how to make it work with repositories other than `the_repository`.

I don't feel like holding this series in hostage though, so if your
analysis and the test suite both say that this is probably fine then we
may want to pursue it. Or we just use a global variable for it for the
time being and then wait until the `repo_config_values()` dust has
settled.

Patrick