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

[PATCH] [diff-lib] Fix check_removed when fsmonitor is on

From
Josip Sokcevic <sokcevic@google.com>
Date
Sep 6, 2023, 06:02 UTC
Message-ID
<20230906060241.944886-2-sokcevic@google.com>
In-Reply-To
<20230829005606.136615-1-jonathantanmy@google.com>

git-diff-index may return incorrect deleted entries when fsmonitor is used in a repository with git submodules. This can be observed on Mac machines, but it can affect all other supported platforms too.

If fsmonitor is used, `stat *st` may not be initialized. Since `lstat` calls
aren't not desired when fsmonitor is on, skip the entire gitlink check using
the same condition used to initialize `stat *st`.
---
This patch is using jonathantanmy@ test case, which now passes. It's
possible there are similar edge cases with fsmonitor, so I'll do more
extensive e2e testing tomorrow.
 diff-lib.c                   | 2 +-
 t/t7527-builtin-fsmonitor.sh | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/diff-lib.c b/diff-lib.c
index d8aa777a73..c67aa5ff89 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -46,7 +46,7 @@ static int check_removed(const struct index_state *istate, const struct cache_en
 	}
 	if (has_symlink_leading_path(ce->name, ce_namelen(ce)))
 		return 1;
-	if (S_ISDIR(st->st_mode)) {
+	if (!(ce->ce_flags & CE_FSMONITOR_VALID) && S_ISDIR(st->st_mode)) {
 		struct object_id sub;
 
 		/*
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 0c241d6c14..894a80fbe8 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -824,6 +824,10 @@ test_expect_success 'submodule always visited' '
 
 	create_super super &&
 	create_sub sub &&
+	git -C super --no-optional-locks diff-index --name-status HEAD >actual.with &&
+	git -C super --no-optional-locks -c core.fsmonitor=false \
+	    diff-index --name-status HEAD >actual.without &&
+	    test_cmp actual.with actual.without &&
 
 	git -C super submodule add ../sub ./dir_1/dir_2/sub &&
 	git -C super commit -m "add sub" &&
@@ -837,6 +841,8 @@ test_expect_success 'submodule always visited' '
 	# some dirt in the submodule and confirm matching output.
 
 	# Completely clean status.
+	echo Now running for clean status &&
+
 	my_match_and_clean &&
 
 	# .M S..U
-- 
2.42.0.283.g2d96d420d3-goog
Previous: Jonathan TanNext: Jonathan Tan
Message 6 of 18 in “Strange diff-index with fsmonitor, submodules”
  1. Jonathan TanAug 29, 2023
  2. Junio C HamanoAug 29, 2023
  3. Jeff HostetlerAug 29, 2023
  4. Jeff HostetlerAug 29, 2023
  5. Jonathan TanAug 29, 2023
  6. [diff-lib] Fix check_removed when fsmonitor is onJosip Sokcevic, Sep 6, 2023
  7. Jonathan TanSep 6, 2023
  8. diff-lib: Fix check_removed when fsmonitor is onJosip Sokcevic, Sep 7, 2023
  9. Jonathan TanSep 7, 2023
  10. Junio C HamanoSep 7, 2023
  11. Josip SokcevicSep 7, 2023
  12. Junio C HamanoSep 8, 2023
  13. diff-lib: Fix check_removed when fsmonitor is onJosip Sokcevic, Sep 11, 2023
  14. Junio C HamanoSep 11, 2023
  15. Josip SokcevicSep 12, 2023
  16. Junio C HamanoSep 12, 2023
  17. Josip SokcevicSep 14, 2023
  18. Junio C HamanoSep 18, 2023

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.