fsmonitor: refactor filesystem checks to common interface

Provide a common interface for getting basic filesystem information including filesystem type and whether the filesystem is remote. Refactor existing code for getting basic filesystem info and detecting remote file systems to the new interface. Refactor filesystem checks to leverage new interface. For macOS, error-out if the Unix Domain socket (UDS) file is on a remote filesystem. Signed-off-by: Eric DeCosta <edecosta@mathworks.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Eric DeCosta committed Oct 4, 2022 at 17:32 UTC 508c1a572d633202a220f2902c0d6b4cd1678349
8 files changed +272 -219
Makefile
+1
@@ -2037,6 +2037,7 @@ endif
2037 ifdef FSMONITOR_OS_SETTINGS
2038 COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS
2039 COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o
2040 + COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o
2041 endif
2042
2043 ifeq ($(TCLTK_PATH),)
compat/fsmonitor/fsm-path-utils-darwin.c new
+43
@@ -0,0 +1,43 @@
1 +#include "fsmonitor.h"
2 +#include "fsmonitor-path-utils.h"
3 +#include <sys/param.h>
4 +#include <sys/mount.h>
5 +
6 +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info)
7 +{
8 + struct statfs fs;
9 + if (statfs(path, &fs) == -1) {
10 + int saved_errno = errno;
11 + trace_printf_key(&trace_fsmonitor, "statfs('%s') failed: %s",
12 + path, strerror(saved_errno));
13 + errno = saved_errno;
14 + return -1;
15 + }
16 +
17 + trace_printf_key(&trace_fsmonitor,
18 + "statfs('%s') [type 0x%08x][flags 0x%08x] '%s'",
19 + path, fs.f_type, fs.f_flags, fs.f_fstypename);
20 +
21 + if (!(fs.f_flags & MNT_LOCAL))
22 + fs_info->is_remote = 1;
23 + else
24 + fs_info->is_remote = 0;
25 +
26 + fs_info->typename = xstrdup(fs.f_fstypename);
27 +
28 + trace_printf_key(&trace_fsmonitor,
29 + "'%s' is_remote: %d",
30 + path, fs_info->is_remote);
31 + return 0;
32 +}
33 +
34 +int fsmonitor__is_fs_remote(const char *path)
35 +{
36 + struct fs_info fs;
37 + if (fsmonitor__get_fs_info(path, &fs))
38 + return -1;
39 +
40 + free(fs.typename);
41 +
42 + return fs.is_remote;
43 +}
compat/fsmonitor/fsm-path-utils-win32.c new
+128
@@ -0,0 +1,128 @@
1 +#include "cache.h"
2 +#include "fsmonitor.h"
3 +#include "fsmonitor-path-utils.h"
4 +
5 +/*
6 + * Check remote working directory protocol.
7 + *
8 + * Return -1 if client machine cannot get remote protocol information.
9 + */
10 +static int check_remote_protocol(wchar_t *wpath)
11 +{
12 + HANDLE h;
13 + FILE_REMOTE_PROTOCOL_INFO proto_info;
14 +
15 + h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
16 + FILE_FLAG_BACKUP_SEMANTICS, NULL);
17 +
18 + if (h == INVALID_HANDLE_VALUE) {
19 + error(_("[GLE %ld] unable to open for read '%ls'"),
20 + GetLastError(), wpath);
21 + return -1;
22 + }
23 +
24 + if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
25 + &proto_info, sizeof(proto_info))) {
26 + error(_("[GLE %ld] unable to get protocol information for '%ls'"),
27 + GetLastError(), wpath);
28 + CloseHandle(h);
29 + return -1;
30 + }
31 +
32 + CloseHandle(h);
33 +
34 + trace_printf_key(&trace_fsmonitor,
35 + "check_remote_protocol('%ls') remote protocol %#8.8lx",
36 + wpath, proto_info.Protocol);
37 +
38 + return 0;
39 +}
40 +
41 +/*
42 + * Notes for testing:
43 + *
44 + * (a) Windows allows a network share to be mapped to a drive letter.
45 + * (This is the normal method to access it.)
46 + *
47 + * $ NET USE Z: \\server\share
48 + * $ git -C Z:/repo status
49 + *
50 + * (b) Windows allows a network share to be referenced WITHOUT mapping
51 + * it to drive letter.
52 + *
53 + * $ NET USE \\server\share\dir
54 + * $ git -C //server/share/repo status
55 + *
56 + * (c) Windows allows "SUBST" to create a fake drive mapping to an
57 + * arbitrary path (which may be remote)
58 + *
59 + * $ SUBST Q: Z:\repo
60 + * $ git -C Q:/ status
61 + *
62 + * (d) Windows allows a directory symlink to be created on a local
63 + * file system that points to a remote repo.
64 + *
65 + * $ mklink /d ./link //server/share/repo
66 + * $ git -C ./link status
67 + */
68 +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info)
69 +{
70 + wchar_t wpath[MAX_PATH];
71 + wchar_t wfullpath[MAX_PATH];
72 + size_t wlen;
73 + UINT driveType;
74 +
75 + /*
76 + * Do everything in wide chars because the drive letter might be
77 + * a multi-byte sequence. See win32_has_dos_drive_prefix().
78 + */
79 + if (xutftowcs_path(wpath, path) < 0) {
80 + return -1;
81 + }
82 +
83 + /*
84 + * GetDriveTypeW() requires a final slash. We assume that the
85 + * worktree pathname points to an actual directory.
86 + */
87 + wlen = wcslen(wpath);
88 + if (wpath[wlen - 1] != L'\\' && wpath[wlen - 1] != L'/') {
89 + wpath[wlen++] = L'\\';
90 + wpath[wlen] = 0;
91 + }
92 +
93 + /*
94 + * Normalize the path. If nothing else, this converts forward
95 + * slashes to backslashes. This is essential to get GetDriveTypeW()
96 + * correctly handle some UNC "\\server\share\..." paths.
97 + */
98 + if (!GetFullPathNameW(wpath, MAX_PATH, wfullpath, NULL)) {
99 + return -1;
100 + }
101 +
102 + driveType = GetDriveTypeW(wfullpath);
103 + trace_printf_key(&trace_fsmonitor,
104 + "DriveType '%s' L'%ls' (%u)",
105 + path, wfullpath, driveType);
106 +
107 + if (driveType == DRIVE_REMOTE) {
108 + fs_info->is_remote = 1;
109 + if (check_remote_protocol(wfullpath) < 0)
110 + return -1;
111 + } else {
112 + fs_info->is_remote = 0;
113 + }
114 +
115 + trace_printf_key(&trace_fsmonitor,
116 + "'%s' is_remote: %d",
117 + path, fs_info->is_remote);
118 +
119 + return 0;
120 +}
121 +
122 +int fsmonitor__is_fs_remote(const char *path)
123 +{
124 + struct fs_info fs;
125 + if (fsmonitor__get_fs_info(path, &fs))
126 + return -1;
127 + return fs.is_remote;
128 +}
compat/fsmonitor/fsm-settings-darwin.c
+20 -49
@@ -1,32 +1,10 @@
1 -#include "cache.h"
1 #include "config.h"
3 -#include "repository.h"
4 -#include "fsmonitor-settings.h"
2 #include "fsmonitor.h"
6 -#include <sys/param.h>
7 -#include <sys/mount.h>
3 +#include "fsmonitor-ipc.h"
4 +#include "fsmonitor-settings.h"
5 +#include "fsmonitor-path-utils.h"
6
9 -/*
10 - * [1] Remote working directories are problematic for FSMonitor.
11 - *
12 - * The underlying file system on the server machine and/or the remote
13 - * mount type (NFS, SAMBA, etc.) dictates whether notification events
14 - * are available at all to remote client machines.
15 - *
16 - * Kernel differences between the server and client machines also
17 - * dictate the how (buffering, frequency, de-dup) the events are
18 - * delivered to client machine processes.
19 - *
20 - * A client machine (such as a laptop) may choose to suspend/resume
21 - * and it is unclear (without lots of testing) whether the watcher can
22 - * resync after a resume. We might be able to treat this as a normal
23 - * "events were dropped by the kernel" event and do our normal "flush
24 - * and resync" --or-- we might need to close the existing (zombie?)
25 - * notification fd and create a new one.
26 - *
27 - * In theory, the above issues need to be addressed whether we are
28 - * using the Hook or IPC API.
29 - *
7 + /*
8 * For the builtin FSMonitor, we create the Unix domain socket for the
9 * IPC in the .git directory. If the working directory is remote,
10 * then the socket will be created on the remote file system. This
@@ -38,42 +16,35 @@
16 * be taken to ensure that $HOME is actually local and not a managed
17 * file share.)
18 *
41 - * So (for now at least), mark remote working directories as
42 - * incompatible.
43 - *
44 - *
45 - * [2] FAT32 and NTFS working directories are problematic too.
19 + * FAT32 and NTFS working directories are problematic too.
20 *
21 * The builtin FSMonitor uses a Unix domain socket in the .git
22 * directory for IPC. These Windows drive formats do not support
23 * Unix domain sockets, so mark them as incompatible for the daemon.
24 *
25 */
52 -static enum fsmonitor_reason check_volume(struct repository *r)
26 +static enum fsmonitor_reason check_uds_volume(struct repository *r)
27 {
54 - struct statfs fs;
28 + struct fs_info fs;
29 + const char *ipc_path = fsmonitor_ipc__get_path();
30 + struct strbuf path = STRBUF_INIT;
31 + strbuf_add(&path, ipc_path, strlen(ipc_path));
32
56 - if (statfs(r->worktree, &fs) == -1) {
57 - int saved_errno = errno;
58 - trace_printf_key(&trace_fsmonitor, "statfs('%s') failed: %s",
59 - r->worktree, strerror(saved_errno));
60 - errno = saved_errno;
33 + if (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) {
34 + strbuf_release(&path);
35 return FSMONITOR_REASON_ERROR;
36 }
37
64 - trace_printf_key(&trace_fsmonitor,
65 - "statfs('%s') [type 0x%08x][flags 0x%08x] '%s'",
66 - r->worktree, fs.f_type, fs.f_flags, fs.f_fstypename);
67 -
68 - if (!(fs.f_flags & MNT_LOCAL))
69 - return FSMONITOR_REASON_REMOTE;
38 + strbuf_release(&path);
39
71 - if (!strcmp(fs.f_fstypename, "msdos")) /* aka FAT32 */
72 - return FSMONITOR_REASON_NOSOCKETS;
73 -
74 - if (!strcmp(fs.f_fstypename, "ntfs"))
40 + if (fs.is_remote ||
41 + !strcmp(fs.typename, "msdos") ||
42 + !strcmp(fs.typename, "ntfs")) {
43 + free(fs.typename);
44 return FSMONITOR_REASON_NOSOCKETS;
45 + }
46
47 + free(fs.typename);
48 return FSMONITOR_REASON_OK;
49 }
50
@@ -81,7 +52,7 @@ enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
52 {
53 enum fsmonitor_reason reason;
54
84 - reason = check_volume(r);
55 + reason = check_uds_volume(r);
56 if (reason != FSMONITOR_REASON_OK)
57 return reason;
58
compat/fsmonitor/fsm-settings-win32.c
+2 -170
@@ -1,8 +1,9 @@
1 #include "cache.h"
2 #include "config.h"
3 #include "repository.h"
4 -#include "fsmonitor-settings.h"
4 #include "fsmonitor.h"
5 +#include "fsmonitor-settings.h"
6 +#include "fsmonitor-path-utils.h"
7
8 /*
9 * VFS for Git is incompatible with FSMonitor.
@@ -24,171 +25,6 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
25 return FSMONITOR_REASON_OK;
26 }
27
27 -/*
28 - * Check if monitoring remote working directories is allowed.
29 - *
30 - * By default, monitoring remote working directories is
31 - * disabled. Users may override this behavior in enviroments where
32 - * they have proper support.
33 - */
34 -static int check_config_allowremote(struct repository *r)
35 -{
36 - int allow;
37 -
38 - if (!repo_config_get_bool(r, "fsmonitor.allowremote", &allow))
39 - return allow;
40 -
41 - return -1; /* fsmonitor.allowremote not set */
42 -}
43 -
44 -/*
45 - * Check remote working directory protocol.
46 - *
47 - * Error if client machine cannot get remote protocol information.
48 - */
49 -static int check_remote_protocol(wchar_t *wpath)
50 -{
51 - HANDLE h;
52 - FILE_REMOTE_PROTOCOL_INFO proto_info;
53 -
54 - h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
55 - FILE_FLAG_BACKUP_SEMANTICS, NULL);
56 -
57 - if (h == INVALID_HANDLE_VALUE) {
58 - error(_("[GLE %ld] unable to open for read '%ls'"),
59 - GetLastError(), wpath);
60 - return -1;
61 - }
62 -
63 - if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
64 - &proto_info, sizeof(proto_info))) {
65 - error(_("[GLE %ld] unable to get protocol information for '%ls'"),
66 - GetLastError(), wpath);
67 - CloseHandle(h);
68 - return -1;
69 - }
70 -
71 - CloseHandle(h);
72 -
73 - trace_printf_key(&trace_fsmonitor,
74 - "check_remote_protocol('%ls') remote protocol %#8.8lx",
75 - wpath, proto_info.Protocol);
76 -
77 - return 0;
78 -}
79 -
80 -/*
81 - * Remote working directories are problematic for FSMonitor.
82 - *
83 - * The underlying file system on the server machine and/or the remote
84 - * mount type dictates whether notification events are available at
85 - * all to remote client machines.
86 - *
87 - * Kernel differences between the server and client machines also
88 - * dictate the how (buffering, frequency, de-dup) the events are
89 - * delivered to client machine processes.
90 - *
91 - * A client machine (such as a laptop) may choose to suspend/resume
92 - * and it is unclear (without lots of testing) whether the watcher can
93 - * resync after a resume. We might be able to treat this as a normal
94 - * "events were dropped by the kernel" event and do our normal "flush
95 - * and resync" --or-- we might need to close the existing (zombie?)
96 - * notification fd and create a new one.
97 - *
98 - * In theory, the above issues need to be addressed whether we are
99 - * using the Hook or IPC API.
100 - *
101 - * So (for now at least), mark remote working directories as
102 - * incompatible.
103 - *
104 - * Notes for testing:
105 - *
106 - * (a) Windows allows a network share to be mapped to a drive letter.
107 - * (This is the normal method to access it.)
108 - *
109 - * $ NET USE Z: \\server\share
110 - * $ git -C Z:/repo status
111 - *
112 - * (b) Windows allows a network share to be referenced WITHOUT mapping
113 - * it to drive letter.
114 - *
115 - * $ NET USE \\server\share\dir
116 - * $ git -C //server/share/repo status
117 - *
118 - * (c) Windows allows "SUBST" to create a fake drive mapping to an
119 - * arbitrary path (which may be remote)
120 - *
121 - * $ SUBST Q: Z:\repo
122 - * $ git -C Q:/ status
123 - *
124 - * (d) Windows allows a directory symlink to be created on a local
125 - * file system that points to a remote repo.
126 - *
127 - * $ mklink /d ./link //server/share/repo
128 - * $ git -C ./link status
129 - */
130 -static enum fsmonitor_reason check_remote(struct repository *r)
131 -{
132 - int ret;
133 - wchar_t wpath[MAX_PATH];
134 - wchar_t wfullpath[MAX_PATH];
135 - size_t wlen;
136 - UINT driveType;
137 -
138 - /*
139 - * Do everything in wide chars because the drive letter might be
140 - * a multi-byte sequence. See win32_has_dos_drive_prefix().
141 - */
142 - if (xutftowcs_path(wpath, r->worktree) < 0)
143 - return FSMONITOR_REASON_ERROR;
144 -
145 - /*
146 - * GetDriveTypeW() requires a final slash. We assume that the
147 - * worktree pathname points to an actual directory.
148 - */
149 - wlen = wcslen(wpath);
150 - if (wpath[wlen - 1] != L'\\' && wpath[wlen - 1] != L'/') {
151 - wpath[wlen++] = L'\\';
152 - wpath[wlen] = 0;
153 - }
154 -
155 - /*
156 - * Normalize the path. If nothing else, this converts forward
157 - * slashes to backslashes. This is essential to get GetDriveTypeW()
158 - * correctly handle some UNC "\\server\share\..." paths.
159 - */
160 - if (!GetFullPathNameW(wpath, MAX_PATH, wfullpath, NULL))
161 - return FSMONITOR_REASON_ERROR;
162 -
163 - driveType = GetDriveTypeW(wfullpath);
164 - trace_printf_key(&trace_fsmonitor,
165 - "DriveType '%s' L'%ls' (%u)",
166 - r->worktree, wfullpath, driveType);
167 -
168 - if (driveType == DRIVE_REMOTE) {
169 - trace_printf_key(&trace_fsmonitor,
170 - "check_remote('%s') true",
171 - r->worktree);
172 -
173 - ret = check_remote_protocol(wfullpath);
174 - if (ret < 0)
175 - return FSMONITOR_REASON_ERROR;
176 -
177 - switch (check_config_allowremote(r)) {
178 - case 0: /* config overrides and disables */
179 - return FSMONITOR_REASON_REMOTE;
180 - case 1: /* config overrides and enables */
181 - return FSMONITOR_REASON_OK;
182 - default:
183 - break; /* config has no opinion */
184 - }
185 -
186 - return FSMONITOR_REASON_REMOTE;
187 - }
188 -
189 - return FSMONITOR_REASON_OK;
190 -}
191 -
28 enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
29 {
30 enum fsmonitor_reason reason;
@@ -197,9 +33,5 @@ enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
33 if (reason != FSMONITOR_REASON_OK)
34 return reason;
35
200 - reason = check_remote(r);
201 - if (reason != FSMONITOR_REASON_OK)
202 - return reason;
203 -
36 return FSMONITOR_REASON_OK;
37 }
contrib/buildsystems/CMakeLists.txt
+2
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC)
308 add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND)
309 list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c)
310 list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c)
311 + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c)
312
313 add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
314 list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c)
@@ -315,6 +316,7 @@ if(SUPPORTS_SIMPLE_IPC)
316 add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND)
317 list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c)
318 list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c)
319 + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c)
320
321 add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
322 list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c)
fsmonitor-path-utils.h new
+26
@@ -0,0 +1,26 @@
1 +#ifndef FSM_PATH_UTILS_H
2 +#define FSM_PATH_UTILS_H
3 +
4 +struct fs_info {
5 + int is_remote;
6 + char *typename;
7 +};
8 +
9 +/*
10 + * Get some basic filesystem informtion for the given path
11 + *
12 + * The caller owns the storage that is occupied by fs_info and
13 + * is responsible for releasing it.
14 + *
15 + * Returns -1 on error, zero otherwise.
16 + */
17 +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);
18 +
19 +/*
20 + * Determines if the filesystem that path resides on is remote.
21 + *
22 + * Returns -1 on error, 0 if not remote, 1 if remote.
23 + */
24 +int fsmonitor__is_fs_remote(const char *path);
25 +
26 +#endif
fsmonitor-settings.c
+50
@@ -2,6 +2,7 @@
2 #include "config.h"
3 #include "repository.h"
4 #include "fsmonitor-settings.h"
5 +#include "fsmonitor-path-utils.h"
6
7 /*
8 * We keep this structure defintion private and have getters
@@ -13,6 +14,52 @@ struct fsmonitor_settings {
14 char *hook_path;
15 };
16
17 +/*
18 + * Remote working directories are problematic for FSMonitor.
19 + *
20 + * The underlying file system on the server machine and/or the remote
21 + * mount type dictates whether notification events are available at
22 + * all to remote client machines.
23 + *
24 + * Kernel differences between the server and client machines also
25 + * dictate the how (buffering, frequency, de-dup) the events are
26 + * delivered to client machine processes.
27 + *
28 + * A client machine (such as a laptop) may choose to suspend/resume
29 + * and it is unclear (without lots of testing) whether the watcher can
30 + * resync after a resume. We might be able to treat this as a normal
31 + * "events were dropped by the kernel" event and do our normal "flush
32 + * and resync" --or-- we might need to close the existing (zombie?)
33 + * notification fd and create a new one.
34 + *
35 + * In theory, the above issues need to be addressed whether we are
36 + * using the Hook or IPC API.
37 + *
38 + * So (for now at least), mark remote working directories as
39 + * incompatible unless 'fsmonitor.allowRemote' is true.
40 + *
41 + */
42 +#ifdef HAVE_FSMONITOR_OS_SETTINGS
43 +static enum fsmonitor_reason check_remote(struct repository *r)
44 +{
45 + int allow_remote = -1; /* -1 unset, 0 not allowed, 1 allowed */
46 + int is_remote = fsmonitor__is_fs_remote(r->worktree);
47 +
48 + switch (is_remote) {
49 + case 0:
50 + return FSMONITOR_REASON_OK;
51 + case 1:
52 + repo_config_get_bool(r, "fsmonitor.allowremote", &allow_remote);
53 + if (allow_remote < 1)
54 + return FSMONITOR_REASON_REMOTE;
55 + else
56 + return FSMONITOR_REASON_OK;
57 + default:
58 + return FSMONITOR_REASON_ERROR;
59 + }
60 +}
61 +#endif
62 +
63 static enum fsmonitor_reason check_for_incompatible(struct repository *r)
64 {
65 if (!r->worktree) {
@@ -27,6 +74,9 @@ static enum fsmonitor_reason check_for_incompatible(struct repository *r)
74 {
75 enum fsmonitor_reason reason;
76
77 + reason = check_remote(r);
78 + if (reason != FSMONITOR_REASON_OK)
79 + return reason;
80 reason = fsm_os__incompatible(r);
81 if (reason != FSMONITOR_REASON_OK)
82 return reason;