Subject: [PATCH v2 00/27] Builtin FSMonitor Part 3

From: "Jeff Hostetler via GitGitGadget" <[email-masked]>
Date: Tue, 08 Mar 2022 22:15:00 +0000

Here is V2 of part 3 of my builtin FSMonitor series.

I think I have addressed all of the feedback from V1. This includes:

[] AEvar's suggestion to simplify the API to get the repo incompatibility
message.

[] I updated the daemon on MacOS to report both the NFC and NFD spellings of
a pathname when appropriate. This is a little more general than the
"core.precomposeUnicode" setting, since the daemon does not know how the
client has (or will have) it set when they make a query.

[] I replaced my Unicode NFC/NFD test for MacOS to focus exclusively on
Unicode composition/decomposition sensitivity and to not confuse that with
case sensitivity.

[] Added code on MacOS to mark repos on FAT32 and NTFS volumes as
incompatible with FSMonitor, since they don't support Unix domain sockets.

[] I reordered some of the static functions in the Windows version of the
health monitoring code to reduce the need for forward declarations.

[] Lots of typos and cleanup.

This version has been rebased upon V6 of Part 2 which is currently in
"next".

I'm not going to repeat the perf test results in the V2 version of the cover
letter; interested readers should look at the cover letter for V1.

Jeff Hostetler (27):
  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 platform-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
  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-settings: NTFS and FAT32 on MacOS are incompatible

 Makefile                               |  19 +-
 builtin/fsmonitor--daemon.c            | 107 ++++++-
 builtin/update-index.c                 |   7 +
 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                   |  81 +++++
 fsmonitor-settings.h                   |  29 ++
 fsmonitor.c                            |  73 ++++-
 fsmonitor.h                            |  11 +
 t/helper/test-fsmonitor-client.c       | 106 +++++++
 t/lib-unicode-nfc-nfd.sh               | 159 ++++++++++
 t/perf/p7527-builtin-fsmonitor.sh      | 257 +++++++++++++++
 t/t7519-status-fsmonitor.sh            |  32 ++
 t/t7527-builtin-fsmonitor.sh           | 314 +++++++++++++++++++
 unpack-trees.c                         |   1 +
 24 files changed, 2208 insertions(+), 124 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: 1a9241e1fee9d418e436849853f031329e792192
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1143%2Fjeffhostetler%2Fbuiltin-fsmonitor-part3-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1143/jeffhostetler/builtin-fsmonitor-part3-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1143

