* Regression in git diff with stdin, with -R
@ 2023-09-09 20:42 Martin Storsjö
2023-09-09 22:12 ` [PATCH] diff --no-index: fix -R with stdin René Scharfe
0 siblings, 1 reply; 6+ messages in thread
From: Martin Storsjö @ 2023-09-09 20:42 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
Hi,
Since 1e3f26542a6ecd3006c2c0d5ccc0bae4a700f7e5, "diff --no-index: support
reading from named pipes", one usecase about diffing with stdin has
broken.
I see that this patch was preceded by adding some extra tests around
diffing with stdin - but one case seem to have been missed.
"git diff --no-index - regularfile" still works fine as it did before,
also "git diff --no-index regularfile -" also still works. (I.e. stdin can
either be the first or second file argument - both work.)
However if using the -R option to reverse the diff direction, i.e. "git
diff --no-index -R - regularfile" or "git diff --no-index -R regularfile
-", I'm now getting the following error:
fatal: stat '-': No such file or directory
// Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] diff --no-index: fix -R with stdin
2023-09-09 20:42 Regression in git diff with stdin, with -R Martin Storsjö
@ 2023-09-09 22:12 ` René Scharfe
2023-09-10 10:00 ` Phillip Wood
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: René Scharfe @ 2023-09-09 22:12 UTC (permalink / raw)
To: Martin Storsjö, git; +Cc: Phillip Wood, Junio C Hamano
When -R is given, queue_diff() swaps the mode and name variables of the
two files to produce a reverse diff. 1e3f26542a (diff --no-index:
support reading from named pipes, 2023-07-05) added variables that
indicate whether files are special, i.e named pipes or - for stdin.
These new variables were not swapped, though, which broke the handling
of stdin with with -R. Swap them like the other metadata variables.
Reported-by: Martin Storsjö <martin@martin.st>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Great bug report, thank you!
diff-no-index.c | 1 +
t/t4053-diff-no-index.sh | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/diff-no-index.c b/diff-no-index.c
index 8aead3e332..e7041b89e3 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,
if (o->flags.reverse_diff) {
SWAP(mode1, mode2);
SWAP(name1, name2);
+ SWAP(special1, special2);
}
d1 = noindex_filespec(name1, mode1, special1);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 6781cc9078..5f059f65fc 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -224,6 +224,25 @@ test_expect_success "diff --no-index treats '-' as stdin" '
test_must_be_empty actual
'
+test_expect_success "diff --no-index -R treats '-' as stdin" '
+ cat >expect <<-EOF &&
+ diff --git b/a/1 a/-
+ index $(git hash-object --stdin <a/1)..$ZERO_OID 100644
+ --- b/a/1
+ +++ a/-
+ @@ -1 +1 @@
+ -1
+ +x
+ EOF
+
+ test_write_lines x | test_expect_code 1 \
+ git -c core.abbrev=no diff --no-index -R -- - a/1 >actual &&
+ test_cmp expect actual &&
+
+ test_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&
+ test_must_be_empty actual
+'
+
test_expect_success 'diff --no-index refuses to diff stdin and a directory' '
test_must_fail git diff --no-index -- - a </dev/null 2>err &&
grep "fatal: cannot compare stdin to a directory" err
--
2.42.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] diff --no-index: fix -R with stdin
2023-09-09 22:12 ` [PATCH] diff --no-index: fix -R with stdin René Scharfe
@ 2023-09-10 10:00 ` Phillip Wood
2023-09-10 18:41 ` Taylor Blau
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Phillip Wood @ 2023-09-10 10:00 UTC (permalink / raw)
To: René Scharfe, Martin Storsjö, git; +Cc: Phillip Wood, Junio C Hamano
On 09/09/2023 23:12, René Scharfe wrote:
> When -R is given, queue_diff() swaps the mode and name variables of the
> two files to produce a reverse diff. 1e3f26542a (diff --no-index:
> support reading from named pipes, 2023-07-05) added variables that
> indicate whether files are special, i.e named pipes or - for stdin.
> These new variables were not swapped, though, which broke the handling
> of stdin with with -R. Swap them like the other metadata variables.
>
> Reported-by: Martin Storsjö <martin@martin.st>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Great bug report, thank you!
Yes thank you Martin for reporting this and thank you to René for fixing
it - I saw the report just before I went to bed and it was already fixed
by the time I got up! The patch looks good and thanks for adding a test.
Best Wishes
Phillip
> diff-no-index.c | 1 +
> t/t4053-diff-no-index.sh | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 8aead3e332..e7041b89e3 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,
> if (o->flags.reverse_diff) {
> SWAP(mode1, mode2);
> SWAP(name1, name2);
> + SWAP(special1, special2);
> }
>
> d1 = noindex_filespec(name1, mode1, special1);
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 6781cc9078..5f059f65fc 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -224,6 +224,25 @@ test_expect_success "diff --no-index treats '-' as stdin" '
> test_must_be_empty actual
> '
>
> +test_expect_success "diff --no-index -R treats '-' as stdin" '
> + cat >expect <<-EOF &&
> + diff --git b/a/1 a/-
> + index $(git hash-object --stdin <a/1)..$ZERO_OID 100644
> + --- b/a/1
> + +++ a/-
> + @@ -1 +1 @@
> + -1
> + +x
> + EOF
> +
> + test_write_lines x | test_expect_code 1 \
> + git -c core.abbrev=no diff --no-index -R -- - a/1 >actual &&
> + test_cmp expect actual &&
> +
> + test_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&
> + test_must_be_empty actual
> +'
> +
> test_expect_success 'diff --no-index refuses to diff stdin and a directory' '
> test_must_fail git diff --no-index -- - a </dev/null 2>err &&
> grep "fatal: cannot compare stdin to a directory" err
> --
> 2.42.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] diff --no-index: fix -R with stdin
2023-09-09 22:12 ` [PATCH] diff --no-index: fix -R with stdin René Scharfe
2023-09-10 10:00 ` Phillip Wood
@ 2023-09-10 18:41 ` Taylor Blau
2023-09-11 19:04 ` Junio C Hamano
2023-09-11 19:20 ` Martin Storsjö
3 siblings, 0 replies; 6+ messages in thread
From: Taylor Blau @ 2023-09-10 18:41 UTC (permalink / raw)
To: René Scharfe; +Cc: Martin Storsjö, git, Phillip Wood, Junio C Hamano
On Sun, Sep 10, 2023 at 12:12:52AM +0200, René Scharfe wrote:
> When -R is given, queue_diff() swaps the mode and name variables of the
> two files to produce a reverse diff. 1e3f26542a (diff --no-index:
> support reading from named pipes, 2023-07-05) added variables that
> indicate whether files are special, i.e named pipes or - for stdin.
> These new variables were not swapped, though, which broke the handling
> of stdin with with -R. Swap them like the other metadata variables.
>
> Reported-by: Martin Storsjö <martin@martin.st>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Great bug report, thank you!
Great patch ;-). LGTM!
Thanks,
Taylor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] diff --no-index: fix -R with stdin
2023-09-09 22:12 ` [PATCH] diff --no-index: fix -R with stdin René Scharfe
2023-09-10 10:00 ` Phillip Wood
2023-09-10 18:41 ` Taylor Blau
@ 2023-09-11 19:04 ` Junio C Hamano
2023-09-11 19:20 ` Martin Storsjö
3 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2023-09-11 19:04 UTC (permalink / raw)
To: René Scharfe; +Cc: Martin Storsjö, git, Phillip Wood
René Scharfe <l.s.r@web.de> writes:
> When -R is given, queue_diff() swaps the mode and name variables of the
> two files to produce a reverse diff. 1e3f26542a (diff --no-index:
> support reading from named pipes, 2023-07-05) added variables that
> indicate whether files are special, i.e named pipes or - for stdin.
> These new variables were not swapped, though, which broke the handling
> of stdin with with -R. Swap them like the other metadata variables.
>
> Reported-by: Martin Storsjö <martin@martin.st>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Great bug report, thank you!
Looking good. I wish all our bugs are this easy and obvious ;-)
> diff-no-index.c | 1 +
> t/t4053-diff-no-index.sh | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 8aead3e332..e7041b89e3 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,
> if (o->flags.reverse_diff) {
> SWAP(mode1, mode2);
> SWAP(name1, name2);
> + SWAP(special1, special2);
> }
>
> d1 = noindex_filespec(name1, mode1, special1);
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 6781cc9078..5f059f65fc 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -224,6 +224,25 @@ test_expect_success "diff --no-index treats '-' as stdin" '
> test_must_be_empty actual
> '
>
> +test_expect_success "diff --no-index -R treats '-' as stdin" '
> + cat >expect <<-EOF &&
> + diff --git b/a/1 a/-
> + index $(git hash-object --stdin <a/1)..$ZERO_OID 100644
> + --- b/a/1
> + +++ a/-
> + @@ -1 +1 @@
> + -1
> + +x
> + EOF
> +
> + test_write_lines x | test_expect_code 1 \
> + git -c core.abbrev=no diff --no-index -R -- - a/1 >actual &&
> + test_cmp expect actual &&
> +
> + test_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&
> + test_must_be_empty actual
> +'
> +
> test_expect_success 'diff --no-index refuses to diff stdin and a directory' '
> test_must_fail git diff --no-index -- - a </dev/null 2>err &&
> grep "fatal: cannot compare stdin to a directory" err
> --
> 2.42.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] diff --no-index: fix -R with stdin
2023-09-09 22:12 ` [PATCH] diff --no-index: fix -R with stdin René Scharfe
` (2 preceding siblings ...)
2023-09-11 19:04 ` Junio C Hamano
@ 2023-09-11 19:20 ` Martin Storsjö
3 siblings, 0 replies; 6+ messages in thread
From: Martin Storsjö @ 2023-09-11 19:20 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Phillip Wood, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 665 bytes --]
On Sun, 10 Sep 2023, René Scharfe wrote:
> When -R is given, queue_diff() swaps the mode and name variables of the
> two files to produce a reverse diff. 1e3f26542a (diff --no-index:
> support reading from named pipes, 2023-07-05) added variables that
> indicate whether files are special, i.e named pipes or - for stdin.
> These new variables were not swapped, though, which broke the handling
> of stdin with with -R. Swap them like the other metadata variables.
>
> Reported-by: Martin Storsjö <martin@martin.st>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Great bug report, thank you!
Thanks for the extremely swift response and fix!
// Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-11 21:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-09 20:42 Regression in git diff with stdin, with -R Martin Storsjö
2023-09-09 22:12 ` [PATCH] diff --no-index: fix -R with stdin René Scharfe
2023-09-10 10:00 ` Phillip Wood
2023-09-10 18:41 ` Taylor Blau
2023-09-11 19:04 ` Junio C Hamano
2023-09-11 19:20 ` Martin Storsjö
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).