fsmonitor: invalidate entire cone on directory move/rename

Invalidate the CE_FSMONITOR_VALID bit for all of the files in the
cone under a directory when it is moved or renamed.

Previously, we only invalidated the files immediately contained within
the directory, but that ignored the fact that the pathnames of nested
items also changed.

Signed-off-by: Jeff Hostetler <[email protected]>

Author: jeffhostetler

fsmonitor.c

@@ -198,37 +198,75 @@ int fsmonitor_is_trivial_response(const struct strbuf *query_result)
 static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
 {
 	int i, len = strlen(name);
+	int pos = index_name_pos(istate, name, len);
+
+	trace_printf_key(&trace_fsmonitor,
+			 "fsmonitor_refresh_callback '%s' (pos %d)",
+			 name, pos);
+
 	if (name[len - 1] == '/') {
-		const char *rel;
-		int pos = index_name_pos(istate, name, len);
+		/*
+		 * The daemon can decorate directory events, such as
+		 * moves or renames, with a trailing slash if the OS
+		 * FS Event contains sufficient information, such as
+		 * MacOS.
+		 *
+		 * Use this to invalidate the entire cone under that
+		 * directory.
+		 *
+		 * We do not expect an exact match because the index
+		 * does not normally contain directory entries, so we
+		 * start at the insertion point and scan.
+		 */
 		if (pos < 0)
 			pos = -pos - 1;
 
 		/* Mark all entries for the folder invalid */
 		for (i = pos; i < istate->cache_nr; i++) {
 			if (!starts_with(istate->cache[i]->name, name))
 				break;
-			/* Only mark the immediate children in the folder */
-			rel = istate->cache[i]->name + len;
-			if (!strchr(rel, '/'))
-				istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
+			istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
 		}
-		/* Need to remove the / from the path for the untracked cache */
+
+		/*
+		 * We need to remove the traling "/" from the path
+		 * for the untracked cache.
+		 */
 		name[len - 1] = '\0';
+	} else if (pos >= 0) {
+		/*
+		 * We have an exact match for this path and can just
+		 * invalidate it.
+		 */
+		istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
 	} else {
-		int pos = index_name_pos(istate, name, len);
+		/*
+		 * The path is not a tracked file -or- it is a
+		 * directory event on a platform that cannot
+		 * distinguish between file and directory events in
+		 * the event handler, such as Windows.
+		 *
+		 * Scan as if it is a directory and invalidate the
+		 * cone under it.  (But remember to ignore items
+		 * between "name" and "name/", such as "name-" and
+		 * "name.".
+		 */
+		pos = -pos - 1;
 
-		if (pos >= 0) {
-			struct cache_entry *ce = istate->cache[pos];
-			ce->ce_flags &= ~CE_FSMONITOR_VALID;
+		for (i = pos; i < istate->cache_nr; i++) {
+			if (!starts_with(istate->cache[i]->name, name))
+				break;
+			if ((unsigned char)istate->cache[i]->name[len] > '/')
+				break;
+			if (istate->cache[i]->name[len] == '/')
+				istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
 		}
 	}
 
 	/*
 	 * Mark the untracked cache dirty even if it wasn't found in the index
 	 * as it could be a new untracked file.
 	 */
-	trace_printf_key(&trace_fsmonitor, "fsmonitor_refresh_callback '%s'", name);
 	untracked_cache_invalidate_path(istate, name, 0);
 }
 

t/t7527-builtin-fsmonitor.sh

@@ -261,6 +261,16 @@ test_expect_success 'setup' '
 	trace*
 	EOF
 
+	mkdir -p T1/T2/T3/T4 &&
+	echo 1 >T1/F1 &&
+	echo 1 >T1/T2/F1 &&
+	echo 1 >T1/T2/T3/F1 &&
+	echo 1 >T1/T2/T3/T4/F1 &&
+	echo 2 >T1/F2 &&
+	echo 2 >T1/T2/F2 &&
+	echo 2 >T1/T2/T3/F2 &&
+	echo 2 >T1/T2/T3/T4/F2 &&
+
 	git -c core.useBuiltinFSMonitor= add . &&
 	test_tick &&
 	git -c core.useBuiltinFSMonitor= commit -m initial &&
@@ -354,6 +364,19 @@ verify_status() {
 	echo HELLO AFTER
 }
 
+move_directory_contents_deeper() {
+	mkdir T1/_new_
+	mv T1/[A-Z]* T1/_new_
+}
+
+move_directory_up() {
+	mv T1/T2/T3 T1
+}
+
+move_directory() {
+	mv T1/T2/T3 T1/T2/NewT3
+}
+
 # The next few test cases confirm that our fsmonitor daemon sees each type
 # of OS filesystem notification that we care about.  At this layer we just
 # ensure we are getting the OS notifications and do not try to confirm what
@@ -622,6 +645,7 @@ test_expect_success 'Matrix: setup for untracked-cache,fsmonitor matrix' '
 '
 
 matrix_clean_up_repo () {
+	git clean -fd
 	git reset --hard HEAD
 	git clean -fd
 }
@@ -687,6 +711,9 @@ do
 		matrix_try $uc_val $fsm_val rename_files
 		matrix_try $uc_val $fsm_val file_to_directory
 		matrix_try $uc_val $fsm_val directory_to_file
+		matrix_try $uc_val $fsm_val move_directory_contents_deeper
+		matrix_try $uc_val $fsm_val move_directory_up
+		matrix_try $uc_val $fsm_val move_directory
 
 		if test $fsm_val = true
 		then