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 bagasdotme@xxxxxxxxx cc: Jeff Hostetler git@xxxxxxxxxxxxxxxxx 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 <jeffhost@xxxxxxxxxxxxx> @@ 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
