fsmonitor: config settings are repository-specific

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. 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. Extend the definition of `core.fsmonitor` to be either a boolean or a hook pathname. When true, the builtin FSMonitor is used. When false or unset, no FSMonitor (neither builtin nor hook) is used. 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. 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@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff Hostetler committed Mar 25, 2022 at 18:02 UTC 1e0ea5c4316d2241dd76ef430a2779db9a097dfb
12 files changed +196 -49
Makefile
+1
@@ -908,6 +908,7 @@ LIB_OBJS += fmt-merge-msg.o
908 LIB_OBJS += fsck.o
909 LIB_OBJS += fsmonitor.o
910 LIB_OBJS += fsmonitor-ipc.o
911 +LIB_OBJS += fsmonitor-settings.o
912 LIB_OBJS += gettext.o
913 LIB_OBJS += gpg-interface.o
914 LIB_OBJS += graph.o
builtin/update-index.c
+5 -2
@@ -1236,14 +1236,17 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
1236 }
1237
1238 if (fsmonitor > 0) {
1239 - if (git_config_get_fsmonitor() == 0)
1239 + enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
1240 + if (fsm_mode == FSMONITOR_MODE_DISABLED) {
1241 warning(_("core.fsmonitor is unset; "
1242 "set it if you really want to "
1243 "enable fsmonitor"));
1244 + }
1245 add_fsmonitor(&the_index);
1246 report(_("fsmonitor enabled"));
1247 } else if (!fsmonitor) {
1246 - if (git_config_get_fsmonitor() == 1)
1248 + enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
1249 + if (fsm_mode > FSMONITOR_MODE_DISABLED)
1250 warning(_("core.fsmonitor is set; "
1251 "remove it if you really want to "
1252 "disable fsmonitor"));
cache.h
-1
@@ -999,7 +999,6 @@ extern int core_preload_index;
999 extern int precomposed_unicode;
1000 extern int protect_hfs;
1001 extern int protect_ntfs;
1002 -extern const char *core_fsmonitor;
1002
1003 extern int core_apply_sparse_checkout;
1004 extern int core_sparse_checkout_cone;
config.c
-14
@@ -2626,20 +2626,6 @@ int git_config_get_max_percent_split_change(void)
2626 return -1; /* default value */
2627 }
2628
2629 -int git_config_get_fsmonitor(void)
2630 -{
2631 - if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
2632 - core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
2633 -
2634 - if (core_fsmonitor && !*core_fsmonitor)
2635 - core_fsmonitor = NULL;
2636 -
2637 - if (core_fsmonitor)
2638 - return 1;
2639 -
2640 - return 0;
2641 -}
2642 -
2629 int git_config_get_index_threads(int *dest)
2630 {
2631 int is_bool, val;
config.h
-1
@@ -597,7 +597,6 @@ int git_config_get_pathname(const char *key, const char **dest);
597 int git_config_get_index_threads(int *dest);
598 int git_config_get_split_index(void);
599 int git_config_get_max_percent_split_change(void);
600 -int git_config_get_fsmonitor(void);
600
601 /* This dies if the configured or default date is in the future */
602 int git_config_get_expiry(const char *key, const char **output);
environment.c
-1
@@ -84,7 +84,6 @@ int protect_hfs = PROTECT_HFS_DEFAULT;
84 #define PROTECT_NTFS_DEFAULT 1
85 #endif
86 int protect_ntfs = PROTECT_NTFS_DEFAULT;
87 -const char *core_fsmonitor;
87
88 /*
89 * The character that begins a commented line in user-editable file
fsmonitor-settings.c new
+114
@@ -0,0 +1,114 @@
1 +#include "cache.h"
2 +#include "config.h"
3 +#include "repository.h"
4 +#include "fsmonitor-settings.h"
5 +
6 +/*
7 + * We keep this structure defintion private and have getters
8 + * for all fields so that we can lazy load it as needed.
9 + */
10 +struct fsmonitor_settings {
11 + enum fsmonitor_mode mode;
12 + char *hook_path;
13 +};
14 +
15 +static void lookup_fsmonitor_settings(struct repository *r)
16 +{
17 + struct fsmonitor_settings *s;
18 + const char *const_str;
19 + int bool_value;
20 +
21 + if (r->settings.fsmonitor)
22 + return;
23 +
24 + CALLOC_ARRAY(s, 1);
25 + s->mode = FSMONITOR_MODE_DISABLED;
26 +
27 + r->settings.fsmonitor = s;
28 +
29 + /*
30 + * Overload the existing "core.fsmonitor" config setting (which
31 + * has historically been either unset or a hook pathname) to
32 + * now allow a boolean value to enable the builtin FSMonitor
33 + * or to turn everything off. (This does imply that you can't
34 + * use a hook script named "true" or "false", but that's OK.)
35 + */
36 + switch (repo_config_get_maybe_bool(r, "core.fsmonitor", &bool_value)) {
37 +
38 + case 0: /* config value was set to <bool> */
39 + if (bool_value)
40 + fsm_settings__set_ipc(r);
41 + return;
42 +
43 + case 1: /* config value was unset */
44 + const_str = getenv("GIT_TEST_FSMONITOR");
45 + break;
46 +
47 + case -1: /* config value set to an arbitrary string */
48 + if (repo_config_get_pathname(r, "core.fsmonitor", &const_str))
49 + return; /* should not happen */
50 + break;
51 +
52 + default: /* should not happen */
53 + return;
54 + }
55 +
56 + if (!const_str || !*const_str)
57 + return;
58 +
59 + fsm_settings__set_hook(r, const_str);
60 +}
61 +
62 +enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
63 +{
64 + if (!r)
65 + r = the_repository;
66 +
67 + lookup_fsmonitor_settings(r);
68 +
69 + return r->settings.fsmonitor->mode;
70 +}
71 +
72 +const char *fsm_settings__get_hook_path(struct repository *r)
73 +{
74 + if (!r)
75 + r = the_repository;
76 +
77 + lookup_fsmonitor_settings(r);
78 +
79 + return r->settings.fsmonitor->hook_path;
80 +}
81 +
82 +void fsm_settings__set_ipc(struct repository *r)
83 +{
84 + if (!r)
85 + r = the_repository;
86 +
87 + lookup_fsmonitor_settings(r);
88 +
89 + r->settings.fsmonitor->mode = FSMONITOR_MODE_IPC;
90 + FREE_AND_NULL(r->settings.fsmonitor->hook_path);
91 +}
92 +
93 +void fsm_settings__set_hook(struct repository *r, const char *path)
94 +{
95 + if (!r)
96 + r = the_repository;
97 +
98 + lookup_fsmonitor_settings(r);
99 +
100 + r->settings.fsmonitor->mode = FSMONITOR_MODE_HOOK;
101 + FREE_AND_NULL(r->settings.fsmonitor->hook_path);
102 + r->settings.fsmonitor->hook_path = strdup(path);
103 +}
104 +
105 +void fsm_settings__set_disabled(struct repository *r)
106 +{
107 + if (!r)
108 + r = the_repository;
109 +
110 + lookup_fsmonitor_settings(r);
111 +
112 + r->settings.fsmonitor->mode = FSMONITOR_MODE_DISABLED;
113 + FREE_AND_NULL(r->settings.fsmonitor->hook_path);
114 +}
fsmonitor-settings.h new
+21
@@ -0,0 +1,21 @@
1 +#ifndef FSMONITOR_SETTINGS_H
2 +#define FSMONITOR_SETTINGS_H
3 +
4 +struct repository;
5 +
6 +enum fsmonitor_mode {
7 + FSMONITOR_MODE_DISABLED = 0,
8 + FSMONITOR_MODE_HOOK = 1, /* core.fsmonitor=<hook_path> */
9 + FSMONITOR_MODE_IPC = 2, /* core.fsmonitor=<true> */
10 +};
11 +
12 +void fsm_settings__set_ipc(struct repository *r);
13 +void fsm_settings__set_hook(struct repository *r, const char *path);
14 +void fsm_settings__set_disabled(struct repository *r);
15 +
16 +enum fsmonitor_mode fsm_settings__get_mode(struct repository *r);
17 +const char *fsm_settings__get_hook_path(struct repository *r);
18 +
19 +struct fsmonitor_settings;
20 +
21 +#endif /* FSMONITOR_SETTINGS_H */
fsmonitor.c
+38 -25
@@ -3,6 +3,7 @@
3 #include "dir.h"
4 #include "ewah/ewok.h"
5 #include "fsmonitor.h"
6 +#include "fsmonitor-ipc.h"
7 #include "run-command.h"
8 #include "strbuf.h"
9
@@ -148,15 +149,18 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
149 /*
150 * Call the query-fsmonitor hook passing the last update token of the saved results.
151 */
151 -static int query_fsmonitor(int version, const char *last_update, struct strbuf *query_result)
152 +static int query_fsmonitor_hook(struct repository *r,
153 + int version,
154 + const char *last_update,
155 + struct strbuf *query_result)
156 {
157 struct child_process cp = CHILD_PROCESS_INIT;
158 int result;
159
156 - if (!core_fsmonitor)
160 + if (fsm_settings__get_mode(r) != FSMONITOR_MODE_HOOK)
161 return -1;
162
159 - strvec_push(&cp.args, core_fsmonitor);
163 + strvec_push(&cp.args, fsm_settings__get_hook_path(r));
164 strvec_pushf(&cp.args, "%d", version);
165 strvec_pushf(&cp.args, "%s", last_update);
166 cp.use_shell = 1;
@@ -225,17 +229,28 @@ void refresh_fsmonitor(struct index_state *istate)
229 char *buf;
230 unsigned int i;
231 int is_trivial = 0;
232 + struct repository *r = istate->repo ? istate->repo : the_repository;
233 + enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
234
229 - if (!core_fsmonitor || istate->fsmonitor_has_run_once)
235 + if (fsm_mode <= FSMONITOR_MODE_DISABLED ||
236 + istate->fsmonitor_has_run_once)
237 return;
238
232 - hook_version = fsmonitor_hook_version();
233 -
239 istate->fsmonitor_has_run_once = 1;
240
241 trace_printf_key(&trace_fsmonitor, "refresh fsmonitor");
242 +
243 + if (fsm_mode == FSMONITOR_MODE_IPC) {
244 + /* TODO */
245 + return;
246 + }
247 +
248 + assert(fsm_mode == FSMONITOR_MODE_HOOK);
249 +
250 + hook_version = fsmonitor_hook_version();
251 +
252 /*
238 - * This could be racy so save the date/time now and query_fsmonitor
253 + * This could be racy so save the date/time now and query_fsmonitor_hook
254 * should be inclusive to ensure we don't miss potential changes.
255 */
256 last_update = getnanotime();
@@ -243,13 +258,14 @@ void refresh_fsmonitor(struct index_state *istate)
258 strbuf_addf(&last_update_token, "%"PRIu64"", last_update);
259
260 /*
246 - * If we have a last update token, call query_fsmonitor for the set of
261 + * If we have a last update token, call query_fsmonitor_hook for the set of
262 * changes since that token, else assume everything is possibly dirty
263 * and check it all.
264 */
265 if (istate->fsmonitor_last_update) {
266 if (hook_version == -1 || hook_version == HOOK_INTERFACE_VERSION2) {
252 - query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION2,
267 + query_success = !query_fsmonitor_hook(
268 + r, HOOK_INTERFACE_VERSION2,
269 istate->fsmonitor_last_update, &query_result);
270
271 if (query_success) {
@@ -280,7 +296,8 @@ void refresh_fsmonitor(struct index_state *istate)
296 }
297
298 if (hook_version == HOOK_INTERFACE_VERSION1) {
283 - query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION1,
299 + query_success = !query_fsmonitor_hook(
300 + r, HOOK_INTERFACE_VERSION1,
301 istate->fsmonitor_last_update, &query_result);
302 if (query_success)
303 is_trivial = query_result.buf[0] == '/';
@@ -290,9 +307,12 @@ void refresh_fsmonitor(struct index_state *istate)
307 trace2_data_intmax("fsm_hook", NULL,
308 "query/trivial-response", 1);
309
293 - trace_performance_since(last_update, "fsmonitor process '%s'", core_fsmonitor);
294 - trace_printf_key(&trace_fsmonitor, "fsmonitor process '%s' returned %s",
295 - core_fsmonitor, query_success ? "success" : "failure");
310 + trace_performance_since(last_update, "fsmonitor process '%s'",
311 + fsm_settings__get_hook_path(r));
312 + trace_printf_key(&trace_fsmonitor,
313 + "fsmonitor process '%s' returned %s",
314 + fsm_settings__get_hook_path(r),
315 + query_success ? "success" : "failure");
316 }
317
318 /*
@@ -429,7 +449,8 @@ void remove_fsmonitor(struct index_state *istate)
449 void tweak_fsmonitor(struct index_state *istate)
450 {
451 unsigned int i;
432 - int fsmonitor_enabled = git_config_get_fsmonitor();
452 + int fsmonitor_enabled = (fsm_settings__get_mode(istate->repo)
453 + > FSMONITOR_MODE_DISABLED);
454
455 if (istate->fsmonitor_dirty) {
456 if (fsmonitor_enabled) {
@@ -449,16 +470,8 @@ void tweak_fsmonitor(struct index_state *istate)
470 istate->fsmonitor_dirty = NULL;
471 }
472
452 - switch (fsmonitor_enabled) {
453 - case -1: /* keep: do nothing */
454 - break;
455 - case 0: /* false */
456 - remove_fsmonitor(istate);
457 - break;
458 - case 1: /* true */
473 + if (fsmonitor_enabled)
474 add_fsmonitor(istate);
460 - break;
461 - default: /* unknown value: do nothing */
462 - break;
463 - }
475 + else
476 + remove_fsmonitor(istate);
477 }
fsmonitor.h
+12 -3
@@ -3,6 +3,7 @@
3
4 #include "cache.h"
5 #include "dir.h"
6 +#include "fsmonitor-settings.h"
7
8 extern struct trace_key trace_fsmonitor;
9
@@ -57,7 +58,10 @@ int fsmonitor_is_trivial_response(const struct strbuf *query_result);
58 */
59 static inline int is_fsmonitor_refreshed(const struct index_state *istate)
60 {
60 - return !core_fsmonitor || istate->fsmonitor_has_run_once;
61 + enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(istate->repo);
62 +
63 + return fsm_mode <= FSMONITOR_MODE_DISABLED ||
64 + istate->fsmonitor_has_run_once;
65 }
66
67 /*
@@ -67,7 +71,10 @@ static inline int is_fsmonitor_refreshed(const struct index_state *istate)
71 */
72 static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache_entry *ce)
73 {
70 - if (core_fsmonitor && !(ce->ce_flags & CE_FSMONITOR_VALID)) {
74 + enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(istate->repo);
75 +
76 + if (fsm_mode > FSMONITOR_MODE_DISABLED &&
77 + !(ce->ce_flags & CE_FSMONITOR_VALID)) {
78 istate->cache_changed = 1;
79 ce->ce_flags |= CE_FSMONITOR_VALID;
80 trace_printf_key(&trace_fsmonitor, "mark_fsmonitor_clean '%s'", ce->name);
@@ -83,7 +90,9 @@ static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache
90 */
91 static inline void mark_fsmonitor_invalid(struct index_state *istate, struct cache_entry *ce)
92 {
86 - if (core_fsmonitor) {
93 + enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(istate->repo);
94 +
95 + if (fsm_mode > FSMONITOR_MODE_DISABLED) {
96 ce->ce_flags &= ~CE_FSMONITOR_VALID;
97 untracked_cache_invalidate_path(istate, ce->name, 1);
98 trace_printf_key(&trace_fsmonitor, "mark_fsmonitor_invalid '%s'", ce->name);
repository.h
+3
@@ -4,6 +4,7 @@
4 #include "path.h"
5
6 struct config_set;
7 +struct fsmonitor_settings;
8 struct git_hash_algo;
9 struct index_state;
10 struct lock_file;
@@ -35,6 +36,8 @@ struct repo_settings {
36 int command_requires_full_index;
37 int sparse_index;
38
39 + struct fsmonitor_settings *fsmonitor; /* lazily loaded */
40 +
41 int index_version;
42 enum untracked_cache_setting core_untracked_cache;
43
t/README
+2 -2
@@ -405,8 +405,8 @@ every 'git commit-graph write', as if the `--changed-paths` option was
405 passed in.
406
407 GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
408 -code path for utilizing a file system monitor to speed up detecting
409 -new or changed files.
408 +code paths for utilizing a (hook based) file system monitor to speed up
409 +detecting new or changed files.
410
411 GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path
412 for the index version specified. Can be set to any valid version