Re: [PATCH v5 06/30] fsmonitor--daemon: add a built-in fsmonitor daemon

> 
> On Fri, Feb 11 2022, Jeff Hostetler via GitGitGadget wrote:
> 
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> Create a built-in file system monitoring daemon that can be used by
>> the existing `fsmonitor` feature (protocol API and index extension)
>> to improve the performance of various Git commands, such as `status`.
>>
>> The `fsmonitor--daemon` feature builds upon the `Simple IPC` API and
>> provides an alternative to hook access to existing fsmonitors such
>> as `watchman`.
>>
>> This commit merely adds the new command without any functionality.
>>

>> +
>> +#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
>> +
>> +int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
>> +{
>> +	const char *subcmd;
>> +
>> +	struct option options[] = {
>> +		OPT_END()
>> +	};
>> +
>> +	git_config(git_default_config, NULL);
>> +
>> +	argc = parse_options(argc, argv, prefix, options,
>> +			     builtin_fsmonitor__daemon_usage, 0);
>> +	if (argc != 1)
>> +		usage_with_options(builtin_fsmonitor__daemon_usage, options);
>> +	subcmd = argv[0];
>> +
>> +	die(_("Unhandled subcommand '%s'"), subcmd);
>> +}
>> +
>> +#else
>> +int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
>> +{
>> +	struct option options[] = {
>> +		OPT_END()
>> +	};
>> +
>> +	if (argc == 2 && !strcmp(argv[1], "-h"))
>> +		usage_with_options(builtin_fsmonitor__daemon_usage, options);
>> +
>> +	die(_("fsmonitor--daemon not supported on this platform"));
>> +}
>> +#endif
> 
> 
> I brought this up in another thread in how this series interacts with
> another, but this patch below on top of "seen" would allow you to catch
> parse_options() BUGs on Linux, even if you don't have a no-OSX
> non-Windows backend yet:
> 	
> 	diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
> 	index 591433e897d..62c0b1d486b 100644
> 	--- a/builtin/fsmonitor--daemon.c
> 	+++ b/builtin/fsmonitor--daemon.c
> 	@@ -18,7 +18,6 @@ static const char * const builtin_fsmonitor__daemon_usage[] = {
> 	 	NULL
> 	 };
> 	
> 	-#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
> 	 /*
> 	  * Global state loaded from config.
> 	  */
> 	@@ -63,6 +62,7 @@ static int fsmonitor_config(const char *var, const char *value, void *cb)
> 	
> 	 	return git_default_config(var, value, cb);
> 	 }
> 	+#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
> 	
> 	 /*
> 	  * Acting as a CLIENT.
> 	@@ -1492,6 +1492,8 @@ static int try_to_start_background_daemon(void)
> 	 	}
> 	 }
> 	
> 	+#endif
> 	+
> 	 int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
> 	 {
> 	 	const char *subcmd;
> 	@@ -1532,6 +1534,7 @@ int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
> 	 		return -1;
> 	 	}
> 	
> 	+#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
> 	 	if (!strcmp(subcmd, "start"))
> 	 		return !!try_to_start_background_daemon();
> 	
> 	@@ -1543,20 +1546,8 @@ int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
> 	
> 	 	if (!strcmp(subcmd, "status"))
> 	 		return !!do_as_client__status();
> 	-
> 	 	die(_("Unhandled subcommand '%s'"), subcmd);
> 	-}
> 	-
> 	 #else
> 	-int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
> 	-{
> 	-	struct option options[] = {
> 	-		OPT_END()
> 	-	};
> 	-
> 	-	if (argc == 2 && !strcmp(argv[1], "-h"))
> 	-		usage_with_options(builtin_fsmonitor__daemon_usage, options);
> 	-
> 	 	die(_("fsmonitor--daemon not supported on this platform"));
> 	-}
> 	 #endif
> 	+}
> 
> I.e. we can be a less zealous when setting the ifdef boundaries, and
> it's actually less code as well.
>