{"thread":{"id":"66460","subject":"[PATCH] fsmonitor: check the untracked cache after a trivial response","startedAt":"2026-10-04T15:19:18Z","lastAt":"2026-10-04T15:19:18Z","messageCount":1,"participants":["Ilia via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"554114","messageId":"pull.2247.git.1791127155614.gitgitgadget@gmail.com","threadId":"66460","inReplyTo":null,"subject":"[PATCH] fsmonitor: check the untracked cache after a trivial response","fromName":"Ilia via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-04T15:19:15Z","receivedAt":"2026-10-04T15:19:18Z","isPatch":true,"body":"From: Ilia K <ki.stfu@gmail.com>\n\nWhen the monitor sends a trivial response (\"/\"), for example after\nthe daemon restarted or lost events, refresh_fsmonitor() marks every\nindex entry dirty, which is saved in the index. For the untracked\ncache it only clears the in-memory flag use_fsmonitor. The valid bits\nof the cached directories are saved unchanged, although nothing\nchecks them anymore: valid_cached_dir() does not lstat() a valid\ndirectory while the monitor is trusted.\n\nA command that does not scan for untracked files, like \"git add\" or\n\"git checkout\", therefore saves the new token next to stale valid\nbits, and every later \"git status\" trusts them. A directory removed\nin the meantime keeps its untracked files listed, or, when its own\nentry is invalid, makes every run print\n```\nwarning: could not open directory 'sub/': No such file or directory\n```\nCheck every cached directory against its stat data before the cache\nis saved next to such a token, as valid_cached_dir() does without a\nmonitor, and invalidate the ones that changed or disappeared. Doing\nthis at write time skips commands that do not write the index, and\nsparse index writes, which drop the fsmonitor extension anyway.\nThe cost is one lstat() per cached directory, once per trivial\nresponse.\n\nSigned-off-by: Ilia K <ki.stfu@gmail.com>\n---\n    fsmonitor: check the untracked cache after a trivial response\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2247%2Fk15tfu%2Funtracked-cache-trivial-response-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2247/k15tfu/untracked-cache-trivial-response-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2247\n\n dir.c                        | 56 ++++++++++++++++++++++++++++++++++++\n dir.h                        |  7 +++++\n read-cache.c                 | 16 +++++++++++\n t/t7519-status-fsmonitor.sh  | 55 +++++++++++++++++++++++++++++++++++\n t/t7527-builtin-fsmonitor.sh | 44 ++++++++++++++++++++++++++++\n 5 files changed, 178 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex d896e7be4b..fc41ca9694 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -4053,6 +4053,62 @@ void untracked_cache_invalidate_trimmed_path(struct index_state *istate,\n \t}\n }\n \n+static int invalidate_stale_dirs(struct untracked_cache *uc,\n+\t\t\t\t struct untracked_cache_dir *ucd,\n+\t\t\t\t struct index_state *istate,\n+\t\t\t\t struct strbuf *path)\n+{\n+\tstruct stat st;\n+\tsize_t len = path->len;\n+\tint nr_invalidated = 0;\n+\tunsigned int i;\n+\n+\tif (ucd->valid &&\n+\t    (lstat(path->buf, &st) ||\n+\t     match_stat_data_racy(istate, &ucd->stat_data, &st))) {\n+\t\tinvalidate_one_directory(uc, ucd);\n+\t\tnr_invalidated++;\n+\t}\n+\n+\tfor (i = 0; i < ucd->dirs_nr; i++) {\n+\t\t/* not written to the index, see write_one_dir() */\n+\t\tif (!ucd->dirs[i]->recurse)\n+\t\t\tcontinue;\n+\t\tstrbuf_addch(path, '/');\n+\t\tstrbuf_addstr(path, ucd->dirs[i]->name);\n+\t\tnr_invalidated += invalidate_stale_dirs(uc, ucd->dirs[i],\n+\t\t\t\t\t\t\tistate, path);\n+\t\tstrbuf_setlen(path, len);\n+\t}\n+\n+\treturn nr_invalidated;\n+}\n+\n+int untracked_cache_invalidate_stale_dirs(struct index_state *istate)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tconst char *worktree;\n+\tint nr_invalidated;\n+\n+\tif (!istate->untracked || !istate->untracked->root)\n+\t\treturn 0;\n+\n+\t/*\n+\t * The index is also read and written by commands that do not\n+\t * run in the top-level directory of the worktree.\n+\t */\n+\tworktree = repo_get_work_tree(istate->repo);\n+\tif (!worktree)\n+\t\treturn 0;\n+\n+\tstrbuf_addstr(&path, worktree);\n+\tnr_invalidated = invalidate_stale_dirs(istate->untracked,\n+\t\t\t\t\t       istate->untracked->root,\n+\t\t\t\t\t       istate, &path);\n+\tstrbuf_release(&path);\n+\treturn nr_invalidated;\n+}\n+\n void untracked_cache_remove_from_index(struct index_state *istate,\n \t\t\t\t       const char *path)\n {\ndiff --git a/dir.h b/dir.h\nindex 83e0f648a8..7af1562b44 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -604,6 +604,13 @@ void untracked_cache_invalidate_path(struct index_state *, const char *, int saf\n void untracked_cache_invalidate_trimmed_path(struct index_state *,\n \t\t\t\t\t     const char *path,\n \t\t\t\t\t     int safe_path);\n+/*\n+ * Invalidate every cached directory that no longer exists or whose\n+ * stat data no longer matches the working tree. valid_cached_dir()\n+ * skips this check while the file system monitor is trusted.\n+ * Returns the number of invalidated directories.\n+ */\n+int untracked_cache_invalidate_stale_dirs(struct index_state *);\n void untracked_cache_remove_from_index(struct index_state *, const char *);\n void untracked_cache_add_to_index(struct index_state *, const char *);\n \ndiff --git a/read-cache.c b/read-cache.c\nindex c4cf08a3a3..00c9960c8e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -3034,6 +3034,22 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t    istate->untracked) {\n \t\tstrbuf_reset(&sb);\n \n+\t\t/*\n+\t\t * The monitor could not say what changed (see the trivial\n+\t\t * response in refresh_fsmonitor()), so nothing kept the\n+\t\t * valid bits up to date. Check them before they are saved\n+\t\t * next to the new token, which later commands trust even\n+\t\t * when this command did not look for untracked files.\n+\t\t */\n+\t\tif (write_extensions & WRITE_FSMONITOR_EXTENSION &&\n+\t\t    istate->fsmonitor_last_update &&\n+\t\t    !istate->untracked->use_fsmonitor) {\n+\t\t\tint nr = untracked_cache_invalidate_stale_dirs(istate);\n+\n+\t\t\ttrace2_data_intmax(\"index\", istate->repo,\n+\t\t\t\t\t   \"extension/untr/invalidated\", nr);\n+\t\t}\n+\n \t\twrite_untracked_extension(&sb, istate->untracked);\n \t\terr = write_index_ext_header(f, eoie_c, CACHE_EXT_UNTRACKED,\n \t\t\t\t\t     sb.len) < 0;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 93973ed25a..2e1d795a4b 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -336,6 +336,61 @@ do\n \tdone\n done\n \n+# After a trivial response (\"/\") the monitor cannot vouch for the\n+# untracked cache. Even a command that does not look for untracked\n+# files must drop the stale entries, or the next \"git status\" trusts them.\n+test_expect_success UNTRACKED_CACHE 'untracked cache is checked after a trivial response' '\n+\ttest_when_finished \"rm -rf trivial err\" &&\n+\tgit init trivial &&\n+\t(\n+\t\tcd trivial &&\n+\t\tmkdir -p dir/sub &&\n+\t\techo tracked >dir/sub/tracked &&\n+\t\tgit add dir &&\n+\t\tgit commit -m initial &&\n+\t\tgit config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-none\" &&\n+\t\t# Version 1 only, or the hook prints a version complaint\n+\t\t# on stderr at every query.\n+\t\tgit config core.fsmonitorHookVersion 1 &&\n+\t\tgit config core.untrackedCache true &&\n+\t\t# With \"normal\", invalidating one path also invalidates\n+\t\t# its parents, and the stale parent below is never seen.\n+\t\tgit config status.showUntrackedFiles all &&\n+\t\techo untracked >dir/sub/untracked &&\n+\t\techo \"?? dir/sub/untracked\" >../expect &&\n+\t\tgit status --porcelain >../actual &&\n+\t\ttest_cmp ../expect ../actual &&\n+\t\tgit status --porcelain >../actual &&\n+\t\ttest_cmp ../expect ../actual &&\n+\n+\t\t# The monitor misses the removal of dir/sub. \"git add other\"\n+\t\t# gets the trivial response and does not touch the entries\n+\t\t# of dir and dir/sub by itself.\n+\t\trm -r dir/sub &&\n+\t\techo other >other &&\n+\t\tgit -c core.fsmonitor=\"$TEST_DIRECTORY/t7519/fsmonitor-all\" \\\n+\t\t\tadd other &&\n+\t\tcat >../expect <<-\\EOF &&\n+\t\t D dir/sub/tracked\n+\t\tA  other\n+\t\tEOF\n+\t\tgit status --porcelain >../actual 2>../err &&\n+\t\ttest_must_be_empty ../err &&\n+\t\ttest_cmp ../expect ../actual &&\n+\n+\t\t# Invalidate the entry of dir/sub. The stale entry of dir then\n+\t\t# makes \"git status\" open the removed directory and warn.\n+\t\tgit update-index --remove dir/sub/tracked &&\n+\t\tcat >../expect <<-\\EOF &&\n+\t\tD  dir/sub/tracked\n+\t\tA  other\n+\t\tEOF\n+\t\tgit status --porcelain >../actual 2>../err &&\n+\t\ttest_must_be_empty ../err &&\n+\t\ttest_cmp ../expect ../actual\n+\t)\n+'\n+\n # test that splitting the index doesn't interfere\n test_expect_success 'splitting the index results in the same state' '\n \twrite_integration_script &&\ndiff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh\nindex 86195770e9..46b97ef784 100755\n--- a/t/t7527-builtin-fsmonitor.sh\n+++ b/t/t7527-builtin-fsmonitor.sh\n@@ -1389,4 +1389,48 @@ test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '\n \ttest_grep -q \" M dir1/dir2/dir4/FILE-4-A\" \"$PWD/file_case_wrong-try3.out\"\n '\n \n+# After a restart the daemon sends a trivial response (\"/\"), because it\n+# cannot know what changed while it was down. Even a command that does\n+# not look for untracked files must then drop the stale untracked cache\n+# entries, or the next \"git status\" trusts them.\n+test_expect_success UNTRACKED_CACHE 'untracked cache is checked after a trivial response' '\n+\ttest_when_finished \"stop_daemon_delete_repo test_trivial\" &&\n+\n+\tgit init test_trivial &&\n+\tmkdir -p test_trivial/dir/sub &&\n+\techo tracked >test_trivial/dir/sub/tracked &&\n+\tgit -C test_trivial add dir &&\n+\tgit -C test_trivial commit -m initial &&\n+\tgit -C test_trivial config core.fsmonitor true &&\n+\tgit -C test_trivial config core.untrackedCache true &&\n+\techo untracked >test_trivial/dir/sub/untracked &&\n+\n+\t# The first status starts the daemon and builds the untracked\n+\t# cache, the second one trusts it.\n+\techo \"?? dir/sub/untracked\" >expect &&\n+\tgit -C test_trivial status --porcelain >actual &&\n+\ttest_cmp expect actual &&\n+\tgit -C test_trivial status --porcelain >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Remove dir/sub while no daemon is running. \"git add\" then\n+\t# starts a new daemon, receives its trivial response, and does\n+\t# not look for untracked files.\n+\tgit -C test_trivial fsmonitor--daemon stop &&\n+\trm -r test_trivial/dir/sub &&\n+\techo other >test_trivial/other &&\n+\tGIT_TRACE2_EVENT=\"$PWD/trace_trivial\" \\\n+\t\tgit -C test_trivial add other &&\n+\thave_t2_data_event fsm_client query/trivial-response <trace_trivial &&\n+\tgit -C test_trivial fsmonitor--daemon status &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D dir/sub/tracked\n+\tA  other\n+\tEOF\n+\tgit -C test_trivial status --porcelain >actual 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\nbase-commit: c46c1e37724f0478939de636ab8ea5a89086d532\n-- \ngitgitgadget\n"}]}