Re: [PATCH] t7800: fix racy "difftool --dir-diff syncs worktree" test

> 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
> 
>