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

>> 	/* we do not care if pipe_command() succeeds or not */
>> 	(void) pipe_command(...);
>> 
>>         /*
>>          * we check ourselves if we do have a usable daemon 
>>          * and that is the authoritative answer.  we were asked
>>          * to ensure that usable daemon exists, and we answer
>>          * if we do or don't.
>>          */
>> 	if (fsmonitor_ipc__get_state() != IPC_STATE__LISTENING)
>> 		res = error(...);
>> 
>> may be more true to the spirit of the code.
>
> This is an unintentional artifact of some minor refactoring of the original
> versions in 'microsoft/git'. Previously [1], there was no
> 'fsmonitor_ipc__get_state()' check before calling 'git fsmonitor--daemon
> start', so we'd expect failures whenever FSMonitor was already running. To
> avoid making that 'pipe_command()' call when FSMonitor was already running,
> I added an earlier call to 'fsmonitor_ipc__get_state()'. But, because I
> didn't remove the later check, the code now implies that 'pipe_command()'
> may give us "false negatives" (that is, fail but still manage to start the
> FSMonitor).
>
> I left the extraneous check in to be overly cautious, but realistically I
> don't actually expect 'git fsmonitor--daemon start' to give us any false
> positives or negatives. The code should reflect that:
>
> 	int res = 0;
> 	if (fsmonitor_ipc__is_supported() &&
> 	    fsmonitor_ipc__get_state() != IPC_STATE__LISTENING) {
> 		struct strbuf err = STRBUF_INIT;
> 		struct child_process cp = CHILD_PROCESS_INIT;
>
> 		/* Try to start the FSMonitor daemon */
> 		cp.git_cmd = 1;
> 		strvec_pushl(&cp.args, "fsmonitor--daemon", "start", NULL);
> 		if (pipe_command(&cp, NULL, 0, NULL, 0, &err, 0))
> 			res = error(_("could not start the FSMonitor daemon: %s"),
> 				    err.buf);
>
> 		strbuf_release(&err);
> 	}
>
> 	return res;
>
> I'll re-roll with this shortly.