Range-diff vs v1:

  1:  23f38338cec !  1:  34619e0652b fsm-listen-win32: handle shortnames
     @@ compat/fsmonitor/fsm-listen-win32.c: normalize:
      + * to longname conversion on every notification event.
      + *
      + * We do not want to create a file to test this, so we assume that the
     -+ * root directory contains a ".git" file or directory.  (Out caller
     ++ * root directory contains a ".git" file or directory.  (Our caller
      + * only calls us for the worktree root, so this should be fine.)
      + *
      + * Remember the spelling of the shortname for ".git" if it exists.
     @@ compat/fsmonitor/fsm-listen-win32.c: normalize:
      +		 * lookup the longname for it.  Likewise, for moves
      +		 * and renames where we are given the old name.)
      +		 *
     -+		 * NEEDSWORK: Since deleting or moving a file or
     -+		 * directory by shortname is rather obscure, I'm going
     -+		 * ignore the failure and ask the caller to report the
     -+		 * original relative path.  This seemds kinder than
     -+		 * failing here and forcing a resync.
     ++		 * Since deleting or moving a file or directory by its
     ++		 * shortname is rather obscure, I'm going ignore the
     ++		 * failure and ask the caller to report the original
     ++		 * relative path.  This seems kinder than failing here
     ++		 * and forcing a resync.  Besides, forcing a resync on
     ++		 * every file/directory delete would effectively
     ++		 * cripple monitoring.
     ++		 *
     ++		 * We might revisit this in the future.
      +		 */
      +		return GRR_NO_CONVERSION_NEEDED;
      +	}
  3:  9af952e4d17 !  2:  3a0f30b849a t7527: test builtin FSMonitor watching repos with unicode paths
     @@ Metadata
      Author: Jeff Hostetler <[email-masked]>
      
       ## Commit message ##
     -    t7527: test builtin FSMonitor watching repos with unicode paths
     +    t7527: test FSMonitor on repos with Unicode root paths
      
     -    Create some test repos with UTF8 pathnames and verify that
     -    the builtin FSMonitor can watch them.  This test is mainly
     -    for Windows where we need to avoid `*A()` routines.
     +    Create some test repos with UTF8 characters in the pathname of the
     +    root directory and verify that the builtin FSMonitor can watch them.
     +
     +    This test is mainly for Windows where we need to avoid `*A()`
     +    routines.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
      
     @@ t/t7527-builtin-fsmonitor.sh: do
       done
       
      +# Test Unicode UTF-8 characters in the pathname of the working
     -+# directory.  Use of "*A()" routines rather than "*W()" routines
     ++# directory root.  Use of "*A()" routines rather than "*W()" routines
      +# on Windows can sometimes lead to odd failures.
      +#
      +u1=$(printf "u_c3_a6__\xC3\xA6")
     @@ t/t7527-builtin-fsmonitor.sh: do
      +u_values="$u1 $u2"
      +for u in $u_values
      +do
     -+	test_expect_success "Unicode path: $u" '
     ++	test_expect_success "Unicode in repo root path: $u" '
      +		test_when_finished "stop_daemon_delete_repo $u" &&
      +
      +		git init "$u" &&
  4:  9efdbe28223 !  3:  87d1c0b6f2a t/helper/fsmonitor-client: create stress test
     @@ Commit message
          Create a client-side thread pool of n threads and have
          each of them make m requests as fast as they can.
      
     -    NEEDSWORK: This is just the client-side thread pool and
     -    is useful for interactive testing and experimentation.
     -    We need to add a script test to drive this.
     +    We do not currently inspect the contents of the response.
     +    We're only interested in placing a heavy request load on
     +    the daemon.
     +
     +    This test is useful for interactive testing and various
     +    experimentation.  For example, to place additional load
     +    on the daemon while another test is running.  We currently
     +    do not have a test script that actually uses this helper.
     +    We might add such a test in the future.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
      
     @@ t/helper/test-fsmonitor-client.c: static int do_send_flush(void)
      +	free(data);
      +
      +	/*
     -+	 * TODO Decide if/when to return an error or call die().
     ++	 * Return an error if any of the _send_query requests failed.
     ++	 * We don't care about thread create/join errors.
      +	 */
     -+	return 0;
     ++	return sum_errors > 0;
      +}
      +
       int cmd__fsmonitor_client(int argc, const char **argv)
  5:  44cc61e186c !  4:  8c4f90ae4fd fsmonitor-settings: bare repos are incompatible with FSMonitor
     @@ builtin/fsmonitor--daemon.c: int cmd_fsmonitor__daemon(int argc, const char **ar
      +	fsm_settings__set_ipc(the_repository);
      +
      +	if (fsm_settings__get_mode(the_repository) == FSMONITOR_MODE_INCOMPATIBLE) {
     -+		struct strbuf buf_reason = STRBUF_INIT;
     -+		fsm_settings__get_reason(the_repository, &buf_reason);
     -+		error("%s '%s'", buf_reason.buf, xgetcwd());
     -+		strbuf_release(&buf_reason);
     -+		return -1;
     ++		const char *msg = fsm_settings__get_reason_msg(the_repository);
     ++
     ++		return error("%s '%s'", msg ? msg : "???", xgetcwd());
      +	}
      +
       	if (!strcmp(subcmd, "start"))
     @@ builtin/update-index.c: int cmd_update_index(int argc, const char **argv, const
       		enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
      +
      +		if (fsm_mode == FSMONITOR_MODE_INCOMPATIBLE) {
     -+			struct strbuf buf_reason = STRBUF_INIT;
     -+			fsm_settings__get_reason(r, &buf_reason);
     -+			error("%s", buf_reason.buf);
     -+			strbuf_release(&buf_reason);
     -+			return -1;
     ++			const char *msg = fsm_settings__get_reason_msg(r);
     ++
     ++			return error("%s '%s'", msg ? msg : "???", xgetcwd());
      +		}
      +
       		if (fsm_mode == FSMONITOR_MODE_DISABLED) {
     @@ fsmonitor-settings.c
       {
       	struct fsmonitor_settings *s;
      @@ fsmonitor-settings.c: void fsm_settings__set_ipc(struct repository *r)
     - {
     + 
       	lookup_fsmonitor_settings(r);
       
      +	if (check_for_incompatible(r))
     @@ fsmonitor-settings.c: void fsm_settings__set_ipc(struct repository *r)
       	FREE_AND_NULL(r->settings.fsmonitor->hook_path);
       }
      @@ fsmonitor-settings.c: void fsm_settings__set_hook(struct repository *r, const char *path)
     - {
     + 
       	lookup_fsmonitor_settings(r);
       
      +	if (check_for_incompatible(r))
     @@ fsmonitor-settings.c: void fsm_settings__set_disabled(struct repository *r)
       	lookup_fsmonitor_settings(r);
       
       	r->settings.fsmonitor->mode = FSMONITOR_MODE_DISABLED;
     -+	r->settings.fsmonitor->reason = FSMONITOR_REASON_ZERO;
     ++	r->settings.fsmonitor->reason = FSMONITOR_REASON_OK;
       	FREE_AND_NULL(r->settings.fsmonitor->hook_path);
       }
      +
     -+static void create_reason_message(struct repository *r,
     -+				  struct strbuf *buf_reason)
     ++enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)
      +{
     -+	struct fsmonitor_settings *s = r->settings.fsmonitor;
     -+
     -+	switch (s->reason) {
     -+	case FSMONITOR_REASON_ZERO:
     -+		return;
     ++	if (!r)
     ++		r = the_repository;
      +
     -+	case FSMONITOR_REASON_BARE:
     -+		strbuf_addstr(buf_reason,
     -+			      _("bare repos are incompatible with fsmonitor"));
     -+		return;
     ++	lookup_fsmonitor_settings(r);
      +
     -+	default:
     -+		BUG("Unhandled case in create_reason_message '%d'", s->reason);
     -+	}
     ++	return r->settings.fsmonitor->reason;
      +}
      +
     -+enum fsmonitor_reason fsm_settings__get_reason(struct repository *r,
     -+					       struct strbuf *buf_reason)
     ++const char *fsm_settings__get_reason_msg(struct repository *r)
      +{
     -+	lookup_fsmonitor_settings(r);
     ++	enum fsmonitor_reason reason = fsm_settings__get_reason(r);
      +
     -+	strbuf_reset(buf_reason);
     -+	if (r->settings.fsmonitor->mode == FSMONITOR_MODE_INCOMPATIBLE)
     -+		create_reason_message(r, buf_reason);
     ++	switch (reason) {
     ++	case FSMONITOR_REASON_OK:
     ++		return NULL;
      +
     -+	return r->settings.fsmonitor->reason;
     ++	case FSMONITOR_REASON_BARE:
     ++		return _("bare repos are incompatible with fsmonitor");
     ++	}
     ++
     ++	BUG("Unhandled case in fsm_settings__get_reason_msg '%d'",
     ++	    reason);
      +}
      
       ## fsmonitor-settings.h ##
     @@ fsmonitor-settings.h
      + * Incompatibility reasons.
      + */
      +enum fsmonitor_reason {
     -+	FSMONITOR_REASON_ZERO = 0,
     -+	FSMONITOR_REASON_BARE = 1,
     ++	FSMONITOR_REASON_OK = 0, /* no incompatibility or when disbled */
     ++	FSMONITOR_REASON_BARE,
      +};
      +
       void fsm_settings__set_ipc(struct repository *r);
       void fsm_settings__set_hook(struct repository *r, const char *path);
       void fsm_settings__set_disabled(struct repository *r);
     - 
     +@@ fsmonitor-settings.h: void fsm_settings__set_disabled(struct repository *r);
       enum fsmonitor_mode fsm_settings__get_mode(struct repository *r);
       const char *fsm_settings__get_hook_path(struct repository *r);
     -+enum fsmonitor_reason fsm_settings__get_reason(struct repository *r,
     -+					       struct strbuf *buf_reason);
       
     ++enum fsmonitor_reason fsm_settings__get_reason(struct repository *r);
     ++const char *fsm_settings__get_reason_msg(struct repository *r);
     ++
       struct fsmonitor_settings;
       
     + #endif /* FSMONITOR_SETTINGS_H */
      
       ## t/t7519-status-fsmonitor.sh ##
      @@ t/t7519-status-fsmonitor.sh: test_lazy_prereq UNTRACKED_CACHE '
     @@ t/t7519-status-fsmonitor.sh: test_lazy_prereq UNTRACKED_CACHE '
      +test_expect_success 'incompatible bare repo' '
      +	test_when_finished "rm -rf ./bare-clone actual expect" &&
      +	git init --bare bare-clone &&
     -+	cat >expect <<-\EOF &&
     -+	error: bare repos are incompatible with fsmonitor
     -+	EOF
      +
      +	test_must_fail \
      +		git -C ./bare-clone -c core.fsmonitor=foo \
      +			update-index --fsmonitor 2>actual &&
     -+	test_cmp expect actual &&
     ++	grep "bare repos are incompatible with fsmonitor" actual &&
      +
      +	test_must_fail \
      +		git -C ./bare-clone -c core.fsmonitor=true \
      +			update-index --fsmonitor 2>actual &&
     -+	test_cmp expect actual
     ++	grep "bare repos are incompatible with fsmonitor" actual
      +'
      +
      +test_expect_success FSMONITOR_DAEMON 'run fsmonitor-daemon in bare repo' '
  6:  4715677f85f !  5:  6329328d185 fsmonitor-settings: stub in platform-specific incompatibility checking
     @@ Commit message
      
          In the existing fsmonitor-settings code we have a way to mark
          types of repos as incompatible with fsmonitor (whether via the
     -    hook and ipc APIs).  For example, we do this for bare repos,
     +    hook and IPC APIs).  For example, we do this for bare repos,
          since there are no files to watch.
      
     -    Extend this exclusion mechanism for platfor-specific reasons.
     +    Extend this exclusion mechanism for platform-specific reasons.
          This commit just creates the framework and adds a stub for Win32.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     @@ Makefile: all::
       # `compat/fsmonitor/fsm-listen-<name>.c` that implements the
       # `fsm_listen__*()` routines.
       #
     -+# If your platform has os-specific ways to tell if a repo is incompatible with
     -+# fsmonitor (whether the hook or ipc daemon version), set FSMONITOR_OS_SETTINGS
     ++# If your platform has OS-specific ways to tell if a repo is incompatible with
     ++# fsmonitor (whether the hook or IPC daemon version), set FSMONITOR_OS_SETTINGS
      +# to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.c`
      +# that implements the `fsm_os_settings__*()` routines.
      +#
     @@ compat/fsmonitor/fsm-settings-win32.c (new)
      +
      +enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
      +{
     -+	return FSMONITOR_REASON_ZERO;
     ++	return FSMONITOR_REASON_OK;
      +}
      
       ## config.mak.uname ##
     @@ fsmonitor-settings.c: static int check_for_incompatible(struct repository *r)
      +		enum fsmonitor_reason reason;
      +
      +		reason = fsm_os__incompatible(r);
     -+		if (reason != FSMONITOR_REASON_ZERO) {
     ++		if (reason != FSMONITOR_REASON_OK) {
      +			set_incompatible(r, reason);
      +			return 1;
      +		}
     @@ fsmonitor-settings.c: static int check_for_incompatible(struct repository *r)

       ## fsmonitor-settings.h ##
     -@@ fsmonitor-settings.h: enum fsmonitor_reason fsm_settings__get_reason(struct repository *r,
     +@@ fsmonitor-settings.h: const char *fsm_settings__get_reason_msg(struct repository *r);
       
       struct fsmonitor_settings;
       
     @@ fsmonitor-settings.h: enum fsmonitor_reason fsm_settings__get_reason(struct repo
      + * Ask platform-specific code whether the repository is incompatible
      + * with fsmonitor (both hook and ipc modes).  For example, if the working
      + * directory is on a remote volume and mounted via a technology that does
     -+ * not support notification events.
     ++ * not support notification events, then we should not pretend to watch it.
      + *
      + * fsm_os__* routines should considered private to fsm_settings__
      + * routines.
  7:  4e856d60e38 !  6:  fa9e86e7de7 fsmonitor-settings: virtual repos are incompatible with FSMonitor
     @@ Metadata
      Author: Jeff Hostetler <[email-masked]>
      
       ## Commit message ##
     -    fsmonitor-settings: virtual repos are incompatible with FSMonitor
     +    fsmonitor-settings: VFS for Git virtual repos are incompatible
      
     -    Virtual repos, such as GVFS (aka VFS for Git), are incompatible
     -    with FSMonitor.
     +    VFS for Git virtual repositories are incompatible with FSMonitor.
     +
     +    VFS for Git is a downstream fork of Git.  It contains its own custom
     +    file system watcher that is aware of the virtualization.  If a working
     +    directory is being managed by VFS for Git, we should not try to watch
     +    it because we may get incomplete results.
     +
     +    We do not know anything about how VFS for Git works, but we do
     +    know that VFS for Git working directories contain a well-defined
     +    config setting.  If it is set, mark the working directory as
     +    incompatible.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
      
     @@ compat/fsmonitor/fsm-settings-win32.c
       #include "fsmonitor-settings.h"
       
      +/*
     -+ * GVFS (aka VFS for Git) is incompatible with FSMonitor.
     ++ * VFS for Git is incompatible with FSMonitor.
      + *
     -+ * Granted, core Git does not know anything about GVFS and we
     ++ * Granted, core Git does not know anything about VFS for Git and we
      + * shouldn't make assumptions about a downstream feature, but users
      + * can install both versions.  And this can lead to incorrect results
     -+ * from core Git commands.  So, without bringing in any of the GVFS
     -+ * code, do a simple config test for a published config setting.  (We
     -+ * do not look at the various *_TEST_* environment variables.)
     ++ * from core Git commands.  So, without bringing in any of the VFS for
     ++ * Git code, do a simple config test for a published config setting.
     ++ * (We do not look at the various *_TEST_* environment variables.)
      + */
     -+static enum fsmonitor_reason is_virtual(struct repository *r)
     ++static enum fsmonitor_reason check_vfs4git(struct repository *r)
      +{
      +	const char *const_str;
      +
      +	if (!repo_config_get_value(r, "core.virtualfilesystem", &const_str))
     -+		return FSMONITOR_REASON_VIRTUAL;
     ++		return FSMONITOR_REASON_VFS4GIT;
      +
     -+	return FSMONITOR_REASON_ZERO;
     ++	return FSMONITOR_REASON_OK;
      +}
      +
       enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
       {
      +	enum fsmonitor_reason reason;
      +
     -+	reason = is_virtual(r);
     -+	if (reason)
     ++	reason = check_vfs4git(r);
     ++	if (reason != FSMONITOR_REASON_OK)
      +		return reason;
      +
     - 	return FSMONITOR_REASON_ZERO;
     + 	return FSMONITOR_REASON_OK;
       }
      
       ## fsmonitor-settings.c ##
     -@@ fsmonitor-settings.c: static void create_reason_message(struct repository *r,
     - 			      _("bare repos are incompatible with fsmonitor"));
     - 		return;
     +@@ fsmonitor-settings.c: const char *fsm_settings__get_reason_msg(struct repository *r)
       
     -+	case FSMONITOR_REASON_VIRTUAL:
     -+		strbuf_addstr(buf_reason,
     -+			      _("virtual repos are incompatible with fsmonitor"));
     -+		return;
     + 	case FSMONITOR_REASON_BARE:
     + 		return _("bare repos are incompatible with fsmonitor");
      +
     - 	default:
     - 		BUG("Unhandled case in create_reason_message '%d'", s->reason);
     ++	case FSMONITOR_REASON_VFS4GIT:
     ++		return _("virtual repos are incompatible with fsmonitor");
       	}
     + 
     + 	BUG("Unhandled case in fsm_settings__get_reason_msg '%d'",
      
       ## fsmonitor-settings.h ##
      @@ fsmonitor-settings.h: enum fsmonitor_mode {
       enum fsmonitor_reason {
     - 	FSMONITOR_REASON_ZERO = 0,
     - 	FSMONITOR_REASON_BARE = 1,
     -+	FSMONITOR_REASON_VIRTUAL = 2,
     + 	FSMONITOR_REASON_OK = 0, /* no incompatibility or when disbled */
     + 	FSMONITOR_REASON_BARE,
     ++	FSMONITOR_REASON_VFS4GIT, /* VFS for Git virtualization */
       };
       
       void fsm_settings__set_ipc(struct repository *r);
  8:  e5511ef0f8b !  7:  c1802410410 fsmonitor-settings: stub in macOS-specific incompatibility checking
     @@ compat/fsmonitor/fsm-settings-darwin.c (new)
      +
      +enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
      +{
     -+	return FSMONITOR_REASON_ZERO;
     ++	return FSMONITOR_REASON_OK;
      +}
      
       ## config.mak.uname ##
  9:  412fbc45868 !  8:  e3bfa0bd69d fsmonitor-settings: remote repos on macOS are incompatible with FSMonitor
     @@ Metadata
      Author: Jeff Hostetler <[email-masked]>
      
       ## Commit message ##
     -    fsmonitor-settings: remote repos on macOS are incompatible with FSMonitor
     +    fsmonitor-settings: remote repos on macOS are incompatible
      
          Teach Git to detect remote working directories on macOS and mark them as
          incompatible with FSMonitor.
     @@ compat/fsmonitor/fsm-settings-darwin.c
      + * So (for now at least), mark remote working directories as
      + * incompatible.
      + */
     -+static enum fsmonitor_reason is_remote(struct repository *r)
     ++static enum fsmonitor_reason check_remote(struct repository *r)
      +{
      +	struct statfs fs;
      +
     @@ compat/fsmonitor/fsm-settings-darwin.c
      +		trace_printf_key(&trace_fsmonitor, "statfs('%s') failed: %s",
      +				 r->worktree, strerror(saved_errno));
      +		errno = saved_errno;
     -+		return FSMONITOR_REASON_ZERO;
     ++		return FSMONITOR_REASON_ERROR;
      +	}
      +
      +	trace_printf_key(&trace_fsmonitor,
     @@ compat/fsmonitor/fsm-settings-darwin.c
      +	if (!(fs.f_flags & MNT_LOCAL))
      +		return FSMONITOR_REASON_REMOTE;
      +
     -+	return FSMONITOR_REASON_ZERO;
     ++	return FSMONITOR_REASON_OK;
      +}
       
       enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
       {
      +	enum fsmonitor_reason reason;
      +
     -+	reason = is_remote(r);
     -+	if (reason)
     ++	reason = check_remote(r);
     ++	if (reason != FSMONITOR_REASON_OK)
      +		return reason;
      +
     - 	return FSMONITOR_REASON_ZERO;
     + 	return FSMONITOR_REASON_OK;
       }
      
       ## fsmonitor-settings.c ##
     -@@ fsmonitor-settings.c: static void create_reason_message(struct repository *r,
     - 			      _("virtual repos are incompatible with fsmonitor"));
     - 		return;
     +@@ fsmonitor-settings.c: const char *fsm_settings__get_reason_msg(struct repository *r)
     + 	case FSMONITOR_REASON_BARE:
     + 		return _("bare repos are incompatible with fsmonitor");
       
     ++	case FSMONITOR_REASON_ERROR:
     ++		return _("repo incompatible with fsmonitor due to errors");
     ++
      +	case FSMONITOR_REASON_REMOTE:
     -+		strbuf_addstr(buf_reason,
     -+			      _("remote repos are incompatible with fsmonitor"));
     -+		return;
     ++		return _("remote repos are incompatible with fsmonitor");
      +
     - 	default:
     - 		BUG("Unhandled case in create_reason_message '%d'", s->reason);
     + 	case FSMONITOR_REASON_VFS4GIT:
     + 		return _("virtual repos are incompatible with fsmonitor");
       	}
      
       ## fsmonitor-settings.h ##
     -@@ fsmonitor-settings.h: enum fsmonitor_reason {
     - 	FSMONITOR_REASON_ZERO = 0,
     - 	FSMONITOR_REASON_BARE = 1,
     - 	FSMONITOR_REASON_VIRTUAL = 2,
     -+	FSMONITOR_REASON_REMOTE = 3,
     +@@ fsmonitor-settings.h: enum fsmonitor_mode {
     + enum fsmonitor_reason {
     + 	FSMONITOR_REASON_OK = 0, /* no incompatibility or when disbled */
     + 	FSMONITOR_REASON_BARE,
     ++	FSMONITOR_REASON_ERROR, /* FS error probing for compatibility */
     ++	FSMONITOR_REASON_REMOTE,
     + 	FSMONITOR_REASON_VFS4GIT, /* VFS for Git virtualization */
       };
       
     - void fsm_settings__set_ipc(struct repository *r);
 10:  ae09fb10c3a !  9:  e32da3118fb fsmonitor-settings: remote repos on Windows are incompatible with FSMonitor
     @@ Metadata
      Author: Jeff Hostetler <[email-masked]>
      
       ## Commit message ##
     -    fsmonitor-settings: remote repos on Windows are incompatible with FSMonitor
     +    fsmonitor-settings: remote repos on Windows are incompatible
      
          Teach Git to detect remote working directories on Windows and mark them as
          incompatible with FSMonitor.
     @@ compat/fsmonitor/fsm-settings-win32.c
      +#include "fsmonitor.h"
       
       /*
     -  * GVFS (aka VFS for Git) is incompatible with FSMonitor.
     -@@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason is_virtual(struct repository *r)
     - 	return FSMONITOR_REASON_ZERO;
     +  * VFS for Git is incompatible with FSMonitor.
     +@@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4git(struct repository *r)
     + 	return FSMONITOR_REASON_OK;
       }
       
      +/*
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason is_virtual(s
      + *     $ mklink /d ./link //server/share/repo
      + *     $ git -C ./link status
      + */
     -+static enum fsmonitor_reason is_remote(struct repository *r)
     ++static enum fsmonitor_reason check_remote(struct repository *r)
      +{
      +	wchar_t wpath[MAX_PATH];
      +	wchar_t wfullpath[MAX_PATH];
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason is_virtual(s
      +	 * a multi-byte sequence.  See win32_has_dos_drive_prefix().
      +	 */
      +	if (xutftowcs_path(wpath, r->worktree) < 0)
     -+		return FSMONITOR_REASON_ZERO;
     ++		return FSMONITOR_REASON_ERROR;
      +
      +	/*
      +	 * GetDriveTypeW() requires a final slash.  We assume that the
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason is_virtual(s
      +	 * correctly handle some UNC "\\server\share\..." paths.
      +	 */
      +	if (!GetFullPathNameW(wpath, MAX_PATH, wfullpath, NULL))
     -+		return FSMONITOR_REASON_ZERO;
     ++		return FSMONITOR_REASON_ERROR;
      +
      +	driveType = GetDriveTypeW(wfullpath);
      +	trace_printf_key(&trace_fsmonitor,
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason is_virtual(s
      +
      +	if (driveType == DRIVE_REMOTE) {
      +		trace_printf_key(&trace_fsmonitor,
     -+				 "is_remote('%s') true",
     ++				 "check_remote('%s') true",
      +				 r->worktree);
      +		return FSMONITOR_REASON_REMOTE;
      +	}
      +
     -+	return FSMONITOR_REASON_ZERO;
     ++	return FSMONITOR_REASON_OK;
      +}
      +
       enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
       {
       	enum fsmonitor_reason reason;
      @@ compat/fsmonitor/fsm-settings-win32.c: enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
     - 	if (reason)
     + 	if (reason != FSMONITOR_REASON_OK)
       		return reason;
       
     -+	reason = is_remote(r);
     -+	if (reason)
     ++	reason = check_remote(r);
     ++	if (reason != FSMONITOR_REASON_OK)
      +		return reason;
      +
     - 	return FSMONITOR_REASON_ZERO;
     + 	return FSMONITOR_REASON_OK;
       }
 11:  be1672e32b2 = 10:  f63de4eda31 unpack-trees: initialize fsmonitor_has_run_once in o->result
 12:  71babe7243a = 11:  fe305f5f287 fsm-listen-darwin: ignore FSEvents caused by xattr changes on macOS
 14:  95cdff22be0 ! 12:  c8f3e251b1f fsmonitor--daemon: cd out of worktree root
     @@ builtin/fsmonitor--daemon.c: done:
       	strbuf_release(&state.path_cookie_prefix);
      +	strbuf_release(&state.path_ipc);
       
     - 	/*
     - 	 * NEEDSWORK: Consider "rm -rf <gitdir>/<fsmonitor-dir>"
     + 	return err;
     + }
      
       ## compat/fsmonitor/fsm-listen-win32.c ##
      @@ compat/fsmonitor/fsm-listen-win32.c: static int recv_rdcw_watch(struct one_watch *watch)
 15:  6b5642f7770 = 13:  71673be2da5 fsmonitor--daemon: prepare for adding health thread
 16:  63c502c20bd = 14:  5387baaf5d7 fsmonitor--daemon: rename listener thread related variables
 17:  4a77f5b1fde ! 15:  f78e4ad87c0 fsmonitor--daemon: stub in health thread
     @@ Commit message
          file system events outside of the watched worktree root or if
          we want to have an idle-timeout auto-shutdown feature.
      
     +    This commit creates the health thread itself, defines the thread-proc
     +    and sets up the thread's event loop.  It integrates this new thread
     +    into the existing IPC and Listener thread models.
     +
     +    This commit defines the API to the platform-specific code where all of
     +    the monitoring will actually happen.
     +
     +    The platform-specific code for MacOS is just stubs.  Meaning that the
     +    health thread will immediately exit on MacOS, but that is OK and
     +    expected.  Future work can define MacOS-specific monitoring.
     +
     +    The platform-specific code for Windows sets up enough of the
     +    WaitForMultipleObjects() machinery to watch for system and/or custom
     +    events.  Currently, the set of wait handles only includes our custom
     +    shutdown event (sent from our other theads).  Later commits in this
     +    series will extend the set of wait handles to monitor other
     +    conditions.
     +
          Signed-off-by: Jeff Hostetler <[email-masked]>
      
       ## Makefile ##
     @@ Makefile: all::
      +# `compat/fsmonitor/fsm-health-<name>.c` files
      +# that implement the `fsm_listen__*()` and `fsm_health__*()` routines.
       #
     - # If your platform has os-specific ways to tell if a repo is incompatible with
     - # fsmonitor (whether the hook or ipc daemon version), set FSMONITOR_OS_SETTINGS
     + # If your platform has OS-specific ways to tell if a repo is incompatible with
     + # fsmonitor (whether the hook or IPC daemon version), set FSMONITOR_OS_SETTINGS
      @@ Makefile: endif
       ifdef FSMONITOR_DAEMON_BACKEND
       	COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND
 18:  a398cdb8a04 ! 16:  bb72f911a05 fsm-health-win32: add framework to monitor daemon health
     @@ Metadata
      Author: Jeff Hostetler <[email-masked]>
      
       ## Commit message ##
     -    fsm-health-win32: add framework to monitor daemon health
     +    fsm-health-win32: add polling framework to monitor daemon health
      
     -    Create framework in Win32 version of the "health" thread to
     -    periodically inspect the system and shutdown if warranted.
     +    Extend the Windows version of the "health" thread to periodically
     +    inspect the system and shutdown if warranted.
      
     -    This version just includes the setup for the timeout in
     -    WaitForMultipleObjects() and calls the (currently empty) table
     -    of functions.
     +    This commit updates the thread's wait loop to use a timeout and
     +    defines a (currently empty) table of functions to poll the system.
      
          A later commit will add functions to the table to actually
          inspect the system.
     @@ compat/fsmonitor/fsm-health-win32.c
      + */
      +#define WAIT_FREQ_MS (60 * 1000)
      +
     -+enum interval_fn_ctx { CTX_INIT = 0, CTX_TERM, CTX_TIMER };
     ++/*
     ++ * State machine states for each of the interval functions
     ++ * used for polling our health.
     ++ */
     ++enum interval_fn_ctx {
     ++	CTX_INIT = 0,
     ++	CTX_TERM,
     ++	CTX_TIMER
     ++};
      +
      +typedef int (interval_fn)(struct fsmonitor_daemon_state *state,
      +			  enum interval_fn_ctx ctx);
      +
     + struct fsm_health_data
     + {
     + 	HANDLE hEventShutdown;
     +@@ compat/fsmonitor/fsm-health-win32.c: void fsm_health__dtor(struct fsmonitor_daemon_state *state)
     + 	FREE_AND_NULL(state->health_data);
     + }
     + 
     ++/*
     ++ * A table of the polling functions.
     ++ */
      +static interval_fn *table[] = {
      +	NULL, /* must be last */
      +};
      +
      +/*
     -+ * Call all of the functions in the table.
     ++ * Call all of the polling functions in the table.
      + * Shortcut and return first error.
      + *
      + * Return 0 if all succeeded.
     @@ compat/fsmonitor/fsm-health-win32.c
      +	return 0;
      +}
      +
     - struct fsm_health_data
     - {
     - 	HANDLE hEventShutdown;
     -@@ compat/fsmonitor/fsm-health-win32.c: void fsm_health__dtor(struct fsmonitor_daemon_state *state)
       void fsm_health__loop(struct fsmonitor_daemon_state *state)
       {
       	struct fsm_health_data *data = state->health_data;
 19:  023fcd6e2b1 ! 17:  baf8c031a97 fsm-health-win32: force shutdown daemon if worktree root moves
     @@ Commit message
          Force shutdown fsmonitor daemon if the worktree root directory
          is moved, renamed, or deleted.
      
     +    Use Windows low-level GetFileInformationByHandle() to get and
     +    compare the Windows system unique ID for the directory with a
     +    cached version when we started up.  This lets us detect the
     +    case where someone renames the directory that we are watching
     +    and then creates a new directory with the original pathname.
     +
     +    This is important because we are listening to a named pipe for
     +    requests and they are stored in the Named Pipe File System (NPFS)
     +    which a kernel-resident pseudo filesystem not associated with
     +    the actual NTFS directory.
     +
     +    For example, if the daemon was watching "~/foo/", it would have
     +    a directory-watch handle on that directory and a named-pipe
     +    handle for "//./pipe/...foo".  Moving the directory to "~/bar/"
     +    does not invalidate the directory handle.  (So the daemon would
     +    actually be watching "~/bar" but listening on "//./pipe/...foo".
     +    If the user then does "git init ~/foo" and causes another daemon
     +    to start, the first daemon will still have ownership of the pipe
     +    and the second daemon instance will fail to start.  "git status"
     +    clients in "~/foo" will ask "//./pipe/...foo" about changes and
     +    the first daemon instance will tell them about "~/bar".
     +
     +    This commit causes the first daemon to shutdown if the system unique
     +    ID for "~/foo" changes (changes from what it was when the daemon
     +    started).  Shutdown occurs after a periodic poll.  After the
     +    first daemon exits and releases the lock on the named pipe,
     +    subsequent Git commands may cause another daemon to be started
     +    on "~/foo".  Similarly, a subsequent Git command may cause another
     +    daemon to be started on "~/bar".
     +
          Signed-off-by: Jeff Hostetler <[email-masked]>
      
       ## compat/fsmonitor/fsm-health-win32.c ##
     -@@ compat/fsmonitor/fsm-health-win32.c: enum interval_fn_ctx { CTX_INIT = 0, CTX_TERM, CTX_TIMER };
     - typedef int (interval_fn)(struct fsmonitor_daemon_state *state,
     - 			  enum interval_fn_ctx ctx);
     - 
     -+static interval_fn has_worktree_moved;
     -+
     - static interval_fn *table[] = {
     -+	has_worktree_moved,
     - 	NULL, /* must be last */
     - };
     - 
      @@ compat/fsmonitor/fsm-health-win32.c: struct fsm_health_data
       	HANDLE hHandles[1]; /* the array does not own these handles */
       #define HEALTH_SHUTDOWN 0
     @@ compat/fsmonitor/fsm-health-win32.c: struct fsm_health_data
      +	} wt_moved;
       };
       
     - int fsm_health__ctor(struct fsmonitor_daemon_state *state)
     -@@ compat/fsmonitor/fsm-health-win32.c: void fsm_health__dtor(struct fsmonitor_daemon_state *state)
     - 	FREE_AND_NULL(state->health_data);
     - }
     - 
     ++/*
     ++ * Lookup the system unique ID for the path.  This is as close as
     ++ * we get to an inode number, but this also contains volume info,
     ++ * so it is a little stronger.
     ++ */
      +static int lookup_bhfi(wchar_t *wpath,
      +		       BY_HANDLE_FILE_INFORMATION *bhfi)
      +{
     @@ compat/fsmonitor/fsm-health-win32.c: void fsm_health__dtor(struct fsmonitor_daem
      +	return 0;
      +}
      +
     ++/*
     ++ * Compare the relevant fields from two system unique IDs.
     ++ * We use this to see if two different handles to the same
     ++ * path actually refer to the same *instance* of the file
     ++ * or directory.
     ++ */
      +static int bhfi_eq(const BY_HANDLE_FILE_INFORMATION *bhfi_1,
      +		   const BY_HANDLE_FILE_INFORMATION *bhfi_2)
      +{
     @@ compat/fsmonitor/fsm-health-win32.c: void fsm_health__dtor(struct fsmonitor_daem
      +	return 0;
      +}
      +
     - void fsm_health__loop(struct fsmonitor_daemon_state *state)
     ++
     + int fsm_health__ctor(struct fsmonitor_daemon_state *state)
       {
     - 	struct fsm_health_data *data = state->health_data;
     + 	struct fsm_health_data *data;
     +@@ compat/fsmonitor/fsm-health-win32.c: void fsm_health__dtor(struct fsmonitor_daemon_state *state)
     +  * A table of the polling functions.
     +  */
     + static interval_fn *table[] = {
     ++	has_worktree_moved,
     + 	NULL, /* must be last */
     + };
     + 
 20:  496b3da35d0 ! 18:  796b6591393 fsm-listen-darwin: shutdown daemon if worktree root is moved/renamed
     @@ compat/fsmonitor/fsm-listen-darwin.c: static void fsevent_callback(ConstFSEventS
      +			/*
      +			 * The spelling of the pathname of the root directory
      +			 * has changed.  This includes the name of the root
     -+			 * directory itself of of any parent directory in the
     ++			 * directory itself or of any parent directory in the
      +			 * path.
      +			 *
      +			 * (There may be other conditions that throw this,
 21:  07a9c7542be = 19:  24591920878 fsmonitor: optimize processing of directory events
 22:  f065e8c9a90 = 20:  06a32413854 t7527: FSMonitor tests for directory moves
 23:  e5a12832afa = 21:  4b59013cadd t/perf/p7527: add perf test for builtin FSMonitor
  -:  ----------- > 22:  524d449ed64 fsmonitor: never set CE_FSMONITOR_VALID on submodules
  2:  ad8cf6d9a47 ! 23:  c7264decaf6 t7527: test FS event reporing on macOS WRT case and Unicode
     @@ Metadata
      Author: Jeff Hostetler <[email-masked]>
      
       ## Commit message ##
     -    t7527: test FS event reporing on macOS WRT case and Unicode
     +    t7527: test FSMonitor on case insensitive+preserving file system
      
     -    Confirm that macOS FS events are reported with a normalized spelling.
     -
     -    APFS (and/or HFS+) is case-insensitive.  This means that case-independent
     -    lookups ( [ -d .git ] and [ -d .GIT ] ) should both succeed.  But that
     -    doesn't tell us how FS events are reported if we try "rm -rf .git" versus
     -    "rm -rf .GIT".  Are the events reported using the on-disk spelling of the
     -    pathname or in the spelling used by the command.
     -
     -    NEEDSWORK: I was only able to test case.  It would be nice to add tests
     -    that use different Unicode spellings/normalizations and understand the
     -    differences between APFS and HFS+ in this area.  We should confirm that
     -    the spelling of the workdir paths that the daemon sends to clients are
     -    always properly normalized.
     +    Test that FS events from the OS are received using the preserved,
     +    on-disk spelling of files/directories rather than spelling used
     +    to make the change.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
      
       ## t/t7527-builtin-fsmonitor.sh ##
     -@@ t/t7527-builtin-fsmonitor.sh: test_expect_success MINGW,SHORTNAMES 'implicit daemon stop (rename GIT~2)' '
     - 	test_must_fail git -C test_implicit_1s2 fsmonitor--daemon status
     +@@ t/t7527-builtin-fsmonitor.sh: test_expect_success "Submodule" '
     + 	my_match_and_clean
       '
       
     -+# Confirm that MacOS hides all of the Unicode normalization and/or
     -+# case folding from the FS events.  That is, are the pathnames in the
     -+# FS events reported using the spelling on the disk or in the spelling
     -+# used by the other process.
     -+#
     -+# Note that we assume that the filesystem is set to case insensitive.
     -+#
     -+# NEEDSWORK: APFS handles Unicode and Unicode normalization
     -+# differently than HFS+.  I only have an APFS partition, so
     -+# more testing here would be helpful.
     ++# On a case-insensitive file system, confirm that the daemon
     ++# notices when the .git directory is moved/renamed/deleted
     ++# regardless of how it is spelled in the the FS event.
     ++# That is, does the FS event receive the spelling of the
     ++# operation or does it receive the spelling preserved with
     ++# the file/directory.
      +#
     ++test_expect_success CASE_INSENSITIVE_FS 'case insensitive+preserving' '
     ++#	test_when_finished "stop_daemon_delete_repo test_insensitive" &&
     ++
     ++	git init test_insensitive &&
     ++	(
     ++		GIT_TRACE_FSMONITOR="$(pwd)/insensitive.trace" &&
     ++		export GIT_TRACE_FSMONITOR &&
      +
     -+# Rename .git using alternate spelling and confirm that the daemon
     -+# sees the event using the correct spelling and shutdown.
     -+test_expect_success UTF8_NFD_TO_NFC 'MacOS event spelling (rename .GIT)' '
     -+	test_when_finished "stop_daemon_delete_repo test_apfs" &&
     ++		start_daemon test_insensitive
     ++	) &&
      +
     -+	git init test_apfs &&
     -+	start_daemon test_apfs &&
     ++	mkdir -p test_insensitive/abc/def &&
     ++	echo xyz >test_insensitive/ABC/DEF/xyz &&
      +
     -+	test_path_is_dir test_apfs/.git &&
     -+	test_path_is_dir test_apfs/.GIT &&
     ++	test_path_is_dir test_insensitive/.git &&
     ++	test_path_is_dir test_insensitive/.GIT &&
      +
     -+	mv test_apfs/.GIT test_apfs/.FOO &&
     ++	# Rename .git using an alternate spelling to verify that that
     ++	# daemon detects it and automatically shuts down.
     ++	mv test_insensitive/.GIT test_insensitive/.FOO &&
      +	sleep 1 &&
     -+	mv test_apfs/.FOO test_apfs/.git &&
     ++	mv test_insensitive/.FOO test_insensitive/.git &&
     ++	test_must_fail git -C test_insensitive fsmonitor--daemon status &&
      +
     -+	test_must_fail git -C test_apfs fsmonitor--daemon status
     ++	# Verify that events were reported using on-disk spellings of the
     ++	# directories and files that we touched.  We may or may not get a
     ++	# trailing slash on modified directories.
     ++	#
     ++	egrep "^event: abc/?$"       ./insensitive.trace &&
     ++	egrep "^event: abc/def/?$"   ./insensitive.trace &&
     ++	egrep "^event: abc/def/xyz$" ./insensitive.trace
      +'
      +
     - test_expect_success 'cannot start multiple daemons' '
     - 	test_when_finished "stop_daemon_delete_repo test_multiple" &&
     - 
     + test_done
 13:  f19671f7def <  -:  ----------- fsmonitor--daemon: print start message only if fsmonitor.announceStartup
  -:  ----------- > 24:  95b9d4210d2 fsmonitor: on macOS also emit NFC spelling for NFD pathname
  -:  ----------- > 25:  5a0c1b7a287 t/lib-unicode-nfc-nfd: helper prereqs for testing unicode nfc/nfd
  -:  ----------- > 26:  a45c1fd3000 t7527: test Unicode NFC/NFD handling on MacOS
  -:  ----------- > 27:  e3e01677d93 fsmonitor-settings: NTFS and FAT32 on MacOS are incompatible

-- 
gitgitgadget