Re: [PATCH v1 6/7] correct blame for files commited with CRLF

On 29.03.16 19:21, Junio C Hamano wrote:

tboegi@web.de writes:

quoted

From: Torsten Bögershausen <redacted>

git blame reports lines as not "Not Committed Yet" when they have
CRLF in the index, CRLF in the worktree and e.g. core.autocrlf is true.

Since commit c48053 "new safer autocrlf handling", files that have CRLF
in the index are not normalized at commit when e.g. core.autocrl is set.

Whenever a CLRF conversion is needed (or any filter us set), load the
index temporally, before calling convert_to_git()

Sorry, but I do not understand what problem you are trying to
address here.

Under the same condition described in the first paragraph, what
would "git diff" and "git diff HEAD" say?  They should show that you
would be making a commit that corrects the line ending of the blob
recorded in the history.

Let's make an experiment, Git v2.8.0


$ printf "Line1\r\nLine2\r\n" >test_CRLF.txt
$ git add test_CRLF.txt 
$ git commit -m "add test_CRLF.txt"
 [detached HEAD 719c166] add test_CRLF.txt
 1 file changed, 2 insertions(+)
 create mode 100644 test_CRLF.txt

$ git ls-files --eol test_CRLF.txt 
i/crlf  w/crlf  attr/                   test_CRLF.txt
# So far, so good.

git config core.autocrlf true

# Now lets patch Git to debug the safer CRLF handling

diff --git a/convert.c b/convert.c
index f524b8d..fcf7653 100644
--- a/convert.c
+++ b/convert.c

@@ -260,8 +260,11 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
                         * If the file in the index has any CR in it, do not convert.
                         * This is the new safer autocrlf handling.
                         */
-                       if (has_cr_in_index(path))
+                       if (has_cr_in_index(path)) {
+                               fprintf(stderr, "%s/%s:%d has_cr_in_index(%s)\n",
+                                       __FILE__, __FUNCTION__, __LINE__, path);
                                return 0;
+                       }

# Of course, run make
$ make
#
printf "Line3\r\n" >>test_CRLF.txt

# Lets see what diff says:
./git diff test_CRLF.txt | od -c
convert.c/crlf_to_git:265 has_cr_in_index(test_CRLF.txt)
convert.c/crlf_to_git:265 has_cr_in_index(test_CRLF.txt)
0000000    d   i   f   f       -   -   g   i   t       a   /   t   e   s
0000020    t   _   C   R   L   F   .   t   x   t       b   /   t   e   s
0000040    t   _   C   R   L   F   .   t   x   t  \n   i   n   d   e   x
0000060        4   a   a   5   5   1   d   .   .   d   0   f   a   f   1
0000100    d       1   0   0   6   4   4  \n   -   -   -       a   /   t
0000120    e   s   t   _   C   R   L   F   .   t   x   t  \n   +   +   +
0000140        b   /   t   e   s   t   _   C   R   L   F   .   t   x   t
0000160   \n   @   @       -   1   ,   2       +   1   ,   3       @   @
0000200   \n       L   i   n   e   1  \r  \n       L   i   n   e   2  \r
0000220   \n   +   l   i   n   e   3  \r  \n                            
0000231
# Here the lines are not going to be normalized at the next commit.
# They stay CRLF.
# But git blame doesn't know that, because has_cr_in_index doesn't work
# without an index.

$ ./git blame test_CRLF.txt 
00000000 (Not Committed Yet 2016-03-29 21:44:48 +0200 1) Line1
00000000 (Not Committed Yet 2016-03-29 21:44:48 +0200 2) Line2
00000000 (Not Committed Yet 2016-03-29 21:44:48 +0200 3) line3



$ git commit -m "Add line3" test_CRLF.txt

The "Not Committed Yet" output from "git blame" is the same thing.
It is telling you that the commit you would be making by adding
that path from the working tree in its current state will become
the one that is responsible for the final state of the line.

So it is absolutely the right thing that these lines are shown as
"Not Commited Yet".  You will be making the line-ending correction
for the entire blob, and you should be made aware of it.

If we had made the CRLF -> LF conversion, yes. But we don't do that.
crlf_to_git() returns without touching the line endings.