Subject: Re: [PATCH 06/23] fsmonitor-settings: stub in platform-specific incompatibility checking

From: Derrick Stolee <[email-masked]>
Date: Thu, 24 Feb 2022 10:05:39 -0500

On 2/15/2022 10:59 AM, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <[email-masked]>
> 
> Extend generic incompatibility checkout with platform-specific
> mechanism.  Stub in Win32 version.
> 
> In the existing fsmonitor-settings code we have a way to mark
> types of repos as incompatible with fsmonitor (whether via the
> hook and ipc APIs).  For example, we do this for bare repos,
> since there are no files to watch.
> 
> Extend this exclusion mechanism for platfor-specific reasons.

s/platfor-specific/platform-specific/

> +# If your platform has os-specific ways to tell if a repo is incompatible with
> +# fsmonitor (whether the hook or ipc daemon version), set FSMONITOR_OS_SETTINGS
> +# to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.c`
> +# that implements the `fsm_os_settings__*()` routines.

nit: It's just a comment, but I think "OS" and "IPC" should probably
be capitalized.

> +#ifdef HAVE_FSMONITOR_OS_SETTINGS
> +	{
> +		enum fsmonitor_reason reason;
> +
> +		reason = fsm_os__incompatible(r);
> +		if (reason != FSMONITOR_REASON_ZERO) {

A naming nit about FSMONITOR_REASON_ZERO. It seems named ZERO
on purpose so a non-zero reason signals incompatibility. That
would mean you could use

		if (reason)

here. Alternatively, FSMONITOR_REASON_COMPATIBLE would signal
the meaning better here:

		if (reason != FSMONITOR_REASON_COMPATIBLE)

> +			set_incompatible(r, reason);
> +			return 1;
> +		}

Thanks,
-Stolee