FSMonitor: deepening a directory causes confusing events (Take 2)

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,8 +645,8 @@ test_expect_success 'Matrix: setup for untracked-cache,fsmonitor matrix' '
 '
 
 matrix_clean_up_repo () {
-	git reset --hard HEAD
 	git clean -fd
+	git reset --hard HEAD
 }
 
 matrix_try () {
@@ -687,6 +710,31 @@ 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
+
+		# NEEDSWORK: On Windows the untracked-cache is buggy when FSMonitor
+		# is DISABLED.  Turn off a few test that cause it problems until
+		# we can debug it.
+		#
+		try_moves="true"
+		test_have_prereq UNTRACKED_CACHE,WINDOWS && \
+			test $uc_val = true && \
+			test $fsm_val = false && \
+			try_moves="false"
+		if test $try_moves = true
+		then
+			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
+		fi
+
+		if test $fsm_val = true
+		then
+			test_expect_success "Matrix[uc:$uc_val][fsm:$fsm_val] disable fsmonitor at end" '
+				test_might_fail git config --unset core.useBuiltinFSMonitor &&
+				git update-index --no-fsmonitor &&
+				test_might_fail git fsmonitor--daemon stop 2>/dev/null
+			'
+		fi
 	done
 done