Skip to content

Commit e0c2aa5

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 9c6947c commit e0c2aa5

File tree

2 files changed

+112
-0
lines changed

2 files changed

+112
-0
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod change_id;
33
mod cmd;
44
mod commit;
55
mod diff;
6+
mod merge;
67
mod extract_remote_name_and_short_name;
78
mod json_samples;
89
mod ref_metadata;
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
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+
builtin_driver::text::{Conflict, ConflictStyle, Labels, Options},
8+
Resolution,
9+
};
10+
11+
fn fixtures() -> std::path::PathBuf {
12+
let dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
13+
.join("tests/fixtures/merge");
14+
assert!(dir.exists(), "fixtures directory missing: {dir:?}");
15+
dir
16+
}
17+
18+
/// gix's Myers blob merge produces a false conflict on certain inputs where
19+
/// `git merge-file` (also Myers-based) resolves cleanly.
20+
///
21+
/// Scenario
22+
/// --------
23+
/// * **base** (71 lines) – original version of a Svelte component
24+
/// * **ours** (164 lines) – heavily expanded version: ~70 new lines inserted
25+
/// above a section comment, everything below the comment unchanged
26+
/// * **theirs** (70 lines) – identical to base except the section comment is
27+
/// deleted (single-line deletion)
28+
///
29+
/// git resolves this cleanly: keep the new block from ours, apply the comment
30+
/// deletion from theirs. `git merge-file -p ours base theirs` exits 0.
31+
/// imara-diff's Myers implementation produces different hunk boundaries for the
32+
/// large base→ours expansion, which makes the 3-way merge see the small
33+
/// theirs deletion as overlapping with ours' insertion — a false conflict.
34+
///
35+
/// Upstream issue: https://github.com/GitoxideLabs/gitoxide/issues/2475
36+
///
37+
/// Remove `#[should_panic]` once the upstream fix lands.
38+
#[test]
39+
#[should_panic(
40+
expected = "gix Myers blob merge should resolve cleanly (upstream bug: \
41+
https://github.com/GitoxideLabs/gitoxide/issues/2475)"
42+
)]
43+
fn myers_blob_merge_false_conflict_with_large_insertion_and_adjacent_deletion() {
44+
let dir = fixtures();
45+
let base = std::fs::read(dir.join("base.svelte")).unwrap();
46+
let ours = std::fs::read(dir.join("ours.svelte")).unwrap();
47+
let theirs = std::fs::read(dir.join("theirs.svelte")).unwrap();
48+
49+
let labels = Labels {
50+
ancestor: Some("base".into()),
51+
current: Some("ours".into()),
52+
other: Some("theirs".into()),
53+
};
54+
let options = Options {
55+
diff_algorithm: gix::diff::blob::Algorithm::Myers,
56+
conflict: Conflict::Keep {
57+
style: ConflictStyle::Merge,
58+
marker_size: std::num::NonZeroU8::new(7).unwrap(),
59+
},
60+
};
61+
62+
let mut out = Vec::new();
63+
let mut input = gix::diff::blob::intern::InternedInput::new(&[][..], &[][..]);
64+
let resolution =
65+
gix::merge::blob::builtin_driver::text(&mut out, &mut input, labels, &ours, &base, &theirs, options);
66+
67+
assert_eq!(
68+
resolution,
69+
Resolution::Complete,
70+
"gix Myers blob merge should resolve cleanly (upstream bug: \
71+
https://github.com/GitoxideLabs/gitoxide/issues/2475)"
72+
);
73+
}
74+
75+
/// Sanity check: the Histogram algorithm already resolves the same input
76+
/// without conflicts. This test must always pass.
77+
#[test]
78+
fn histogram_blob_merge_resolves_large_insertion_and_adjacent_deletion() {
79+
let dir = fixtures();
80+
let base = std::fs::read(dir.join("base.svelte")).unwrap();
81+
let ours = std::fs::read(dir.join("ours.svelte")).unwrap();
82+
let theirs = std::fs::read(dir.join("theirs.svelte")).unwrap();
83+
84+
let labels = Labels {
85+
ancestor: Some("base".into()),
86+
current: Some("ours".into()),
87+
other: Some("theirs".into()),
88+
};
89+
let options = Options {
90+
diff_algorithm: gix::diff::blob::Algorithm::Histogram,
91+
conflict: Conflict::Keep {
92+
style: ConflictStyle::Merge,
93+
marker_size: std::num::NonZeroU8::new(7).unwrap(),
94+
},
95+
};
96+
97+
let mut out = Vec::new();
98+
let mut input = gix::diff::blob::intern::InternedInput::new(&[][..], &[][..]);
99+
let resolution =
100+
gix::merge::blob::builtin_driver::text(&mut out, &mut input, labels, &ours, &base, &theirs, options);
101+
102+
assert_eq!(
103+
resolution,
104+
Resolution::Complete,
105+
"Histogram should merge without conflicts"
106+
);
107+
assert!(
108+
!String::from_utf8_lossy(&out).contains("<<<<<<<"),
109+
"Histogram merge output should contain no conflict markers"
110+
);
111+
}

0 commit comments

Comments
 (0)