On 8/16/2022 7:58 PM, Matthew John Cheetham via GitGitGadget wrote:
> +#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
> + /*
> + * Enable the built-in FSMonitor on supported platforms.
> + */
> + { "core.fsmonitor", "true" },
> +#endif
> + if (fsmonitor_ipc__is_supported() && start_fsmonitor_daemon())
> + return error(_("could not start the FSMonitor daemon"));
> +
I initially worried if fsmonitor_ipc__is_supported() could use some
run-time information to detect if FS Monitor is supported (say, existence
of a network share or something). However, that implementation is
currently defined as a constant depending on
HAVE_FSMONITOR_DAEMON_BACKEND.
The reason I was worried is that we could enable core.fsmonitor=true based
on the compile-time macro, but then avoid starting the daemon based on the
run-time results. If we get into this state, would the user's 'git status'
calls start complaining about the core.fsmonitor=true config because it is
not supported?
The most future-proof thing to do might be to move the config write out of
the set_recommended_config() and into start_fsmonitor_daemon(). Perhaps
rename it to enable_fsmonitor() so it can fail due to writing the config
_or_ for starting the daemon. The error message would change, then, too.
Or maybe I'm making a mountain out of a mole hill and what exists here is
perfectly fine.
> +test_lazy_prereq BUILTIN_FSMONITOR '
> + git version --build-options | grep -q "feature:.*fsmonitor--daemon"
> +'
It looks like we already have a FSMONITOR_DAEMON prereq in test-lib.sh.
Should we use that instead?
Thanks,
-Stolee
