fsmonitor: handle version 2 of the hooks that will use opaque token
Some file monitors like watchman will use something other than a timestamp to keep better track of what changes happen in between calls to query the fsmonitor. The clockid in watchman is a string. Now that the index is storing an opaque token for the last update the code needs to be updated to pass that opaque token to a verion 2 of the fsmonitor hook. Because there are repos that already have version 1 of the hook and we want them to continue to work when git is updated, we need to handle both version 1 and version 2 of the hook. In order to do that a config value is being added core.fsmonitorHookVersion to force what version of the hook should be used. When this is not set it will default to -1 and then the code will attempt to call version 2 of the hook first. If that fails it will fallback to trying version 1. Signed-off-by: Kevin Willford <Kevin.Willford@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Kevin Willford committed
Jan 7, 2020 at 19:04 UTC
8da2c57629a16e148b0f94282ac787c1503e4779
4 files changed
+71
-15
fsmonitor.c
+64
-11
@@ -8,7 +8,8 @@
8
9
#define INDEX_EXTENSION_VERSION1 (1)
10
#define INDEX_EXTENSION_VERSION2 (2)
11
-#define HOOK_INTERFACE_VERSION (1)
11
+#define HOOK_INTERFACE_VERSION1 (1)
12
+#define HOOK_INTERFACE_VERSION2 (2)
13
14
struct trace_key trace_fsmonitor = TRACE_KEY_INIT(FSMONITOR);
15
@@ -25,6 +26,22 @@ static void fsmonitor_ewah_callback(size_t pos, void *is)
26
ce->ce_flags &= ~CE_FSMONITOR_VALID;
27
}
28
29
+static int fsmonitor_hook_version(void)
30
+{
31
+ int hook_version;
32
+
33
+ if (git_config_get_int("core.fsmonitorhookversion", &hook_version))
34
+ return -1;
35
+
36
+ if (hook_version == HOOK_INTERFACE_VERSION1 ||
37
+ hook_version == HOOK_INTERFACE_VERSION2)
38
+ return hook_version;
39
+
40
+ warning("Invalid hook version '%i' in core.fsmonitorhookversion. "
41
+ "Must be 1 or 2.", hook_version);
42
+ return -1;
43
+}
44
+
45
int read_fsmonitor_extension(struct index_state *istate, const void *data,
46
unsigned long sz)
47
{
@@ -158,8 +175,8 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n
175
void refresh_fsmonitor(struct index_state *istate)
176
{
177
struct strbuf query_result = STRBUF_INIT;
161
- int query_success = 0;
162
- size_t bol; /* beginning of line */
178
+ int query_success = 0, hook_version = -1;
179
+ size_t bol = 0; /* beginning of line */
180
uint64_t last_update;
181
struct strbuf last_update_token = STRBUF_INIT;
182
char *buf;
@@ -167,6 +184,9 @@ void refresh_fsmonitor(struct index_state *istate)
184
185
if (!core_fsmonitor || istate->fsmonitor_has_run_once)
186
return;
187
+
188
+ hook_version = fsmonitor_hook_version();
189
+
190
istate->fsmonitor_has_run_once = 1;
191
192
trace_printf_key(&trace_fsmonitor, "refresh fsmonitor");
@@ -175,27 +195,60 @@ void refresh_fsmonitor(struct index_state *istate)
195
* should be inclusive to ensure we don't miss potential changes.
196
*/
197
last_update = getnanotime();
178
- strbuf_addf(&last_update_token, "%"PRIu64"", last_update);
198
+ if (hook_version == HOOK_INTERFACE_VERSION1)
199
+ strbuf_addf(&last_update_token, "%"PRIu64"", last_update);
200
201
/*
181
- * If we have a last update time, call query_fsmonitor for the set of
182
- * changes since that time, else assume everything is possibly dirty
202
+ * If we have a last update token, call query_fsmonitor for the set of
203
+ * changes since that token, else assume everything is possibly dirty
204
* and check it all.
205
*/
206
if (istate->fsmonitor_last_update) {
186
- query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION,
187
- istate->fsmonitor_last_update, &query_result);
207
+ if (hook_version == -1 || hook_version == HOOK_INTERFACE_VERSION2) {
208
+ query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION2,
209
+ istate->fsmonitor_last_update, &query_result);
210
+
211
+ if (query_success) {
212
+ if (hook_version < 0)
213
+ hook_version = HOOK_INTERFACE_VERSION2;
214
+
215
+ /*
216
+ * First entry will be the last update token
217
+ * Need to use a char * variable because static
218
+ * analysis was suggesting to use strbuf_addbuf
219
+ * but we don't want to copy the entire strbuf
220
+ * only the the chars up to the first NUL
221
+ */
222
+ buf = query_result.buf;
223
+ strbuf_addstr(&last_update_token, buf);
224
+ if (!last_update_token.len) {
225
+ warning("Empty last update token.");
226
+ query_success = 0;
227
+ } else {
228
+ bol = last_update_token.len + 1;
229
+ }
230
+ } else if (hook_version < 0) {
231
+ hook_version = HOOK_INTERFACE_VERSION1;
232
+ if (!last_update_token.len)
233
+ strbuf_addf(&last_update_token, "%"PRIu64"", last_update);
234
+ }
235
+ }
236
+
237
+ if (hook_version == HOOK_INTERFACE_VERSION1) {
238
+ query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION1,
239
+ istate->fsmonitor_last_update, &query_result);
240
+ }
241
+
242
trace_performance_since(last_update, "fsmonitor process '%s'", core_fsmonitor);
243
trace_printf_key(&trace_fsmonitor, "fsmonitor process '%s' returned %s",
244
core_fsmonitor, query_success ? "success" : "failure");
245
}
246
247
/* a fsmonitor process can return '/' to indicate all entries are invalid */
194
- if (query_success && query_result.buf[0] != '/') {
248
+ if (query_success && query_result.buf[bol] != '/') {
249
/* Mark all entries returned by the monitor as dirty */
250
buf = query_result.buf;
197
- bol = 0;
198
- for (i = 0; i < query_result.len; i++) {
251
+ for (i = bol; i < query_result.len; i++) {
252
if (buf[i] != '\0')
253
continue;
254
fsmonitor_refresh_callback(istate, buf + bol);
t/t7519-status-fsmonitor.sh
+6
-1
@@ -32,11 +32,12 @@ write_integration_script () {
32
echo "$0: exactly 2 arguments expected"
33
exit 2
34
fi
35
- if test "$1" != 1
35
+ if test "$1" != 2
36
then
37
echo "Unsupported core.fsmonitor hook version." >&2
38
exit 1
39
fi
40
+ printf "last_update_token\0"
41
printf "untracked\0"
42
printf "dir1/untracked\0"
43
printf "dir2/untracked\0"
@@ -107,6 +108,7 @@ EOF
108
# test that "update-index --fsmonitor-valid" sets the fsmonitor valid bit
109
test_expect_success 'update-index --fsmonitor-valid" sets the fsmonitor valid bit' '
110
write_script .git/hooks/fsmonitor-test<<-\EOF &&
111
+ printf "last_update_token\0"
112
EOF
113
git update-index --fsmonitor &&
114
git update-index --fsmonitor-valid dir1/modified &&
@@ -167,6 +169,7 @@ EOF
169
# test that newly added files are marked valid
170
test_expect_success 'newly added files are marked valid' '
171
write_script .git/hooks/fsmonitor-test<<-\EOF &&
172
+ printf "last_update_token\0"
173
EOF
174
git add new &&
175
git add dir1/new &&
@@ -207,6 +210,7 @@ EOF
210
# test that *only* files returned by the integration script get flagged as invalid
211
test_expect_success '*only* files returned by the integration script get flagged as invalid' '
212
write_script .git/hooks/fsmonitor-test<<-\EOF &&
213
+ printf "last_update_token\0"
214
printf "dir1/modified\0"
215
EOF
216
clean_repo &&
@@ -276,6 +280,7 @@ do
280
# (if enabled) files unless it is told about them.
281
test_expect_success "status doesn't detect unreported modifications" '
282
write_script .git/hooks/fsmonitor-test<<-\EOF &&
283
+ printf "last_update_token\0"
284
:>marker
285
EOF
286
clean_repo &&
t/t7519/fsmonitor-all
-1
@@ -17,7 +17,6 @@ fi
17
18
if test "$1" != 1
19
then
20
- echo "Unsupported core.fsmonitor hook version." >&2
20
exit 1
21
fi
22
t/t7519/fsmonitor-watchman
+1
-2
@@ -26,8 +26,7 @@ if ($version == 1) {
26
# subtract one second to make sure watchman will return all changes
27
$time = int ($time / 1000000000) - 1;
28
} else {
29
- die "Unsupported query-fsmonitor hook version '$version'.\n" .
30
- "Falling back to scanning...\n";
29
+ exit 1;
30
}
31
32
my $git_work_tree;