# [PATCH] diff: bugfix: binary file permission regression 4 messages from 2010-06-10 to 2010-06-11. Participants: Nazri Ramliy, Christian Couder. Thread: https://gitlist.dev/t/24076 ## Nazri Ramliy, 2010-06-10 18:31 Subject: [PATCH] diff: bugfix: binary file permission regression Message-ID: <1276194685-28098-1-git-send-email-ayiehere@gmail.com> URL: https://gitlist.dev/e/1276194685-28098-1-git-send-email-ayiehere%40gmail.com ``` Regression at 3e97c7c6af2901cec63bf35fcd43ae3472e24af8 "No diff -b/-w output for all-whitespace changes" When a binary file's permission is modified, git status reports the file as modified, but git diff shows nothing. Add a test file too. --- diff.c | 4 ++-- t/t4043-diff-binary-permission.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100755 t/t4043-diff-binary-permission.sh diff --git a/diff.c b/diff.c index 494f560..0aa1a2d 100644 --- a/diff.c +++ b/diff.c @@ -1745,12 +1745,12 @@ static void builtin_diff(const char *name_a, (!textconv_two && diff_filespec_is_binary(two)) )) { if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) die("unable to read files to diff"); + fprintf(o->file, "%s", header.buf); + strbuf_reset(&header); /* Quite common confusing case */ if (mf1.size == mf2.size && !memcmp(mf1.ptr, mf2.ptr, mf1.size)) goto free_ab_and_return; - fprintf(o->file, "%s", header.buf); - strbuf_reset(&header); if (DIFF_OPT_TST(o, BINARY)) emit_binary_diff(o->file, &mf1, &mf2); else diff --git a/t/t4043-diff-binary-permission.sh b/t/t4043-diff-binary-permission.sh new file mode 100755 index 0000000..7b77fb3 --- /dev/null +++ b/t/t4043-diff-binary-permission.sh @@ -0,0 +1,26 @@ +#!/bin/sh +# +# Copyright (c) 2010 Nazri Ramliy +# + +test_description='Test permission change on binary files + +' +. ./test-lib.sh + +/bin/echo -e "\0\0\0\0" > a.bin +chmod 644 a.bin +git update-index --add a.bin +chmod 755 a.bin + +cat << EOF > expect +diff --git a/a.bin b/a.bin +old mode 100644 +new mode 100755 +EOF + +git diff > out + +test_expect_success 'diff-binary-permission' 'test_cmp expect out' + +test_done -- 1.7.1.245.g7c42e.dirty ``` ## Nazri Ramliy, 2010-06-11 07:06 Subject: Re: [PATCH] diff: bugfix: binary file permission regression Message-ID: URL: https://gitlist.dev/e/AANLkTimwmkMnaqMY44SeHz1L8hE2Lp324PXPY4eqvTGb%40mail.gmail.com In-Reply-To: <1276194685-28098-1-git-send-email-ayiehere@gmail.com> ``` On Fri, Jun 11, 2010 at 2:31 AM, Nazri Ramliy wrote: > (!textconv_two && diff_filespec_is_binary(two)) )) { > if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) > die("unable to read files to diff"); > + fprintf(o->file, "%s", header.buf); > + strbuf_reset(&header); Since the fill_mmfile()s could result in a die maybe it's better if the header is printed before the read attempt?: (!textconv_two && diff_filespec_is_binary(two)) )) { + fprintf(o->file, "%s", header.buf); + strbuf_reset(&header); if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) die("unable to read files to diff"); I did this on my work tree and ran 'make' in the test directory and no errors were reported. nazri. ``` ## Christian Couder, 2010-06-11 07:24 Subject: Re: [PATCH] diff: bugfix: binary file permission regression Message-ID: URL: https://gitlist.dev/e/AANLkTikWNaEY5aErPF7OkBMleN_hiFRholfdFXLF1cJO%40mail.gmail.com In-Reply-To: ``` On Fri, Jun 11, 2010 at 9:06 AM, Nazri Ramliy wrote: > On Fri, Jun 11, 2010 at 2:31 AM, Nazri Ramliy wrote: >>              (!textconv_two && diff_filespec_is_binary(two)) )) { >>                if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) >>                        die("unable to read files to diff"); >> +               fprintf(o->file, "%s", header.buf); >> +               strbuf_reset(&header); > >  Since the fill_mmfile()s could result in a die maybe it's >  better if the header is printed before the read attempt?: > >              (!textconv_two && diff_filespec_is_binary(two)) )) { > +               fprintf(o->file, "%s", header.buf); > +               strbuf_reset(&header); >                if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) >                        die("unable to read files to diff"); > > I did this on my work tree and ran 'make' in the test directory and no errors > were reported. Hi, Please have a look at this thread: http://thread.gmane.org/gmane.comp.version-control.git/147732/ The patch resulting from the thread is currently in next and pu. Thanks, Christian. ``` ## Nazri Ramliy, 2010-06-11 09:45 Subject: Re: [PATCH] diff: bugfix: binary file permission regression Message-ID: URL: https://gitlist.dev/e/AANLkTikTyEjheFwI0x3svV8SWMkhksE6q2aspI2rww-l%40mail.gmail.com In-Reply-To: ``` On Fri, Jun 11, 2010 at 3:24 PM, Christian Couder wrote: > Please have a look at this thread: > > http://thread.gmane.org/gmane.comp.version-control.git/147732/ > > The patch resulting from the thread is currently in next and pu. Thank you for the heads-up. nazri. Note to self: google before bugging list. ```