Skip to content

Commit 0e5a4a6

Browse files
committed
Add failing gix blob-merge tests for Myers algorithm issue
gix's blob merge (gix_merge::blob::builtin_driver::text) with the Myers diff algorithm produces a false conflict on inputs where git merge-file resolves cleanly. The trigger is a 3-way merge where base→ours is a large expansion (~70 new lines inserted above a section comment) and base→theirs is a single-line deletion of that same comment. imara-diff's Myers implementation produces different hunk boundaries than git's xdiff for this input, causing the merge to see the deletion as overlapping with the insertion — a false conflict. The Histogram algorithm in imara-diff handles the same input correctly. This directly impacts GitButler's commit amend operation: create_tree() uses gix's tree merge for the cherry-pick step, and the false conflict causes valid changes to be rejected with CherryPickMergeConflict. Add a #[should_panic] test that documents the upstream bug and will start failing (reminding us to remove the annotation) once gix is fixed, plus a Histogram-based test that verifies the working alternative. Upstream: GitoxideLabs/gitoxide#2475
1 parent cd0d1e3 commit 0e5a4a6

5 files changed

Lines changed: 122 additions & 0 deletions

File tree

crates/but-core/tests/core/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ mod commit;
55
mod diff;
66
mod extract_remote_name_and_short_name;
77
mod json_samples;
8+
mod merge;
89
mod ref_metadata;
910
mod settings;
1011
mod snapshot;
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/// Tests for gix blob-merge behaviour.
2+
///
3+
/// Some tests in this module are expected to **fail** until an upstream gix bug
4+
/// is resolved. Those tests are marked `#[should_panic]` so the test suite
5+
/// stays green in the interim; once gix is fixed the annotation must be removed.
6+
use gix::merge::blob::{
7+
Resolution,
8+
builtin_driver::text::{Conflict, ConflictStyle, Labels, Options},
9+
};
10+
11+
fn fixtures() -> std::path::PathBuf {
12+
let dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/merge");
13+
assert!(dir.exists(), "fixtures directory missing: {dir:?}");
14+
dir
15+
}
16+
17+
/// gix's Myers blob merge produces a false conflict on certain inputs where
18+
/// `git merge-file` (also Myers-based) resolves cleanly.
19+
///
20+
/// Scenario (minimal reproduction – 5/4/4 lines)
21+
/// -----------------------------------------------
22+
/// * **base** – `alpha_x`, blank, `bravo_x`, `charlie_x`, blank
23+
/// * **ours** – blank, blank, `bravo_x`, `charlie_x`
24+
/// (i.e. `alpha_x` deleted, one blank added, trailing blank removed)
25+
/// * **theirs** – `alpha_x`, blank, `charlie_x`, blank
26+
/// (i.e. `bravo_x` deleted)
27+
///
28+
/// `git merge-file -p ours base theirs` exits 0 (clean merge).
29+
/// imara-diff's Myers implementation chooses different hunk boundaries when
30+
/// blank lines create alignment ambiguity, causing the 3-way merge to see
31+
/// the `bravo_x` deletion as overlapping with the `alpha_x` area changes.
32+
///
33+
/// Upstream issue: https://github.com/GitoxideLabs/gitoxide/issues/2475
34+
///
35+
/// Remove `#[should_panic]` once the upstream fix lands.
36+
#[test]
37+
#[should_panic(expected = "https://github.com/GitoxideLabs/gitoxide/issues/2475")]
38+
fn myers_blob_merge_false_conflict_with_large_insertion_and_adjacent_deletion() {
39+
let dir = fixtures();
40+
let base = std::fs::read(dir.join("base.txt")).unwrap();
41+
let ours = std::fs::read(dir.join("ours.txt")).unwrap();
42+
let theirs = std::fs::read(dir.join("theirs.txt")).unwrap();
43+
44+
let labels = Labels {
45+
ancestor: Some("base".into()),
46+
current: Some("ours".into()),
47+
other: Some("theirs".into()),
48+
};
49+
let options = Options {
50+
diff_algorithm: gix::diff::blob::Algorithm::Myers,
51+
conflict: Conflict::Keep {
52+
style: ConflictStyle::Merge,
53+
marker_size: std::num::NonZeroU8::new(7).unwrap(),
54+
},
55+
};
56+
57+
let mut out = Vec::new();
58+
let mut input = gix::diff::blob::intern::InternedInput::new(&[][..], &[][..]);
59+
let resolution = gix::merge::blob::builtin_driver::text(
60+
&mut out, &mut input, labels, &ours, &base, &theirs, options,
61+
);
62+
63+
assert_eq!(
64+
resolution,
65+
Resolution::Complete,
66+
"gix Myers blob merge should resolve cleanly (upstream bug: \
67+
https://github.com/GitoxideLabs/gitoxide/issues/2475)"
68+
);
69+
}
70+
71+
/// Sanity check: the Histogram algorithm already resolves the same input
72+
/// without conflicts. This test must always pass.
73+
#[test]
74+
fn histogram_blob_merge_resolves_large_insertion_and_adjacent_deletion() {
75+
let dir = fixtures();
76+
let base = std::fs::read(dir.join("base.txt")).unwrap();
77+
let ours = std::fs::read(dir.join("ours.txt")).unwrap();
78+
let theirs = std::fs::read(dir.join("theirs.txt")).unwrap();
79+
80+
let labels = Labels {
81+
ancestor: Some("base".into()),
82+
current: Some("ours".into()),
83+
other: Some("theirs".into()),
84+
};
85+
let options = Options {
86+
diff_algorithm: gix::diff::blob::Algorithm::Histogram,
87+
conflict: Conflict::Keep {
88+
style: ConflictStyle::Merge,
89+
marker_size: std::num::NonZeroU8::new(7).unwrap(),
90+
},
91+
};
92+
93+
let mut out = Vec::new();
94+
let mut input = gix::diff::blob::intern::InternedInput::new(&[][..], &[][..]);
95+
let resolution = gix::merge::blob::builtin_driver::text(
96+
&mut out, &mut input, labels, &ours, &base, &theirs, options,
97+
);
98+
99+
assert_eq!(
100+
resolution,
101+
Resolution::Complete,
102+
"Histogram should merge without conflicts"
103+
);
104+
assert!(
105+
!String::from_utf8_lossy(&out).contains("<<<<<<<"),
106+
"Histogram merge output should contain no conflict markers"
107+
);
108+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
alpha_x
2+
3+
bravo_x
4+
charlie_x
5+
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
3+
bravo_x
4+
charlie_x
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
alpha_x
2+
3+
charlie_x
4+

0 commit comments

Comments
 (0)