{"thread":{"id":"24076","subject":"[PATCH] diff: bugfix: binary file permission regression","startedAt":"2010-06-10T18:31:25Z","lastAt":"2010-06-11T09:45:30Z","messageCount":4,"participants":["Nazri Ramliy","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"143467","messageId":"1276194685-28098-1-git-send-email-ayiehere@gmail.com","threadId":"24076","inReplyTo":null,"subject":"[PATCH] diff: bugfix: binary file permission regression","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-06-10T18:31:25Z","receivedAt":"2010-06-10T18:31:25Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"Regression at 3e97c7c6af2901cec63bf35fcd43ae3472e24af8\n\"No diff -b/-w output for all-whitespace changes\"\n\nWhen a binary file's permission is modified, git status\nreports the file as modified, but git diff shows nothing.\n\nAdd a test file too.\n---\n diff.c | 4 ++--\n t/t4043-diff-binary-permission.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 28 insertions(+), 2 deletions(-)\n create mode 100755 t/t4043-diff-binary-permission.sh\n\ndiff --git a/diff.c b/diff.c\nindex 494f560..0aa1a2d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1745,12 +1745,12 @@ static void builtin_diff(const char *name_a,\n \t (!textconv_two && diff_filespec_is_binary(two)) )) {\n \t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\t\tdie(\"unable to read files to diff\");\n+\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\tstrbuf_reset(&header);\n \t\t/* Quite common confusing case */\n \t\tif (mf1.size == mf2.size &&\n \t\t !memcmp(mf1.ptr, mf2.ptr, mf1.size))\n \t\t\tgoto free_ab_and_return;\n-\t\tfprintf(o->file, \"%s\", header.buf);\n-\t\tstrbuf_reset(&header);\n \t\tif (DIFF_OPT_TST(o, BINARY))\n \t\t\temit_binary_diff(o->file, &mf1, &mf2);\n \t\telse\ndiff --git a/t/t4043-diff-binary-permission.sh b/t/t4043-diff-binary-permission.sh\nnew file mode 100755\nindex 0000000..7b77fb3\n--- /dev/null\n+++ b/t/t4043-diff-binary-permission.sh\n@@ -0,0 +1,26 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2010 Nazri Ramliy\n+#\n+\n+test_description='Test permission change on binary files\n+\n+'\n+. ./test-lib.sh\n+\n+/bin/echo -e \"\\0\\0\\0\\0\" > a.bin\n+chmod 644 a.bin\n+git update-index --add a.bin\n+chmod 755 a.bin\n+\n+cat << EOF > expect\n+diff --git a/a.bin b/a.bin\n+old mode 100644\n+new mode 100755\n+EOF\n+\n+git diff > out\n+\n+test_expect_success 'diff-binary-permission' 'test_cmp expect out'\n+\n+test_done\n-- \n1.7.1.245.g7c42e.dirty\n"},{"id":"143487","messageId":"AANLkTimwmkMnaqMY44SeHz1L8hE2Lp324PXPY4eqvTGb@mail.gmail.com","threadId":"24076","inReplyTo":"1276194685-28098-1-git-send-email-ayiehere@gmail.com","subject":"Re: [PATCH] diff: bugfix: binary file permission regression","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-06-11T07:06:02Z","receivedAt":"2010-06-11T07:06:02Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Fri, Jun 11, 2010 at 2:31 AM, Nazri Ramliy wrote:\n> (!textconv_two && diff_filespec_is_binary(two)) )) {\n> if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n> die(\"unable to read files to diff\");\n> + fprintf(o->file, \"%s\", header.buf);\n> + strbuf_reset(&header);\n\n Since the fill_mmfile()s could result in a die maybe it's\n better if the header is printed before the read attempt?:\n\n (!textconv_two && diff_filespec_is_binary(two)) )) {\n+ fprintf(o->file, \"%s\", header.buf);\n+ strbuf_reset(&header);\n if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n die(\"unable to read files to diff\");\n\nI did this on my work tree and ran 'make' in the test directory and no errors\nwere reported.\n\nnazri.\n"},{"id":"143489","messageId":"AANLkTikWNaEY5aErPF7OkBMleN_hiFRholfdFXLF1cJO@mail.gmail.com","threadId":"24076","inReplyTo":"AANLkTimwmkMnaqMY44SeHz1L8hE2Lp324PXPY4eqvTGb@mail.gmail.com","subject":"Re: [PATCH] diff: bugfix: binary file permission regression","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2010-06-11T07:24:37Z","receivedAt":"2010-06-11T07:24:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jun 11, 2010 at 9:06 AM, Nazri Ramliy wrote:\n> On Fri, Jun 11, 2010 at 2:31 AM, Nazri Ramliy wrote:\n>>              (!textconv_two && diff_filespec_is_binary(two)) )) {\n>>                if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n>>                        die(\"unable to read files to diff\");\n>> +               fprintf(o->file, \"%s\", header.buf);\n>> +               strbuf_reset(&header);\n>\n>  Since the fill_mmfile()s could result in a die maybe it's\n>  better if the header is printed before the read attempt?:\n>\n>              (!textconv_two && diff_filespec_is_binary(two)) )) {\n> +               fprintf(o->file, \"%s\", header.buf);\n> +               strbuf_reset(&header);\n>                if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n>                        die(\"unable to read files to diff\");\n>\n> I did this on my work tree and ran 'make' in the test directory and no errors\n> were reported.\n\nHi,\n\nPlease have a look at this thread:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/147732/\n\nThe patch resulting from the thread is currently in next and pu.\n\nThanks,\nChristian.\n"},{"id":"143494","messageId":"AANLkTikTyEjheFwI0x3svV8SWMkhksE6q2aspI2rww-l@mail.gmail.com","threadId":"24076","inReplyTo":"AANLkTikWNaEY5aErPF7OkBMleN_hiFRholfdFXLF1cJO@mail.gmail.com","subject":"Re: [PATCH] diff: bugfix: binary file permission regression","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-06-11T09:45:30Z","receivedAt":"2010-06-11T09:45:30Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Fri, Jun 11, 2010 at 3:24 PM, Christian Couder\n wrote:\n> Please have a look at this thread:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/147732/\n>\n> The patch resulting from the thread is currently in next and pu.\n\nThank you for the heads-up.\n\nnazri.\n\nNote to self: google before bugging list.\n"}]}