Thread (66 messages) flat view 66 messages, 5 authors, 6d ago

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

From: Ben Knoble <hidden>
Date: 2026-08-14 11:29:35

Le 14 août 2026 à 07:07, Patrick Steinhardt [off-list ref] a écrit :

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

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
Makes sense. I should have also mentioned that, of the nodes I’ve analyzed so far, they all terminate in a path that uses the_repository (some intermediate nodes are also exposed, though, as written previously).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help