cherry-pick/fsmonitor-icase-corner-case-fix #632
Merged
+522
−56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
index_dir_exists() returns a boolean to indicate if there is a case-insensitive match in the directory name-hash, but does not provide the caller with the exact spelling of that match. Create index_dir_find() to do the case-insensitive search *and* optionally return the spelling of the matched directory prefix in a provided strbuf. To avoid code duplication, convert index_dir_exists() to be a trivial wrapper around the new index_dir_find(). Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The FSMonitor client code trusts the spelling of the pathnames in the FSEvents received from the FSMonitor daemon. On case-insensitive file systems, these OBSERVED pathnames may be spelled differently than the EXPECTED pathnames listed in the .git/index. This causes a miss when using `index_name_pos()` which expects the given case to be correct. When this happens, the FSMonitor client code does not update the state of the CE_FSMONITOR_VALID bit when refreshing the index (and before starting to scan the worktree). This results in modified files NOT being reported by `git status` when there is a discrepancy in the case-spelling of a tracked file's pathname. This commit contains a (rather contrived) test case to demonstrate this. A later commit in this series will update the FSMonitor client code to recognize these discrepancies and update the CE_ bit accordingly. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the code to handle directory FSEvents (containing pathnames with a trailing slash) into a helper function. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Improve documentation of the refresh callback helper function used for directory FSEvents. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the code that handles unqualified FSEvents (without a trailing slash) into a helper function. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create a wrapper function for untracked_cache_invalidate_path() that silently trims a trailing slash, if present, before calling the wrapped function. The untracked cache expects to be called with a pathname that does not contain a trailing slash. This can make it inconvenient for callers that have a directory path. Lets hide this complexity. This will be used by a later commit in the FSMonitor code which may receive directory pathnames from an FSEvent. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update fsmonitor_refresh_callback() to use the new untracked_cache_invalidate_trimmed_path() to invalidate the cache using the observed pathname without needing to modify the caller's buffer. Previously, we modified the caller's buffer when the observed pathname contained a trailing slash (and did not restore it). This wasn't a problem for the single use-case caller, but felt dirty nontheless. In a later commit we will want to invalidate case-corrected versions of the pathname (using possibly borrowed pathnames from the name-hash or dir-name-hash) and we may not want to keep the tradition of altering the passed-in pathname. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the call to invalidate the untracked-cache for the FSEvent pathname into the two helper functions. In a later commit in this series, we will call these helpers from other contexts and it safer to include the UC invalidation in the helpers than to remember to also add it to each helper call-site. This has the side-effect of invalidating the UC *before* we invalidate the ce_flags in the cache-entry. These activities are independent and do not affect each other. Also, by doing the UC work first, we can avoid worrying about "early returns" or the need for the usual "goto the end" in each of the handler functions. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach the refresh callback helper function for directory FSEvents to return the number of cache-entries that were invalidated in response to a directory event. This will be used in a later commit to help determine if the observed pathname in the FSEvent was a (possibly) case-incorrect directory prefix (on a case-insensitive filesystem) of one or more actual cache-entries. If there exists at least one case-insensitive prefix match, then we can assume that the directory is a (case-incorrect) prefix of at least one tracked item rather than a completely unknown/untracked file or directory. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor the code that handles refresh events for pathnames that do not contain a trailing slash. Instead of using a custom loop to try to scan the index and detect if the FSEvent named a file or might be a directory prefix, use the recently created helper function to do that. Also update the comments to describe what and why we are doing this. On platforms that DO NOT annotate FS events with a trailing slash, if we fail to find an exact match for the pathname in the index, we do not know if the pathname represents a directory or simply an untracked file. Pretend that the pathname is a directory and try again before assuming it is an untracked file. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach the refresh callback helper function for unqualified FSEvents (pathnames without a trailing slash) to return the number of cache-entries that were invalided in response to the event. This will be used in a later commit to help determine if the observed pathname was (possibly) case-incorrect when (on a case-insensitive file system). Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Consolidate the directory/non-directory calls to the refresh handler code. Log the resulting count of invalidated cache-entries. The nr_in_cone value will be used in a later commit to decide if we also need to try to do case-insensitive lookups. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor code in the fsmonitor_refresh_callback() call chain dealing with invalidating the CE_FSMONITOR_VALID bit and add a trace message. During the refresh, we clear the CE_FSMONITOR_VALID bit in response to data from the FSMonitor daemon (so that a later phase will lstat() and verify the true state of the file). Create a new function to clear the bit and add some unique tracing for it to help debug edge cases. This is similar to the existing `mark_fsmonitor_invalid()` function, but it also does untracked-cache invalidation and we've already handled that in the refresh-callback handlers, so but we don't need to repeat that. Signed-off-by: Jeff Hostetler <jeffhostetler@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
dscho
approved these changes
Mar 21, 2024
There was a problem hiding this comment.
This looks like a straight backport to me:
$ git range-diff 29c139ce7895e1c14be119300a7f99fbdc90c5e1~14..29c139ce7895e1c14be119300a7f99fbdc90c5e1 \
microsoft/cherry-pick/fsmonitor-icase-corner-case-fix~14..microsoft/cherry-pick/fsmonitor-icase-corner-case-fix-
1: b316552 ! 1: e35106f name-hash: add index_dir_find()
@@ name-hash.h + void adjust_dirname_case(struct index_state *istate, char *name); struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase); - + struct cache_entry *index_file_next_match(struct index_state *istate, struct cache_entry *ce, int igncase);
-
2: 32ca706 = 2: 15e25b5 t7527: add case-insensitve test for FSMonitor
-
3: e5da3dd = 3: ac5f796 fsmonitor: refactor refresh callback on directory events
-
4: 7a15a62 = 4: 64c9b50 fsmonitor: clarify handling of directory events in callback helper
-
5: 8687c2b = 5: 7c8609c fsmonitor: refactor refresh callback for non-directory events
-
6: 3e4ffda = 6: f4e4019 dir: create untracked_cache_invalidate_trimmed_path()
-
7: 48f4cd7 = 7: 5fdd6fb fsmonitor: refactor untracked-cache invalidation
-
8: 7c97174 = 8: 9628e45 fsmonitor: move untracked-cache invalidation into helper functions
-
9: a524820 = 9: c3c49b4 fsmonitor: return invalidated cache-entry count on directory event
-
10: 558d146 = 10: 8e9dfb8 fsmonitor: remove custom loop from non-directory path handler
-
11: 9e34e56 = 11: 405d568 fsmonitor: return invalidated cache-entry count on non-directory event
-
12: 84d441f = 12: 428ed9d fsmonitor: trace the new invalidated cache-entry count
-
13: b0dba50 = 13: 3357ac3 fsmonitor: refactor bit invalidation in refresh callback
-
14: 29c139c = 14: b1b11c1 fsmonitor: support case-insensitive events
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-pick
jh/fsmonitor-icase-corner-case-fixfrom core Git.29c139c (gitster/jh/fsmonitor-icase-corner-case-fix)