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

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <derrickstolee@github.com> writes:

> 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?

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.