git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PTPaul Tarjan <paul@paultarjan.com>
Date
Jan 3, 2026, 16:30 UTC
Message-ID
<CALvWuB79v3i3zU_g1swqQVS-fH1f-U8Ptr9Z9ObAUgeFJHx++A@mail.gmail.com>
In-Reply-To
<02749b7d-e9a4-4894-a50c-91a7c1a22d84@gmail.com>

I've updated the commit and PR summary for your comments. Should I re-run /submit to send a no-op patch or leave it as is until code changes are needed?

On Fri, Jan 2, 2026 at 11:39 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 11 quoted lines
>
> 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?

I did a cursory grep through the Github actions and didn't find any other failures for this. The fact that you have seen it too means this is more widespread. I was merely reacting to the fact that it failed on my unrelated diff.

My guess is this will start failing more once my fsmonitor for linux merges in since it will be yet another platform to fail on.

Show 25 quoted lines
>
> > 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?

You're right, the UNRELIABLE_FSTAT reference was misapplied here. That flag addresses a different issue (fstat vs lstat discrepancies on open files). The actual problem is simpler: when file size and mtime both match, stat-based detection fails entirely. I'll remove this from the commit message.

Show 7 quoted lines
>
> >   - 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?

You're right - I conflated unrelated documentation. Looking at CI history, the failure was only observed on Windows (win test (8)). The root cause is Windows-specific: Git relies on inode changes as a fallback when other stat fields match, but Windows lacks inodes. Johannes linked git-for-windows#5132 showing this affects real users, not just tests.

Show 11 quoted lines
>
> >   - 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/

This was speculation on my part that doesn't hold up. The actual mechanism is straightforward: changed_files() in difftool runs update-index --really-refresh and diff-files against a temporary index. When size and mtime match, no change is detected. The Windows API details aren't relevant. I'll remove this from the explanation.

Show 10 quoted lines
>
> > 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.

Agreed completely. This fix was to make the lives of git developers easier, not its users. The fix addresses the symptom, not the cause. The difftool creates its wtindex via make_cache_entry() and the subsequent refresh/diff-files path doesn't trigger content comparison when stat data matches. Anyone scripting difftool with modifications that preserve file size could hit this silently.

Show 112 quoted lines
>
> 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
>
>
Previous: Phillip WoodNext: Johannes Schindelin
Message 6 of 11 in “t7800: fix racy "difftool --dir-diff syncs worktree" test”
  1. t7800: fix racy "difftool --dir-diff syncs worktree" testPaul Tarjan via GitGitGadget, Dec 31, 2025
  2. Johannes SchindelinJan 1, 2026
  3. t7800: fix racy "difftool --dir-diff syncs worktree" testPaul Tarjan via GitGitGadget, Jan 1, 2026
  4. Junio C HamanoJan 1, 2026
  5. Phillip WoodJan 3, 2026
  6. Paul TarjanJan 3, 2026
  7. Johannes SchindelinJan 3, 2026
  8. Junio C HamanoJan 4, 2026
  9. t7800: fix racy "difftool --dir-diff syncs worktree" testPaul Tarjan via GitGitGadget, Jan 3, 2026
  10. Junio C HamanoJan 4, 2026
  11. Phillip WoodJan 5, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.