Skip to content

Commit 75203d0

Browse files
committed
blame_git: fix coalescing step never being executed
Since blame has been imported from git.git and had its first share of refactorings in b6f60a4 (Clean up ported code, 2013-09-21), the code is actually not doing the coalescing step of the generated blame. While the code to do the coalescing does exist, it is never being called as the function `git_blame__like_git` will directly return from its `while (true)` loop. The function that was being imported from git.git was the `assign_blame` function from "builtin/blame.c" from 717d1462b (git-blame --incremental, 2007-01-28), which hasn't really changed much. Upon taking an initial look, one can seet hat `coalesce` is actually never getting called in `assign_blame`, as well, so one may assume that not calling `coalesce` by accident is actually the right thing. But it is not, as `coalesce` is being called ever since cee7f245d (git-pickaxe: blame rewritten., 2006-10-19) after the blame has been done in the caller of `assign_blame`. Thus we can conclude the code of libgit2 is actually buggy since forever. To fix the issue, simply break out of the loop instead of doing a direct return. Note that this does not alter behaviour in any way visible to our tests, which is unfortunate. But in order to not diverge from what git.git does, I'd rather adapt to how it is being done upstream in order to avoid breaking certain edge cases than to just remove that code.
1 parent 9e8bc72 commit 75203d0

File tree

1 file changed

+8
-5
lines changed

1 file changed

+8
-5
lines changed

src/blame_git.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,8 @@ static void coalesce(git_blame *blame)
623623

624624
int git_blame__like_git(git_blame *blame, uint32_t opt)
625625
{
626+
int error = 0;
627+
626628
while (true) {
627629
git_blame__entry *ent;
628630
git_blame__origin *suspect = NULL;
@@ -632,13 +634,13 @@ int git_blame__like_git(git_blame *blame, uint32_t opt)
632634
if (!ent->guilty)
633635
suspect = ent->suspect;
634636
if (!suspect)
635-
return 0; /* all done */
637+
break;
636638

637639
/* We'll use this suspect later in the loop, so hold on to it for now. */
638640
origin_incref(suspect);
639641

640-
if (pass_blame(blame, suspect, opt) < 0)
641-
return -1;
642+
if ((error = pass_blame(blame, suspect, opt)) < 0)
643+
break;
642644

643645
/* Take responsibility for the remaining entries */
644646
for (ent = blame->ent; ent; ent = ent->next) {
@@ -652,9 +654,10 @@ int git_blame__like_git(git_blame *blame, uint32_t opt)
652654
origin_decref(suspect);
653655
}
654656

655-
coalesce(blame);
657+
if (!error)
658+
coalesce(blame);
656659

657-
return 0;
660+
return error;
658661
}
659662

660663
void git_blame__free_entry(git_blame__entry *ent)

0 commit comments

Comments
 (0)