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

From: Jeff King <peff@peff.net>
Date: Sep 2, 2026, 07:26 UTC
Message-ID: <20260902072646.GB70165@coredump.intra.peff.net>
In-Reply-To: <CALnO6CAZYvnv3fMWkU0pqY+XN3ncBqVav49ZEvzV0LMtmkYO0Q@mail.gmail.com>

On Tue, Sep 01, 2026 at 08:36:22AM -0400, D. Ben Knoble wrote:

Show 10 quoted lines

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

Show 8 quoted lines

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

Show 5 quoted lines

> 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