From: Ben Knoble Date: Wed, 02 Sep 2026 11:45:38 GMT Subject: Re: [PATCH v6 3/3] core: convert build-time USE_NSEC into runtime core.useNanosec Message-ID: In-Reply-To: <20260902072646.GB70165@coredump.intra.peff.net> > Le 2 sept. 2026 à 03:26, Jeff King a écrit : > > On Tue, Sep 01, 2026 at 08:36:22AM -0400, D. Ben Knoble wrote: > >>> 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 Right on. Always good to find myself nodding along with your explanations :)