Subject: [PATCH v3 0/5] Builtin FSMonitor Part 2

From: "Jeff Hostetler via GitGitGadget" <[email-masked]>
Date: Wed, 13 Oct 2021 20:31:42 +0000

Here is V3 of Part 2 of my Builtin FSMonitor series. Like V2, it is built
upon "next" because it requires "ab/repo-settings-cleanup" and
"jh/builtin-fsmonitor-part1" series.

V3 removes the explicit initialization of r->repo_settings->fsmonitor in
repo-settings.c as requested. It also includes a more detailed commit
message for the 3 commit to explain the rationale for putting fsmonitor
settings in its own source file rather than adding it repo-settings.c

There was a comment on the V2 series about integrating the fsmonitor hook
path with the on-going "hook config" effort that I did not address. I think
it would be best to do that in a follow-on if it makes sense to do so.

cc: Bagas Sanjaya [email-masked] cc: Jeff Hostetler
[email-masked]

Jeff Hostetler (5):
  fsmonitor: enhance existing comments
  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: update fsmonitor config documentation

 Documentation/config/core.txt      |  56 ++++++---
 Documentation/git-update-index.txt |  27 +++--
 Documentation/githooks.txt         |   3 +-
 Makefile                           |   2 +
 builtin/update-index.c             |  19 +++-
 cache.h                            |   1 -
 config.c                           |  14 ---
 config.h                           |   1 -
 environment.c                      |   1 -
 fsmonitor-ipc.c                    | 176 +++++++++++++++++++++++++++++
 fsmonitor-ipc.h                    |  48 ++++++++
 fsmonitor-settings.c               |  97 ++++++++++++++++
 fsmonitor-settings.h               |  21 ++++
 fsmonitor.c                        | 130 +++++++++++++++------
 fsmonitor.h                        |  18 ++-
 repository.h                       |   3 +
 t/README                           |   4 +-
 17 files changed, 536 insertions(+), 85 deletions(-)
 create mode 100644 fsmonitor-ipc.c
 create mode 100644 fsmonitor-ipc.h
 create mode 100644 fsmonitor-settings.c
 create mode 100644 fsmonitor-settings.h

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

Range-diff vs v2:

 1:  cb25eeaf72d = 1:  cb25eeaf72d fsmonitor: enhance existing comments
 2:  df81a63acee = 2:  df81a63acee fsmonitor-ipc: create client routines for git-fsmonitor--daemon
 3:  7d5a353e74d ! 3:  a1d606aa622 fsmonitor: config settings are repository-specific
     @@ Metadata
       ## Commit message ##
          fsmonitor: config settings are repository-specific
      
     -    Move FSMonitor config settings to a new `struct fsmonitor_settings`
     -    structure.  Add a lazily-loaded pointer to `struct repo_settings`.
     +    Move fsmonitor config settings to a new and opaque
     +    `struct fsmonitor_settings` structure.  Add a lazily-loaded pointer
     +    to this into `struct repo_settings`
     +
     +    Create an `enum fsmonitor_mode` type in `struct fsmonitor_settings` to
     +    represent the state of fsmonitor.  This lets us represent which, if
     +    any, fsmonitor provider (hook or IPC) is enabled.
     +
          Create `fsm_settings__get_*()` getters to lazily look up fsmonitor-
          related config settings.
      
     -    Get rid of the `core_fsmonitor` global variable, and add support for
     -    the new `core.useBuiltinFSMonitor` config setting.  Move config code
     -    to lookup the existing `core.fsmonitor` value to `fsmonitor-settings.[ch]`.
     +    Add support for the new `core.useBuiltinFSMonitor` config setting.
     +
     +    Get rid of the `core_fsmonitor` global variable.  Move the code to
     +    lookup the existing `core.fsmonitor` config value into the fsmonitor
     +    settings.
     +
     +    Create a hook pathname variable in `struct fsmonitor-settings` and
     +    only set it when in hook mode.
      
     -    The `core_fsmonitor` global variable was used to store the pathname to
     -    the FSMonitor hook and it was used as a boolean to see if FSMonitor
     -    was enabled.  This dual usage will lead to confusion when we add
     -    support for a builtin FSMonitor based on IPC, since the builtin
     -    FSMonitor doesn't need the hook pathname.
     +    The existing `core_fsmonitor` global variable was used to store the
     +    pathname to the fsmonitor hook *and* it was used as a boolean to see
     +    if fsmonitor was enabled.  This dual usage and global visibility leads
     +    to confusion when we add the IPC-based provider.  So lets hide the
     +    details in fsmonitor-settings.c and let it decide which provider to
     +    use in the case of multiple settings.  This avoids cluttering up
     +    repo-settings.c with these private details.
      
     -    Replace the boolean usage with an `enum fsmonitor_mode` to represent
     -    the state of FSMonitor.  And only set the pathname when in HOOK mode.
     +    A future commit in builtin-fsmonitor series will add the ability to
     +    disqualify worktrees for various reasons, such as being mounted from a
     +    remote volume, where fsmonitor should not be started.  Having the
     +    config settings hidden in fsmonitor-settings.c allows such worktree
     +    restrictions to override the config values used.
      
          Signed-off-by: Jeff Hostetler <[email-masked]>
      
     @@ fsmonitor.h: static inline void mark_fsmonitor_valid(struct index_state *istate,
       		untracked_cache_invalidate_path(istate, ce->name, 1);
       		trace_printf_key(&trace_fsmonitor, "mark_fsmonitor_invalid '%s'", ce->name);
      
     - ## repo-settings.c ##
     -@@ repo-settings.c: void prepare_repo_settings(struct repository *r)
     - 	if (r->settings.initialized++)
     - 		return;
     - 
     -+	r->settings.fsmonitor = NULL; /* lazy loaded */
     -+
     - 	/* Defaults */
     - 	r->settings.index_version = -1;
     - 	r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
     -
       ## repository.h ##
      @@
       #include "path.h"
 4:  8608c8718d8 = 4:  4d8d812be08 fsmonitor: use IPC to query the builtin FSMonitor daemon
 5:  7c22ce53377 = 5:  45a86cef8d7 fsmonitor: update fsmonitor config documentation

-- 
gitgitgadget