All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul Tarjan via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Paul Tarjan <github@paulisageek.com>,
	Paul Tarjan <github@paulisageek.com>
Subject: [PATCH] t7800: fix racy "difftool --dir-diff syncs worktree" test
Date: Wed, 31 Dec 2025 22:19:59 +0000	[thread overview]
Message-ID: <pull.2149.git.git.1767219599334.gitgitgadget@gmail.com> (raw)

From: Paul Tarjan <github@paulisageek.com>

The "difftool --dir-diff syncs worktree without unstaged change" test
fails intermittently, particularly on Windows CI.

The test modifies a file in difftool's temp directory via an extcmd
script and expects the change to be synced back to the worktree. The
sync-back detection relies on git's change detection mechanisms.

The root cause is that the original file content and the replacement
content have identical sizes:

  - Original: "main\ntest\na\n" = 12 bytes
  - New:      "new content\n"   = 12 bytes

When difftool creates the temporary index (wtindex), the cache entries
have sd_size = 0 (zero-initialized via make_cache_entry with no
refresh). Git's ie_modified() is designed to handle this by calling
ce_modified_check_fs() for content hashing when sd_size is 0.

However, Windows has known filesystem issues that may cause this to
fail intermittently:

 - UNRELIABLE_FSTAT: Windows fstat() on open files may not return the
   same information as lstat() after close (config.mak.uname:506)

 - NTFS timestamp issues: The racy-git documentation notes that NTFS
   is "still broken" regarding timestamp granularity between in-core
   and on-disk representations (Documentation/technical/racy-git.adoc)

 - Attribute caching: Windows GetFileAttributesExW may cache results

Fix this by changing the replacement content to "modified content\n"
(17 bytes), ensuring the change is detected at the earliest size
comparison in match_stat_data(), bypassing any platform-specific edge
cases in the more complex code paths.

Note: Other tests with same-size file patterns (t0010-racy-git.sh,
t2200-add-update.sh, t1701-racy-split-index.sh) are not vulnerable
because they use normal Git index operations with proper racy git
detection. The difftool case is unique due to its ephemeral wtindex
created via make_cache_entry() without full stat refresh.

Signed-off-by: Paul Tarjan <github@paulisageek.com>
---
    t7800: fix racy "difftool --dir-diff syncs worktree" test
    
    In
    https://github.com/git/git/actions/runs/20624095002/job/59231745784#step:5:416
    this test failed for me on an unrelated commit. I had Claude look into
    it and it thought that this could be a racy git problem. I'm skeptical
    but a) I don't know the source well enough and b) the fix is low risk so
    I thought I'd send it to you folks. Everything below is the AI generated
    explanation.
    
    The "difftool --dir-diff syncs worktree without unstaged change" test
    fails intermittently, particularly on Windows CI.
    
    The test modifies a file in difftool's temp directory via an extcmd
    script and expects the change to be synced back to the worktree. The
    sync-back detection relies on git's change detection mechanisms.
    
    The root cause is that the original file content and the replacement
    content have identical sizes:
    
     * Original: "main\ntest\na\n" = 12 bytes
     * New: "new content\n" = 12 bytes
    
    When difftool creates the temporary index (wtindex), the cache entries
    have sd_size = 0 (zero-initialized via make_cache_entry with no
    refresh). Git's ie_modified() is designed to handle this by calling
    ce_modified_check_fs() for content hashing when sd_size is 0.
    
    However, Windows has known filesystem issues that may cause this to fail
    intermittently:
    
     * UNRELIABLE_FSTAT: Windows fstat() on open files may not return the
       same information as lstat() after close (config.mak.uname:506)
    
     * NTFS timestamp issues: The racy-git documentation notes that NTFS is
       "still broken" regarding timestamp granularity between in-core and
       on-disk representations (Documentation/technical/racy-git.adoc)
    
     * Attribute caching: Windows GetFileAttributesExW may cache results
    
    Fix this by changing the replacement content to "modified content\n" (17
    bytes), ensuring the change is detected at the earliest size comparison
    in match_stat_data(), bypassing any platform-specific edge cases in the
    more complex code paths.
    
    Note: Other tests with same-size file patterns (t0010-racy-git.sh,
    t2200-add-update.sh, t1701-racy-split-index.sh) are not vulnerable
    because they use normal Git index operations with proper racy git
    detection. The difftool case is unique due to its ephemeral wtindex
    created via make_cache_entry() without full stat refresh.
    
    Signed-off-by: Paul Tarjan github@paulisageek.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2149%2Fptarjan%2Fclaude%2Ffix-difftool-test-DDxDC-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2149/ptarjan/claude/fix-difftool-test-DDxDC-v1
Pull-Request: https://github.com/git/git/pull/2149

 t/t7800-difftool.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index bf0f67378d..8a91ff3603 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -647,21 +647,21 @@ test_expect_success SYMLINKS 'difftool --dir-diff --symlinks without unstaged ch
 '
 
 write_script modify-right-file <<\EOF
-echo "new content" >"$2/file"
+echo "modified content" >"$2/file"
 EOF
 
 run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged change' '
 	test_when_finished git reset --hard &&
 	echo "orig content" >file &&
 	git difftool -d $symlinks --extcmd "$PWD/modify-right-file" branch &&
-	echo "new content" >expect &&
+	echo "modified content" >expect &&
 	test_cmp expect file
 '
 
 run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged change' '
 	test_when_finished git reset --hard &&
 	git difftool -d $symlinks --extcmd "$PWD/modify-right-file" branch &&
-	echo "new content" >expect &&
+	echo "modified content" >expect &&
 	test_cmp expect file
 '
 

base-commit: 68cb7f9e92a5d8e9824f5b52ac3d0a9d8f653dbe
-- 
gitgitgadget

             reply	other threads:[~2025-12-31 22:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-31 22:19 Paul Tarjan via GitGitGadget [this message]
2026-01-01 14:49 ` [PATCH] t7800: fix racy "difftool --dir-diff syncs worktree" test Johannes Schindelin
2026-01-01 18:27 ` [PATCH v2] " Paul Tarjan via GitGitGadget
2026-01-01 22:49   ` Junio C Hamano
2026-01-03  9:39   ` Phillip Wood
2026-01-03 16:30     ` Paul Tarjan
2026-01-03 20:29       ` Johannes Schindelin
2026-01-04  2:27         ` Junio C Hamano
2026-01-03 20:40   ` [PATCH v3] " Paul Tarjan via GitGitGadget
2026-01-04  2:34     ` Junio C Hamano
2026-01-05 10:33     ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.2149.git.git.1767219599334.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=github@paulisageek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.