{"thread":{"id":"60215","subject":"Regression in git diff with stdin, with -R","startedAt":"2023-09-09T20:58:03Z","lastAt":"2023-09-11T21:38:59Z","messageCount":6,"participants":["Martin Storsjö","René Scharfe","Phillip Wood","Taylor Blau","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"481630","messageId":"d42579a0-f438-9b4c-97e4-58724dbe4a4@martin.st","threadId":"60215","inReplyTo":null,"subject":"Regression in git diff with stdin, with -R","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2023-09-09T20:42:41Z","receivedAt":"2023-09-09T20:58:03Z","isPatch":false,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"Hi,\n\nSince 1e3f26542a6ecd3006c2c0d5ccc0bae4a700f7e5, \"diff --no-index: support \nreading from named pipes\", one usecase about diffing with stdin has \nbroken.\n\nI see that this patch was preceded by adding some extra tests around \ndiffing with stdin - but one case seem to have been missed.\n\n\"git diff --no-index - regularfile\" still works fine as it did before, \nalso \"git diff --no-index regularfile -\" also still works. (I.e. stdin can \neither be the first or second file argument - both work.)\n\nHowever if using the -R option to reverse the diff direction, i.e. \"git \ndiff --no-index -R - regularfile\" or \"git diff --no-index -R regularfile \n-\", I'm now getting the following error:\n\n fatal: stat '-': No such file or directory\n\n// Martin\n\n"},{"id":"481634","messageId":"22fdfa3b-f90e-afcc-667c-705fb7670245@web.de","threadId":"60215","inReplyTo":"d42579a0-f438-9b4c-97e4-58724dbe4a4@martin.st","subject":"[PATCH] diff --no-index: fix -R with stdin","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-09-09T22:12:52Z","receivedAt":"2023-09-09T22:13:09Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"When -R is given, queue_diff() swaps the mode and name variables of the\ntwo files to produce a reverse diff. 1e3f26542a (diff --no-index:\nsupport reading from named pipes, 2023-07-05) added variables that\nindicate whether files are special, i.e named pipes or - for stdin.\nThese new variables were not swapped, though, which broke the handling\nof stdin with with -R. Swap them like the other metadata variables.\n\nReported-by: Martin Storsjö \nSigned-off-by: René Scharfe \n---\nGreat bug report, thank you!\n\n diff-no-index.c | 1 +\n t/t4053-diff-no-index.sh | 19 +++++++++++++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 8aead3e332..e7041b89e3 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,\n \t\tif (o->flags.reverse_diff) {\n \t\t\tSWAP(mode1, mode2);\n \t\t\tSWAP(name1, name2);\n+\t\t\tSWAP(special1, special2);\n \t\t}\n\n \t\td1 = noindex_filespec(name1, mode1, special1);\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 6781cc9078..5f059f65fc 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -224,6 +224,25 @@ test_expect_success \"diff --no-index treats '-' as stdin\" '\n \ttest_must_be_empty actual\n '\n\n+test_expect_success \"diff --no-index -R treats '-' as stdin\" '\n+\tcat >expect <<-EOF &&\n+\tdiff --git b/a/1 a/-\n+\tindex $(git hash-object --stdin actual &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'diff --no-index refuses to diff stdin and a directory' '\n \ttest_must_fail git diff --no-index -- - a err &&\n \tgrep \"fatal: cannot compare stdin to a directory\" err\n--\n2.42.0\n"},{"id":"481648","messageId":"fe550a42-4d78-4e57-a0bc-cb558e8ac1ab@gmail.com","threadId":"60215","inReplyTo":"22fdfa3b-f90e-afcc-667c-705fb7670245@web.de","subject":"Re: [PATCH] diff --no-index: fix -R with stdin","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-10T10:00:17Z","receivedAt":"2023-09-10T10:05:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 09/09/2023 23:12, René Scharfe wrote:\n> When -R is given, queue_diff() swaps the mode and name variables of the\n> two files to produce a reverse diff. 1e3f26542a (diff --no-index:\n> support reading from named pipes, 2023-07-05) added variables that\n> indicate whether files are special, i.e named pipes or - for stdin.\n> These new variables were not swapped, though, which broke the handling\n> of stdin with with -R. Swap them like the other metadata variables.\n> \n> Reported-by: Martin Storsjö \n> Signed-off-by: René Scharfe \n> ---\n> Great bug report, thank you!\n\nYes thank you Martin for reporting this and thank you to René for fixing \nit - I saw the report just before I went to bed and it was already fixed \nby the time I got up! The patch looks good and thanks for adding a test.\n\nBest Wishes\n\nPhillip\n\n> diff-no-index.c | 1 +\n> t/t4053-diff-no-index.sh | 19 +++++++++++++++++++\n> 2 files changed, 20 insertions(+)\n> \n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 8aead3e332..e7041b89e3 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,\n> \t\tif (o->flags.reverse_diff) {\n> \t\t\tSWAP(mode1, mode2);\n> \t\t\tSWAP(name1, name2);\n> +\t\t\tSWAP(special1, special2);\n> \t\t}\n> \n> \t\td1 = noindex_filespec(name1, mode1, special1);\n> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\n> index 6781cc9078..5f059f65fc 100755\n> --- a/t/t4053-diff-no-index.sh\n> +++ b/t/t4053-diff-no-index.sh\n> @@ -224,6 +224,25 @@ test_expect_success \"diff --no-index treats '-' as stdin\" '\n> \ttest_must_be_empty actual\n> '\n> \n> +test_expect_success \"diff --no-index -R treats '-' as stdin\" '\n> +\tcat >expect <<-EOF &&\n> +\tdiff --git b/a/1 a/-\n> +\tindex $(git hash-object --stdin +\t--- b/a/1\n> +\t+++ a/-\n> +\t@@ -1 +1 @@\n> +\t-1\n> +\t+x\n> +\tEOF\n> +\n> +\ttest_write_lines x | test_expect_code 1 \\\n> +\t\tgit -c core.abbrev=no diff --no-index -R -- - a/1 >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\ttest_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&\n> +\ttest_must_be_empty actual\n> +'\n> +\n> test_expect_success 'diff --no-index refuses to diff stdin and a directory' '\n> \ttest_must_fail git diff --no-index -- - a err &&\n> \tgrep \"fatal: cannot compare stdin to a directory\" err\n> --\n> 2.42.0\n"},{"id":"481658","messageId":"ZP4N7c92ra5ZOKl8@nand.local","threadId":"60215","inReplyTo":"22fdfa3b-f90e-afcc-667c-705fb7670245@web.de","subject":"Re: [PATCH] diff --no-index: fix -R with stdin","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-09-10T18:41:49Z","receivedAt":"2023-09-10T18:41:53Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sun, Sep 10, 2023 at 12:12:52AM +0200, René Scharfe wrote:\n> When -R is given, queue_diff() swaps the mode and name variables of the\n> two files to produce a reverse diff. 1e3f26542a (diff --no-index:\n> support reading from named pipes, 2023-07-05) added variables that\n> indicate whether files are special, i.e named pipes or - for stdin.\n> These new variables were not swapped, though, which broke the handling\n> of stdin with with -R. Swap them like the other metadata variables.\n>\n> Reported-by: Martin Storsjö \n> Signed-off-by: René Scharfe \n> ---\n> Great bug report, thank you!\n\nGreat patch ;-). LGTM!\n\nThanks,\nTaylor\n"},{"id":"481674","messageId":"c9d3184f-24ac-af75-f31e-aa63c788bfb3@martin.st","threadId":"60215","inReplyTo":"22fdfa3b-f90e-afcc-667c-705fb7670245@web.de","subject":"Re: [PATCH] diff --no-index: fix -R with stdin","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2023-09-11T19:20:33Z","receivedAt":"2023-09-11T21:38:10Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Sun, 10 Sep 2023, René Scharfe wrote:\n\n> When -R is given, queue_diff() swaps the mode and name variables of the\n> two files to produce a reverse diff. 1e3f26542a (diff --no-index:\n> support reading from named pipes, 2023-07-05) added variables that\n> indicate whether files are special, i.e named pipes or - for stdin.\n> These new variables were not swapped, though, which broke the handling\n> of stdin with with -R. Swap them like the other metadata variables.\n>\n> Reported-by: Martin Storsjö \n> Signed-off-by: René Scharfe \n> ---\n> Great bug report, thank you!\n\nThanks for the extremely swift response and fix!\n\n// Martin\n"},{"id":"481688","messageId":"xmqqil8gv8e0.fsf@gitster.g","threadId":"60215","inReplyTo":"22fdfa3b-f90e-afcc-667c-705fb7670245@web.de","subject":"Re: [PATCH] diff --no-index: fix -R with stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-11T19:04:39Z","receivedAt":"2023-09-11T21:38:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> When -R is given, queue_diff() swaps the mode and name variables of the\n> two files to produce a reverse diff. 1e3f26542a (diff --no-index:\n> support reading from named pipes, 2023-07-05) added variables that\n> indicate whether files are special, i.e named pipes or - for stdin.\n> These new variables were not swapped, though, which broke the handling\n> of stdin with with -R. Swap them like the other metadata variables.\n>\n> Reported-by: Martin Storsjö \n> Signed-off-by: René Scharfe \n> ---\n> Great bug report, thank you!\n\nLooking good. I wish all our bugs are this easy and obvious ;-)\n\n> diff-no-index.c | 1 +\n> t/t4053-diff-no-index.sh | 19 +++++++++++++++++++\n> 2 files changed, 20 insertions(+)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 8aead3e332..e7041b89e3 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,\n> \t\tif (o->flags.reverse_diff) {\n> \t\t\tSWAP(mode1, mode2);\n> \t\t\tSWAP(name1, name2);\n> +\t\t\tSWAP(special1, special2);\n> \t\t}\n>\n> \t\td1 = noindex_filespec(name1, mode1, special1);\n> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\n> index 6781cc9078..5f059f65fc 100755\n> --- a/t/t4053-diff-no-index.sh\n> +++ b/t/t4053-diff-no-index.sh\n> @@ -224,6 +224,25 @@ test_expect_success \"diff --no-index treats '-' as stdin\" '\n> \ttest_must_be_empty actual\n> '\n>\n> +test_expect_success \"diff --no-index -R treats '-' as stdin\" '\n> +\tcat >expect <<-EOF &&\n> +\tdiff --git b/a/1 a/-\n> +\tindex $(git hash-object --stdin +\t--- b/a/1\n> +\t+++ a/-\n> +\t@@ -1 +1 @@\n> +\t-1\n> +\t+x\n> +\tEOF\n> +\n> +\ttest_write_lines x | test_expect_code 1 \\\n> +\t\tgit -c core.abbrev=no diff --no-index -R -- - a/1 >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\ttest_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&\n> +\ttest_must_be_empty actual\n> +'\n> +\n> test_expect_success 'diff --no-index refuses to diff stdin and a directory' '\n> \ttest_must_fail git diff --no-index -- - a err &&\n> \tgrep \"fatal: cannot compare stdin to a directory\" err\n> --\n> 2.42.0\n"}]}