Correctly pass blame for some merge commits

- Merge commits where a file was added in one chain of ancestors
- Merge commits where a conflict was resolved

Author: cruessler

gix-blame/src/lib.rs

@@ -407,7 +407,13 @@ pub fn process_change(
             }
         }
         (Some(hunk), Some(Change::Added(added, number_of_lines_deleted))) => {
-            let range_in_suspect = hunk.suspects.get(&suspect).expect("TODO").clone();
+            let Some(range_in_suspect) = hunk.suspects.get(&suspect) else {
+                new_hunks_to_blame.push(hunk);
+
+                return (None, Some(Change::Added(added, number_of_lines_deleted)));
+            };
+
+            let range_in_suspect = range_in_suspect.clone();
 
             match (
                 range_in_suspect.contains(&added.start),
@@ -853,10 +859,37 @@ pub fn blame_file<E>(
             }
         } else {
             for parent_id in parent_ids {
-                // Pass blame to parent.
-                hunks_to_blame
-                    .iter_mut()
-                    .for_each(|unblamed_hunk| unblamed_hunk.clone_blame(suspect, parent_id));
+                let changes_for_file_path = get_changes_for_file_path(&odb, file_path, item.id, parent_id);
+
+                let [ref modification]: [gix_diff::tree::recorder::Change] = changes_for_file_path[..] else {
+                    // None of the changes affected the file we’re currently blaming. Pass blame
+                    // to parent.
+                    hunks_to_blame
+                        .iter_mut()
+                        .for_each(|unblamed_hunk| unblamed_hunk.clone_blame(suspect, parent_id));
+
+                    continue;
+                };
+
+                match modification {
+                    gix_diff::tree::recorder::Change::Addition { .. } => {
+                        // Do nothing under the assumption that this always (or almost always)
+                        // implies that the file comes from a different parent, compared to which
+                        // it was modified, not added.
+                        //
+                        // TODO: I still have to figure out whether this is correct in all cases.
+                    }
+                    gix_diff::tree::recorder::Change::Deletion { .. } => todo!(),
+                    gix_diff::tree::recorder::Change::Modification { previous_oid, oid, .. } => {
+                        let changes = get_changes(&odb, resource_cache, *oid, *previous_oid, file_path);
+
+                        hunks_to_blame = process_changes(&mut out, &hunks_to_blame, &changes, suspect);
+
+                        hunks_to_blame
+                            .iter_mut()
+                            .for_each(|unblamed_hunk| unblamed_hunk.pass_blame(suspect, parent_id));
+                    }
+                }
             }
 
             hunks_to_blame

gix-blame/tests/blame.rs

@@ -235,36 +235,13 @@ mktest!(added_line_before_changed_line, "added-line-before-changed-line", 3);
 mktest!(same_line_changed_twice, "same-line-changed-twice", 2);
 mktest!(coalesce_adjacent_hunks, "coalesce-adjacent-hunks", 1);
 
-#[test]
-#[ignore = "TODO: as of October 2024, passing blames to more than one parent is not fully implemented yet"]
-fn merge_commits() {
-    for case in ["resolved-conflict"] {
-        let Fixture {
-            worktree_path,
-            odb,
-            mut resource_cache,
-            suspect,
-            commits,
-        } = Fixture::new().unwrap();
-
-        let lines_blamed = blame_file(
-            &odb,
-            commits,
-            &mut resource_cache,
-            suspect,
-            worktree_path,
-            format!("{case}.txt").as_str().into(),
-        )
-        .unwrap();
-
-        assert_ne!(lines_blamed.len(), 0);
-
-        let git_dir = fixture_path().join(".git");
-        let baseline = Baseline::collect(git_dir.join(format!("{case}.baseline"))).unwrap();
-
-        assert_eq!(lines_blamed, baseline, "{case}");
-    }
-}
+mktest!(resolved_conflict, "resolved-conflict", 2);
+mktest!(file_in_one_chain_of_ancestors, "file-in-one-chain-of-ancestors", 1);
+mktest!(
+    different_file_in_another_chain_of_ancestors,
+    "different-file-in-another-chain-of-ancestors",
+    1
+);
 
 #[test]
 #[ignore = "TBD: figure out what the problem is"]

gix-blame/tests/fixtures/make_blame_repo.sh

@@ -154,6 +154,20 @@ echo -e "line 1 conflict resolved\nline 2\n line 3" > resolved-conflict.txt
 git add resolved-conflict.txt
 git commit -q -m c9
 
+echo -e "line 1\nline 2\n line 3" > file-in-one-chain-of-ancestors.txt
+git add file-in-one-chain-of-ancestors.txt
+git commit -q -m c10
+
+git checkout -b different-branch-that-does-not-contain-file
+git reset --hard HEAD~1
+
+echo -e "line 4\nline 5\n line 6" > different-file-in-another-chain-of-ancestors.txt
+git add different-file-in-another-chain-of-ancestors.txt
+git commit -q -m c11
+
+git checkout main
+git merge different-branch-that-does-not-contain-file || true
+
 git blame --porcelain simple.txt > .git/simple.baseline
 git blame --porcelain multiline-hunks.txt > .git/multiline-hunks.baseline
 git blame --porcelain deleted-lines.txt > .git/deleted-lines.baseline
@@ -168,6 +182,8 @@ git blame --porcelain same-line-changed-twice.txt > .git/same-line-changed-twice
 git blame --porcelain coalesce-adjacent-hunks.txt > .git/coalesce-adjacent-hunks.baseline
 
 git blame --porcelain resolved-conflict.txt > .git/resolved-conflict.baseline
+git blame --porcelain file-in-one-chain-of-ancestors.txt > .git/file-in-one-chain-of-ancestors.baseline
+git blame --porcelain different-file-in-another-chain-of-ancestors.txt > .git/different-file-in-another-chain-of-ancestors.baseline
 
 git blame --porcelain empty-lines-histogram.txt > .git/empty-lines-histogram.baseline