From: Phillip Wood Date: Mon, 05 Jan 2026 10:33:34 GMT Subject: Re: [PATCH v3] t7800: fix racy "difftool --dir-diff syncs worktree" test Message-ID: In-Reply-To: Hi Paul Thanks for rewording the commit message. This looks good - it will be nice to have one less flaky test to worry about when the CI fails Phillip On 03/01/2026 20:40, Paul Tarjan via GitGitGadget wrote: > From: Paul Tarjan > > The "difftool --dir-diff syncs worktree without unstaged change" test > fails intermittently on Windows CI, as seen at: > > https://github.com/git/git/actions/runs/20624095002/job/59231745784#step:5:416 > > 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's sync-back mechanism checks for changes, it compares > stat data between the temporary index and the modified files. If the > modification happens within the same timestamp granularity window and > file size stays the same, the change goes undetected. > > On Windows, this is more likely to manifest because Git relies on > inode changes as a fallback when other stat fields match, but Windows > filesystems lack inodes. This is a real bug that could affect users > scripting difftool similarly, as seen at: > > https://github.com/git-for-windows/git/issues/5132 > > Fix the test by changing the replacement content to "modified content" > (17 bytes), ensuring the size difference is detected regardless of > timestamp resolution or platform-specific stat behavior. > > Note: This fixes the test flakiness but not the underlying issue in > difftool's change detection. Other tests with same-size file patterns > (t0010-racy-git.sh, t2200-add-update.sh) are not affected because they > use normal index operations with proper racy-git detection. > > Signed-off-by: Paul Tarjan > Reviewed-by: Johannes Schindelin > --- > t7800: fix racy "difftool --dir-diff syncs worktree" test > > The "difftool --dir-diff syncs worktree without unstaged change" test > fails intermittently on Windows CI, as seen at: > > https://github.com/git/git/actions/runs/20624095002/job/59231745784#step:5:416 > > 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's sync-back mechanism checks for changes, it compares stat > data between the temporary index and the modified files. If the > modification happens within the same timestamp granularity window and > file size stays the same, the change goes undetected. > > On Windows, this is more likely to manifest because Git relies on inode > changes as a fallback when other stat fields match, but Windows > filesystems lack inodes. This is a real bug that could affect users > scripting difftool similarly (see > https://github.com/git-for-windows/git/issues/5132 for a related > real-world report). > > Fix the test by changing the replacement content to "modified content" > (17 bytes), ensuring the size difference is detected regardless of > timestamp resolution or platform-specific stat behavior. > > Note: This fixes the test flakiness but not the underlying issue in > difftool's change detection. Other tests with same-size file patterns > (t0010-racy-git.sh, t2200-add-update.sh) are not affected because they > use normal index operations with proper racy-git detection. > > Changes since v2 > > * Added Reviewed-by to the commit message > * Updated URL to be a full link to github > * Reduced speculation from commit message > > Changes since v1 > > * Added Reviewed-by > > Signed-off-by: Paul Tarjan github@paulisageek.com Reviewed-by: Johannes > Schindelin Johannes.Schindelin@gmx.de > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2149%2Fptarjan%2Fclaude%2Ffix-difftool-test-DDxDC-v3 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2149/ptarjan/claude/fix-difftool-test-DDxDC-v3 > Pull-Request: https://github.com/git/git/pull/2149 > > Range-diff vs v2: > > 1: 98bc88f336 ! 1: 3e43dcc7fd t7800: fix racy "difftool --dir-diff syncs worktree" test > @@ Commit message > t7800: fix racy "difftool --dir-diff syncs worktree" test > > The "difftool --dir-diff syncs worktree without unstaged change" test > - fails intermittently, particularly on Windows CI. > + fails intermittently on Windows CI, as seen at: > > - 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. > + https://github.com/git/git/actions/runs/20624095002/job/59231745784#step:5:416 > > The root cause is that the original file content and the replacement > content have identical sizes: > @@ Commit message > - 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. > + When difftool's sync-back mechanism checks for changes, it compares > + stat data between the temporary index and the modified files. If the > + modification happens within the same timestamp granularity window and > + file size stays the same, the change goes undetected. > > - However, Windows has known filesystem issues that may cause this to > - fail intermittently: > + On Windows, this is more likely to manifest because Git relies on > + inode changes as a fallback when other stat fields match, but Windows > + filesystems lack inodes. This is a real bug that could affect users > + scripting difftool similarly, as seen at: > > - - UNRELIABLE_FSTAT: Windows fstat() on open files may not return the > - same information as lstat() after close (config.mak.uname:506) > + https://github.com/git-for-windows/git/issues/5132 > > - - 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) > + Fix the test by changing the replacement content to "modified content" > + (17 bytes), ensuring the size difference is detected regardless of > + timestamp resolution or platform-specific stat behavior. > > - - 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. > + Note: This fixes the test flakiness but not the underlying issue in > + difftool's change detection. Other tests with same-size file patterns > + (t0010-racy-git.sh, t2200-add-update.sh) are not affected because they > + use normal index operations with proper racy-git detection. > > Signed-off-by: Paul Tarjan > + Reviewed-by: Johannes Schindelin > > ## t/t7800-difftool.sh ## > @@ t/t7800-difftool.sh: test_expect_success SYMLINKS 'difftool --dir-diff --symlinks without unstaged ch > > > 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