patchdiff --no-index: fix -R with file/directory conflicts

4 messages between Sep 18, 2026 and Sep 21, 2026, from Haokai Ding via GitGitGadget, René Scharfe, Jacob Keller, Junio C Hamano.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Haokai Ding via GitGitGadgetSep 18, 2026, 07:19 UTC on lore

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(+)

Show changes to 2 files +20 −0

diff-no-index.c, t/t4053-diff-no-index.sh

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

René ScharfeSep 20, 2026, 15:23 UTC in reply to Haokai Ding via GitGitGadget on lore

Re: [PATCH] diff --no-index: fix -R with file/directory conflicts

On 9/18/26 9:19 AM, Haokai Ding via GitGitGadget wrote:

Show 37 quoted lines

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

Show 27 quoted lines

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

Show 7 quoted lines

> +'
> +
>  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

Jacob KellerSep 20, 2026, 23:04 UTC in reply to Haokai Ding via GitGitGadget on lore

Re: [PATCH] diff --no-index: fix -R with file/directory conflicts

On 9/18/2026 12:19 AM, Haokai Ding via GitGitGadget wrote:

Show 17 quoted lines

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

Show 54 quoted lines

>     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

Junio C HamanoSep 21, 2026, 18:01 UTC in reply to Jacob Keller on lore

Re: [PATCH] diff --no-index: fix -R with file/directory conflicts

Jacob Keller <jacob.e.keller@intel.com> writes:

Show 22 quoted lines

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