Blame showing all lines as NCY

[frogonwheels]

When doing a git blame (current master 8768113 ) with core.eol unset (native) and autocrlf=true, the entire file is always under 'work NCY'.

I suspect that there's a crlf translation issue here. This happens in git gui blame as well as git blame (which I use via fugitive.vim).

[t-b]

Thanks for taking the task!

I can't reprocude that here though:

thomas@THOMAS-WIN7-X64 /git (master)
$ git blame convert.h | head
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  1) /*
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  2)  * Copyright (c) 2011, Google Inc.
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  3)  */
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  4) #ifndef CONVERT_H
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  5) #define CONVERT_H
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  6)
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  7) enum safe_crlf {
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  8)  SAFE_CRLF_FALSE = 0,
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700  9)  SAFE_CRLF_FAIL = 1,
d1bf0e08 (Junio C Hamano 2011-05-20 12:59:01 -0700 10)  SAFE_CRLF_WARN = 2

thomas@THOMAS-WIN7-X64 /git (master)
$ git config core.autocrlf
true

thomas@THOMAS-WIN7-X64 /git (master)
$ git config core.eol

[frogonwheels]

I've finally found some time to work on this:

Try with
$ git config core.autocrlf=false
$ git config core.eol=crlf

I'm assuming you agree that this should work?

This also has similar issues with diff. The weird thing is that I can see where diff does it's crlf translation - and blame is using the textconv code from diff, but doesn't seem to be using the crlf translation code! I'm probably missing something though. Anyway, I've got a couple of things on the way.

[dscho]

This probably happens with Vagrant, too?

[frogonwheels]

I haven't used Vagrant, so can't comment.

What I've found is that in git diff, the crlf_action is set to CRLF_GUESS and so, because_ autocrlf_ is False, the crlf conversion is skipped. I've fixed this by setting it from CRLF_GUESS to CRLF_CRLF in the case where core_eol is EOL_CRLF (convert_attrs). (*note I found a simpler fix than this)

The next issue is that in blame, it appears that diff_populate_filespec() is not used, which means that convert_to_git() is not called (which does the crlf conversion in diff). This can be fixed by calling it in `textconv_object()in a similar way tofill_textconv`` which is used in diff.c

[frogonwheels]

Wow.. without looking at the history, I'm guessing there's been a restructure of all these functions in diff.c, forgetting that they are used in blame.c. It's assumed within diff code, that the lifetime of `diff_filespec *dfis longer than the lifetime of the buffer returned infill_textconv``. Moreover, this value returned in``*outbuf``can be static (const) or from``df``, which in turn can either be allocated memory or a memory-mapped file. The problem with using this memory in``blame.c`` is that all of these issues are ignored, and the assumption made that the returned memory is allocated and thus can be assigned to a stringbuf. In the case where there's a textconv, this is freed before being passed back anyway! Youch. Am I missing something?

[frogonwheels]

I've put changes up here:

https://github.com/frogonwheels/git/tree/mrg/blame-crlf-v1

What I seem to have missed is how blame worked at all. When constructing the '00000000' fake commit which is the current working file, if the textconv failed (ie there wasn't one), then it seemed to just stream the file straight to memory! This is what I was seeing. Maybe in those other cases the textconv was behaving differently somehow.

[frogonwheels]

There were still a few problems, so I've made a new version

https://github.com/frogonwheels/git/tree/mrg/blame-crlf-v2

Some feedback on the filespec blob commits would be good. It seems like there's some potential for quite bad behaviour there, as well as just missing out on the conversion. I guess this is also one for upstream.

Now the only time I get issues is when I have binary nulls in my text file, and I'm happy to file that under expected behaviour.. though it's rather hard to diagnose (as in I didn't work it out until I was in the debugger and it triggered the 'is binary' heuristic).

[dscho]

@frogonwheels I commented a bit on your v2. Please note, though, that it might be better to use GitHub to keep track of the current Pull Request version than to add -v1, -v2, etc to branch names (it is too easy to lose track).

[dscho]

@frogonwheels would you terribly mind switching to Git for Windows 2.x' SDK, rebase your branch, and open a Pull Request? Even if you think there is more work to be done; in that case, at least others can see your current state and help out.