Thread (72 messages) flat view 72 messages, 5 authors, 7d ago

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

From: Jeff King <hidden>
Date: 2026-09-02 07:26:48

On Tue, Sep 01, 2026 at 08:36:22AM -0400, D. Ben Knoble wrote:
quoted
This hunk made me wonder if we even need to do any build-time magic here
at all. If your platform doesn't support nanosecond stat entries, then
you're probably not going to ask for core.usenanosec in the first place.
But if you do, I think the code still works; we fake the entries as "0",
so they'd always yield a racy tie, just as if core.usenanosec was
disabled.
At first I thought you meant we fake the cfg->use_nanosec as 0; it
took me a moment to realize you mean that we fake the index entries as
0ns. (That is what you mean, right?)
Yeah, sorry to be unclear. I meant that we still have this code:

  #ifdef NO_NSEC
  #define ST_CTIME_NSEC(st) 0
  #define ST_MTIME_NSEC(st) 0

So we are free to pretend that stat nsecs exist and compare them.
In that case, yes, I suppose it would work. Might be confusing in a
debugger to see use_nanosec set and checked, though?
Maybe. Looking at the list of NO_NSEC flags in config.mak.uname, I
suspect it's a pretty small population in the first place.
Hm, yeah. I haven't thought too hard either about the interactions
where you toggle core.usenanosec on and off, but giving it an initial
think they seem fine. Unlike this hypothetical case, when it's off we
don't look at the ns fields, so I don't think we end up with any false
negatives.

And in this hypothetical, by restricting the option parsing we avoid
reading the ns values on unsupported platforms, I think?
I'd have to double check, but I thought that even without USE_NSEC (and
thus even with your new core.usenanosec off) we still read and store the
nanosecond values in the index, as long as the platform supports it (and
if not, then we use those "0" fallback values).

So they are always there in the index. I guess the same odd sequence
applies even today. If you:

  1. Build with NO_NSEC and get "fake" 0 values in your index.

  2. Re-build without NO_NSEC, and also enable USE_NSEC. Now we get
     _real_ values when we stat(), and compare them to the fake values
     in the index.

Now the index values appear up to 1-second older than they actually are.
Which could maybe yield a racy miss of an update? Probably not for
stat-freshness (where we want an exact match), but maybe for some index
vs entry racy-git comparison. I didn't think that hard about it, because
at some point this sequence is just kind of insane.
The build-time conditional _does_ mean that if your distro (e.g.)
provides a NO_NSEC build, you can't access the core.usenanosec feature
without compiling yourself, even if your platform supports it. But I
haven't thought too hard either about what it looks like to get rid of
NO_NSEC entirely, and I'm not totally sure if that's a good idea.
You couldn't access it even if core.usenanosec is supported in the
build, because your fake nsec values would all be "0" and it's
effectively a noop. ;)

My suggestion wasn't really about supporting more cases, but just about
making the code simpler by having one less #ifdef. But like I said
earlier, we can't get rid of the NO_NSEC knob entirely, so it's probably
not worth worrying about the one #ifdef either way.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help