fsmonitor: support case-insensitive events
Teach fsmonitor_refresh_callback() to handle case-insensitive lookups if case-sensitive lookups fail on case-insensitive systems. This can cause 'git status' to report stale status for files if there are case issues/errors in the worktree. The FSMonitor daemon sends FSEvents using the observed spelling of each pathname. On case-insensitive file systems this may be different than the expected case spelling. The existing code uses index_name_pos() to find the cache-entry for the pathname in the FSEvent and clear the CE_FSMONITOR_VALID bit so that the worktree scan/index refresh will revisit and revalidate the path. On a case-insensitive file system, the exact match lookup may fail to find the associated cache-entry. This causes status to think that the cached CE flags are correct and skip over the file. Update event handling to optionally use the name-hash and dir-name-hash if necessary. Also update t7527 to convert the "test_expect_failure" to "_success" now that we have fixed the bug. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff Hostetler committed
Feb 26, 2024 at 21:39 UTC
29c139ce7895e1c14be119300a7f99fbdc90c5e1
2 files changed
+137
-10
fsmonitor.c
+121
@@ -5,6 +5,7 @@
5
#include "ewah/ewok.h"
6
#include "fsmonitor.h"
7
#include "fsmonitor-ipc.h"
8
+#include "name-hash.h"
9
#include "run-command.h"
10
#include "strbuf.h"
11
#include "trace2.h"
@@ -202,6 +203,113 @@ static void invalidate_ce_fsm(struct cache_entry *ce)
203
static size_t handle_path_with_trailing_slash(
204
struct index_state *istate, const char *name, int pos);
205
206
+/*
207
+ * Use the name-hash to do a case-insensitive cache-entry lookup with
208
+ * the pathname and invalidate the cache-entry.
209
+ *
210
+ * Returns the number of cache-entries that we invalidated.
211
+ */
212
+static size_t handle_using_name_hash_icase(
213
+ struct index_state *istate, const char *name)
214
+{
215
+ struct cache_entry *ce = NULL;
216
+
217
+ ce = index_file_exists(istate, name, strlen(name), 1);
218
+ if (!ce)
219
+ return 0;
220
+
221
+ /*
222
+ * A case-insensitive search in the name-hash using the
223
+ * observed pathname found a cache-entry, so the observed path
224
+ * is case-incorrect. Invalidate the cache-entry and use the
225
+ * correct spelling from the cache-entry to invalidate the
226
+ * untracked-cache. Since we now have sparse-directories in
227
+ * the index, the observed pathname may represent a regular
228
+ * file or a sparse-index directory.
229
+ *
230
+ * Note that we should not have seen FSEvents for a
231
+ * sparse-index directory, but we handle it just in case.
232
+ *
233
+ * Either way, we know that there are not any cache-entries for
234
+ * children inside the cone of the directory, so we don't need to
235
+ * do the usual scan.
236
+ */
237
+ trace_printf_key(&trace_fsmonitor,
238
+ "fsmonitor_refresh_callback MAP: '%s' '%s'",
239
+ name, ce->name);
240
+
241
+ /*
242
+ * NEEDSWORK: We used the name-hash to find the correct
243
+ * case-spelling of the pathname in the cache-entry[], so
244
+ * technically this is a tracked file or a sparse-directory.
245
+ * It should not have any entries in the untracked-cache, so
246
+ * we should not need to use the case-corrected spelling to
247
+ * invalidate the the untracked-cache. So we may not need to
248
+ * do this. For now, I'm going to be conservative and always
249
+ * do it; we can revisit this later.
250
+ */
251
+ untracked_cache_invalidate_trimmed_path(istate, ce->name, 0);
252
+
253
+ invalidate_ce_fsm(ce);
254
+ return 1;
255
+}
256
+
257
+/*
258
+ * Use the dir-name-hash to find the correct-case spelling of the
259
+ * directory. Use the canonical spelling to invalidate all of the
260
+ * cache-entries within the matching cone.
261
+ *
262
+ * Returns the number of cache-entries that we invalidated.
263
+ */
264
+static size_t handle_using_dir_name_hash_icase(
265
+ struct index_state *istate, const char *name)
266
+{
267
+ struct strbuf canonical_path = STRBUF_INIT;
268
+ int pos;
269
+ size_t len = strlen(name);
270
+ size_t nr_in_cone;
271
+
272
+ if (name[len - 1] == '/')
273
+ len--;
274
+
275
+ if (!index_dir_find(istate, name, len, &canonical_path))
276
+ return 0; /* name is untracked */
277
+
278
+ if (!memcmp(name, canonical_path.buf, canonical_path.len)) {
279
+ strbuf_release(&canonical_path);
280
+ /*
281
+ * NEEDSWORK: Our caller already tried an exact match
282
+ * and failed to find one. They called us to do an
283
+ * ICASE match, so we should never get an exact match,
284
+ * so we could promote this to a BUG() here if we
285
+ * wanted to. It doesn't hurt anything to just return
286
+ * 0 and go on because we should never get here. Or we
287
+ * could just get rid of the memcmp() and this "if"
288
+ * clause completely.
289
+ */
290
+ BUG("handle_using_dir_name_hash_icase(%s) did not exact match",
291
+ name);
292
+ }
293
+
294
+ trace_printf_key(&trace_fsmonitor,
295
+ "fsmonitor_refresh_callback MAP: '%s' '%s'",
296
+ name, canonical_path.buf);
297
+
298
+ /*
299
+ * The dir-name-hash only tells us the corrected spelling of
300
+ * the prefix. We have to use this canonical path to do a
301
+ * lookup in the cache-entry array so that we repeat the
302
+ * original search using the case-corrected spelling.
303
+ */
304
+ strbuf_addch(&canonical_path, '/');
305
+ pos = index_name_pos(istate, canonical_path.buf,
306
+ canonical_path.len);
307
+ nr_in_cone = handle_path_with_trailing_slash(
308
+ istate, canonical_path.buf, pos);
309
+ strbuf_release(&canonical_path);
310
+ return nr_in_cone;
311
+}
312
+
313
/*
314
* The daemon sent an observed pathname without a trailing slash.
315
* (This is the normal case.) We do not know if it is a tracked or
@@ -335,6 +443,19 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
443
else
444
nr_in_cone = handle_path_without_trailing_slash(istate, name, pos);
445
446
+ /*
447
+ * If we did not find an exact match for this pathname or any
448
+ * cache-entries with this directory prefix and we're on a
449
+ * case-insensitive file system, try again using the name-hash
450
+ * and dir-name-hash.
451
+ */
452
+ if (!nr_in_cone && ignore_case) {
453
+ nr_in_cone = handle_using_name_hash_icase(istate, name);
454
+ if (!nr_in_cone)
455
+ nr_in_cone = handle_using_dir_name_hash_icase(
456
+ istate, name);
457
+ }
458
+
459
if (nr_in_cone)
460
trace_printf_key(&trace_fsmonitor,
461
"fsmonitor_refresh_callback CNT: %d",
t/t7527-builtin-fsmonitor.sh
+16
-10
@@ -1051,7 +1051,7 @@ test_expect_success 'split-index and FSMonitor work well together' '
1051
#
1052
# The setup is a little contrived.
1053
#
1054
-test_expect_failure CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
1054
+test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
1055
test_when_finished "stop_daemon_delete_repo subdir_case_wrong" &&
1056
1057
git init subdir_case_wrong &&
@@ -1116,19 +1116,19 @@ test_expect_failure CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
1116
1117
grep -q "dir1/DIR2/dir3/file3.*pos -3" "$PWD/subdir_case_wrong.log1" &&
1118
1119
+ # Verify that we get a mapping event to correct the case.
1120
+ grep -q "MAP:.*dir1/DIR2/dir3/file3.*dir1/dir2/dir3/file3" \
1121
+ "$PWD/subdir_case_wrong.log1" &&
1122
+
1123
# The refresh-callbacks should have caused "git status" to clear
1124
# the CE_FSMONITOR_VALID bit on each of those files and caused
1125
# the worktree scan to visit them and mark them as modified.
1126
grep -q " M AAA" "$PWD/subdir_case_wrong.out" &&
1127
grep -q " M zzz" "$PWD/subdir_case_wrong.out" &&
1124
-
1125
- # Expect Breakage: with the case confusion, the "(pos -3)" causes
1126
- # the client to not clear the CE_FSMONITOR_VALID bit and therefore
1127
- # status will not rescan the file and therefore not report it as dirty.
1128
grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
1129
'
1130
1131
-test_expect_failure CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
1131
+test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
1132
test_when_finished "stop_daemon_delete_repo file_case_wrong" &&
1133
1134
git init file_case_wrong &&
@@ -1242,14 +1242,20 @@ test_expect_failure CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
1242
GIT_TRACE_FSMONITOR="$PWD/file_case_wrong-try3.log" \
1243
git -C file_case_wrong --no-optional-locks status --short \
1244
>"$PWD/file_case_wrong-try3.out" &&
1245
+
1246
+ # Verify that we get a mapping event to correct the case.
1247
+ grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir3/FILE-3-A.*dir1/dir2/dir3/file-3-a" \
1248
+ "$PWD/file_case_wrong-try3.log" &&
1249
+ grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir4/file-4-a.*dir1/dir2/dir4/FILE-4-A" \
1250
+ "$PWD/file_case_wrong-try3.log" &&
1251
+
1252
# FSEvents are in observed case.
1253
grep -q "fsmonitor_refresh_callback.*FILE-3-A.*pos -3" "$PWD/file_case_wrong-try3.log" &&
1254
grep -q "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try3.log" &&
1255
1249
- # Expect Breakage: with the case confusion, the "(pos-3)" and
1250
- # "(pos -9)" causes the client to not clear the CE_FSMONITOR_VALID
1251
- # bit and therefore status will not rescan the files and therefore
1252
- # not report them as dirty.
1256
+ # The refresh-callbacks should have caused "git status" to clear
1257
+ # the CE_FSMONITOR_VALID bit on each of those files and caused
1258
+ # the worktree scan to visit them and mark them as modified.
1259
grep -q " M dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-try3.out" &&
1260
grep -q " M dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-try3.out"
1261
'