git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 08/12] fsmonitor: support case-insensitive directory events

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 15, 2024, 09:32 UTC
Message-ID
<Zc3aO95bvuXXENLj@tanuki>
In-Reply-To
<e0029a2aad68a60f672e74368a384f68a343e21c.1707857541.git.gitgitgadget@gmail.com>
On Tue, Feb 13, 2024 at 08:52:17PM +0000, Jeff Hostetler via GitGitGadget wrote:
Show 63 quoted lines
> From: Jeff Hostetler <jeffhostetler@github.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 the handling of directory-style FSEvents (ones containing a
> path with a trailing slash) to optionally use the name-hash if the
> case-correct search does not find a match.
> 
> (The FSMonitor daemon can send directory FSEvents if the OS provides
> that information.)
> 
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
>  fsmonitor.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 120 insertions(+), 2 deletions(-)
> 
> diff --git a/fsmonitor.c b/fsmonitor.c
> index 14585b6c516..73e6ac82af7 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -5,6 +5,7 @@
>  #include "ewah/ewok.h"
>  #include "fsmonitor.h"
>  #include "fsmonitor-ipc.h"
> +#include "name-hash.h"
>  #include "run-command.h"
>  #include "strbuf.h"
>  #include "trace2.h"
> @@ -183,6 +184,9 @@ static int query_fsmonitor_hook(struct repository *r,
>  	return result;
>  }
>  
> +static int fsmonitor_refresh_callback_slash(
> +	struct index_state *istate, const char *name, int len, int pos);
> +
>  /*
>   * Invalidate the untracked cache for the given pathname.  Copy the
>   * buffer to a proper null-terminated string (since the untracked
> @@ -205,6 +209,84 @@ static void my_invalidate_untracked_cache(
>  	strbuf_release(&work_path);
>  }
>  
> +/*
> + * Use the name-hash to lookup the pathname.
> + *
> + * Returns the number of cache-entries that we invalidated.
> + */

The function not only looks up the path name, but also invalidates the corresponding cache entry. You imply this with the second sentence, but this could be a bit more explicit.

