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

From

Torsten Bögershausen <tboegi@web.de>

Date

Mar 29, 2016, 19:51 UTC

Message-ID

<56FADCAB.1010201@web.de>

In-Reply-To

<xmqqa8lhyzbq.fsf@gitster.mtv.corp.google.com>

On 29.03.16 19:21, Junio C Hamano wrote:

> tboegi@web.de writes:
> 
>> From: Torsten Bögershausen <tboegi@web.de>
>>
>> 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 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.