Subject: [PATCH v6 00/30] Builtin FSMonitor Part 2

From: "Jeff Hostetler via GitGitGadget" <[email-masked]>
Date: Tue, 01 Mar 2022 18:43:02 +0000

Here is V6 of Part 2 of my builtin FSmonitor series.

This version contains mostly cleanup based on feedback from V5. Of note:

 * I squashed in the 1_file fix for p7519.
 * I squashed in a commit from part 3 to optionally print the "running
   daemon on..." message on stderr.
 * I added a note to the documentation about incompatible changes around
   core.fsmonitor.
 * Removed/rephrased some obsolete NEEDSWORK items.

Tao has an ongoing parallel series to fix test-chmtime on Windows.
https://lore.kernel.org/all/[email-masked]/

If that lands first, we should be able to drop my 't/helper/test-chmtime:
skip directories on Windows' commit.

A followup Part 3 will contain additional refinements to the daemon and
additional tests. I drew the line here between Part 2 and 3 to make it
easier to review.

Jeff Hostetler (30):
  fsmonitor: enhance existing comments, clarify trivial response
    handling
  fsmonitor-ipc: create client routines for git-fsmonitor--daemon
  fsmonitor: config settings are repository-specific
  fsmonitor: use IPC to query the builtin FSMonitor daemon
  fsmonitor: document builtin fsmonitor
  fsmonitor--daemon: add a built-in fsmonitor daemon
  fsmonitor--daemon: implement 'stop' and 'status' commands
  compat/fsmonitor/fsm-listen-win32: stub in backend for Windows
  compat/fsmonitor/fsm-listen-darwin: stub in backend for Darwin
  fsmonitor--daemon: implement 'run' command
  fsmonitor--daemon: implement 'start' command
  fsmonitor--daemon: add pathname classification
  fsmonitor--daemon: define token-ids
  fsmonitor--daemon: create token-based changed path cache
  compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on
    Windows
  compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent
  compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on
    MacOS
  fsmonitor--daemon: implement handle_client callback
  help: include fsmonitor--daemon feature flag in version info
  t/helper/fsmonitor-client: create IPC client to talk to FSMonitor
    Daemon
  t7527: create test for fsmonitor--daemon
  t/perf: avoid copying builtin fsmonitor files into test repo
  t/helper/test-chmtime: skip directories on Windows
  t/perf/p7519: speed up test on Windows
  t/perf/p7519: add fsmonitor--daemon test cases
  fsmonitor--daemon: periodically truncate list of modified files
  fsmonitor--daemon: use a cookie file to sync with file system
  fsmonitor: force update index after large responses
  t7527: test status with untracked-cache and fsmonitor--daemon
  update-index: convert fsmonitor warnings to advise

 .gitignore                              |    1 +
 Documentation/config/core.txt           |   60 +-
 Documentation/git-fsmonitor--daemon.txt |   75 ++
 Documentation/git-update-index.txt      |    8 +-
 Makefile                                |   17 +
 builtin.h                               |    1 +
 builtin/fsmonitor--daemon.c             | 1473 +++++++++++++++++++++++
 builtin/update-index.c                  |   19 +-
 cache.h                                 |    1 -
 compat/fsmonitor/fsm-listen-darwin.c    |  499 ++++++++
 compat/fsmonitor/fsm-listen-win32.c     |  586 +++++++++
 compat/fsmonitor/fsm-listen.h           |   49 +
 config.c                                |   14 -
 config.h                                |    1 -
 config.mak.uname                        |   20 +
 contrib/buildsystems/CMakeLists.txt     |   10 +
 environment.c                           |    1 -
 fsmonitor--daemon.h                     |  166 +++
 fsmonitor-ipc.c                         |  171 +++
 fsmonitor-ipc.h                         |   48 +
 fsmonitor-settings.c                    |  115 ++
 fsmonitor-settings.h                    |   21 +
 fsmonitor.c                             |  216 +++-
 fsmonitor.h                             |   15 +-
 git.c                                   |    1 +
 help.c                                  |    4 +
 repo-settings.c                         |    1 +
 repository.h                            |    3 +
 t/README                                |    4 +-
 t/helper/test-chmtime.c                 |   15 +
 t/helper/test-fsmonitor-client.c        |  121 ++
 t/helper/test-tool.c                    |    1 +
 t/helper/test-tool.h                    |    1 +
 t/perf/p7519-fsmonitor.sh               |   61 +-
 t/perf/perf-lib.sh                      |    2 +-
 t/t7527-builtin-fsmonitor.sh            |  604 ++++++++++
 t/test-lib.sh                           |    6 +
 37 files changed, 4301 insertions(+), 110 deletions(-)
 create mode 100644 Documentation/git-fsmonitor--daemon.txt
 create mode 100644 builtin/fsmonitor--daemon.c
 create mode 100644 compat/fsmonitor/fsm-listen-darwin.c
 create mode 100644 compat/fsmonitor/fsm-listen-win32.c
 create mode 100644 compat/fsmonitor/fsm-listen.h
 create mode 100644 fsmonitor--daemon.h
 create mode 100644 fsmonitor-ipc.c
 create mode 100644 fsmonitor-ipc.h
 create mode 100644 fsmonitor-settings.c
 create mode 100644 fsmonitor-settings.h
 create mode 100644 t/helper/test-fsmonitor-client.c
 create mode 100755 t/t7527-builtin-fsmonitor.sh

