From: Phillip Wood <phillip.wood123@gmail•com>
To: Paul Tarjan via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org
Cc: Paul Tarjan <github@paulisageek•com>
Subject: Re: [PATCH v2] t7800: fix racy "difftool --dir-diff syncs worktree" test
Date: Sat, 3 Jan 2026 09:39:04 +0000 [thread overview]
Message-ID: <02749b7d-e9a4-4894-a50c-91a7c1a22d84@gmail.com> (raw)
In-Reply-To: <pull.2149.v2.git.git.1767292068036.gitgitgadget@gmail.com>
Hi Paul
On 01/01/2026 18:27, Paul Tarjan via GitGitGadget wrote:
> From: Paul Tarjan <github@paulisageek•com>
>
> The "difftool --dir-diff syncs worktree without unstaged change" test
> fails intermittently, particularly on Windows CI.
Thanks for working on this. I've seen it fail a lot in Windows CI runs -
does it fail on other platforms as well?
> 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)
As I understand it the test is flaky because the file is updated without
changing any of the stat fields that git looks at. How does that relate
to fstat() returning different data to lstat()? Also doesn't
UNRELIABLE_FSTAT exist so that we can work around the problem?
> - 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)
That comment is specifically talking about linux so how does it relate
to a test that is flaky on Windows?
> - Attribute caching: Windows GetFileAttributesExW may cache results
When git refreshes the index it calls lstat() on each path in the index.
GitFileAttributesExW() provides an API like readir() which returns paths
in an arbitary order and it also resolves symbolic links so I'm having a
hard time understating where it is called by git. (There was a post [1]
on reddit recently about using GitFileAttributesExW in this context)
[1]
https://www.reddit.com/r/rust/comments/1prkzqg/writing_the_fastest_implementation_of_git_status/
> 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.
This stops the test from being flaky but it is a real bug. If the user
is modifying the files interactively then they're unlikely to be able to
update the file fast enough to be affected but if anyone is scripting
like the test does then they might be affected.
Thanks
Phillip
> 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 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-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2149/ptarjan/claude/fix-difftool-test-DDxDC-v2
> Pull-Request: https://github.com/git/git/pull/2149
>
> Range-diff vs v1:
>
> 1: dd5b774451 = 1: 98bc88f336 t7800: fix racy "difftool --dir-diff syncs worktree" test
>
>
> 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
next prev parent reply other threads:[~2026-01-03 9:39 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-31 22:19 [PATCH] t7800: fix racy "difftool --dir-diff syncs worktree" test Paul Tarjan via GitGitGadget
2026-01-01 14:49 ` 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 [this message]
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=02749b7d-e9a4-4894-a50c-91a7c1a22d84@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=github@paulisageek$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox