Subject: Re: [PATCH 1/7] fsmonitor: rebase with master

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 15 Feb 2024 14:49:22 +0100

On Thu, Feb 15, 2024 at 10:29:32AM +0000, Eric DeCosta via GitGitGadget wrote:
> From: Eric DeCosta <[email-masked]>
> 
> rebased with master, and resolved conflicts

It would be a lot more useful if you adopted the original phrasing of
the commit:

```
fsmonitor: prepare to share code between Mac OS and Linux 

Linux and Mac OS can share some of the code originally developed for Mac OS.

Mac OS and Linux can share fsm-ipc-unix.c and fsm-settings-unix.c

Signed-off-by: Eric DeCosta <[email-masked]>
```

Depending on whether or not you have made significant changes during the
rebase I'd also convert the trailers to:

Patch-originally-by: Eric DeCoste <[email-masked]>
Signed-off-by: Marzieh Esipreh <[email-masked]>

Furthermore, I think it would be useful if this commit was split up even
further than it already is. Having a preparatory patch that moves around
shareable code is a different topic than introducing the code skeleton
for Linux support, and as far as I can see the shared code does not end
up requiring anything from the new "*-linux.c" files.

Patrick

> Signed-off-by: Eric DeCosta <[email-masked]>
> ---
>  compat/fsmonitor/fsm-health-linux.c    | 24 ++++++++++
>  compat/fsmonitor/fsm-ipc-darwin.c      | 57 +----------------------
>  compat/fsmonitor/fsm-ipc-linux.c       |  1 +
>  compat/fsmonitor/fsm-ipc-unix.c        | 53 +++++++++++++++++++++
>  compat/fsmonitor/fsm-settings-darwin.c | 64 +-------------------------
>  compat/fsmonitor/fsm-settings-linux.c  |  1 +
>  compat/fsmonitor/fsm-settings-unix.c   | 61 ++++++++++++++++++++++++
>  7 files changed, 142 insertions(+), 119 deletions(-)
>  create mode 100644 compat/fsmonitor/fsm-health-linux.c
>  create mode 100644 compat/fsmonitor/fsm-ipc-linux.c
>  create mode 100644 compat/fsmonitor/fsm-ipc-unix.c
>  create mode 100644 compat/fsmonitor/fsm-settings-linux.c
>  create mode 100644 compat/fsmonitor/fsm-settings-unix.c
> 
> diff --git a/compat/fsmonitor/fsm-health-linux.c b/compat/fsmonitor/fsm-health-linux.c
> new file mode 100644
> index 00000000000..b9f709e8548
> --- /dev/null
> +++ b/compat/fsmonitor/fsm-health-linux.c
> @@ -0,0 +1,24 @@
> +#include "cache.h"
> +#include "config.h"
> +#include "fsmonitor.h"
> +#include "fsm-health.h"
> +#include "fsmonitor--daemon.h"
> +
> +int fsm_health__ctor(struct fsmonitor_daemon_state *state)
> +{
> +	return 0;
> +}
> +
> +void fsm_health__dtor(struct fsmonitor_daemon_state *state)
> +{
> +	return;
> +}
> +
> +void fsm_health__loop(struct fsmonitor_daemon_state *state)
> +{
> +	return;
> +}
> +
> +void fsm_health__stop_async(struct fsmonitor_daemon_state *state)
> +{
> +}
> diff --git a/compat/fsmonitor/fsm-ipc-darwin.c b/compat/fsmonitor/fsm-ipc-darwin.c
> index 6f3a95410cc..4c3c92081ee 100644
> --- a/compat/fsmonitor/fsm-ipc-darwin.c
> +++ b/compat/fsmonitor/fsm-ipc-darwin.c
> @@ -1,56 +1 @@
> -#include "git-compat-util.h"
> -#include "config.h"
> -#include "gettext.h"
> -#include "hex.h"
> -#include "path.h"
> -#include "repository.h"
> -#include "strbuf.h"
> -#include "fsmonitor-ll.h"
> -#include "fsmonitor-ipc.h"
> -#include "fsmonitor-path-utils.h"
> -
> -static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, "fsmonitor--daemon.ipc")
> -
> -const char *fsmonitor_ipc__get_path(struct repository *r)
> -{
> -	static const char *ipc_path = NULL;
> -	git_SHA_CTX sha1ctx;
> -	char *sock_dir = NULL;
> -	struct strbuf ipc_file = STRBUF_INIT;
> -	unsigned char hash[GIT_MAX_RAWSZ];
> -
> -	if (!r)
> -		BUG("No repository passed into fsmonitor_ipc__get_path");
> -
> -	if (ipc_path)
> -		return ipc_path;
> -
> -
> -	/* By default the socket file is created in the .git directory */
> -	if (fsmonitor__is_fs_remote(r->gitdir) < 1) {
> -		ipc_path = fsmonitor_ipc__get_default_path();
> -		return ipc_path;
> -	}
> -
> -	git_SHA1_Init(&sha1ctx);
> -	git_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));
> -	git_SHA1_Final(hash, &sha1ctx);
> -
> -	repo_config_get_string(r, "fsmonitor.socketdir", &sock_dir);
> -
> -	/* Create the socket file in either socketDir or $HOME */
> -	if (sock_dir && *sock_dir) {
> -		strbuf_addf(&ipc_file, "%s/.git-fsmonitor-%s",
> -					sock_dir, hash_to_hex(hash));
> -	} else {
> -		strbuf_addf(&ipc_file, "~/.git-fsmonitor-%s", hash_to_hex(hash));
> -	}
> -	free(sock_dir);
> -
> -	ipc_path = interpolate_path(ipc_file.buf, 1);
> -	if (!ipc_path)
> -		die(_("Invalid path: %s"), ipc_file.buf);
> -
> -	strbuf_release(&ipc_file);
> -	return ipc_path;
> -}
> +#include "fsm-ipc-unix.c"
> diff --git a/compat/fsmonitor/fsm-ipc-linux.c b/compat/fsmonitor/fsm-ipc-linux.c
> new file mode 100644
> index 00000000000..4c3c92081ee
> --- /dev/null
> +++ b/compat/fsmonitor/fsm-ipc-linux.c
> @@ -0,0 +1 @@
> +#include "fsm-ipc-unix.c"
> diff --git a/compat/fsmonitor/fsm-ipc-unix.c b/compat/fsmonitor/fsm-ipc-unix.c
> new file mode 100644
> index 00000000000..eb25123fa12
> --- /dev/null
> +++ b/compat/fsmonitor/fsm-ipc-unix.c
> @@ -0,0 +1,53 @@
> +#include "cache.h"
> +#include "config.h"
> +#include "hex.h"
> +#include "strbuf.h"
> +#include "fsmonitor.h"
> +#include "fsmonitor-ipc.h"
> +#include "fsmonitor-path-utils.h"
> +
> +static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, "fsmonitor--daemon.ipc")
> +
> +const char *fsmonitor_ipc__get_path(struct repository *r)
> +{
> +	static const char *ipc_path = NULL;
> +	git_SHA_CTX sha1ctx;
> +	char *sock_dir = NULL;
> +	struct strbuf ipc_file = STRBUF_INIT;
> +	unsigned char hash[GIT_MAX_RAWSZ];
> +
> +	if (!r)
> +		BUG("No repository passed into fsmonitor_ipc__get_path");
> +
> +	if (ipc_path)
> +		return ipc_path;
> +
> +
> +	/* By default the socket file is created in the .git directory */
> +	if (fsmonitor__is_fs_remote(r->gitdir) < 1) {
> +		ipc_path = fsmonitor_ipc__get_default_path();
> +		return ipc_path;
> +	}
> +
> +	git_SHA1_Init(&sha1ctx);
> +	git_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));
> +	git_SHA1_Final(hash, &sha1ctx);
> +
> +	repo_config_get_string(r, "fsmonitor.socketdir", &sock_dir);
> +
> +	/* Create the socket file in either socketDir or $HOME */
> +	if (sock_dir && *sock_dir) {
> +		strbuf_addf(&ipc_file, "%s/.git-fsmonitor-%s",
> +					sock_dir, hash_to_hex(hash));
> +	} else {
> +		strbuf_addf(&ipc_file, "~/.git-fsmonitor-%s", hash_to_hex(hash));
> +	}
> +	free(sock_dir);
> +
> +	ipc_path = interpolate_path(ipc_file.buf, 1);
> +	if (!ipc_path)
> +		die(_("Invalid path: %s"), ipc_file.buf);
> +
> +	strbuf_release(&ipc_file);
> +	return ipc_path;
> +}
> diff --git a/compat/fsmonitor/fsm-settings-darwin.c b/compat/fsmonitor/fsm-settings-darwin.c
> index a3825906351..14baf9f0603 100644
> --- a/compat/fsmonitor/fsm-settings-darwin.c
> +++ b/compat/fsmonitor/fsm-settings-darwin.c
> @@ -1,63 +1 @@
> -#include "git-compat-util.h"
> -#include "config.h"
> -#include "fsmonitor-ll.h"
> -#include "fsmonitor-ipc.h"
> -#include "fsmonitor-settings.h"
> -#include "fsmonitor-path-utils.h"
> -
> - /*
> - * For the builtin FSMonitor, we create the Unix domain socket for the
> - * IPC in the .git directory.  If the working directory is remote,
> - * then the socket will be created on the remote file system.  This
> - * can fail if the remote file system does not support UDS file types
> - * (e.g. smbfs to a Windows server) or if the remote kernel does not
> - * allow a non-local process to bind() the socket.  (These problems
> - * could be fixed by moving the UDS out of the .git directory and to a
> - * well-known local directory on the client machine, but care should
> - * be taken to ensure that $HOME is actually local and not a managed
> - * file share.)
> - *
> - * FAT32 and NTFS working directories are problematic too.
> - *
> - * The builtin FSMonitor uses a Unix domain socket in the .git
> - * directory for IPC.  These Windows drive formats do not support
> - * Unix domain sockets, so mark them as incompatible for the daemon.
> - *
> - */
> -static enum fsmonitor_reason check_uds_volume(struct repository *r)
> -{
> -	struct fs_info fs;
> -	const char *ipc_path = fsmonitor_ipc__get_path(r);
> -	struct strbuf path = STRBUF_INIT;
> -	strbuf_add(&path, ipc_path, strlen(ipc_path));
> -
> -	if (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) {
> -		strbuf_release(&path);
> -		return FSMONITOR_REASON_ERROR;
> -	}
> -
> -	strbuf_release(&path);
> -
> -	if (fs.is_remote ||
> -		!strcmp(fs.typename, "msdos") ||
> -		!strcmp(fs.typename, "ntfs")) {
> -		free(fs.typename);
> -		return FSMONITOR_REASON_NOSOCKETS;
> -	}
> -
> -	free(fs.typename);
> -	return FSMONITOR_REASON_OK;
> -}
> -
> -enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc)
> -{
> -	enum fsmonitor_reason reason;
> -
> -	if (ipc) {
> -		reason = check_uds_volume(r);
> -		if (reason != FSMONITOR_REASON_OK)
> -			return reason;
> -	}
> -
> -	return FSMONITOR_REASON_OK;
> -}
> +#include "fsm-settings-unix.c"
> diff --git a/compat/fsmonitor/fsm-settings-linux.c b/compat/fsmonitor/fsm-settings-linux.c
> new file mode 100644
> index 00000000000..14baf9f0603
> --- /dev/null
> +++ b/compat/fsmonitor/fsm-settings-linux.c
> @@ -0,0 +1 @@
> +#include "fsm-settings-unix.c"
> diff --git a/compat/fsmonitor/fsm-settings-unix.c b/compat/fsmonitor/fsm-settings-unix.c
> new file mode 100644
> index 00000000000..d16dca89416
> --- /dev/null
> +++ b/compat/fsmonitor/fsm-settings-unix.c
> @@ -0,0 +1,61 @@
> +#include "fsmonitor.h"
> +#include "fsmonitor-ipc.h"
> +#include "fsmonitor-path-utils.h"
> +
> + /*
> + * For the builtin FSMonitor, we create the Unix domain socket for the
> + * IPC in the .git directory.  If the working directory is remote,
> + * then the socket will be created on the remote file system.  This
> + * can fail if the remote file system does not support UDS file types
> + * (e.g. smbfs to a Windows server) or if the remote kernel does not
> + * allow a non-local process to bind() the socket.  (These problems
> + * could be fixed by moving the UDS out of the .git directory and to a
> + * well-known local directory on the client machine, but care should
> + * be taken to ensure that $HOME is actually local and not a managed
> + * file share.)
> + *
> + * FAT32 and NTFS working directories are problematic too.
> + *
> + * The builtin FSMonitor uses a Unix domain socket in the .git
> + * directory for IPC.  These Windows drive formats do not support
> + * Unix domain sockets, so mark them as incompatible for the daemon.
> + *
> + */
> +static enum fsmonitor_reason check_uds_volume(struct repository *r)
> +{
> +	struct fs_info fs;
> +	const char *ipc_path = fsmonitor_ipc__get_path(r);
> +	struct strbuf path = STRBUF_INIT;
> +	strbuf_addstr(&path, ipc_path);
> +
> +	if (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) {
> +		free(fs.typename);
> +		strbuf_release(&path);
> +		return FSMONITOR_REASON_ERROR;
> +	}
> +
> +	strbuf_release(&path);
> +
> +	if (fs.is_remote ||
> +		!strcmp(fs.typename, "msdos") ||
> +		!strcmp(fs.typename, "ntfs")) {
> +		free(fs.typename);
> +		return FSMONITOR_REASON_NOSOCKETS;
> +	}
> +
> +	free(fs.typename);
> +	return FSMONITOR_REASON_OK;
> +}
> +
> +enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc)
> +{
> +	enum fsmonitor_reason reason;
> +
> +	if (ipc) {
> +		reason = check_uds_volume(r);
> +		if (reason != FSMONITOR_REASON_OK)
> +			return reason;
> +	}
> +
> +	return FSMONITOR_REASON_OK;
> +}
> -- 
> gitgitgadget
> 
>