CODEWITHSUNDEEP
gitblame
ksok
ksok

Reputation: 21

Git blame -M: unexpected behavior

When using git blame -M in order to detect code movements inside one file, I get results which I cannot explain to myself.

First I commit the following file(file.cpp):

void func1(){return;}[CR][LF]
int func2(){return 23;}[CR][LF]

Then I modify it by moving what was in the first line and adding something new instead:

float newFunc(){return 23.0;}[CR][LF]
int func2(){return 23;}[CR][LF]
[CR][LF]
[CR][LF]
void func1(){return;}[CR][LF]

The log now looks as follows:

>git log --oneline -2
18c670f modified file.cpp
92b4186 added file.cpp

Now I run blame:

git blame -s -w -M file.cpp
18c670fa 1) float newFunc(){return 23.0;}
92b4186d 2) int func2(){return 23;}
18c670fa 3)
18c670fa 4)
18c670fa 5) void func1(){return;}

I wonder why the line containing func1() isn't recognized as moved. I've tried to reduce the number of required characters (i.e. -M4 etc.). Furthermore spaces should not matter because of the -w option.

Upvotes: 2

Views: 164

Answers (1)

VonC
VonC

Reputation: 1328982

(3 years later)

I wonder why the line containing func1() isn't recognized as moved.

That could be related to a recently (July 2016) fixed bug for the upcomming git 2.10:

See commit 17a07e2 (28 May 2016) by David Kastrup (fedelibre).
(Merged by Junio C Hamano -- gitster -- in commit 7418a6b, 19 Jul 2016)

blame: require 0 context lines while finding moved lines with -M

The core part of git blame -M required 1 context line, but there is no rationale to be found in the code; it causes artifacts like discussed in that thread "Understanding behavior of git blame -M".

The function diff_hunks is a wrapper for the diff engine.
Putting the context length explicitly into this wrapper (rather than not passing an argument and just setting the context length to zero anyway in the function) clearly indicates that somebody wanted it called with different values.

There is no documentation or rationale in the file why as far as I remember.
Maybe it can crash or end up in an infinite loop. Maybe it could do so at one point of time but no longer does.

Not sure if it helps, but ctxlen = 1 seems to be added back in d24bba8 (git-pickaxe -M: blame line movements within a file).

Seems like not detecting single line movements is per design and just the documentation is not precise about this. Could such an enhancement be considered as a feature request?

This thread sheds some more light on the history of the previous choice.

The only thing that remotely hints why we thought a non-zero context was a good idea was this, in which I said:

We may need to use a handful surrounding context lines for better identification of copy source by the "ciff" algorithm but that is a minor implementation detail.

Upvotes: 1

Related Questions