From: Phillip Wood <phillip.wood123@gmail•com>
To: Paul Tarjan via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org
Cc: Paul Tarjan <paul@paultarjan•com>, Paul Tarjan <github@paulisageek•com>
Subject: Re: [PATCH v3] t7800: fix racy "difftool --dir-diff syncs worktree" test
Date: Mon, 5 Jan 2026 10:33:34 +0000 [thread overview]
Message-ID: <c0ccc3e8-7863-47b0-abf9-99c3dba0f4eb@gmail.com> (raw)
In-Reply-To: <pull.2149.v3.git.git.1767472809897.gitgitgadget@gmail.com>
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 <github@paulisageek•com>
>
> 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 <github@paulisageek•com>
> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx•de>
> ---
> 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 <github@paulisageek•com>
> + Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx•de>
>
> ## 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
prev parent reply other threads:[~2026-01-05 10:33 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
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 [this message]
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=c0ccc3e8-7863-47b0-abf9-99c3dba0f4eb@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=paul@paultarjan$(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