Hi Torsten, Your patch has been superseded, but I thought I ought to answer your questions rather than leave them hanging. On Thursday 02 March 2017 at 19:17:00 +0100, Torsten Bögershausen wrote: > On 2017-03-01 22:25, Mike Crowe wrote: > > On Wednesday 01 March 2017 at 18:04:44 +0100, tboegi@xxxxxx wrote: > >> From: Junio C Hamano <gitster@xxxxxxxxx> > >> > >> git diff --quiet may take a short-cut to see if a file is changed > >> in the working tree: > >> Whenever the file size differs from what is recorded in the index, > >> the file is assumed to be changed and git diff --quiet returns > >> exit with code 1 > >> > >> This shortcut must be suppressed whenever the line endings are converted > >> or a filter is in use. > >> The attributes say "* text=auto" and a file has > >> "Hello\nWorld\n" in the index with a length of 12. > >> The file in the working tree has "Hello\r\nWorld\r\n" with a length of 14. > >> (Or even "Hello\r\nWorld\n"). > >> In this case "git add" will not do any changes to the index, and > >> "git diff -quiet" should exit 0. > >> > >> Add calls to would_convert_to_git() before blindly saying that a different > >> size means different content. > >> > >> Reported-By: Mike Crowe <mac@xxxxxxxxxx> > >> Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx> > >> --- > >> This is what I can come up with, collecting all the loose ends. > >> I'm not sure if Mike wan't to have the Reported-By with a > >> Signed-off-by ? > >> The other question is, if the commit message summarizes the discussion > >> well enough ? > >> > >> diff.c | 18 ++++++++++++++---- > >> t/t0028-diff-converted.sh | 27 +++++++++++++++++++++++++++ > >> 2 files changed, 41 insertions(+), 4 deletions(-) > >> create mode 100755 t/t0028-diff-converted.sh > >> > >> diff --git a/diff.c b/diff.c > >> index 051761b..c264758 100644 > >> --- a/diff.c > >> +++ b/diff.c > >> @@ -4921,9 +4921,10 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p) > >> * differences. > >> * > >> * 2. At this point, the file is known to be modified, > >> - * with the same mode and size, and the object > >> - * name of one side is unknown. Need to inspect > >> - * the identical contents. > >> + * with the same mode and size, the object > >> + * name of one side is unknown, or size comparison > >> + * cannot be depended upon. Need to inspect the > >> + * contents. > >> */ > >> if (!DIFF_FILE_VALID(p->one) || /* (1) */ > >> !DIFF_FILE_VALID(p->two) || > >> @@ -4931,7 +4932,16 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p) > >> (p->one->mode != p->two->mode) || > >> diff_populate_filespec(p->one, CHECK_SIZE_ONLY) || > >> diff_populate_filespec(p->two, CHECK_SIZE_ONLY) || > >> - (p->one->size != p->two->size) || > >> + > >> + /* > >> + * only if eol and other conversions are not involved, > >> + * we can say that two contents of different sizes > >> + * cannot be the same without checking their contents. > >> + */ > >> + (!would_convert_to_git(p->one->path) && > >> + !would_convert_to_git(p->two->path) && > >> + (p->one->size != p->two->size)) || > >> + > >> !diff_filespec_is_identical(p->one, p->two)) /* (2) */ > >> p->skip_stat_unmatch_result = 1; > >> return p->skip_stat_unmatch_result; > >> diff --git a/t/t0028-diff-converted.sh b/t/t0028-diff-converted.sh > >> new file mode 100755 > >> index 0000000..3d5ab95 > >> --- /dev/null > >> +++ b/t/t0028-diff-converted.sh > >> @@ -0,0 +1,27 @@ > >> +#!/bin/sh > >> +# > >> +# Copyright (c) 2017 Mike Crowe > >> +# > >> +# These tests ensure that files changing line endings in the presence > >> +# of .gitattributes to indicate that line endings should be ignored > >> +# don't cause 'git diff' or 'git diff --quiet' to think that they have > >> +# been changed. > >> + > >> +test_description='git diff with files that require CRLF conversion' > >> + > >> +. ./test-lib.sh > >> + > >> +test_expect_success setup ' > >> + echo "* text=auto" >.gitattributes && > >> + printf "Hello\r\nWorld\r\n" >crlf.txt && > >> + git add .gitattributes crlf.txt && > >> + git commit -m "initial" > >> +' > >> + > >> +test_expect_success 'quiet diff works on file with line-ending change that has no effect on repository' ' > >> + printf "Hello\r\nWorld\n" >crlf.txt && > >> + git status && > >> + git diff --quiet > >> +' > >> + > >> +test_done > > [snip] > > Also, I think I've found a behaviour change with this fix. Consider: > > > > echo "* text=auto" >.gitattributes > > printf "Hello\r\nWorld\r\n" >crlf.txt > That should give > "Hello\nWorld\n" in the index: > > git add .gitattributes crlf.txt > warning: CRLF will be replaced by LF in ttt/crlf.txt. > The file will have its original line endings in your working directory. > tb@mac:/tmp/ttt> git commit -m "initial" > [master (root-commit) 354f657] initial > 2 files changed, 3 insertions(+) > create mode 100644 ttt/.gitattributes > create mode 100644 ttt/crlf.txt > tb@mac:/tmp/ttt> git ls-files --eol > i/lf w/lf attr/text=auto .gitattributes > i/lf w/crlf attr/text=auto crlf.txt > tb@mac:/tmp/ttt> > > > git add .gitattributes crlf.txt > > git commit -m "initial" > > > > printf "\r\n" >>crlf.txt > > > > With the above patch, both "git diff" and "git diff --quiet" report that > > there are no changes. Previously Git would report the extra newline > > correctly. > Wait a second. > Which extra newline "correctly" ? The extra newline I appended with the printf "\r\n" >> crlf.txt > The "git diff" command is about the changes which will be done to the index. > Regardless if you have any of these in the working tree on disk: > > "Hello\nWorld\n" > "Hello\nWorld\r\n" > "Hello\r\nWorld\n" > "Hello\r\nWorld\r\n" > > "git status" and "git diff --quiet" > should not report any changes. But I didn't have any of those. I ended up with: "Hello\nWorld\n" in the index, and "Hello\r\nWorld\r\n\r\n" in the working tree, but the extra newline was not reported by git diff. > So I don't know if there is a mis-understanding about "git diff" on your side, > or if I miss something. I don't think it matters any more since Junio's patch didn't suffer from this problem. Thanks. Mike.