All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: Junio C Hamano <gitster@pobox.com>, tboegi@web.de
Cc: git@vger.kernel.org
Subject: Re: [PATCH v1 6/7] correct blame for files commited with CRLF
Date: Tue, 29 Mar 2016 21:51:07 +0200	[thread overview]
Message-ID: <56FADCAB.1010201@web.de> (raw)
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 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.

  reply	other threads:[~2016-03-29 19:51 UTC|newest]

Thread overview: 126+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <Message-Id=xmqqio26nqk8.fsf@gitster.mtv.corp.google.com>
2016-02-11 16:16 ` [PATCH 1/3] git reset --hard gives clean working tree tboegi
2016-02-11 18:49   ` Junio C Hamano
2016-03-05  7:23     ` Torsten Bögershausen
2016-03-05  8:05       ` Junio C Hamano
2016-03-05  8:27         ` Torsten Bögershausen
2016-03-05 21:18           ` Junio C Hamano
2016-03-07  8:14             ` Junio C Hamano
2016-03-07  8:51               ` Junio C Hamano
2016-03-07  8:58                 ` Torsten Bögershausen
2016-03-07 22:34                   ` Junio C Hamano
2016-03-29 13:25                     ` [PATCH v1 1/7] Make it possible to get sha1 for a path from the index tboegi
2016-03-29 13:28                       ` Duy Nguyen
2016-03-29 13:31                         ` Duy Nguyen
2016-03-29 15:05                           ` Torsten Bögershausen
2016-03-29 19:32                       ` Eric Sunshine
2016-03-29 13:25                     ` [PATCH v1 2/7] convert.c: stream and early out tboegi
2016-03-29 13:25                     ` [PATCH v1 3/7] Allow core.autocrlf=input and core.eol=crlf tboegi
2016-03-29 13:25                     ` [PATCH v1 4/7] t0027: TC for combined attributes tboegi
2016-03-29 13:25                     ` [PATCH v1 5/7] CRLF: unify the "auto" handling tboegi
2016-03-29 19:42                       ` Eric Sunshine
2016-03-29 13:25                     ` [PATCH v1 6/7] correct blame for files commited with CRLF tboegi
2016-03-29 17:21                       ` Junio C Hamano
2016-03-29 19:51                         ` Torsten Bögershausen [this message]
2016-03-29 19:58                           ` Junio C Hamano
2016-03-29 20:25                           ` Junio C Hamano
2016-03-29 20:32                             ` Junio C Hamano
2016-03-29 20:50                               ` Junio C Hamano
2016-03-30 17:48                                 ` Torsten Bögershausen
2016-03-29 13:25                     ` [PATCH v1 7/7] convert.c: more safer crlf handling with text attribute tboegi
2016-03-29 18:37                       ` Junio C Hamano
2016-04-01 16:08                     ` [PATCH v2 1/7] Make it possible to get sha1 for a path from the index tboegi
2016-04-01 16:08                     ` [PATCH v2 2/7] convert.c: stream and early out tboegi
2016-04-01 16:08                     ` [PATCH v2 3/7] Allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-01 22:20                       ` Junio C Hamano
2016-04-01 16:08                     ` [PATCH v2 4/7] t0027: TC for combined attributes tboegi
2016-04-01 22:22                       ` Junio C Hamano
2016-04-01 16:08                     ` [PATCH v2 5/7] CRLF: unify the "auto" handling tboegi
2016-04-01 22:25                       ` Junio C Hamano
2016-04-01 16:08                     ` [PATCH v2 6/7] correct blame for files commited with CRLF tboegi
2016-04-01 22:29                       ` Junio C Hamano
2016-04-03  9:29                         ` Torsten Bögershausen
2016-04-01 16:08                     ` [PATCH v2 7/7] convert.c: more safer crlf handling with text attribute tboegi
2016-04-05 19:23                     ` [PATCH v1] correct blame for files commited with CRLF tboegi
2016-04-05 20:57                       ` Junio C Hamano
2016-04-05 21:12                       ` Junio C Hamano
2016-04-06  4:17                         ` Torsten Bögershausen
2016-04-19 13:24                     ` [PATCH v5 1/4] t0027: Make more reliable tboegi
2016-04-19 13:26                     ` [PATCH v5 2/4] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-19 13:26                     ` [PATCH v5 3/4] t0027: test cases for combined attributes tboegi
2016-04-19 21:32                       ` Junio C Hamano
2016-04-20 15:52                         ` Torsten Bögershausen
2016-04-19 13:26                     ` [PATCH v5 4/4] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-20 22:27                       ` Junio C Hamano
2016-04-22 14:38                     ` [PATCH v6 01/10] t0027: Make more reliable tboegi
2016-04-22 22:03                       ` Junio C Hamano
2016-04-24  3:45                         ` Torsten Bögershausen
2016-04-22 14:53                     ` [PATCH v6 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-22 14:53                     ` [PATCH v6 03/10] t0027: test cases for combined attributes tboegi
2016-04-22 14:53                     ` [PATCH v6 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-22 14:53                     ` [PATCH v6 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-22 14:53                     ` [PATCH v6 06/10] convert.c: stream and early out tboegi
2016-04-22 14:53                     ` [PATCH v6 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-04-22 14:53                     ` [PATCH v6 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-22 14:53                     ` [PATCH v6 09/10] t6038; use crlf on all platforms tboegi
2016-04-22 14:53                     ` [PATCH v6 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-24 15:10                     ` [PATCH v6b 01/10] t0027: Make commit_chk_wrnNNO() reliable tboegi
2016-04-24 15:11                     ` [PATCH v6b 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-24 15:11                     ` [PATCH v6b 03/10] t0027: test cases for combined attributes tboegi
2016-04-24 15:11                     ` [PATCH v6b 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-24 15:11                     ` [PATCH v6b 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-24 15:11                     ` [PATCH v6b 06/10] convert.c: stream and early out tboegi
2016-04-24 15:11                     ` [PATCH v6b 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-04-24 15:11                     ` [PATCH v6b 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-24 15:11                     ` [PATCH v6b 09/10] t6038; use crlf on all platforms tboegi
2016-04-24 15:11                     ` [PATCH v6b 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-25 16:56                     ` [PATCH v7 01/10] t0027: Make commit_chk_wrnNNO() reliable tboegi
2016-04-25 19:15                       ` Junio C Hamano
2016-04-25 16:56                     ` [PATCH v7 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-25 16:56                     ` [PATCH v7 03/10] t0027: test cases for combined attributes tboegi
2016-04-25 16:56                     ` [PATCH v7 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-25 16:56                     ` [PATCH v7 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-25 16:56                     ` [PATCH v7 06/10] convert.c: stream and early out tboegi
2016-04-25 16:56                     ` [PATCH v7 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-04-25 19:37                       ` Junio C Hamano
2016-04-26 16:33                         ` Torsten Bögershausen
2016-04-26 17:42                           ` Junio C Hamano
2016-04-25 16:56                     ` [PATCH v7 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-25 16:56                     ` [PATCH v7 09/10] t6038; use crlf on all platforms tboegi
2016-04-25 16:56                     ` [PATCH v7 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-29 15:01                     ` [PATCH v8 01/10] t0027: make commit_chk_wrnNNO() reliable tboegi
2016-04-29 15:01                     ` [PATCH v8 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-29 15:01                     ` [PATCH v8 03/10] t0027: test cases for combined attributes tboegi
2016-04-29 15:01                     ` [PATCH v8 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-29 15:02                     ` [PATCH v8 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-29 15:02                     ` [PATCH v8 06/10] convert.c: stream and early out tboegi
2016-04-29 15:02                     ` [PATCH v8 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-11-25 15:48                       ` Torsten Bögershausen
2016-11-27 16:22                         ` [PATCH/RFC v1 1/1] New way to normalize the line endings tboegi
2016-11-29 19:15                           ` Junio C Hamano
2017-04-12 11:48                         ` [PATCH v2 1/1] Document how " tboegi
2016-04-29 15:02                     ` [PATCH v8 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-29 15:02                     ` [PATCH v8 09/10] t6038; use crlf on all platforms tboegi
2016-04-29 15:02                     ` [PATCH v8 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-29 18:20                       ` Junio C Hamano
2016-04-29 21:09                       ` Junio C Hamano
2016-05-01 16:27                         ` Torsten Bögershausen
2016-05-02 18:16                           ` Junio C Hamano
2016-05-02 19:33                             ` Junio C Hamano
2016-05-03 16:02                               ` Torsten Bögershausen
2016-05-03 18:31                                 ` Junio C Hamano
2016-05-04  4:07                                   ` Torsten Bögershausen
2016-05-04  7:23                                     ` Junio C Hamano
2016-05-06  8:54                                       ` Torsten Bögershausen
2016-05-06 17:11                                         ` Junio C Hamano
2016-05-07  6:10                     ` [PATCH v9 0/6] convert-eol-autocrlf, old 5..10 now 1..6 tboegi
2016-05-07  6:10                     ` [PATCH v9 1/6] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-09 19:54                       ` Junio C Hamano
2016-05-07  6:11                     ` [PATCH v9 2/6] convert.c: stream and early out tboegi
2016-05-09 20:29                       ` Junio C Hamano
2016-05-11  4:30                         ` Torsten Bögershausen
2016-05-07  6:11                     ` [PATCH v9 3/6] convert: unify the "auto" handling of CRLF tboegi
2016-05-07  6:11                     ` [PATCH v9 4/6] convert.c: more safer crlf handling with text attribute tboegi
2016-05-07  6:11                     ` [PATCH v9 5/6] t6038; use crlf on all platforms tboegi
2016-05-07  6:11                     ` [PATCH v9 6/6] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-02-11 16:16 ` [PATCH 2/3] Factor out convert_cmp_checkout() into convert.c tboegi
2016-02-11 16:16 ` [PATCH 3/3] convert.c: Optimize convert_cmp_checkout() for changed file len tboegi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56FADCAB.1010201@web.de \
    --to=tboegi@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.