Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 0fce40d

Browse files
committedOct 22, 2024··
Finish the entire merge implementation and cover everything with tests.
1 parent 37bc21b commit 0fce40d

File tree

8 files changed

+1145
-180
lines changed

8 files changed

+1145
-180
lines changed
 

‎Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crate-status.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,11 @@ Check out the [performance discussion][gix-diff-performance] as well.
340340

341341
* [x] three-way merge analysis of **blobs** with choice of how to resolve conflicts
342342
- [ ] choose how to resolve conflicts on the data-structure
343-
- [ ] produce a new blob based on data-structure containing possible resolutions
343+
- [x] produce a new blob based on data-structure containing possible resolutions
344344
- [x] `merge` style
345345
- [x] `diff3` style
346346
- [x] `zdiff` style
347+
- [ ] various newlines-related options during the merge (see https://git-scm.com/docs/git-merge#Documentation/git-merge.txt-ignore-space-change).
347348
- [ ] a way to control inter-hunk merging based on proximity (maybe via `gix-diff` feature which could use the same)
348349
* [ ] diff-heuristics match Git perfectly
349350
* [x] API documentation

‎gix-merge/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ document-features = { version = "0.2.0", optional = true }
4343
[dev-dependencies]
4444
gix-testtools = { path = "../tests/tools" }
4545
gix-odb = { path = "../gix-odb" }
46+
gix-utils = { version = "^0.1.12", path = "../gix-utils" }
47+
termtree = "0.5.1"
4648
pretty_assertions = "1.4.0"
4749

4850
[package.metadata.docs.rs]

‎gix-merge/src/commit.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@ pub enum Error {
1818
}
1919

2020
/// A way to configure [`commit()`](crate::commit()).
21-
#[derive(Default, Debug, Copy, Clone)]
21+
#[derive(Default, Debug, Clone)]
2222
pub struct Options {
2323
/// If `true`, merging unrelated commits is allowed, with the merge-base being assumed as empty tree.
2424
pub allow_missing_merge_base: bool,
2525
/// Options to define how trees should be merged.
2626
pub tree_merge: crate::tree::Options,
27-
/// Options to define how to merge blobs.
28-
///
29-
/// Note that these are temporarily overwritten if multiple merge-bases are merged into one.
30-
pub blob_merge: crate::blob::platform::merge::Options,
27+
/// If `true`, do not merge multiple merge-bases into one. Instead, just use the first one.
28+
// TODO: test
29+
#[doc(alias = "no_recursive", alias = "git2")]
30+
pub use_first_merge_base: bool,
3131
}
3232

3333
pub(super) mod function {
@@ -51,21 +51,29 @@ pub(super) mod function {
5151
///
5252
/// Note that `objects` *should* have an object cache to greatly accelerate tree-retrieval.
5353
#[allow(clippy::too_many_arguments)]
54-
pub fn commit<'objects>(
54+
pub fn commit<'objects, E>(
5555
our_commit: gix_hash::ObjectId,
5656
their_commit: gix_hash::ObjectId,
5757
mut labels: crate::blob::builtin_driver::text::Labels<'_>,
5858
graph: &mut gix_revwalk::Graph<'_, '_, gix_revwalk::graph::Commit<gix_revision::merge_base::Flags>>,
5959
diff_resource_cache: &mut gix_diff::blob::Platform,
6060
blob_merge: &mut crate::blob::Platform,
6161
objects: &'objects impl gix_object::FindObjectOrHeader,
62+
write_blob_to_odb: impl FnMut(&[u8]) -> Result<gix_hash::ObjectId, E>,
6263
options: Options,
63-
) -> Result<crate::tree::Outcome<'objects>, Error> {
64+
) -> Result<crate::tree::Outcome<'objects>, Error>
65+
where
66+
E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
67+
{
6468
let merge_bases_commit_ids = gix_revision::merge_base(our_commit, &[their_commit], graph)?;
6569
let (merge_base_commit_id, ancestor_name) = match merge_bases_commit_ids {
6670
Some(base_commit) if base_commit.len() == 1 => (base_commit[0], None),
6771
Some(base_commits) => {
68-
let virtual_base_tree = *base_commits.first().expect("TODO: merge multiple bases into one");
72+
let virtual_base_tree = if options.use_first_merge_base {
73+
*base_commits.first().expect("TODO: merge multiple bases into one")
74+
} else {
75+
todo!("merge multiple merge bases")
76+
};
6977
(virtual_base_tree, Some("merged common ancestors".into()))
7078
}
7179
None => {
@@ -97,6 +105,7 @@ pub(super) mod function {
97105
&their_tree_id,
98106
labels,
99107
objects,
108+
write_blob_to_odb,
100109
&mut state,
101110
diff_resource_cache,
102111
blob_merge,

‎gix-merge/src/tree.rs

Lines changed: 840 additions & 150 deletions
Large diffs are not rendered by default.
Binary file not shown.

‎gix-merge/tests/fixtures/tree-baseline.sh

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,12 @@ function baseline () (
3131
their_commit_id="$(git rev-parse "$their_committish")"
3232

3333
local merge_info="${output_name}.merge-info"
34-
git merge-tree -z --write-tree "$our_commit_id" "$their_commit_id" > "$merge_info" || :
35-
echo "$dir" "$our_commit_id" "$their_commit_id" "$merge_info" >> ../baseline.cases
34+
git merge-tree -z --write-tree "$our_committish" "$their_committish" > "$merge_info" || :
35+
echo "$dir" "$our_commit_id" "$our_committish" "$their_commit_id" "$their_committish" "$merge_info" >> ../baseline.cases
36+
37+
local merge_info="${output_name}-reversed.merge-info"
38+
git merge-tree -z --write-tree "$their_committish" "$our_committish" > "$merge_info" || :
39+
echo "$dir" "$their_commit_id" "$their_committish" "$our_commit_id" "$our_committish" "$merge_info" >> ../baseline.cases
3640
)
3741

3842
git init simple
@@ -87,4 +91,32 @@ git init simple
8791
git commit -m first-commit
8892
)
8993

94+
git init dir-rename-and-content
95+
(cd dir-rename-and-content
96+
write_lines 1 2 3 4 5 >foo
97+
mkdir olddir
98+
for i in a b c; do echo $i >olddir/$i; done
99+
git add foo olddir
100+
git commit -m "original"
101+
102+
git branch O
103+
git branch A
104+
git branch B
105+
106+
git checkout A
107+
write_lines 1 2 3 4 5 6 >foo
108+
git add foo
109+
git mv olddir newdir
110+
git commit -m "Modify foo, rename olddir to newdir"
111+
112+
git checkout B
113+
write_lines 1 2 3 4 5 six >foo
114+
git add foo
115+
git mv foo olddir/bar
116+
git commit -m "Modify foo & rename foo -> olddir/bar"
117+
)
118+
90119
baseline simple without-conflict side1 side3
120+
baseline simple various-conflicts side1 side2
121+
baseline simple single-content-conflict side1 side4
122+
baseline dir-rename-and-content dirrename A B

‎gix-merge/tests/merge/tree.rs

Lines changed: 248 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ fn run_baseline() -> crate::Result {
99
root,
1010
odb,
1111
our_commit_id,
12+
our_side_name,
1213
their_commit_id,
14+
their_side_name,
1315
merge_info,
1416
case_name,
1517
} in baseline::Expectations::new(&root, &cases)
@@ -19,56 +21,136 @@ fn run_baseline() -> crate::Result {
1921
let mut diff_resource_cache = baseline::new_diff_resource_cache(&root);
2022
let options = gix_merge::commit::Options {
2123
allow_missing_merge_base: false,
24+
use_first_merge_base: false,
2225
tree_merge: gix_merge::tree::Options {
2326
rewrites: Some(Rewrites {
2427
copies: None,
2528
percentage: Some(0.5),
2629
limit: 0,
2730
}),
31+
blob_merge: Default::default(),
32+
blob_merge_command_ctx: Default::default(),
33+
fail_on_conflict: false,
2834
},
29-
blob_merge: Default::default(),
3035
};
36+
dbg!(&case_name);
3137
let mut actual = gix_merge::commit(
3238
our_commit_id,
3339
their_commit_id,
3440
gix_merge::blob::builtin_driver::text::Labels {
3541
ancestor: None,
36-
current: Some("ours".into()),
37-
other: Some("theirs".into()),
42+
current: Some(our_side_name.as_str().into()),
43+
other: Some(their_side_name.as_str().into()),
3844
},
3945
&mut graph,
4046
&mut diff_resource_cache,
4147
&mut blob_merge,
4248
&odb,
49+
|content| odb.write_buf(gix_object::Kind::Blob, content),
4350
options,
4451
)?;
4552

46-
match merge_info {
47-
Ok(expected_tree_id) => {
48-
let actual_id = actual.tree.write(|tree| odb.write(tree))?;
49-
assert_eq!(actual_id, expected_tree_id, "{case_name}: merged tree mismatch");
50-
}
51-
Err(_conflicts) => {
52-
todo!("compare conflicts")
53-
}
53+
let actual_id = actual.tree.write(|tree| odb.write(tree))?;
54+
if actual_id != merge_info.merged_tree {
55+
baseline::show_diff_and_fail(&case_name, actual_id, &actual, &merge_info, &odb);
5456
}
57+
dbg!(actual.conflicts);
5558
}
5659

5760
Ok(())
5861
}
5962

63+
// TODO: make sure everything is read eventually, even if only to improve debug messages in case of failure.
64+
#[allow(dead_code)]
6065
mod baseline {
66+
use bstr::{BStr, ByteSlice};
67+
use gix_hash::ObjectId;
68+
use gix_object::tree::EntryMode;
69+
use gix_object::FindExt;
6170
use gix_worktree::stack::state::attributes;
6271
use std::path::{Path, PathBuf};
6372

64-
pub struct Conflict;
73+
/// An entry in the conflict
74+
#[derive(Debug)]
75+
pub struct Entry {
76+
/// The relative path in the repository
77+
pub location: String,
78+
/// The content id.
79+
pub id: gix_hash::ObjectId,
80+
/// The kind of entry.
81+
pub mode: EntryMode,
82+
}
83+
84+
/// Keep track of all the sides of a conflict. Some might not be set to indicate removal, including the ancestor.
85+
#[derive(Default, Debug)]
86+
pub struct Conflict {
87+
pub ancestor: Option<Entry>,
88+
pub ours: Option<Entry>,
89+
pub theirs: Option<Entry>,
90+
}
91+
92+
#[derive(Debug)]
93+
pub enum ConflictKind {
94+
/// The conflict was resolved by automatically merging the content.
95+
AutoMerging,
96+
/// The content could not be resolved so it's conflicting.
97+
ConflictContents,
98+
/// Directory in theirs in the way of our file.
99+
ConflictDirectoryBlocksFile,
100+
/// Modified in ours but deleted in theirs.
101+
ConflictModifyDelete,
102+
/// Modified in ours but parent directory renamed in theirs.
103+
DirectoryRenamedWithModificationInside,
104+
}
105+
106+
/// More loosely structured information about the `Conflict`.
107+
#[derive(Debug)]
108+
pub struct ConflictInfo {
109+
/// All the paths involved in the informational message
110+
pub paths: Vec<String>,
111+
/// The type of the conflict, further described in `message`.
112+
pub kind: ConflictKind,
113+
/// An arbitrary message formed from paths and kind
114+
pub message: String,
115+
}
116+
117+
impl Conflict {
118+
fn any_location(&self) -> Option<&str> {
119+
self.ancestor
120+
.as_ref()
121+
.or(self.ours.as_ref())
122+
.or(self.theirs.as_ref())
123+
.map(|a| a.location.as_str())
124+
}
125+
fn storage_for(&mut self, side: Side, location: &str) -> Option<&mut Option<Entry>> {
126+
let current_location = self.any_location();
127+
let location_is_same = current_location.is_none() || current_location == Some(location);
128+
let side = match side {
129+
Side::Ancestor => &mut self.ancestor,
130+
Side::Ours => &mut self.ours,
131+
Side::Theirs => &mut self.theirs,
132+
};
133+
(!side.is_some() && location_is_same).then_some(side)
134+
}
135+
}
136+
137+
pub struct MergeInfo {
138+
/// The hash of the merged tree - it may contain intermediate files if the merge didn't succeed entirely.
139+
pub merged_tree: gix_hash::ObjectId,
140+
/// If there were conflicts, this is the conflicting paths.
141+
pub conflicts: Option<Vec<Conflict>>,
142+
/// Structured details which to some extent can be compared to our own conflict information.
143+
pub information: Vec<ConflictInfo>,
144+
}
65145

66146
pub struct Expectation {
67147
pub root: PathBuf,
68148
pub odb: gix_odb::memory::Proxy<gix_odb::Handle>,
69149
pub our_commit_id: gix_hash::ObjectId,
150+
pub our_side_name: String,
70151
pub their_commit_id: gix_hash::ObjectId,
71-
pub merge_info: Result<gix_hash::ObjectId, Conflict>,
152+
pub their_side_name: String,
153+
pub merge_info: MergeInfo,
72154
pub case_name: String,
73155
}
74156

@@ -92,8 +174,21 @@ mod baseline {
92174
fn next(&mut self) -> Option<Self::Item> {
93175
let line = self.lines.next()?;
94176
let mut tokens = line.split(' ');
95-
let (Some(subdir), Some(our_commit_id), Some(their_commit_id), Some(merge_info_filename)) =
96-
(tokens.next(), tokens.next(), tokens.next(), tokens.next())
177+
let (
178+
Some(subdir),
179+
Some(our_commit_id),
180+
Some(our_side_name),
181+
Some(their_commit_id),
182+
Some(their_side_name),
183+
Some(merge_info_filename),
184+
) = (
185+
tokens.next(),
186+
tokens.next(),
187+
tokens.next(),
188+
tokens.next(),
189+
tokens.next(),
190+
tokens.next(),
191+
)
97192
else {
98193
unreachable!("invalid line: {line:?}")
99194
};
@@ -109,7 +204,9 @@ mod baseline {
109204
root: subdir_path,
110205
odb: objects,
111206
our_commit_id,
207+
our_side_name: our_side_name.to_owned(),
112208
their_commit_id,
209+
their_side_name: their_side_name.to_owned(),
113210
merge_info,
114211
case_name: format!(
115212
"{subdir}-{}",
@@ -122,11 +219,94 @@ mod baseline {
122219
}
123220
}
124221

125-
fn parse_merge_info(content: String) -> Result<gix_hash::ObjectId, Conflict> {
126-
let mut lines = content.split('\0').filter(|t| !t.is_empty());
222+
fn parse_merge_info(content: String) -> MergeInfo {
223+
let mut lines = content.split('\0').filter(|t| !t.is_empty()).peekable();
127224
let tree_id = gix_hash::ObjectId::from_hex(lines.next().unwrap().as_bytes()).unwrap();
128-
assert_eq!(lines.next(), None, "TODO: implement multi-line answer");
129-
Ok(tree_id)
225+
let mut out = MergeInfo {
226+
merged_tree: tree_id,
227+
conflicts: None,
228+
information: Vec::new(),
229+
};
230+
231+
let mut conflicts = Vec::new();
232+
let mut conflict = Conflict::default();
233+
while let Some(line) = lines.peek() {
234+
let (entry, side) = match parse_conflict_file_info(line) {
235+
Some(t) => t,
236+
None => break,
237+
};
238+
lines.next();
239+
let field = match conflict.storage_for(side, &entry.location) {
240+
None => {
241+
conflicts.push(conflict);
242+
conflict = Conflict::default();
243+
conflict
244+
.storage_for(side, &entry.location)
245+
.expect("always available for new side")
246+
}
247+
Some(field) => field,
248+
};
249+
*field = Some(entry);
250+
}
251+
252+
while lines.peek().is_some() {
253+
out.information
254+
.push(parse_info(&mut lines).expect("if there are lines, it should be valid info"));
255+
}
256+
assert_eq!(lines.next(), None, "TODO: conflict messages");
257+
out.conflicts = (!conflicts.is_empty()).then_some(conflicts);
258+
out
259+
}
260+
261+
#[derive(Copy, Clone)]
262+
enum Side {
263+
Ancestor,
264+
Ours,
265+
Theirs,
266+
}
267+
268+
fn parse_conflict_file_info(line: &str) -> Option<(Entry, Side)> {
269+
let (info, mut path) = line.split_at(line.find('\t')?);
270+
path = &path[1..];
271+
let mut tokens = info.split(' ');
272+
let (oct_mode, hex_id, stage) = (
273+
tokens.next().expect("mode"),
274+
tokens.next().expect("id"),
275+
tokens.next().expect("stage"),
276+
);
277+
assert_eq!(
278+
tokens.next(),
279+
None,
280+
"info line not understood, expected three fields only"
281+
);
282+
Some((
283+
Entry {
284+
location: path.to_owned(),
285+
id: gix_hash::ObjectId::from_hex(hex_id.as_bytes()).unwrap(),
286+
mode: EntryMode(gix_utils::btoi::to_signed_with_radix::<usize>(oct_mode.as_bytes(), 8).unwrap() as u16),
287+
},
288+
match stage {
289+
"1" => Side::Ancestor,
290+
"2" => Side::Ours,
291+
"3" => Side::Theirs,
292+
invalid => panic!("{invalid} is an unexpected side"),
293+
},
294+
))
295+
}
296+
297+
fn parse_info<'a>(mut lines: impl Iterator<Item = &'a str>) -> Option<ConflictInfo> {
298+
let num_paths: usize = lines.next()?.parse().ok()?;
299+
let paths: Vec<_> = lines.by_ref().take(num_paths).map(ToOwned::to_owned).collect();
300+
let kind = match lines.next()? {
301+
"Auto-merging" => ConflictKind::AutoMerging,
302+
"CONFLICT (contents)" => ConflictKind::ConflictContents,
303+
"CONFLICT (file/directory)" => ConflictKind::ConflictDirectoryBlocksFile,
304+
"CONFLICT (modify/delete)" => ConflictKind::ConflictModifyDelete,
305+
"CONFLICT (directory rename suggested)" => ConflictKind::DirectoryRenamedWithModificationInside,
306+
conflict_type => panic!("Unkonwn conflict type: {conflict_type}"),
307+
};
308+
let message = lines.next()?.to_owned();
309+
Some(ConflictInfo { paths, kind, message })
130310
}
131311

132312
pub fn new_platform(
@@ -170,4 +350,53 @@ mod baseline {
170350
),
171351
)
172352
}
353+
354+
fn visualize_tree(id: &gix_hash::oid, odb: &impl gix_object::Find, name: Option<&BStr>) -> termtree::Tree<String> {
355+
fn short_id(id: &gix_hash::oid) -> String {
356+
id.to_string()[..7].to_string()
357+
}
358+
let entry_name = |id: &gix_hash::oid, name: Option<&BStr>| -> String {
359+
let mut buf = Vec::new();
360+
match name {
361+
None => short_id(id),
362+
Some(name) => {
363+
format!(
364+
"{name}:{} {:?}",
365+
short_id(id),
366+
match odb.find_blob(id, &mut buf) {
367+
Ok(blob) => blob.data.as_bstr(),
368+
Err(_) => "".into(),
369+
}
370+
)
371+
}
372+
}
373+
};
374+
375+
let mut tree = termtree::Tree::new(entry_name(id, name));
376+
let mut buf = Vec::new();
377+
for entry in odb.find_tree(id, &mut buf).unwrap().entries {
378+
if entry.mode.is_tree() {
379+
tree.push(visualize_tree(entry.oid, odb, entry.filename.into()));
380+
} else {
381+
tree.push(entry_name(entry.oid, entry.filename.into()));
382+
}
383+
}
384+
tree
385+
}
386+
387+
pub fn show_diff_and_fail(
388+
case_name: &str,
389+
actual_id: ObjectId,
390+
actual: &gix_merge::tree::Outcome<'_>,
391+
expected: &MergeInfo,
392+
odb: &gix_odb::memory::Proxy<gix_odb::Handle>,
393+
) {
394+
pretty_assertions::assert_str_eq!(
395+
visualize_tree(&actual_id, odb, None).to_string(),
396+
visualize_tree(&expected.merged_tree, odb, None).to_string(),
397+
"{case_name}: merged tree mismatch {:#?} {:#?}",
398+
actual.conflicts,
399+
expected.information
400+
);
401+
}
173402
}

0 commit comments

Comments
 (0)
Please sign in to comment.