Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo

From: Torsten Bögershausen <tboegi@web.de>
Date: Apr 28, 2015, 07:40 UTC
Message-ID: <553F3959.1060202@web.de>
In-Reply-To: <xmqqbni8vhiz.fsf@gitster.dls.corp.google.com>

On 28/04/15 09:28, Junio C Hamano wrote:

Show 42 quoted lines

> Torsten Bögershausen<tboegi@web.de>  writes:
>
>> What do you think about the following test cases for a V2 patch ?
>>
>> test_expect_success 'create blamerepo' '
>>      test_create_repo blamerepo &&
>>      (
>>          cd blamerepo &&
>>          printf "testcase\r\n" >crlffile &&
>>          git -c core.autocrlf=false add crlffile &&
>>          git commit -m "add files" &&
>>          git -c core.autocrlf=false blame crlffile >crlfclean.txt
>>      )
>> '
>>
>> test_expect_success 'blaming files with CRLF newlines in repo, core.autoclrf=input' '
>>      (
>>          cd blamerepo &&
>>          git -c core.autocrlf=input blame crlffile >actual &&
>>          grep "Not Committed Yet" actual
> Are you interested in seeing just some of the lines to show up as
> "Not commited yet", or all of them?  I think it would be the latter,
> so perhaps
>
>      ! grep -v "Not Committed Yet" actual
>
> or something?
>
>>      )
>> '
>>
>>
> Two blank lines only here?
>
>> test_expect_success 'blaming files with CRLF newlines core.autocrlf=true' '
>>      (
>>          cd blamerepo &&
>>          git -c core.autocrlf=true blame crlffile >actual &&
>>          test_cmp crlfclean.txt actual
>>      )
>> '
> OK

Interestingly this test doesn't pass on one of my systems,
after having stripped t8003 to contain to only have the corner cases.
When core.autocrlf is true, the converting should be suppressed:
  convert.c/has_cr_in_index() should return 1, but doesn't.

  data = read_blob_data_from_cache(path, &sz);
and data is NULL.

Some more digging has to be done here.

On the other hand we want to test blame on a file with LF in the repo and CRLF in the workspace as well.

So all in all I need to send a V2.

Show 17 quoted lines

>
>> test_expect_success 'blaming files with CRLF newlines core.autocrlf=false' '
>>      (
>>          cd blamerepo &&
>>          git -c core.autocrlf=false blame crlffile >actual &&
>>          test_cmp crlfclean.txt actual
>>      )
>> '
> Hmm, how's this blame invocation any different from the one done in
> the set-up step at the very beginning?  In other words, I am not sure
> what kind of breakage could cause this step to fail.
>
> I see there is no "git blame HEAD crlffile" that bypasses the fake
> latest commit altogether.  Wouldn't that be the most appropriate
> thing to compare against (i.e. how to create crlfclean.txt in the
> set-up step)?
>

Jepp,

There is room for improvements.