# Regression in git diff with stdin, with -R 6 messages from 2023-09-09 to 2023-09-11. Participants: Martin Storsjö, René Scharfe, Phillip Wood, Taylor Blau, Junio C Hamano. Thread: https://gitlist.dev/t/60215 ## Martin Storsjö, 2023-09-09 20:42 Subject: Regression in git diff with stdin, with -R Message-ID: URL: https://gitlist.dev/e/d42579a0-f438-9b4c-97e4-58724dbe4a4%40martin.st ``` 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 ``` ## René Scharfe, 2023-09-09 22:12 Subject: [PATCH] diff --no-index: fix -R with stdin Message-ID: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de> URL: https://gitlist.dev/e/22fdfa3b-f90e-afcc-667c-705fb7670245%40web.de In-Reply-To: ``` 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ö Signed-off-by: René Scharfe --- 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 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 err && grep "fatal: cannot compare stdin to a directory" err -- 2.42.0 ``` ## Phillip Wood, 2023-09-10 10:00 Subject: Re: [PATCH] diff --no-index: fix -R with stdin Message-ID: URL: https://gitlist.dev/e/fe550a42-4d78-4e57-a0bc-cb558e8ac1ab%40gmail.com In-Reply-To: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de> ``` 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ö > Signed-off-by: René Scharfe > --- > 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 + --- 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 err && > grep "fatal: cannot compare stdin to a directory" err > -- > 2.42.0 ``` ## Taylor Blau, 2023-09-10 18:41 Subject: Re: [PATCH] diff --no-index: fix -R with stdin Message-ID: URL: https://gitlist.dev/e/ZP4N7c92ra5ZOKl8%40nand.local In-Reply-To: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de> ``` 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ö > Signed-off-by: René Scharfe > --- > Great bug report, thank you! Great patch ;-). LGTM! Thanks, Taylor ``` ## Martin Storsjö, 2023-09-11 19:20 Subject: Re: [PATCH] diff --no-index: fix -R with stdin Message-ID: URL: https://gitlist.dev/e/c9d3184f-24ac-af75-f31e-aa63c788bfb3%40martin.st In-Reply-To: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de> ``` 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ö > Signed-off-by: René Scharfe > --- > Great bug report, thank you! Thanks for the extremely swift response and fix! // Martin ``` ## Junio C Hamano, 2023-09-11 19:04 Subject: Re: [PATCH] diff --no-index: fix -R with stdin Message-ID: URL: https://gitlist.dev/e/xmqqil8gv8e0.fsf%40gitster.g In-Reply-To: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de> ``` René Scharfe 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ö > Signed-off-by: René Scharfe > --- > 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 + --- 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 err && > grep "fatal: cannot compare stdin to a directory" err > -- > 2.42.0 ```