pr-1143/jeffhostetler/builtin-fsmonitor-part3-v6
tagged this
22 Apr 21:29
Here is version 6 of part 3 of FSMonitor.
This version addresses:
1. Junio's comments on V5 23/28 WRT shell variable references and quoting
pathnames in the create_super and create_sub helper functions.
2. Ævar's comments on V4 4/27 (sorry I didn't see them until after I sent
V5) WRT somewhat blurry logic around the fsmonitor-settings and
detecting incompatible worktrees. I simplified things, but not to the
level that Ævar was suggesting. For example, in builtin/update-index.c
the suggestion was to detect incompatible FS before taking the lock on
the index, but the lock is taken before the CL args are parsed (because
update-index uses a custom version of parse_options_start()), so we
don't know yet whether the user passed --fsmonitor until much later and
that is what triggers the error/warning. I did replace the return 1 with
a die() so hopefully, we'll release the lock on the index like all of
the other errors in that function. I did try to better document the code
in update-index.c and in builtin/fsmonitor--daemon.c to explain how
things are supposed to work. So hopefully it'll be easier to review.
3. Also, in update-index and fsmonitor--daemon, I redid how the error
messages are printed, so that I could use die() in the cmd_*() functions
rather than having calls to error() hidden inside fsmonitor-settings.c.
I think that helped with the above cleanup.
Here is a range diff from V5. It is a little noisy because of the untangling
within fsmonitor-settings.c and moving the error messages.
1: 8b7c5f4e23 = 1: 8b7c5f4e23 fsm-listen-win32: handle shortnames
2: 5b246bec24 = 2: 5b246bec24 t7527: test FSMonitor on repos with Unicode root paths
3: 8a474d6999 = 3: 8a474d6999 t/helper/fsmonitor-client: create stress test
4: 004b67b62e < -: ---------- fsmonitor-settings: bare repos are incompatible with FSMonitor
-: ---------- > 4: 72b94acd5f fsmonitor-settings: bare repos are incompatible with FSMonitor
5: e1e55550c1 ! 5: 2e225c3f4f fsmonitor-settings: stub in Win32-specific incompatibility checking
@@ contrib/buildsystems/CMakeLists.txt: if(SUPPORTS_SIMPLE_IPC)
list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c)
## fsmonitor-settings.c ##
-@@ fsmonitor-settings.c: static int check_for_incompatible(struct repository *r)
- return 1;
+@@ fsmonitor-settings.c: static enum fsmonitor_reason check_for_incompatible(struct repository *r)
+ return FSMONITOR_REASON_BARE;
}
+#ifdef HAVE_FSMONITOR_OS_SETTINGS
@@ fsmonitor-settings.c: static int check_for_incompatible(struct repository *r)
+ enum fsmonitor_reason reason;
+
+ reason = fsm_os__incompatible(r);
-+ if (reason != FSMONITOR_REASON_OK) {
-+ set_incompatible(r, reason);
-+ return 1;
-+ }
++ if (reason != FSMONITOR_REASON_OK)
++ return reason;
+ }
+#endif
+
- return 0;
+ return FSMONITOR_REASON_OK;
}
## fsmonitor-settings.h ##
-@@ fsmonitor-settings.h: int fsm_settings__error_if_incompatible(struct repository *r);
+@@ fsmonitor-settings.h: char *fsm_settings__get_incompatible_msg(const struct repository *r,
struct fsmonitor_settings;
6: 2d68fc9a46 ! 6: e0d3bdf755 fsmonitor-settings: VFS for Git virtual repos are incompatible
@@ compat/fsmonitor/fsm-settings-win32.c
}
## fsmonitor-settings.c ##
-@@ fsmonitor-settings.c: int fsm_settings__error_if_incompatible(struct repository *r)
- error(_("bare repository '%s' is incompatible with fsmonitor"),
- xgetcwd());
- return 1;
+@@ fsmonitor-settings.c: char *fsm_settings__get_incompatible_msg(const struct repository *r,
+ _("bare repository '%s' is incompatible with fsmonitor"),
+ xgetcwd());
+ goto done;
+
+ case FSMONITOR_REASON_VFS4GIT:
-+ error(_("virtual repository '%s' is incompatible with fsmonitor"),
-+ r->worktree);
-+ return 1;
++ strbuf_addf(&msg,
++ _("virtual repository '%s' is incompatible with fsmonitor"),
++ r->worktree);
++ goto done;
}
- BUG("Unhandled case in fsm_settings__error_if_incompatible: '%d'",
+ BUG("Unhandled case in fsm_settings__get_incompatible_msg: '%d'",
## fsmonitor-settings.h ##
-@@ fsmonitor-settings.h: enum fsmonitor_mode {
- enum fsmonitor_reason {
- FSMONITOR_REASON_OK = 0, /* no incompatibility or when disbled */
+@@ fsmonitor-settings.h: enum fsmonitor_reason {
+ FSMONITOR_REASON_UNTESTED = 0,
+ FSMONITOR_REASON_OK, /* no incompatibility or when disbled */
FSMONITOR_REASON_BARE,
+ FSMONITOR_REASON_VFS4GIT, /* VFS for Git virtualization */
};
7: 94ae2e424f = 7: c50ed29a31 fsmonitor-settings: stub in macOS-specific incompatibility checking
8: b2ca6c1b20 ! 8: 1f5b772d42 fsmonitor-settings: remote repos on macOS are incompatible
@@ compat/fsmonitor/fsm-settings-darwin.c
}
## fsmonitor-settings.c ##
-@@ fsmonitor-settings.c: int fsm_settings__error_if_incompatible(struct repository *r)
- xgetcwd());
- return 1;
+@@ fsmonitor-settings.c: char *fsm_settings__get_incompatible_msg(const struct repository *r,
+ xgetcwd());
+ goto done;
+ case FSMONITOR_REASON_ERROR:
-+ error(_("repository '%s' is incompatible with fsmonitor due to errors"),
-+ r->worktree);
-+ return 1;
++ strbuf_addf(&msg,
++ _("repository '%s' is incompatible with fsmonitor due to errors"),
++ r->worktree);
++ goto done;
+
+ case FSMONITOR_REASON_REMOTE:
-+ error(_("remote repository '%s' is incompatible with fsmonitor"),
-+ r->worktree);
-+ return 1;
++ strbuf_addf(&msg,
++ _("remote repository '%s' is incompatible with fsmonitor"),
++ r->worktree);
++ goto done;
+
case FSMONITOR_REASON_VFS4GIT:
- error(_("virtual repository '%s' is incompatible with fsmonitor"),
- r->worktree);
+ strbuf_addf(&msg,
+ _("virtual repository '%s' is incompatible with fsmonitor"),
## fsmonitor-settings.h ##
-@@ fsmonitor-settings.h: enum fsmonitor_mode {
- enum fsmonitor_reason {
- FSMONITOR_REASON_OK = 0, /* no incompatibility or when disbled */
+@@ fsmonitor-settings.h: enum fsmonitor_reason {
+ FSMONITOR_REASON_UNTESTED = 0,
+ FSMONITOR_REASON_OK, /* no incompatibility or when disbled */
FSMONITOR_REASON_BARE,
+ FSMONITOR_REASON_ERROR, /* FS error probing for compatibility */
+ FSMONITOR_REASON_REMOTE,
9: a3cc4b3b16 = 9: 495e54049b fsmonitor-settings: remote repos on Windows are incompatible
10: 8f1f484075 ! 10: 4b52083698 fsmonitor-settings: NTFS and FAT32 on MacOS are incompatible
@@ compat/fsmonitor/fsm-settings-darwin.c: enum fsmonitor_reason fsm_os__incompatib
## fsmonitor-settings.c ##
-@@ fsmonitor-settings.c: int fsm_settings__error_if_incompatible(struct repository *r)
- error(_("virtual repository '%s' is incompatible with fsmonitor"),
- r->worktree);
- return 1;
+@@ fsmonitor-settings.c: char *fsm_settings__get_incompatible_msg(const struct repository *r,
+ _("virtual repository '%s' is incompatible with fsmonitor"),
+ r->worktree);
+ goto done;
+
+ case FSMONITOR_REASON_NOSOCKETS:
-+ error(_("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets"),
-+ r->worktree);
-+ return 1;
++ strbuf_addf(&msg,
++ _("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets"),
++ r->worktree);
++ goto done;
}
- BUG("Unhandled case in fsm_settings__error_if_incompatible: '%d'",
+ BUG("Unhandled case in fsm_settings__get_incompatible_msg: '%d'",
## fsmonitor-settings.h ##
@@ fsmonitor-settings.h: enum fsmonitor_reason {
11: 8d48d9c562 = 11: d4a4263d37 unpack-trees: initialize fsmonitor_has_run_once in o->result
12: 088c7b3334 = 12: f4feb00ec2 fsm-listen-darwin: ignore FSEvents caused by xattr changes on macOS
13: 00fab62666 = 13: dbb983fd9d fsmonitor--daemon: cd out of worktree root
14: 6552f51802 = 14: ae90b99ea9 fsmonitor--daemon: prepare for adding health thread
15: f2bf07cd73 = 15: b6c5800095 fsmonitor--daemon: rename listener thread related variables
16: 2a44f2eded = 16: 32fc6ba743 fsmonitor--daemon: stub in health thread
17: 854fb5e365 = 17: 77bc037481 fsm-health-win32: add polling framework to monitor daemon health
18: 3af1fe0d61 = 18: b06edd995e fsm-health-win32: force shutdown daemon if worktree root moves
19: f1365cdd40 = 19: 1bd5f34624 fsm-listen-darwin: shutdown daemon if worktree root is moved/renamed
20: 15698d64ed = 20: 48af0813de fsmonitor: optimize processing of directory events
21: 9d0da8fc22 = 21: a9b35e770f t7527: FSMonitor tests for directory moves
22: 040c00cfd6 = 22: 26308936af t/perf/p7527: add perf test for builtin FSMonitor
23: 5db241f7d2 ! 23: d0e25f6bac fsmonitor: never set CE_FSMONITOR_VALID on submodules
@@ t/t7527-builtin-fsmonitor.sh: do
+# submodule.
+
+create_super () {
-+ super=$1 &&
-+
-+ git init "${super}" &&
-+ echo x >${super}/file_1 &&
-+ echo y >${super}/file_2 &&
-+ echo z >${super}/file_3 &&
-+ mkdir ${super}/dir_1 &&
-+ echo a >${super}/dir_1/file_11 &&
-+ echo b >${super}/dir_1/file_12 &&
-+ mkdir ${super}/dir_1/dir_2 &&
-+ echo a >${super}/dir_1/dir_2/file_21 &&
-+ echo b >${super}/dir_1/dir_2/file_22 &&
-+ git -C ${super} add . &&
-+ git -C ${super} commit -m "initial ${super} commit"
++ super="$1" &&
++
++ git init "$super" &&
++ echo x >"$super/file_1" &&
++ echo y >"$super/file_2" &&
++ echo z >"$super/file_3" &&
++ mkdir "$super/dir_1" &&
++ echo a >"$super/dir_1/file_11" &&
++ echo b >"$super/dir_1/file_12" &&
++ mkdir "$super/dir_1/dir_2" &&
++ echo a >"$super/dir_1/dir_2/file_21" &&
++ echo b >"$super/dir_1/dir_2/file_22" &&
++ git -C "$super" add . &&
++ git -C "$super" commit -m "initial $super commit"
+}
+
+create_sub () {
-+ sub=$1 &&
-+
-+ git init "${sub}" &&
-+ echo x >${sub}/file_x &&
-+ echo y >${sub}/file_y &&
-+ echo z >${sub}/file_z &&
-+ mkdir ${sub}/dir_x &&
-+ echo a >${sub}/dir_x/file_a &&
-+ echo b >${sub}/dir_x/file_b &&
-+ mkdir ${sub}/dir_x/dir_y &&
-+ echo a >${sub}/dir_x/dir_y/file_a &&
-+ echo b >${sub}/dir_x/dir_y/file_b &&
-+ git -C ${sub} add . &&
-+ git -C ${sub} commit -m "initial ${sub} commit"
++ sub="$1" &&
++
++ git init "$sub" &&
++ echo x >"$sub/file_x" &&
++ echo y >"$sub/file_y" &&
++ echo z >"$sub/file_z" &&
++ mkdir "$sub/dir_x" &&
++ echo a >"$sub/dir_x/file_a" &&
++ echo b >"$sub/dir_x/file_b" &&
++ mkdir "$sub/dir_x/dir_y" &&
++ echo a >"$sub/dir_x/dir_y/file_a" &&
++ echo b >"$sub/dir_x/dir_y/file_b" &&
++ git -C "$sub" add . &&
++ git -C "$sub" commit -m "initial $sub commit"
+}
+
+my_match_and_clean () {
@@ t/t7527-builtin-fsmonitor.sh: do
+ rm -rf super; \
+ rm -rf sub" &&
+
-+ create_super "super" &&
-+ create_sub "sub" &&
++ create_super super &&
++ create_sub sub &&
+
+ git -C super submodule add ../sub ./dir_1/dir_2/sub &&
+ git -C super commit -m "add sub" &&
24: 93de3707d2 = 24: 410dd2d292 t7527: test FSMonitor on case insensitive+preserving file system
25: d890c2e2d9 = 25: cd7c55b0d3 fsmonitor: on macOS also emit NFC spelling for NFD pathname
26: 7c60623555 = 26: 8278f32c4d t/lib-unicode-nfc-nfd: helper prereqs for testing unicode nfc/nfd
27: 9724c41d18 = 27: 4efb3a4383 t7527: test Unicode NFC/NFD handling on MacOS
28: b8325fb7c7 ! 28: df1b4f3a80 fsmonitor--daemon: allow --super-prefix argument
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success "Submodule always visited" '
+ rm -rf sub; \
+ rm super-sub.trace" &&
+
-+ create_super "super" &&
-+ create_sub "sub" &&
++ create_super super &&
++ create_sub sub &&
+
+ # Copy rather than submodule add so that we get a .git dir.
+ cp -R ./sub ./super/dir_1/dir_2/sub &&
Jeff Hostetler (28):
fsm-listen-win32: handle shortnames
t7527: test FSMonitor on repos with Unicode root paths
t/helper/fsmonitor-client: create stress test
fsmonitor-settings: bare repos are incompatible with FSMonitor
fsmonitor-settings: stub in Win32-specific incompatibility checking
fsmonitor-settings: VFS for Git virtual repos are incompatible
fsmonitor-settings: stub in macOS-specific incompatibility checking
fsmonitor-settings: remote repos on macOS are incompatible
fsmonitor-settings: remote repos on Windows are incompatible
fsmonitor-settings: NTFS and FAT32 on MacOS are incompatible
unpack-trees: initialize fsmonitor_has_run_once in o->result
fsm-listen-darwin: ignore FSEvents caused by xattr changes on macOS
fsmonitor--daemon: cd out of worktree root
fsmonitor--daemon: prepare for adding health thread
fsmonitor--daemon: rename listener thread related variables
fsmonitor--daemon: stub in health thread
fsm-health-win32: add polling framework to monitor daemon health
fsm-health-win32: force shutdown daemon if worktree root moves
fsm-listen-darwin: shutdown daemon if worktree root is moved/renamed
fsmonitor: optimize processing of directory events
t7527: FSMonitor tests for directory moves
t/perf/p7527: add perf test for builtin FSMonitor
fsmonitor: never set CE_FSMONITOR_VALID on submodules
t7527: test FSMonitor on case insensitive+preserving file system
fsmonitor: on macOS also emit NFC spelling for NFD pathname
t/lib-unicode-nfc-nfd: helper prereqs for testing unicode nfc/nfd
t7527: test Unicode NFC/NFD handling on MacOS
fsmonitor--daemon: allow --super-prefix argument
Makefile | 19 +-
builtin/fsmonitor--daemon.c | 116 ++++++-
builtin/update-index.c | 16 +
compat/fsmonitor/fsm-health-darwin.c | 24 ++
compat/fsmonitor/fsm-health-win32.c | 278 +++++++++++++++++
compat/fsmonitor/fsm-health.h | 47 +++
compat/fsmonitor/fsm-listen-darwin.c | 122 ++++++--
compat/fsmonitor/fsm-listen-win32.c | 413 ++++++++++++++++++++-----
compat/fsmonitor/fsm-listen.h | 2 +-
compat/fsmonitor/fsm-settings-darwin.c | 89 ++++++
compat/fsmonitor/fsm-settings-win32.c | 137 ++++++++
config.mak.uname | 5 +
contrib/buildsystems/CMakeLists.txt | 8 +
fsmonitor--daemon.h | 11 +-
fsmonitor-settings.c | 167 ++++++++--
fsmonitor-settings.h | 33 ++
fsmonitor.c | 73 ++++-
fsmonitor.h | 11 +
git.c | 2 +-
t/helper/test-fsmonitor-client.c | 106 +++++++
t/lib-unicode-nfc-nfd.sh | 167 ++++++++++
t/perf/p7527-builtin-fsmonitor.sh | 257 +++++++++++++++
t/t7519-status-fsmonitor.sh | 32 ++
t/t7527-builtin-fsmonitor.sh | 367 ++++++++++++++++++++++
unpack-trees.c | 1 +
25 files changed, 2358 insertions(+), 145 deletions(-)
create mode 100644 compat/fsmonitor/fsm-health-darwin.c
create mode 100644 compat/fsmonitor/fsm-health-win32.c
create mode 100644 compat/fsmonitor/fsm-health.h
create mode 100644 compat/fsmonitor/fsm-settings-darwin.c
create mode 100644 compat/fsmonitor/fsm-settings-win32.c
create mode 100755 t/lib-unicode-nfc-nfd.sh
create mode 100755 t/perf/p7527-builtin-fsmonitor.sh
base-commit: 5eb696daba2fe108d4d9ba2ccf4b357447ef9946
Submitted-As: https://lore.kernel.org/git/pull.1143.v6.git.1650662994.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1143.git.1644940773.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1143.v2.git.1646777727.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1143.v3.git.1647973380.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1143.v4.git.1648140680.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1143.v5.git.1650487398.gitgitgadget@gmail.com
Assets 2
-
2022-04-22T21:29:54Z -
2022-04-22T21:29:54Z -