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

On Wed, Aug 19, 2026 at 4:24 AM Patrick Steinhardt <[email protected]> wrote:
>
> On Tue, Aug 18, 2026 at 10:59:47AM -0400, D. Ben Knoble wrote:
> > Racy Git problems persist today, manifesting themselves in the
> > performance of commands like "git diff" in new worktrees [1]. We have
> > long had a build knob "USE_NSEC" to tell Git to use in-core nanosecond
> > precision when available, which mitigates most if not all racy issues,
> > but most builds we know about it don't use it. In part, that's because
>
> s/about it/about/

Thanks; fixed locally.

> > diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc
> > index 340329edc3..33104444ab 100644
> > --- a/Documentation/config/core.adoc
> > +++ b/Documentation/config/core.adoc
> > @@ -118,6 +118,12 @@ core.trustctime::
> >       crawlers and some backup systems).
> >       See linkgit:git-update-index[1]. True by default.
> >
> > +core.useNanosec::
> > +     If true, use nanosecond precision for ctime and mtime
> > +     comparisions between the index and the working tree (if Git
> > +     was compiled to store it).
> > +     See link:technical/racy-git.html[Racy Git]. False by default.
>
> Should we mentino here that this may not be safe on all platforms and/or
> filesystems, in addition to linking to racy-hit?

Yeah, a brief mention here is probably warranted.

> And do we really want to link to the HTML page here? The user may be
> reading a manpage, so doing so feels a bit weird to me.

See a variation on the grep done in patch 1; we link lots of HTML
documentation in our manuals (including when rendered to manpage
format).

AFAICT, the idea is that we produce manual pages for commands and a
few other "special" documents; we produce HTML of everything. So there
isn't a good non-HTML link target for, e.g., the Racy Git document. In
particular, even "git help" doesn't know about Racy Git. I have a
script [1] that opens files out of "git --html-path", so that provides
one way to access the Racy Git document (aside: neither of my
systems---Homebrew macOS or Portage Gentoo---install anything into
"git --info-path", so that would not make a good link target even if I
knew how to write it). Patch 1/3 makes it easier to get the correct
link in the manual for folks who can click links in their terminal
emulators (or copy-paste).

[1]: https://github.com/benknoble/Dotfiles/tree/master/links/bin/git-doc
(with completion!
https://github.com/benknoble/Dotfiles/tree/master/links/zshfns/_git_doc)

TBH, I am not sure what other folks do for these HTML links in
manuals. As I mention in patch 1, the Homebrew links are broken. If
you know about "git --html-path" you can find the documents, or use
the Git SCM website's rendered versions.

Anyway, this is the current "normal" style for linking, I think.

> > diff --git a/environment.c b/environment.c
> > index 6676e6f5ae..c7f6b801f4 100644
> > --- a/environment.c
> > +++ b/environment.c
> > @@ -571,6 +571,13 @@ int git_default_core_config(const char *var, const char *value,
> >               return 0;
> >       }
> >
> > +#ifndef NO_NSEC
> > +     if (!strcmp(var, "core.usenanosec")) {
> > +             cfg->use_nanosec = git_config_bool(var, value);
> > +             return 0;
> > +     }
> > +#endif
>
> Do we want to omit a warning in case the config is enabled and we have
> NO_SEC set? Or would that be too obnoxious?

I would say that can always be done later ;) Perhaps it should be
better documented, though, so let me try that, too.

>
> > @@ -769,6 +776,9 @@ void repo_config_values_init(struct repo_config_values *cfg)
> >       cfg->ignore_case = 0;
> >       cfg->trust_executable_bit = 1;
> >       cfg->has_symlinks = platform_has_symlinks();
> > +#ifndef NO_NSEC
> > +     cfg->use_nanosec = 0;
> > +#endif
>
> Can't we set this unconditionally? The respective field exists
> unconditionally, too.

Yep, see reply to Junio.

> > diff --git a/read-cache.c b/read-cache.c
> > index 6c449f393d..31888f77ee 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -353,12 +353,18 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
> >  static int is_racy_stat(const struct index_state *istate,
> >                       const struct stat_data *sd)
> >  {
> > +#ifndef NO_NSEC
> > +     int use_nsec = repo_config_values(istate->repo)->use_nanosec;
> > +#endif
> > +
> >       return (istate->timestamp.sec &&
> > -#ifdef USE_NSEC
> > -              /* nanosecond timestamped files can also be racy! */
> > -             (istate->timestamp.sec < sd->sd_mtime.sec ||
> > -              (istate->timestamp.sec == sd->sd_mtime.sec &&
> > -               istate->timestamp.nsec <= sd->sd_mtime.nsec))
> > +#ifndef NO_NSEC
> > +             /* nanosecond timestamped files can also be racy! */
> > +             use_nsec
> > +             ? (istate->timestamp.sec < sd->sd_mtime.sec ||
> > +                (istate->timestamp.sec == sd->sd_mtime.sec &&
> > +                 istate->timestamp.nsec <= sd->sd_mtime.nsec))
> > +             : istate->timestamp.sec <= sd->sd_mtime.sec
> >  #else
> >               istate->timestamp.sec <= sd->sd_mtime.sec
> >  #endif
>
> I think this would be a bit more readable if we had a single NO_NSEC
> block.

I'm not sure what "single block" means here, but I think the plan (see
reply to Junio) is to make this more readable by not needing
pre-processor directives at all.

[snip]

> There's one more site in "builtin/update-index.c" where we mention
> USE_NSEC that wasn't updated as part of this patch.

Oh, did I miss one? The only spot I saw in builtin/update-index.c that
mentions USE_NSEC is a comment that I'm sure patch 3 updated. Maybe
you were thinking of that, or maybe you know of something I left out?
(That is, locally on this branch, "git grep USE_NSEC" returns one hit
in Documentation/RelNotes/2.5.0.adoc.)

Thanks!

-- 
D. Ben Knoble