Re: [PATCH v2 4/5] scalar unregister: stop FSMonitor daemon

From: Derrick Stolee <derrickstolee@github.com>
Date: Aug 17, 2022, 14:39 UTC
Message-ID: <2cbbd732-b9e7-fe8b-9c77-f86a856d06c7@github.com>
In-Reply-To: <fc4aa1fde31fa0726cde2c1d4e41f3f140fff6f6.1660694290.git.gitgitgadget@gmail.com>

On 8/16/2022 7:58 PM, Johannes Schindelin via GitGitGadget wrote:

Show 5 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> Especially on Windows, we will need to stop that daemon, just in case
> that the directory needs to be removed (the daemon would otherwise hold
> a handle to that directory, preventing it from being deleted).

Show 20 quoted lines

> +static int stop_fsmonitor_daemon(void)
> +{
> +	assert(fsmonitor_ipc__is_supported());
> +
> +	if (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING)
> +		return run_git("fsmonitor--daemon", "stop", NULL);
> +
> +	return 0;
> +}
> +
>  static int register_dir(void)
>  {
>  	if (add_or_remove_enlistment(1))
> @@ -281,6 +291,9 @@ static int unregister_dir(void)
>  	if (add_or_remove_enlistment(0))
>  		res = error(_("could not remove enlistment"));
>  
> +	if (fsmonitor_ipc__is_supported() && stop_fsmonitor_daemon() < 0)
> +		res = error(_("could not stop the FSMonitor daemon"));
> +

One thing that is interesting about 'scalar unregister' is that it does not change config values. At that point, we don't know which config values are valuable to keep or not because the user may have set them before 'scalar register', or otherwise liked the config options.

Here, the reason to stop the daemon is so we unlock the ability to delete the directory on Windows.

Should this become part of cmd_delete() instead of unregister_dir()? Or, do we think that users would opt to run 'scalar unregister' before trying to delete their directory manually?

Thanks, -Stolee