Re: [PATCH v3 3/3] core: convert build-time USE_NSEC into runtime core.useNanosec
- From
D. Ben Knoble <ben.knoble@gmail.com>
- Date
- Aug 19, 2026, 13:09 UTC
- Message-ID
- <CALnO6CDgfT+VXaBqSmStB8vNOwBpr5XMjvmxhMdc7v-ma-YwXg@mail.gmail.com>
- In-Reply-To
- <aoVoJ3Ijoaj3u64e@pks.im>
On Wed, Aug 19, 2026 at 4:24 AM Patrick Steinhardt <ps@pks.im> wrote:
Show 9 quoted lines
> > 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.
Show 16 quoted lines
> > 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.
Show 17 quoted lines
> > 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.
Show 11 quoted lines
> > > @@ -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.
Show 31 quoted lines
> > 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