public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Paul Tarjan via GitGitGadget" <gitgitgadget@gmail•com>
To: git@vger•kernel.org
Cc: Paul Tarjan <github@paulisageek•com>,
	Paul Tarjan <github@paulisageek•com>
Subject: [PATCH v2] t7800: fix racy "difftool --dir-diff syncs worktree" test
Date: Thu, 01 Jan 2026 18:27:48 +0000	[thread overview]
Message-ID: <pull.2149.v2.git.git.1767292068036.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2149.git.git.1767219599334.gitgitgadget@gmail.com>

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

  parent reply	other threads:[~2026-01-01 18:27 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 ` Paul Tarjan via GitGitGadget [this message]
2026-01-01 22:49   ` [PATCH v2] " 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

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=pull.2149.v2.git.git.1767292068036.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=github@paulisageek$(echo .)com \
    /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