Merge branch 'ik/fsmonitor-untracked-cache-trivial-response' into seen

The index writer has been taught to check the untracked cache
against the filesystem when an FSMonitor trivial response is
received, preventing commands like 'git add' from saving a new token
while leaving stale untracked cache entries intact.

* ik/fsmonitor-untracked-cache-trivial-response:
  fsmonitor: check the untracked cache after a trivial response

dir.c

@@ -4085,6 +4085,62 @@ void untracked_cache_invalidate_trimmed_path(struct index_state *istate,
 	}
 }
 
+static int invalidate_stale_dirs(struct untracked_cache *uc,
+				 struct untracked_cache_dir *ucd,
+				 struct index_state *istate,
+				 struct strbuf *path)
+{
+	struct stat st;
+	size_t len = path->len;
+	int nr_invalidated = 0;
+	unsigned int i;
+
+	if (ucd->valid &&
+	    (lstat(path->buf, &st) ||
+	     match_stat_data_racy(istate, &ucd->stat_data, &st))) {
+		invalidate_one_directory(uc, ucd);
+		nr_invalidated++;
+	}
+
+	for (i = 0; i < ucd->dirs_nr; i++) {
+		/* not written to the index, see write_one_dir() */
+		if (!ucd->dirs[i]->recurse)
+			continue;
+		strbuf_addch(path, '/');
+		strbuf_addstr(path, ucd->dirs[i]->name);
+		nr_invalidated += invalidate_stale_dirs(uc, ucd->dirs[i],
+							istate, path);
+		strbuf_setlen(path, len);
+	}
+
+	return nr_invalidated;
+}
+
+int untracked_cache_invalidate_stale_dirs(struct index_state *istate)
+{
+	struct strbuf path = STRBUF_INIT;
+	const char *worktree;
+	int nr_invalidated;
+
+	if (!istate->untracked || !istate->untracked->root)
+		return 0;
+
+	/*
+	 * The index is also read and written by commands that do not
+	 * run in the top-level directory of the worktree.
+	 */
+	worktree = repo_get_work_tree(istate->repo);
+	if (!worktree)
+		return 0;
+
+	strbuf_addstr(&path, worktree);
+	nr_invalidated = invalidate_stale_dirs(istate->untracked,
+					       istate->untracked->root,
+					       istate, &path);
+	strbuf_release(&path);
+	return nr_invalidated;
+}
+
 void untracked_cache_remove_from_index(struct index_state *istate,
 				       const char *path)
 {

dir.h

@@ -600,6 +600,13 @@ void untracked_cache_invalidate_path(struct index_state *, const char *, int saf
 void untracked_cache_invalidate_trimmed_path(struct index_state *,
 					     const char *path,
 					     int safe_path);
+/*
+ * Invalidate every cached directory that no longer exists or whose
+ * stat data no longer matches the working tree. valid_cached_dir()
+ * skips this check while the file system monitor is trusted.
+ * Returns the number of invalidated directories.
+ */
+int untracked_cache_invalidate_stale_dirs(struct index_state *);
 void untracked_cache_remove_from_index(struct index_state *, const char *);
 void untracked_cache_add_to_index(struct index_state *, const char *);
 

read-cache.c

@@ -3034,6 +3034,22 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
 	    istate->untracked) {
 		strbuf_reset(&sb);
 
+		/*
+		 * The monitor could not say what changed (see the trivial
+		 * response in refresh_fsmonitor()), so nothing kept the
+		 * valid bits up to date. Check them before they are saved
+		 * next to the new token, which later commands trust even
+		 * when this command did not look for untracked files.
+		 */
+		if (write_extensions & WRITE_FSMONITOR_EXTENSION &&
+		    istate->fsmonitor_last_update &&
+		    !istate->untracked->use_fsmonitor) {
+			int nr = untracked_cache_invalidate_stale_dirs(istate);
+
+			trace2_data_intmax("index", istate->repo,
+					   "extension/untr/invalidated", nr);
+		}
+
 		write_untracked_extension(&sb, istate->untracked);
 		err = write_index_ext_header(f, eoie_c, CACHE_EXT_UNTRACKED,
 					     sb.len) < 0;

t/t7519-status-fsmonitor.sh

@@ -336,6 +336,61 @@ do
 	done
 done
 
+# After a trivial response ("/") the monitor cannot vouch for the
+# untracked cache. Even a command that does not look for untracked
+# files must drop the stale entries, or the next "git status" trusts them.
+test_expect_success UNTRACKED_CACHE 'untracked cache is checked after a trivial response' '
+	test_when_finished "rm -rf trivial err" &&
+	git init trivial &&
+	(
+		cd trivial &&
+		mkdir -p dir/sub &&
+		echo tracked >dir/sub/tracked &&
+		git add dir &&
+		git commit -m initial &&
+		git config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-none" &&
+		# Version 1 only, or the hook prints a version complaint
+		# on stderr at every query.
+		git config core.fsmonitorHookVersion 1 &&
+		git config core.untrackedCache true &&
+		# With "normal", invalidating one path also invalidates
+		# its parents, and the stale parent below is never seen.
+		git config status.showUntrackedFiles all &&
+		echo untracked >dir/sub/untracked &&
+		echo "?? dir/sub/untracked" >../expect &&
+		git status --porcelain >../actual &&
+		test_cmp ../expect ../actual &&
+		git status --porcelain >../actual &&
+		test_cmp ../expect ../actual &&
+
+		# The monitor misses the removal of dir/sub. "git add other"
+		# gets the trivial response and does not touch the entries
+		# of dir and dir/sub by itself.
+		rm -r dir/sub &&
+		echo other >other &&
+		git -c core.fsmonitor="$TEST_DIRECTORY/t7519/fsmonitor-all" \
+			add other &&
+		cat >../expect <<-\EOF &&
+		 D dir/sub/tracked
+		A  other
+		EOF
+		git status --porcelain >../actual 2>../err &&
+		test_must_be_empty ../err &&
+		test_cmp ../expect ../actual &&
+
+		# Invalidate the entry of dir/sub. The stale entry of dir then
+		# makes "git status" open the removed directory and warn.
+		git update-index --remove dir/sub/tracked &&
+		cat >../expect <<-\EOF &&
+		D  dir/sub/tracked
+		A  other
+		EOF
+		git status --porcelain >../actual 2>../err &&
+		test_must_be_empty ../err &&
+		test_cmp ../expect ../actual
+	)
+'
+
 # test that splitting the index doesn't interfere
 test_expect_success 'splitting the index results in the same state' '
 	write_integration_script &&

t/t7527-builtin-fsmonitor.sh

@@ -1389,4 +1389,48 @@ test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
 	test_grep -q " M dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-try3.out"
 '
 
+# After a restart the daemon sends a trivial response ("/"), because it
+# cannot know what changed while it was down. Even a command that does
+# not look for untracked files must then drop the stale untracked cache
+# entries, or the next "git status" trusts them.
+test_expect_success UNTRACKED_CACHE 'untracked cache is checked after a trivial response' '
+	test_when_finished "stop_daemon_delete_repo test_trivial" &&
+
+	git init test_trivial &&
+	mkdir -p test_trivial/dir/sub &&
+	echo tracked >test_trivial/dir/sub/tracked &&
+	git -C test_trivial add dir &&
+	git -C test_trivial commit -m initial &&
+	git -C test_trivial config core.fsmonitor true &&
+	git -C test_trivial config core.untrackedCache true &&
+	echo untracked >test_trivial/dir/sub/untracked &&
+
+	# The first status starts the daemon and builds the untracked
+	# cache, the second one trusts it.
+	echo "?? dir/sub/untracked" >expect &&
+	git -C test_trivial status --porcelain >actual &&
+	test_cmp expect actual &&
+	git -C test_trivial status --porcelain >actual &&
+	test_cmp expect actual &&
+
+	# Remove dir/sub while no daemon is running. "git add" then
+	# starts a new daemon, receives its trivial response, and does
+	# not look for untracked files.
+	git -C test_trivial fsmonitor--daemon stop &&
+	rm -r test_trivial/dir/sub &&
+	echo other >test_trivial/other &&
+	GIT_TRACE2_EVENT="$PWD/trace_trivial" \
+		git -C test_trivial add other &&
+	have_t2_data_event fsm_client query/trivial-response <trace_trivial &&
+	git -C test_trivial fsmonitor--daemon status &&
+
+	cat >expect <<-\EOF &&
+	 D dir/sub/tracked
+	A  other
+	EOF
+	git -C test_trivial status --porcelain >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
 test_done