base-commit: 715d08a9e51251ad8290b181b6ac3b9e1f9719d7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1041%2Fjeffhostetler%2Fbuiltin-fsmonitor-part2-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1041/jeffhostetler/builtin-fsmonitor-part2-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1041

Range-diff vs v5:

  1:  a5ecabb4d02 !  1:  10a34082fcc fsmonitor: enhance existing comments, clarify trivial response handling
     @@ Commit message
          fsmonitor: enhance existing comments, clarify trivial response handling
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## fsmonitor.c ##
      @@ fsmonitor.c: static int query_fsmonitor(int version, const char *last_update, struct strbuf *
  2:  365964b7664 !  2:  e1c946af2c5 fsmonitor-ipc: create client routines for git-fsmonitor--daemon
     @@ Commit message
          Stub in empty fsmonitor_ipc__*() functions for unsupported platforms.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## Makefile ##
      @@ Makefile: LIB_OBJS += fetch-pack.o
     @@ fsmonitor-ipc.c (new)
      +#include "strbuf.h"
      +#include "trace2.h"
      +
     -+#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
     ++#ifndef HAVE_FSMONITOR_DAEMON_BACKEND
     ++
     ++/*
     ++ * A trivial implementation of the fsmonitor_ipc__ API for unsupported
     ++ * platforms.
     ++ */
     ++
     ++int fsmonitor_ipc__is_supported(void)
     ++{
     ++	return 0;
     ++}
     ++
     ++const char *fsmonitor_ipc__get_path(void)
     ++{
     ++	return NULL;
     ++}
     ++
     ++enum ipc_active_state fsmonitor_ipc__get_state(void)
     ++{
     ++	return IPC_STATE__OTHER_ERROR;
     ++}
     ++
     ++int fsmonitor_ipc__send_query(const char *since_token,
     ++			      struct strbuf *answer)
     ++{
     ++	return -1;
     ++}
     ++
     ++int fsmonitor_ipc__send_command(const char *command,
     ++				struct strbuf *answer)
     ++{
     ++	return -1;
     ++}
     ++
     ++#else
      +
      +int fsmonitor_ipc__is_supported(void)
      +{
     @@ fsmonitor-ipc.c (new)
      +	return 0;
      +}
      +
     -+#else
     -+
     -+/*
     -+ * A trivial implementation of the fsmonitor_ipc__ API for unsupported
     -+ * platforms.
     -+ */
     -+
     -+int fsmonitor_ipc__is_supported(void)
     -+{
     -+	return 0;
     -+}
     -+
     -+const char *fsmonitor_ipc__get_path(void)
     -+{
     -+	return NULL;
     -+}
     -+
     -+enum ipc_active_state fsmonitor_ipc__get_state(void)
     -+{
     -+	return IPC_STATE__OTHER_ERROR;
     -+}
     -+
     -+int fsmonitor_ipc__send_query(const char *since_token,
     -+			      struct strbuf *answer)
     -+{
     -+	return -1;
     -+}
     -+
     -+int fsmonitor_ipc__send_command(const char *command,
     -+				struct strbuf *answer)
     -+{
     -+	return -1;
     -+}
     -+
      +#endif
      
       ## fsmonitor-ipc.h (new) ##
  3:  384516ce1a1 !  3:  ae622a517cf fsmonitor: config settings are repository-specific
     @@ Commit message
          restrictions to override the config values used.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## Makefile ##
      @@ Makefile: LIB_OBJS += fmt-merge-msg.o
     @@ fsmonitor-settings.c (new)
      +
      +enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
      +{
     ++	if (!r)
     ++		r = the_repository;
     ++
      +	lookup_fsmonitor_settings(r);
      +
      +	return r->settings.fsmonitor->mode;
     @@ fsmonitor-settings.c (new)
      +
      +const char *fsm_settings__get_hook_path(struct repository *r)
      +{
     ++	if (!r)
     ++		r = the_repository;
     ++
      +	lookup_fsmonitor_settings(r);
      +
      +	return r->settings.fsmonitor->hook_path;
     @@ fsmonitor-settings.c (new)
      +
      +void fsm_settings__set_ipc(struct repository *r)
      +{
     ++	if (!r)
     ++		r = the_repository;
     ++
      +	lookup_fsmonitor_settings(r);
      +
      +	r->settings.fsmonitor->mode = FSMONITOR_MODE_IPC;
     @@ fsmonitor-settings.c (new)
      +
      +void fsm_settings__set_hook(struct repository *r, const char *path)
      +{
     ++	if (!r)
     ++		r = the_repository;
     ++
      +	lookup_fsmonitor_settings(r);
      +
      +	r->settings.fsmonitor->mode = FSMONITOR_MODE_HOOK;
     @@ fsmonitor-settings.c (new)
      +
      +void fsm_settings__set_disabled(struct repository *r)
      +{
     ++	if (!r)
     ++		r = the_repository;
     ++
      +	lookup_fsmonitor_settings(r);
      +
      +	r->settings.fsmonitor->mode = FSMONITOR_MODE_DISABLED;
     @@ fsmonitor.c: void remove_fsmonitor(struct index_state *istate)
       {
       	unsigned int i;
      -	int fsmonitor_enabled = git_config_get_fsmonitor();
     -+	struct repository *r = istate->repo ? istate->repo : the_repository;
     -+	int fsmonitor_enabled = (fsm_settings__get_mode(r) > FSMONITOR_MODE_DISABLED);
     ++	int fsmonitor_enabled = (fsm_settings__get_mode(istate->repo)
     ++				 > FSMONITOR_MODE_DISABLED);
       
       	if (istate->fsmonitor_dirty) {
       		if (fsmonitor_enabled) {
     @@ fsmonitor.h: int fsmonitor_is_trivial_response(const struct strbuf *query_result
       static inline int is_fsmonitor_refreshed(const struct index_state *istate)
       {
      -	return !core_fsmonitor || istate->fsmonitor_has_run_once;
     -+	struct repository *r = istate->repo ? istate->repo : the_repository;
     -+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
     ++	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(istate->repo);
      +
      +	return fsm_mode <= FSMONITOR_MODE_DISABLED ||
      +		istate->fsmonitor_has_run_once;
     @@ fsmonitor.h: static inline int is_fsmonitor_refreshed(const struct index_state *
       static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache_entry *ce)
       {
      -	if (core_fsmonitor && !(ce->ce_flags & CE_FSMONITOR_VALID)) {
     -+	struct repository *r = istate->repo ? istate->repo : the_repository;
     -+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
     ++	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(istate->repo);
      +
      +	if (fsm_mode > FSMONITOR_MODE_DISABLED &&
      +	    !(ce->ce_flags & CE_FSMONITOR_VALID)) {
     @@ fsmonitor.h: static inline void mark_fsmonitor_valid(struct index_state *istate,
       static inline void mark_fsmonitor_invalid(struct index_state *istate, struct cache_entry *ce)
       {
      -	if (core_fsmonitor) {
     -+	struct repository *r = istate->repo ? istate->repo : the_repository;
     -+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
     ++	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(istate->repo);
      +
      +	if (fsm_mode > FSMONITOR_MODE_DISABLED) {
       		ce->ce_flags &= ~CE_FSMONITOR_VALID;
  4:  8e738a83bc5 !  4:  55974867da5 fsmonitor: use IPC to query the builtin FSMonitor daemon
     @@ Commit message
      
          Signed-off-by: Johannes Schindelin <[email-masked]>
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## fsmonitor.c ##
      @@ fsmonitor.c: void refresh_fsmonitor(struct index_state *istate)
  5:  49e4c146e02 !  5:  ce42d5bbaf6 fsmonitor: document builtin fsmonitor
     @@ Commit message
          Create `git-fsmonitor--daemon` manual page and describe its features.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## Documentation/config/core.txt ##
      @@ Documentation/config/core.txt: core.protectNTFS::
     @@ Documentation/config/core.txt: core.protectNTFS::
      +git by avoiding unnecessary scanning of files that have not changed.
      ++
      +See the "fsmonitor-watchman" section of linkgit:githooks[5].
     +++
     ++Note that if you concurrently use multiple versions of Git, such
     ++as one version on the command line and another version in an IDE
     ++tool, that the definition of `core.fsmonitor` was extended to
     ++allow boolean values in addition to hook pathnames.  Git versions
     ++2.35.1 and prior will not understand the boolean values and will
     ++consider the "true" or "false" values as hook pathnames to be
     ++invoked.  Git versions 2.26 thru 2.35.1 default to hook protocol
     ++V2 and will fall back to no fsmonitor (full scan).  Git versions
     ++prior to 2.26 default to hook protocol V1 and will silently
     ++assume there were no changes to report (no scan), so status
     ++commands may report incomplete results.  For this reason, it is
     ++best to upgrade all of your Git versions before using the built-in
     ++file system monitor.
       
       core.fsmonitorHookVersion::
      -	Sets the version of hook that is to be used when calling fsmonitor.
  6:  bdd7334da31 !  6:  9ce938c69b5 fsmonitor--daemon: add a built-in fsmonitor daemon
     @@ Commit message
      
          Co-authored-by: Johannes Schindelin <[email-masked]>
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## .gitignore ##
      @@
  7:  9f00ada3dd3 !  7:  3bd7e6a051e fsmonitor--daemon: implement 'stop' and 'status' commands
     @@ Commit message
          system.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@
  8:  d6819bdad66 !  8:  92d3e54da7f compat/fsmonitor/fsm-listen-win32: stub in backend for Windows
     @@ Commit message
          Stub in empty filesystem listener backend for fsmonitor--daemon on Windows.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## Makefile ##
      @@ Makefile: all::
  9:  7de3d01cccc !  9:  f1219221270 compat/fsmonitor/fsm-listen-darwin: stub in backend for Darwin
     @@ Commit message
          backend for Darwin (aka MacOS).
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## compat/fsmonitor/fsm-listen-darwin.c (new) ##
      @@
 10:  6fe5a2bc79e ! 10:  a3f94627cf6 fsmonitor--daemon: implement 'run' command
     @@ Commit message
          backends are still just stubs.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@
     @@ builtin/fsmonitor--daemon.c
      +#define FSMONITOR__IPC_THREADS "fsmonitor.ipcthreads"
      +static int fsmonitor__ipc_threads = 8;
      +
     ++#define FSMONITOR__ANNOUNCE_STARTUP "fsmonitor.announcestartup"
     ++static int fsmonitor__announce_startup = 0;
     ++
      +static int fsmonitor_config(const char *var, const char *value, void *cb)
      +{
      +	if (!strcmp(var, FSMONITOR__IPC_THREADS)) {
     @@ builtin/fsmonitor--daemon.c
      +		return 0;
      +	}
      +
     ++	if (!strcmp(var, FSMONITOR__ANNOUNCE_STARTUP)) {
     ++		int is_bool;
     ++		int i = git_config_bool_or_int(var, value, &is_bool);
     ++		if (i < 0)
     ++			return error(_("value of '%s' not bool or int: %d"),
     ++				     var, i);
     ++		fsmonitor__announce_startup = i;
     ++		return 0;
     ++	}
     ++
      +	return git_default_config(var, value, cb);
      +}
      +
     @@ builtin/fsmonitor--daemon.c: static int do_as_client__status(void)
      +		die("fsmonitor--daemon is already running '%s'",
      +		    the_repository->worktree);
      +
     -+	printf(_("running fsmonitor-daemon in '%s'\n"),
     -+	       the_repository->worktree);
     -+	fflush(stdout);
     ++	if (fsmonitor__announce_startup) {
     ++		fprintf(stderr, _("running fsmonitor-daemon in '%s'\n"),
     ++			the_repository->worktree);
     ++		fflush(stderr);
     ++	}
      +
      +	return !!fsmonitor_run_daemon();
      +}
 11:  69fc0998286 ! 11:  8de40b0fe8b fsmonitor--daemon: implement 'start' command
     @@ Commit message
          because they don't have a Win32 console attached to them.)
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@
     @@ builtin/fsmonitor--daemon.c: static const char * const builtin_fsmonitor__daemon
      +#define FSMONITOR__START_TIMEOUT "fsmonitor.starttimeout"
      +static int fsmonitor__start_timeout_sec = 60;
      +
     - static int fsmonitor_config(const char *var, const char *value, void *cb)
     - {
     - 	if (!strcmp(var, FSMONITOR__IPC_THREADS)) {
     + #define FSMONITOR__ANNOUNCE_STARTUP "fsmonitor.announcestartup"
     + static int fsmonitor__announce_startup = 0;
     + 
      @@ builtin/fsmonitor--daemon.c: static int fsmonitor_config(const char *var, const char *value, void *cb)
       		return 0;
       	}
     @@ builtin/fsmonitor--daemon.c: static int fsmonitor_config(const char *var, const
      +		return 0;
      +	}
      +
     - 	return git_default_config(var, value, cb);
     - }
     - 
     + 	if (!strcmp(var, FSMONITOR__ANNOUNCE_STARTUP)) {
     + 		int is_bool;
     + 		int i = git_config_bool_or_int(var, value, &is_bool);
      @@ builtin/fsmonitor--daemon.c: done:
       	return err;
       }
       
      -static int try_to_run_foreground_daemon(void)
     -+static int try_to_run_foreground_daemon(int free_console)
     ++static int try_to_run_foreground_daemon(int detach_console)
       {
       	/*
       	 * Technically, we don't need to probe for an existing daemon
      @@ builtin/fsmonitor--daemon.c: static int try_to_run_foreground_daemon(void)
     - 	       the_repository->worktree);
     - 	fflush(stdout);
     + 		fflush(stderr);
     + 	}
       
      +#ifdef GIT_WINDOWS_NATIVE
     -+	if (free_console)
     ++	if (detach_console)
      +		FreeConsole();
      +#endif
      +
     @@ builtin/fsmonitor--daemon.c: static int try_to_run_foreground_daemon(void)
      +		die("fsmonitor--daemon is already running '%s'",
      +		    the_repository->worktree);
      +
     -+	printf(_("starting fsmonitor-daemon in '%s'\n"),
     -+	       the_repository->worktree);
     -+	fflush(stdout);
     ++	if (fsmonitor__announce_startup) {
     ++		fprintf(stderr, _("starting fsmonitor-daemon in '%s'\n"),
     ++			the_repository->worktree);
     ++		fflush(stderr);
     ++	}
      +
      +	cp.git_cmd = 1;
      +
      +	strvec_push(&cp.args, "fsmonitor--daemon");
      +	strvec_push(&cp.args, "run");
     -+	strvec_push(&cp.args, "--free-console");
     ++	strvec_push(&cp.args, "--detach");
      +	strvec_pushf(&cp.args, "--ipc-threads=%d", fsmonitor__ipc_threads);
      +
      +	cp.no_stdin = 1;
     @@ builtin/fsmonitor--daemon.c: static int try_to_run_foreground_daemon(void)
       int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
       {
       	const char *subcmd;
     -+	int free_console = 0;
     ++	int detach_console = 0;
       
       	struct option options[] = {
     -+		OPT_BOOL(0, "free-console", &free_console, N_("free console")),
     ++		OPT_BOOL(0, "detach", &detach_console, N_("detach from console")),
       		OPT_INTEGER(0, "ipc-threads",
       			    &fsmonitor__ipc_threads,
       			    N_("use <n> ipc worker threads")),
      +		OPT_INTEGER(0, "start-timeout",
      +			    &fsmonitor__start_timeout_sec,
     -+			    N_("Max seconds to wait for background daemon startup")),
     ++			    N_("max seconds to wait for background daemon startup")),
      +
       		OPT_END()
       	};
     @@ builtin/fsmonitor--daemon.c: int cmd_fsmonitor__daemon(int argc, const char **ar
      +
       	if (!strcmp(subcmd, "run"))
      -		return !!try_to_run_foreground_daemon();
     -+		return !!try_to_run_foreground_daemon(free_console);
     ++		return !!try_to_run_foreground_daemon(detach_console);
       
       	if (!strcmp(subcmd, "stop"))
       		return !!do_as_client__send_stop();
 12:  21c099c5197 ! 12:  e5d419b1ea0 fsmonitor--daemon: add pathname classification
     @@ Commit message
          When we register for filesystem notifications on a directory,
          we get events for everything (recursively) in the directory.
          We want to report to clients changes to tracked and untracked
     -    paths within the working directory.  We do not want to report
     -    changes within the .git directory, for example.
     +    paths within the working directory proper.  We do not want to
     +    report changes within the .git directory, for example.
      
          This classification will be used in a later commit by the
          different backends to classify paths as events are received.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@ builtin/fsmonitor--daemon.c: static int handle_client(void *data,
     @@ fsmonitor--daemon.h: struct fsmonitor_daemon_state {
      + * that to decide whether clients are told about them.  (And to watch
      + * for file system synchronization events.)
      + *
     ++ * The daemon only collects and reports on the set of modified paths
     ++ * within the working directory (proper).
     ++ *
      + * The client should only care about paths within the working
      + * directory proper (inside the working directory and not ".git" nor
      + * inside of ".git/").  That is, the client has read the index and is
      + * asking for a list of any paths in the working directory that have
      + * been modified since the last token.  The client does not care about
     -+ * file system changes within the .git directory (such as new loose
     ++ * file system changes within the ".git/" directory (such as new loose
      + * objects or packfiles).  So the client will only receive paths that
      + * are classified as IS_WORKDIR_PATH.
      + *
     ++ * Note that ".git" is usually a directory and is therefore inside
     ++ * the cone of the FS watch that we have on the working directory root,
     ++ * so we will also get FS events for disk activity on and within ".git/"
     ++ * that we need to respond to or filter from the client.
     ++ *
     ++ * But Git also allows ".git" to be a *file* that points to a GITDIR
     ++ * outside of the working directory.  When this happens, we need to
     ++ * create FS watches on both the working directory root *and* on the
     ++ * (external) GITDIR root.  (The latter is required because we put
     ++ * cookie files inside it and use them to sync with the FS event
     ++ * stream.)
     ++ *
     ++ * Note that in the context of this discussion, I'm using "GITDIR"
     ++ * to only mean an external GITDIR referenced by a ".git" file.
     ++ *
     ++ * The platform FS event backends will receive watch-specific
     ++ * relative paths (except for those OS's that always emit absolute
     ++ * paths).  We use the following enum and routines to classify each
     ++ * path so that we know how to handle it.  There is a slight asymmetry
     ++ * here because ".git/" is inside the working directory and the
     ++ * (external) GITDIR is not, and therefore how we handle events may
     ++ * vary slightly, so I have different enums for "IS...DOT_GIT..." and
     ++ * "IS...GITDIR...".
     ++ *
      + * The daemon uses the IS_DOT_GIT and IS_GITDIR internally to mean the
     -+ * exact ".git" directory or GITDIR.  If the daemon receives a delete
     -+ * event for either of these directories, it will automatically
     -+ * shutdown, for example.
     ++ * exact ".git" file/directory or GITDIR directory.  If the daemon
     ++ * receives a delete event for either of these paths, it will
     ++ * automatically shutdown, for example.
      + *
      + * Note that the daemon DOES NOT explicitly watch nor special case the
     -+ * ".git/index" file.  The daemon does not read the index and does not
     -+ * have any internal index-relative state.  The daemon only collects
     -+ * the set of modified paths within the working directory.
     ++ * index.  The daemon does not read the index nor have any internal
     ++ * index-relative state, so there are no "IS...INDEX..." enum values.
      + */
      +enum fsmonitor_path_type {
      +	IS_WORKDIR_PATH = 0,
 13:  72979c35ceb ! 13:  c2f3668a866 fsmonitor--daemon: define token-ids
     @@ Commit message
          overall token naming scheme.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@ builtin/fsmonitor--daemon.c: static int do_as_client__status(void)
 14:  8d1db644409 ! 14:  bd492d13fb1 fsmonitor--daemon: create token-based changed path cache
     @@ Commit message
          paths within these batches for efficient presentation to clients.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@ builtin/fsmonitor--daemon.c: struct fsmonitor_token_data {
     @@ builtin/fsmonitor--daemon.c: static struct fsmonitor_token_data *fsmonitor_new_t
      + *     We should create a new token and start fresh (as if we just
      + *     booted up).
      + *
     -+ * If there are no concurrent threads readering the current token data
     ++ * If there are no concurrent threads reading the current token data
      + * series, we can free it now.  Otherwise, let the last reader free
      + * it.
      + *
 15:  98c5adf8ca0 ! 15:  6b5b3d08227 compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on Windows
     @@ Commit message
          into batches and publish.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## compat/fsmonitor/fsm-listen-win32.c ##
      @@
 16:  a3b881315fa ! 16:  cdef9730b3f compat/fsmonitor/fsm-listen-darwin: add macos header files for FSEvent
     @@ Metadata
      Author: Jeff Hostetler <[email-masked]>
      
       ## Commit message ##
     -    compat/fsmonitor/fsm-listen-darwin: add macos header files for FSEvent
     +    compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent
      
          Include MacOS system declarations to allow us to use FSEvent and
     -    CoreFoundation APIs.  We need GCC and clang versions because of
     -    compiler and header file conflicts.
     +    CoreFoundation APIs.  We need different versions of the declarations
     +    for GCC vs. clang because of compiler and header file conflicts.
      
          While it is quite possible to #include Apple's CoreServices.h when
          compiling C source code with clang, trying to build it with GCC
     @@ Commit message
      
          Signed-off-by: Johannes Schindelin <[email-masked]>
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## compat/fsmonitor/fsm-listen-darwin.c ##
      @@
     -+#if defined(__GNUC__)
     ++#ifndef __clang__
      +/*
      + * It is possible to #include CoreFoundation/CoreFoundation.h when compiling
      + * with clang, but not with GCC as of time of writing.
 17:  162e357db72 ! 17:  aabfebd3a41 compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on MacOS
     @@ Commit message
          Co-authored-by: Kevin Willford <[email-masked]>
          Co-authored-by: Johannes Schindelin <[email-masked]>
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## compat/fsmonitor/fsm-listen-darwin.c ##
     -@@
     --#if defined(__GNUC__)
     -+#ifndef __clang__
     - /*
     -  * It is possible to #include CoreFoundation/CoreFoundation.h when compiling
     -  * with clang, but not with GCC as of time of writing.
      @@ compat/fsmonitor/fsm-listen-darwin.c: void FSEventStreamRelease(FSEventStreamRef stream);
       #include "cache.h"
       #include "fsmonitor.h"
     @@ compat/fsmonitor/fsm-listen-darwin.c: void FSEventStreamRelease(FSEventStreamRef
      +}
      +
      +/*
     -+ * NEEDSWORK: Investigate the proper value for the `latency` argument
     -+ * in the call to `FSEventStreamCreate()`.  I'm not sure that this
     -+ * needs to be a config setting or just something that we tune after
     -+ * some testing.
     ++ * In the call to `FSEventStreamCreate()` to setup our watch, the
     ++ * `latency` argument determines the frequency of calls to our callback
     ++ * with new FS events.  Too slow and events get dropped; too fast and
     ++ * we burn CPU unnecessarily.  Since it is rather obscure, I don't
     ++ * think this needs to be a config setting.  I've done extensive
     ++ * testing on my systems and chosen the value below.  It gives good
     ++ * results and I've not seen any dropped events.
      + *
      + * With a latency of 0.1, I was seeing lots of dropped events during
      + * the "touch 100000" files test within t/perf/p7519, but with a
     -+ * latency of 0.001 I did not see any dropped events.  So the
     -+ * "correct" value may be somewhere in between.
     ++ * latency of 0.001 I did not see any dropped events.  So I'm going
     ++ * to assume that this is the "correct" value.
      + *
      + * https://developer.apple.com/documentation/coreservices/1443980-fseventstreamcreate
      + */
 18:  3de1c43beaf ! 18:  0896e72e2b8 fsmonitor--daemon: implement handle_client callback
     @@ Commit message
          relative to the provided token.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@
 19:  3517c4a3c13 ! 19:  bc240a9e665 help: include fsmonitor--daemon feature flag in version info
     @@ Commit message
      
          This can be used by test scripts for prereq testing.  Granted, tests
          could just try `git fsmonitor--daemon status` and look for a 128 exit
     -    code or grep for a "not supported" message on stderr, but this is
     -    rather obscure.
     +    code or grep for a "not supported" message on stderr, but these
     +    methods are rather obscure.
      
          The main advantage is that the feature message will automatically
          appear in bug reports and other support requests.
      
     +    This concept was also used during the development of Scalar for
     +    similar reasons.
     +
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## help.c ##
      @@
 20:  4ffc2ddf516 ! 20:  2b563c240eb t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon
     @@ Commit message
          Create an IPC client to send query and flush commands to the daemon.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## Makefile ##
      @@ Makefile: TEST_BUILTINS_OBJS += test-dump-split-index.o
 21:  d93310a7c64 ! 21:  db8809da089 t7527: create test for fsmonitor--daemon
     @@ Commit message
          t7527: create test for fsmonitor--daemon
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## t/t7527-builtin-fsmonitor.sh (new) ##
      @@
 22:  27e47108908 ! 22:  15bd5aaff36 t/perf: avoid copying builtin fsmonitor files into test repo
     @@ Commit message
          .git/fsmonitor--daemon*, so it is simple to exclude them.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## t/perf/perf-lib.sh ##
      @@ t/perf/perf-lib.sh: test_perf_copy_repo_contents () {
 23:  6cba1d950b0 ! 23:  4cb97918d20 t/helper/test-chmtime: skip directories on Windows
     @@ Commit message
          an empty directory on disk.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## t/helper/test-chmtime.c ##
      @@ t/helper/test-chmtime.c: int cmd__chmtime(int argc, const char **argv)
 24:  fcf843a0d42 ! 24:  a70748b4640 t/perf/p7519: speed up test on Windows
     @@ Commit message
          on Windows because of process creation overhead.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## t/perf/p7519-fsmonitor.sh ##
      @@ t/perf/p7519-fsmonitor.sh: then
     @@ t/perf/p7519-fsmonitor.sh: test_expect_success "one time repo setup" '
      -	for i in $(test_seq 1 100); do touch 100_files/$i || return 1; done &&
      -	for i in $(test_seq 1 1000); do touch 1000_files/$i || return 1; done &&
      -	for i in $(test_seq 1 10000); do touch 10000_files/$i || return 1; done &&
     -+	touch_files 1 &&
     ++	: 1_file directory should be left empty &&
      +	touch_files 10 &&
      +	touch_files 100 &&
      +	touch_files 1000 &&
 25:  198f47bda5a ! 25:  b0c9c9c7a40 t/perf/p7519: add fsmonitor--daemon test cases
     @@ Commit message
          the "Simple IPC" interface.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## t/perf/p7519-fsmonitor.sh ##
      @@ t/perf/p7519-fsmonitor.sh: test_perf_w_drop_caches () {
 26:  19993c130d2 ! 26:  64a5b741670 fsmonitor--daemon: periodically truncate list of modified files
     @@ Commit message
          they are considered obsolete.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@ builtin/fsmonitor--daemon.c: static void fsmonitor_batch__combine(struct fsmonitor_batch *batch_dest,
 27:  f47a763dc26 ! 27:  5b06eb5d0e6 fsmonitor--daemon: use a cookie file to sync with file system
     @@ Commit message
          of `FSeventStreamCreate()`.  The kernel only signals every `latency`
          seconds, but does not guarantee that the kernel queue is completely
          drained, so we may have to wait more than one interval.  If we
     -    increase the frequency, the system is more likely to drop events.
     +    increase the latency, the system is more likely to drop events.
          We avoid these issues by having each client thread create a unique
          cookie file and then wait until it is seen in the event stream.
      
          Co-authored-by: Kevin Willford <[email-masked]>
          Co-authored-by: Johannes Schindelin <[email-masked]>
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/fsmonitor--daemon.c ##
      @@ builtin/fsmonitor--daemon.c: static int do_as_client__status(void)
     @@ builtin/fsmonitor--daemon.c: static int do_as_client__status(void)
      +		unlink(cookie_pathname.buf);
      +
      +		/*
     -+		 * NEEDSWORK: This is an infinite wait (well, unless another
     ++		 * Technically, this is an infinite wait (well, unless another
      +		 * thread sends us an abort).  I'd like to change this to
      +		 * use `pthread_cond_timedwait()` and return an error/timeout
     -+		 * and let the caller do the trivial response thing.
     ++		 * and let the caller do the trivial response thing, but we
     ++		 * don't have that routine in our thread-utils.
     ++		 *
     ++		 * After extensive beta testing I'm not really worried about
     ++		 * this.  Also note that the above open() and unlink() calls
     ++		 * will cause at least two FS events on that path, so the odds
     ++		 * of getting stuck are pretty slim.
      +		 */
      +		while (cookie->result == FCIR_INIT)
      +			pthread_cond_wait(&state->cookies_cond,
     @@ builtin/fsmonitor--daemon.c: static void fsmonitor_free_token_data(struct fsmoni
      + * [2] Some of those lost events may have been for cookie files.  We
      + *     should assume the worst and abort them rather letting them starve.
      + *
     -  * If there are no concurrent threads readering the current token data
     +  * If there are no concurrent threads reading the current token data
        * series, we can free it now.  Otherwise, let the last reader free
        * it.
      @@ builtin/fsmonitor--daemon.c: static void with_lock__do_force_resync(struct fsmonitor_daemon_state *state)
     @@ builtin/fsmonitor--daemon.c: done:
       	strbuf_release(&state.path_worktree_watch);
       	strbuf_release(&state.path_gitdir_watch);
      +	strbuf_release(&state.path_cookie_prefix);
     -+
     -+	/*
     -+	 * NEEDSWORK: Consider "rm -rf <gitdir>/<fsmonitor-dir>"
     -+	 */
       
       	return err;
       }
 28:  aec44a21afd ! 28:  1fd5439de03 fsmonitor: force update index after large responses
     @@ Commit message
          unnecessarily spend time actually scanning them during each command.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## fsmonitor.c ##
      @@ fsmonitor.c: static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
     @@ fsmonitor.c: static void fsmonitor_refresh_callback(struct index_state *istate,
      + * will be updated automatically the first time the user touches
      + * a tracked file and causes a command like `git status` to
      + * update an mtime to be updated and/or set a flag bit.
     -+ *
     -+ * NEEDSWORK: Does this need to be a config value?
      + */
      +static int fsmonitor_force_update_threshold = 100;
      +
 29:  d6039987df8 ! 29:  6fc1430285f t7527: test status with untracked-cache and fsmonitor--daemon
     @@ Commit message
          identical.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## t/t7527-builtin-fsmonitor.sh ##
      @@ t/t7527-builtin-fsmonitor.sh: test_expect_success 'setup' '
 30:  5117fbdfc63 ! 30:  b915b95cc2f update-index: convert fsmonitor warnings to advise
     @@ Commit message
          update-index: convert fsmonitor warnings to advise
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
     +    Signed-off-by: Junio C Hamano <[email-masked]>
      
       ## builtin/update-index.c ##
      @@ builtin/update-index.c: int cmd_update_index(int argc, const char **argv, const char *prefix)

-- 
gitgitgadget