* [PATCH] diff --no-index: fix -R with file/directory conflicts
@ 2026-09-18 7:19 Haokai Ding via GitGitGadget
2026-09-20 15:23 ` René Scharfe
2026-09-20 23:04 ` Jacob Keller
0 siblings, 2 replies; 4+ messages in thread
From: Haokai Ding via GitGitGadget @ 2026-09-18 7:19 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Jacob Keller, Haokai Ding, Haokai Ding
From: Haokai Ding <hk_ding@outlook.com>
When a path is a file on one side and a directory on the other,
queue_diff() queues the file separately before recursing into the
directory. This early queue entry bypasses the reverse_diff handling
used for ordinary files.
As a result, comparing directories d and e where d/sub is a file and
e/sub/file is another file reports both paths as deleted with -R.
Reversing the operands reports both paths as added instead.
Swap the filespecs of the early queue entry when reverse_diff is set,
so that -R reverses the file change as well as the directory contents.
Add regression tests for both directions.
Signed-off-by: Haokai Ding <hk_ding@outlook.com>
---
diff --no-index: fix -R with file/directory conflicts
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2232%2FHaokaiDing%2Ffix%2Fno-index-reverse-file-directory-mail-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2232/HaokaiDing/fix/no-index-reverse-file-directory-mail-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2232
diff-no-index.c | 2 ++
t/t4053-diff-no-index.sh | 18 ++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/diff-no-index.c b/diff-no-index.c
index f320424f05..a9151cc27a 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -188,6 +188,8 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,
mode1 = 0;
}
/* emit that file */
+ if (o->flags.reverse_diff)
+ SWAP(d1, d2);
diff_queue(&diff_queued_diff, d1, d2);
/* and then let the entire directory be created or deleted */
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 8e0394cf5a..c3c703f4f4 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -120,6 +120,24 @@ test_expect_success 'turning a file into a directory' '
)
'
+test_expect_success 'reverse diff when turning a file into a directory' '
+ (
+ cd non/git &&
+ printf "A\td/sub\nD\te/sub/file\n" >expect &&
+ test_expect_code 1 git diff --no-index -R --name-status d e >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'reverse diff when turning a directory into a file' '
+ (
+ cd non/git &&
+ printf "D\td/sub\nA\te/sub/file\n" >expect &&
+ test_expect_code 1 git diff --no-index -R --name-status e d >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'diff from repo subdir shows real paths (explicit)' '
echo "diff --git a/../../non/git/a b/../../non/git/b" >expect &&
test_expect_code 1 \
base-commit: d38352cd43ab9745686d697872408bc3249a153f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] diff --no-index: fix -R with file/directory conflicts
2026-09-18 7:19 [PATCH] diff --no-index: fix -R with file/directory conflicts Haokai Ding via GitGitGadget
@ 2026-09-20 15:23 ` René Scharfe
2026-09-20 23:04 ` Jacob Keller
1 sibling, 0 replies; 4+ messages in thread
From: René Scharfe @ 2026-09-20 15:23 UTC (permalink / raw)
To: Haokai Ding via GitGitGadget, git; +Cc: Jacob Keller, Haokai Ding
On 9/18/26 9:19 AM, Haokai Ding via GitGitGadget wrote:
> From: Haokai Ding <hk_ding@outlook.com>
>
> When a path is a file on one side and a directory on the other,
> queue_diff() queues the file separately before recursing into the
> directory. This early queue entry bypasses the reverse_diff handling
> used for ordinary files.
>
> As a result, comparing directories d and e where d/sub is a file and
> e/sub/file is another file reports both paths as deleted with -R.
> Reversing the operands reports both paths as added instead.
>
> Swap the filespecs of the early queue entry when reverse_diff is set,
> so that -R reverses the file change as well as the directory contents.
> Add regression tests for both directions.
>
> Signed-off-by: Haokai Ding <hk_ding@outlook.com>
> ---
> diff --no-index: fix -R with file/directory conflicts
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2232%2FHaokaiDing%2Ffix%2Fno-index-reverse-file-directory-mail-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2232/HaokaiDing/fix/no-index-reverse-file-directory-mail-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2232
>
> diff-no-index.c | 2 ++
> t/t4053-diff-no-index.sh | 18 ++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index f320424f05..a9151cc27a 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -188,6 +188,8 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,
> mode1 = 0;
> }
> /* emit that file */
> + if (o->flags.reverse_diff)
> + SWAP(d1, d2);
Good find!
> diff_queue(&diff_queued_diff, d1, d2);
>
> /* and then let the entire directory be created or deleted */
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 8e0394cf5a..c3c703f4f4 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -120,6 +120,24 @@ test_expect_success 'turning a file into a directory' '
> )
> '
>
> +test_expect_success 'reverse diff when turning a file into a directory' '
> + (
> + cd non/git &&
> + printf "A\td/sub\nD\te/sub/file\n" >expect &&
> + test_expect_code 1 git diff --no-index -R --name-status d e >actual &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success 'reverse diff when turning a directory into a file' '
> + (
> + cd non/git &&
> + printf "D\td/sub\nA\te/sub/file\n" >expect &&
> + test_expect_code 1 git diff --no-index -R --name-status e d >actual &&
> + test_cmp expect actual
> + )
Looks good. I like the explicit exit code expectation.
> +'
> +
> test_expect_success 'diff from repo subdir shows real paths (explicit)' '
> echo "diff --git a/../../non/git/a b/../../non/git/b" >expect &&
> test_expect_code 1 \
>
> base-commit: d38352cd43ab9745686d697872408bc3249a153f
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] diff --no-index: fix -R with file/directory conflicts
2026-09-18 7:19 [PATCH] diff --no-index: fix -R with file/directory conflicts Haokai Ding via GitGitGadget
2026-09-20 15:23 ` René Scharfe
@ 2026-09-20 23:04 ` Jacob Keller
2026-09-21 18:01 ` Junio C Hamano
1 sibling, 1 reply; 4+ messages in thread
From: Jacob Keller @ 2026-09-20 23:04 UTC (permalink / raw)
To: Haokai Ding via GitGitGadget, git; +Cc: René Scharfe, Haokai Ding
On 9/18/2026 12:19 AM, Haokai Ding via GitGitGadget wrote:
> From: Haokai Ding <hk_ding@outlook.com>
>
> When a path is a file on one side and a directory on the other,
> queue_diff() queues the file separately before recursing into the
> directory. This early queue entry bypasses the reverse_diff handling
> used for ordinary files.
>
> As a result, comparing directories d and e where d/sub is a file and
> e/sub/file is another file reports both paths as deleted with -R.
> Reversing the operands reports both paths as added instead.
>
> Swap the filespecs of the early queue entry when reverse_diff is set,
> so that -R reverses the file change as well as the directory contents.
> Add regression tests for both directions.
>
> Signed-off-by: Haokai Ding <hk_ding@outlook.com>
> ---
Thanks for fixing!
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> diff --no-index: fix -R with file/directory conflicts
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2232%2FHaokaiDing%2Ffix%2Fno-index-reverse-file-directory-mail-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2232/HaokaiDing/fix/no-index-reverse-file-directory-mail-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2232
>
> diff-no-index.c | 2 ++
> t/t4053-diff-no-index.sh | 18 ++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index f320424f05..a9151cc27a 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -188,6 +188,8 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,
> mode1 = 0;
> }
> /* emit that file */
> + if (o->flags.reverse_diff)
> + SWAP(d1, d2);
> diff_queue(&diff_queued_diff, d1, d2);
>
> /* and then let the entire directory be created or deleted */
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 8e0394cf5a..c3c703f4f4 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -120,6 +120,24 @@ test_expect_success 'turning a file into a directory' '
> )
> '
>
> +test_expect_success 'reverse diff when turning a file into a directory' '
> + (
> + cd non/git &&
> + printf "A\td/sub\nD\te/sub/file\n" >expect &&
> + test_expect_code 1 git diff --no-index -R --name-status d e >actual &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success 'reverse diff when turning a directory into a file' '
> + (
> + cd non/git &&
> + printf "D\td/sub\nA\te/sub/file\n" >expect &&
> + test_expect_code 1 git diff --no-index -R --name-status e d >actual &&
> + test_cmp expect actual
> + )
> +'
> +
> test_expect_success 'diff from repo subdir shows real paths (explicit)' '
> echo "diff --git a/../../non/git/a b/../../non/git/b" >expect &&
> test_expect_code 1 \
>
> base-commit: d38352cd43ab9745686d697872408bc3249a153f
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] diff --no-index: fix -R with file/directory conflicts
2026-09-20 23:04 ` Jacob Keller
@ 2026-09-21 18:01 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2026-09-21 18:01 UTC (permalink / raw)
To: Jacob Keller
Cc: Haokai Ding via GitGitGadget, git, René Scharfe, Haokai Ding
Jacob Keller <jacob.e.keller@intel.com> writes:
> On 9/18/2026 12:19 AM, Haokai Ding via GitGitGadget wrote:
>> From: Haokai Ding <hk_ding@outlook.com>
>>
>> When a path is a file on one side and a directory on the other,
>> queue_diff() queues the file separately before recursing into the
>> directory. This early queue entry bypasses the reverse_diff handling
>> used for ordinary files.
>>
>> As a result, comparing directories d and e where d/sub is a file and
>> e/sub/file is another file reports both paths as deleted with -R.
>> Reversing the operands reports both paths as added instead.
>>
>> Swap the filespecs of the early queue entry when reverse_diff is set,
>> so that -R reverses the file change as well as the directory contents.
>> Add regression tests for both directions.
>>
>> Signed-off-by: Haokai Ding <hk_ding@outlook.com>
>> ---
>
> Thanks for fixing!
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Thanks, both. Will queue.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-21 18:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-18 7:19 [PATCH] diff --no-index: fix -R with file/directory conflicts Haokai Ding via GitGitGadget
2026-09-20 15:23 ` René Scharfe
2026-09-20 23:04 ` Jacob Keller
2026-09-21 18:01 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox