Thread (34 messages) 34 messages, 4 authors, 2022-08-19

Re: [PATCH v2 3/5] scalar: enable built-in FSMonitor on `register`

From: Junio C Hamano <hidden>
Date: 2022-08-17 15:54:33

Derrick Stolee [off-list ref] writes:
On 8/16/2022 7:58 PM, Matthew John Cheetham via GitGitGadget wrote:
quoted
+#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?
Ah, I didn't consider the possibility where the user uses the
configuration to say "enable it if you are able, but it is OK if you
cannot".  Whether the "is supported" is dynamic or compiled-in, that
may be a valid issue to consider.  An easy way out may be to declare
that the value "true" for "core.fsmonitor" variable means exactly
that, i.e. the user asks to run it, but it is not an error if it
cannot run.

A variant that may need slightly more work would be to introduce a
separate value (perhaps "when-able") that means that, while keeping
the "true" to mean "run the built-in one, or error out to let me
know otherwise" as before.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help