> +static int my_callback_name_hash(
> +	struct index_state *istate, const char *name, int len)
I find the naming conventions here to be weird with the `my_` prefix.
Show 6 quoted lines
> +{
> +	struct cache_entry *ce = NULL;
> +
> +	ce = index_file_exists(istate, name, len, 1);
> +	if (!ce)
> +		return 0;

Okay, `index_file_exists()` is called with `icase == 1` here. But is that the correct thing to do on case-sensitive platforms? I would have expected us to honor `core.ignoreCase` here.

Turns out, we only end up calling this function when `ignore_case` is set, so we already do. I think this can be clarified both by giving the function a better name and by documenting this in the comment. Also, neither of this or the next function really are callbacks -- they only happen to be called by a callback function.

I'd think something like `lookup_and_invalidate_path_icase()` and `lookup_and_invalidate_dir_icase()` could help to clarify intent.

Show 43 quoted lines
> +	/*
> +	 * The index contains a case-insensitive match for the pathname.
> +	 * This could either be a regular file or a sparse-index directory.
> +	 *
> +	 * We should not have seen FSEvents for a sparse-index directory,
> +	 * but we handle it just in case.
> +	 *
> +	 * Either way, we know that there are not any cache-entries for
> +	 * children inside the cone of the directory, so we don't need to
> +	 * do the usual scan.
> +	 */
> +	trace_printf_key(&trace_fsmonitor,
> +			 "fsmonitor_refresh_callback map '%s' '%s'",
> +			 name, ce->name);
> +
> +	my_invalidate_untracked_cache(istate, ce->name, ce->ce_namelen);
> +
> +	ce->ce_flags &= ~CE_FSMONITOR_VALID;
> +	return 1;
> +}
> +
> +/*
> + * Use the directory name-hash to find the correct-case spelling
> + * of the directory.  Use the canonical spelling to invalidate all
> + * of the cache-entries within the matching cone.
> + *
> + * The pathname MUST NOT have a trailing slash.
> + *
> + * Returns the number of cache-entries that we invalidated.
> + */
> +static int my_callback_dir_name_hash(
> +	struct index_state *istate, const char *name, int len)
> +{
> +	struct strbuf canonical_path = STRBUF_INIT;
> +	int pos;
> +	int nr_in_cone;
> +
> +	if (!index_dir_exists2(istate, name, len, &canonical_path))
> +		return 0; /* name is untracked */
> +	if (!memcmp(name, canonical_path.buf, len)) {
> +		strbuf_release(&canonical_path);
> +		return 0; /* should not happen */
> +	}

So in other words, this function should only be called when we know that casing differs, and thus the passed-in name and the canonical name should never be the same? If this case shouldn't ever happen, shouldn't we report this as an error or use `BUG()` instead of silently ignoring this mismatch of expectations?

Patrick
Show 91 quoted lines
> +	trace_printf_key(&trace_fsmonitor,
> +			 "fsmonitor_refresh_callback map '%s' '%s'",
> +			 name, canonical_path.buf);
> +
> +	/*
> +	 * The directory name-hash only tells us the corrected
> +	 * spelling of the prefix.  We have to use this canonical
> +	 * path to do a lookup in the cache-entry array so that we
> +	 * we repeat the original search using the case-corrected
> +	 * spelling.
> +	 */
> +	strbuf_addch(&canonical_path, '/');
> +	pos = index_name_pos(istate, canonical_path.buf,
> +			     canonical_path.len);
> +	nr_in_cone = fsmonitor_refresh_callback_slash(
> +		istate, canonical_path.buf, canonical_path.len, pos);
> +	strbuf_release(&canonical_path);
> +	return nr_in_cone;
> +}
> +
>  static void fsmonitor_refresh_callback_unqualified(
>  	struct index_state *istate, const char *name, int len, int pos)
>  {
> @@ -269,7 +351,10 @@ static void fsmonitor_refresh_callback_unqualified(
>   *
>   * Return the number of cache-entries that we invalidated.  We will
>   * use this later to determine if we need to attempt a second
> - * case-insensitive search.
> + * case-insensitive search.  That is, if a observed-case search yields
> + * any results, we assume the prefix is case-correct.  If there are
> + * no matches, we still don't know if the observed path is simply
> + * untracked or case-incorrect.
>   */
>  static int fsmonitor_refresh_callback_slash(
>  	struct index_state *istate, const char *name, int len, int pos)
> @@ -293,17 +378,50 @@ static int fsmonitor_refresh_callback_slash(
>  	return nr_in_cone;
>  }
>  
> +/*
> + * On a case-insensitive FS, use the name-hash and directory name-hash
> + * to map the case of the observed path to the canonical case expected
> + * by the index.
> + *
> + * The given pathname includes the trailing slash.
> + *
> + * Return the number of cache-entries that we invalidated.
> + */
> +static int fsmonitor_refresh_callback_slash_icase(
> +	struct index_state *istate, const char *name, int len)
> +{
> +	int nr_in_cone;
> +
> +	/*
> +	 * Look for a case-incorrect sparse-index directory.
> +	 */
> +	nr_in_cone = my_callback_name_hash(istate, name, len);
> +	if (nr_in_cone)
> +		return nr_in_cone;
> +
> +	/*
> +	 * (len-1) because we do not include the trailing slash in the
> +	 * pathname.
> +	 */
> +	nr_in_cone = my_callback_dir_name_hash(istate, name, len-1);
> +	return nr_in_cone;
> +}
> +
>  static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
>  {
>  	int len = strlen(name);
>  	int pos = index_name_pos(istate, name, len);
> +	int nr_in_cone;
> +
>  
>  	trace_printf_key(&trace_fsmonitor,
>  			 "fsmonitor_refresh_callback '%s' (pos %d)",
>  			 name, pos);
>  
>  	if (name[len - 1] == '/') {
> -		fsmonitor_refresh_callback_slash(istate, name, len, pos);
> +		nr_in_cone = fsmonitor_refresh_callback_slash(istate, name, len, pos);
> +		if (ignore_case && !nr_in_cone)
> +			fsmonitor_refresh_callback_slash_icase(istate, name, len);
>  	} else {
>  		fsmonitor_refresh_callback_unqualified(istate, name, len, pos);
>  	}
> -- 
> gitgitgadget
> 
> 
Previous: Jeff Hostetler via GitGitGadgetNext: Jeff Hostetler via GitGitGadget
Message 28 of 91 in “FSMonitor edge cases on case-insensitive file systems”
  1. 00/12 FSMonitor edge cases on case-insensitive file systemsJeff Hostetler via GitGitGadget, Feb 13, 2024
  2. 01/12 sparse-index: pass string length to index_file_exists()Jeff Hostetler via GitGitGadget, Feb 13, 2024
  3. Junio C HamanoFeb 13, 2024
  4. Jeff HostetlerFeb 20, 2024
  5. 02/12 name-hash: add index_dir_exists2()Jeff Hostetler via GitGitGadget, Feb 13, 2024
  6. Junio C HamanoFeb 13, 2024
  7. Jeff HostetlerFeb 20, 2024
  8. Junio C HamanoFeb 20, 2024
  9. Patrick SteinhardtFeb 15, 2024
  10. 03/12 t7527: add case-insensitve test for FSMonitorJeff Hostetler via GitGitGadget, Feb 13, 2024
  11. 04/12 fsmonitor: refactor refresh callback on directory eventsJeff Hostetler via GitGitGadget, Feb 13, 2024
  12. Patrick SteinhardtFeb 15, 2024
  13. Jeff HostetlerFeb 20, 2024
  14. Patrick SteinhardtFeb 21, 2024
  15. 05/12 fsmonitor: refactor refresh callback for non-directory eventsJeff Hostetler via GitGitGadget, Feb 13, 2024
  16. Junio C HamanoFeb 14, 2024
  17. Patrick SteinhardtFeb 15, 2024
  18. 06/12 fsmonitor: clarify handling of directory events in callbackJeff Hostetler via GitGitGadget, Feb 13, 2024
  19. Junio C HamanoFeb 14, 2024
  20. Jeff HostetlerFeb 20, 2024
  21. Junio C HamanoFeb 20, 2024
  22. Patrick SteinhardtFeb 15, 2024
  23. Jeff HostetlerFeb 20, 2024
  24. 07/12 fsmonitor: refactor untracked-cache invalidationJeff Hostetler via GitGitGadget, Feb 13, 2024
  25. Junio C HamanoFeb 14, 2024
  26. Patrick SteinhardtFeb 15, 2024
  27. 08/12 fsmonitor: support case-insensitive directory eventsJeff Hostetler via GitGitGadget, Feb 13, 2024
  28. Patrick SteinhardtFeb 15, 2024
  29. 09/12 fsmonitor: refactor non-directory callbackJeff Hostetler via GitGitGadget, Feb 13, 2024
  30. Patrick SteinhardtFeb 15, 2024
  31. 10/12 fsmonitor: support case-insensitive non-directory eventsJeff Hostetler via GitGitGadget, Feb 13, 2024
  32. 11/12 fsmonitor: refactor bit invalidation in refresh callbackJeff Hostetler via GitGitGadget, Feb 13, 2024
  33. Patrick SteinhardtFeb 15, 2024
  34. 12/12 t7527: update case-insenstive fsmonitor testJeff Hostetler via GitGitGadget, Feb 13, 2024
  35. 00/16 FSMonitor edge cases on case-insensitive file systemsJeff Hostetler via GitGitGadget, Feb 23, 2024
  36. 01/16 name-hash: add index_dir_find()Jeff Hostetler via GitGitGadget, Feb 23, 2024
  37. Junio C HamanoFeb 23, 2024
  38. 03/16 t7527: temporarily disable case-insensitive testsJeff Hostetler via GitGitGadget, Feb 23, 2024
  39. Junio C HamanoFeb 23, 2024
  40. Jeff HostetlerFeb 26, 2024
  41. 02/16 t7527: add case-insensitve test for FSMonitorJeff Hostetler via GitGitGadget, Feb 23, 2024
  42. 05/16 fsmonitor: clarify handling of directory events in callback helperJeff Hostetler via GitGitGadget, Feb 23, 2024
  43. 04/16 fsmonitor: refactor refresh callback on directory eventsJeff Hostetler via GitGitGadget, Feb 23, 2024
  44. Junio C HamanoFeb 23, 2024
  45. 06/16 fsmonitor: refactor refresh callback for non-directory eventsJeff Hostetler via GitGitGadget, Feb 23, 2024
  46. Junio C HamanoFeb 23, 2024
  47. Torsten BögershausenFeb 25, 2024
  48. Junio C HamanoFeb 25, 2024
  49. 07/16 dir: create untracked_cache_invalidate_trimmed_path()Jeff Hostetler via GitGitGadget, Feb 23, 2024
  50. Torsten BögershausenFeb 25, 2024
  51. 08/16 fsmonitor: refactor untracked-cache invalidationJeff Hostetler via GitGitGadget, Feb 23, 2024
  52. 09/16 fsmonitor: move untracked invalidation into helper functionsJeff Hostetler via GitGitGadget, Feb 23, 2024
  53. Junio C HamanoFeb 23, 2024
  54. Jeff HostetlerFeb 26, 2024
  55. 10/16 fsmonitor: return invalidated cache-entry count on directory eventJeff Hostetler via GitGitGadget, Feb 23, 2024
  56. 11/16 fsmonitor: remove custom loop from non-directory path handlerJeff Hostetler via GitGitGadget, Feb 23, 2024
  57. Junio C HamanoFeb 23, 2024
  58. 13/16 fsmonitor: trace the new invalidated cache-entry countJeff Hostetler via GitGitGadget, Feb 23, 2024
  59. Junio C HamanoFeb 23, 2024
  60. 12/16 fsmonitor: return invalided cache-entry count on non-directory eventJeff Hostetler via GitGitGadget, Feb 23, 2024
  61. Junio C HamanoFeb 23, 2024
  62. 14/16 fsmonitor: support case-insensitive eventsJeff Hostetler via GitGitGadget, Feb 23, 2024
  63. Junio C HamanoFeb 23, 2024
  64. Jeff HostetlerFeb 26, 2024
  65. Junio C HamanoFeb 26, 2024
  66. Torsten BögershausenFeb 25, 2024
  67. Jeff HostetlerFeb 26, 2024
  68. 15/16 fsmonitor: refactor bit invalidation in refresh callbackJeff Hostetler via GitGitGadget, Feb 23, 2024
  69. Junio C HamanoFeb 23, 2024
  70. 16/16 t7527: update case-insenstive fsmonitor testJeff Hostetler via GitGitGadget, Feb 23, 2024
  71. 00/14 FSMonitor edge cases on case-insensitive file systemsJeff Hostetler via GitGitGadget, Feb 26, 2024
  72. 01/14 name-hash: add index_dir_find()Jeff Hostetler via GitGitGadget, Feb 26, 2024
  73. 02/14 t7527: add case-insensitve test for FSMonitorJeff Hostetler via GitGitGadget, Feb 26, 2024
  74. 03/14 fsmonitor: refactor refresh callback on directory eventsJeff Hostetler via GitGitGadget, Feb 26, 2024
  75. 04/14 fsmonitor: clarify handling of directory events in callback helperJeff Hostetler via GitGitGadget, Feb 26, 2024
  76. 05/14 fsmonitor: refactor refresh callback for non-directory eventsJeff Hostetler via GitGitGadget, Feb 26, 2024
  77. 06/14 dir: create untracked_cache_invalidate_trimmed_path()Jeff Hostetler via GitGitGadget, Feb 26, 2024
  78. 07/14 fsmonitor: refactor untracked-cache invalidationJeff Hostetler via GitGitGadget, Feb 26, 2024
  79. 08/14 fsmonitor: move untracked-cache invalidation into helper functionsJeff Hostetler via GitGitGadget, Feb 26, 2024
  80. 09/14 fsmonitor: return invalidated cache-entry count on directory eventJeff Hostetler via GitGitGadget, Feb 26, 2024
  81. 10/14 fsmonitor: remove custom loop from non-directory path handlerJeff Hostetler via GitGitGadget, Feb 26, 2024
  82. 11/14 fsmonitor: return invalided cache-entry count on non-directory eventJeff Hostetler via GitGitGadget, Feb 26, 2024
  83. Patrick SteinhardtMar 6, 2024
  84. 12/14 fsmonitor: trace the new invalidated cache-entry countJeff Hostetler via GitGitGadget, Feb 26, 2024
  85. 13/14 fsmonitor: refactor bit invalidation in refresh callbackJeff Hostetler via GitGitGadget, Feb 26, 2024
  86. 14/14 fsmonitor: support case-insensitive eventsJeff Hostetler via GitGitGadget, Feb 26, 2024
  87. Patrick SteinhardtMar 6, 2024
  88. Junio C HamanoFeb 27, 2024
  89. Patrick SteinhardtMar 6, 2024
  90. Junio C HamanoMar 6, 2024
  91. Jeff HostetlerMar 6, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.