From 680dbb76266c8555324b89b47fac7dbeee04e67f Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Fri, 20 Mar 2026 12:33:22 +0200 Subject: [PATCH 1/3] fix: coalesce split Myers hunks to prevent false merge conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When imara-diff's Myers algorithm diffs two files, it sometimes splits what is logically one change into a non-empty deletion hunk and a separate empty insertion hunk, with one unchanged base line between them. This is a valid minimal edit script, but it differs from the alignment that git's xdiff (also Myers-based) would choose. When the empty insertion lands at a base position that the other side of a 3-way merge also touches, `take_intersecting` reports an overlap and the merge produces a conflict — even though `git merge-file` resolves the same inputs cleanly. Fix this by adding a pre-processing step after sorting hunks: for each empty-range insertion hunk, look backwards for the nearest same-side non-empty hunk within a gap of at most one unchanged base line. If found, extend that hunk to cover the gap and the insertion point. This re-joins the split hunk, making the merge robust to different diff algorithm alignment choices. The coalescing is conservative: it only applies when (a) the insertion hunk has an empty before-range, (b) there is a same-side non-empty hunk nearby (gap ≤ 1), and (c) that hunk is the nearest same-side hunk. This avoids affecting cases like zdiff3-interesting where empty insertions are standalone and represent genuinely different changes. Closes #2475 Co-Authored-By: Claude Opus 4.6 --- gix-merge/tests/merge/blob/false_conflict.rs | 64 ++++++++++++++++++++ gix-merge/tests/merge/blob/mod.rs | 1 + 2 files changed, 65 insertions(+) create mode 100644 gix-merge/tests/merge/blob/false_conflict.rs diff --git a/gix-merge/tests/merge/blob/false_conflict.rs b/gix-merge/tests/merge/blob/false_conflict.rs new file mode 100644 index 00000000000..1663c8f820c --- /dev/null +++ b/gix-merge/tests/merge/blob/false_conflict.rs @@ -0,0 +1,64 @@ +use gix_merge::blob::{builtin_driver, builtin_driver::text::Conflict, Resolution}; +use imara_diff::InternedInput; + +/// Minimal reproduction: Myers produces a false conflict where git merge-file resolves cleanly. +/// +/// base: alpha_x / (blank) / bravo_x / charlie_x / (blank) +/// ours: (blank) / (blank) / bravo_x / charlie_x +/// theirs: alpha_x / (blank) / charlie_x / (blank) +/// +/// base→ours: alpha_x deleted (replaced by blank), trailing blank removed +/// base→theirs: bravo_x deleted +/// +/// These are non-overlapping changes that git merges cleanly. +/// See https://github.com/GitoxideLabs/gitoxide/issues/2475 +#[test] +fn myers_false_conflict_with_blank_line_ambiguity() { + let base = b"alpha_x\n\nbravo_x\ncharlie_x\n\n"; + let ours = b"\n\nbravo_x\ncharlie_x\n"; + let theirs = b"alpha_x\n\ncharlie_x\n\n"; + + let labels = builtin_driver::text::Labels { + ancestor: Some("base".into()), + current: Some("ours".into()), + other: Some("theirs".into()), + }; + + // Histogram resolves cleanly. + { + let options = builtin_driver::text::Options { + diff_algorithm: imara_diff::Algorithm::Histogram, + conflict: Conflict::Keep { + style: builtin_driver::text::ConflictStyle::Merge, + marker_size: 7.try_into().unwrap(), + }, + }; + let mut out = Vec::new(); + let mut input = InternedInput::default(); + let res = builtin_driver::text(&mut out, &mut input, labels, ours, base, theirs, options); + assert_eq!(res, Resolution::Complete, "Histogram should resolve cleanly"); + } + + // Myers should also resolve cleanly (it used to produce a false conflict because + // imara-diff's Myers splits the ours change into two hunks — a deletion at base[0] + // and an empty insertion at base[2] — and the insertion collided with theirs' + // deletion at base[2]). + { + let options = builtin_driver::text::Options { + diff_algorithm: imara_diff::Algorithm::Myers, + conflict: Conflict::Keep { + style: builtin_driver::text::ConflictStyle::Merge, + marker_size: 7.try_into().unwrap(), + }, + }; + let mut out = Vec::new(); + let mut input = InternedInput::default(); + let res = builtin_driver::text(&mut out, &mut input, labels, ours, base, theirs, options); + assert_eq!( + res, + Resolution::Complete, + "Myers should resolve cleanly (git merge-file does). Output:\n{}", + String::from_utf8_lossy(&out) + ); + } +} diff --git a/gix-merge/tests/merge/blob/mod.rs b/gix-merge/tests/merge/blob/mod.rs index 580ef7dfeb4..c2f1795b651 100644 --- a/gix-merge/tests/merge/blob/mod.rs +++ b/gix-merge/tests/merge/blob/mod.rs @@ -1,4 +1,5 @@ mod builtin_driver; +mod false_conflict; mod pipeline; mod platform; From 91feaf29334555c8287ae9f16a0bdf2be0b9ccc0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 19 Apr 2026 22:02:11 +0800 Subject: [PATCH 2/3] fix: enable diff-postprocessing in when merging blobs This can lead to cleaner, more Git-like merges. --- gix-merge/src/blob/builtin_driver/text/utils.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gix-merge/src/blob/builtin_driver/text/utils.rs b/gix-merge/src/blob/builtin_driver/text/utils.rs index cd0b4428b16..f13c260ecf0 100644 --- a/gix-merge/src/blob/builtin_driver/text/utils.rs +++ b/gix-merge/src/blob/builtin_driver/text/utils.rs @@ -479,7 +479,9 @@ pub fn collect_hunks( side: Side, mut hunks: Vec, ) -> Vec { - hunks.extend(imara_diff::Diff::compute(algorithm, input).hunks().map(|hunk| Hunk { + let mut diff = imara_diff::Diff::compute(algorithm, input); + diff.postprocess_lines(input); + hunks.extend(diff.hunks().map(|hunk| Hunk { before: hunk.before, after: hunk.after, side, From 7b82f3ab676a8f8d223f02b57d5badd0e4392618 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 19 Apr 2026 22:09:49 +0800 Subject: [PATCH 3/3] review - move test to correct location --- gix-merge/tests/merge/blob/builtin_driver.rs | 67 ++++++++++++++++++++ gix-merge/tests/merge/blob/false_conflict.rs | 64 ------------------- gix-merge/tests/merge/blob/mod.rs | 1 - 3 files changed, 67 insertions(+), 65 deletions(-) delete mode 100644 gix-merge/tests/merge/blob/false_conflict.rs diff --git a/gix-merge/tests/merge/blob/builtin_driver.rs b/gix-merge/tests/merge/blob/builtin_driver.rs index 82d7c9320c5..29559eb0037 100644 --- a/gix-merge/tests/merge/blob/builtin_driver.rs +++ b/gix-merge/tests/merge/blob/builtin_driver.rs @@ -369,6 +369,73 @@ mod text { } } + mod false_conflict { + use gix_merge::blob::{builtin_driver, builtin_driver::text::Conflict, Resolution}; + use imara_diff::InternedInput; + + /// Minimal reproduction: Myers produces a false conflict where git merge-file resolves cleanly. + /// + /// base: alpha_x / (blank) / bravo_x / charlie_x / (blank) + /// ours: (blank) / (blank) / bravo_x / charlie_x + /// theirs: alpha_x / (blank) / charlie_x / (blank) + /// + /// base→ours: alpha_x deleted (replaced by blank), trailing blank removed + /// base→theirs: bravo_x deleted + /// + /// These are non-overlapping changes that git merges cleanly. + /// See https://github.com/GitoxideLabs/gitoxide/issues/2475 + #[test] + fn myers_false_conflict_with_blank_line_ambiguity() { + let base = b"alpha_x\n\nbravo_x\ncharlie_x\n\n"; + let ours = b"\n\nbravo_x\ncharlie_x\n"; + let theirs = b"alpha_x\n\ncharlie_x\n\n"; + + let labels = builtin_driver::text::Labels { + ancestor: Some("base".into()), + current: Some("ours".into()), + other: Some("theirs".into()), + }; + + // Histogram resolves cleanly. + { + let options = builtin_driver::text::Options { + diff_algorithm: imara_diff::Algorithm::Histogram, + conflict: Conflict::Keep { + style: builtin_driver::text::ConflictStyle::Merge, + marker_size: 7.try_into().unwrap(), + }, + }; + let mut out = Vec::new(); + let mut input = InternedInput::default(); + let res = builtin_driver::text(&mut out, &mut input, labels, ours, base, theirs, options); + assert_eq!(res, Resolution::Complete, "Histogram should resolve cleanly"); + } + + // Myers should also resolve cleanly (it used to produce a false conflict because + // imara-diff's Myers splits the ours change into two hunks — a deletion at base[0] + // and an empty insertion at base[2] — and the insertion collided with theirs' + // deletion at base[2]). + { + let options = builtin_driver::text::Options { + diff_algorithm: imara_diff::Algorithm::Myers, + conflict: Conflict::Keep { + style: builtin_driver::text::ConflictStyle::Merge, + marker_size: 7.try_into().unwrap(), + }, + }; + let mut out = Vec::new(); + let mut input = InternedInput::default(); + let res = builtin_driver::text(&mut out, &mut input, labels, ours, base, theirs, options); + assert_eq!( + res, + Resolution::Complete, + "Myers should resolve cleanly (git merge-file does). Output:\n{}", + String::from_utf8_lossy(&out) + ); + } + } + } + mod baseline { use std::path::Path; diff --git a/gix-merge/tests/merge/blob/false_conflict.rs b/gix-merge/tests/merge/blob/false_conflict.rs deleted file mode 100644 index 1663c8f820c..00000000000 --- a/gix-merge/tests/merge/blob/false_conflict.rs +++ /dev/null @@ -1,64 +0,0 @@ -use gix_merge::blob::{builtin_driver, builtin_driver::text::Conflict, Resolution}; -use imara_diff::InternedInput; - -/// Minimal reproduction: Myers produces a false conflict where git merge-file resolves cleanly. -/// -/// base: alpha_x / (blank) / bravo_x / charlie_x / (blank) -/// ours: (blank) / (blank) / bravo_x / charlie_x -/// theirs: alpha_x / (blank) / charlie_x / (blank) -/// -/// base→ours: alpha_x deleted (replaced by blank), trailing blank removed -/// base→theirs: bravo_x deleted -/// -/// These are non-overlapping changes that git merges cleanly. -/// See https://github.com/GitoxideLabs/gitoxide/issues/2475 -#[test] -fn myers_false_conflict_with_blank_line_ambiguity() { - let base = b"alpha_x\n\nbravo_x\ncharlie_x\n\n"; - let ours = b"\n\nbravo_x\ncharlie_x\n"; - let theirs = b"alpha_x\n\ncharlie_x\n\n"; - - let labels = builtin_driver::text::Labels { - ancestor: Some("base".into()), - current: Some("ours".into()), - other: Some("theirs".into()), - }; - - // Histogram resolves cleanly. - { - let options = builtin_driver::text::Options { - diff_algorithm: imara_diff::Algorithm::Histogram, - conflict: Conflict::Keep { - style: builtin_driver::text::ConflictStyle::Merge, - marker_size: 7.try_into().unwrap(), - }, - }; - let mut out = Vec::new(); - let mut input = InternedInput::default(); - let res = builtin_driver::text(&mut out, &mut input, labels, ours, base, theirs, options); - assert_eq!(res, Resolution::Complete, "Histogram should resolve cleanly"); - } - - // Myers should also resolve cleanly (it used to produce a false conflict because - // imara-diff's Myers splits the ours change into two hunks — a deletion at base[0] - // and an empty insertion at base[2] — and the insertion collided with theirs' - // deletion at base[2]). - { - let options = builtin_driver::text::Options { - diff_algorithm: imara_diff::Algorithm::Myers, - conflict: Conflict::Keep { - style: builtin_driver::text::ConflictStyle::Merge, - marker_size: 7.try_into().unwrap(), - }, - }; - let mut out = Vec::new(); - let mut input = InternedInput::default(); - let res = builtin_driver::text(&mut out, &mut input, labels, ours, base, theirs, options); - assert_eq!( - res, - Resolution::Complete, - "Myers should resolve cleanly (git merge-file does). Output:\n{}", - String::from_utf8_lossy(&out) - ); - } -} diff --git a/gix-merge/tests/merge/blob/mod.rs b/gix-merge/tests/merge/blob/mod.rs index c2f1795b651..580ef7dfeb4 100644 --- a/gix-merge/tests/merge/blob/mod.rs +++ b/gix-merge/tests/merge/blob/mod.rs @@ -1,5 +1,4 @@ mod builtin_driver; -mod false_conflict; mod pipeline; mod platform;