Skip to content

Commit b76a538

Browse files
committed
implement support for resolving irreconcilable tree conflicts with 'ours' or 'ancestor'
1 parent 3228de6 commit b76a538

File tree

8 files changed

+1035
-220
lines changed

8 files changed

+1035
-220
lines changed

crate-status.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,11 @@ Check out the [performance discussion][gix-diff-performance] as well.
352352
- [ ] a way to control inter-hunk merging based on proximity (maybe via `gix-diff` feature which could use the same)
353353
* [x] **tree**-diff-heuristics match Git for its test-cases
354354
- [x] a way to generate an index with stages, mostly conforming with Git.
355+
- [ ] resolve to be *ours* or the *ancestors* version of the tree.
355356
- [ ] submodule merges (*right now they count as conflicts if they differ*)
356357
- [ ] assure sparse indices are handled correctly during application - right now we refuse.
358+
- [ ] rewrite so that the whole logic can be proven to be correct - it's too insane now and probably has way
359+
more possible states than are tested, despite best attempts.
357360
* [x] **commits** - with handling of multiple merge bases by recursive merge-base merge
358361
* [x] API documentation
359362
* [ ] Examples

gix-merge/src/tree/function.rs

Lines changed: 416 additions & 198 deletions
Large diffs are not rendered by default.

gix-merge/src/tree/mod.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ enum ConflictIndexEntryPathHint {
157157
}
158158

159159
/// A utility to help define which side is what in the [`Conflict`] type.
160-
#[derive(Debug, Clone, Copy)]
160+
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
161161
enum ConflictMapping {
162162
/// The sides are as described in the field documentation, i.e. `ours` is `ours`.
163163
Original,
@@ -175,13 +175,19 @@ impl ConflictMapping {
175175
ConflictMapping::Swapped => ConflictMapping::Original,
176176
}
177177
}
178+
fn to_global(self, global: ConflictMapping) -> ConflictMapping {
179+
match global {
180+
ConflictMapping::Original => self,
181+
ConflictMapping::Swapped => self.swapped(),
182+
}
183+
}
178184
}
179185

180186
impl Conflict {
181187
/// Return `true` if this instance is considered unresolved based on the criterion specified by `how`.
182188
pub fn is_unresolved(&self, how: TreatAsUnresolved) -> bool {
183189
use crate::blob;
184-
let content_merge_matches = |info: &ContentMerge| match how.content_merge {
190+
let content_merge_unresolved = |info: &ContentMerge| match how.content_merge {
185191
treat_as_unresolved::ContentMerge::Markers => matches!(info.resolution, blob::Resolution::Conflict),
186192
treat_as_unresolved::ContentMerge::ForcedResolution => {
187193
matches!(
@@ -192,20 +198,28 @@ impl Conflict {
192198
};
193199
match how.tree_merge {
194200
treat_as_unresolved::TreeMerge::Undecidable => {
195-
self.resolution.is_err() || self.content_merge().map_or(false, |info| content_merge_matches(&info))
201+
self.resolution.is_err()
202+
|| self
203+
.content_merge()
204+
.map_or(false, |info| content_merge_unresolved(&info))
196205
}
197206
treat_as_unresolved::TreeMerge::EvasiveRenames | treat_as_unresolved::TreeMerge::ForcedResolution => {
198207
match &self.resolution {
199208
Ok(success) => match success {
200209
Resolution::SourceLocationAffectedByRename { .. } => false,
201-
Resolution::Forced(_) => how.tree_merge == treat_as_unresolved::TreeMerge::ForcedResolution,
210+
Resolution::Forced(_) => {
211+
how.tree_merge == treat_as_unresolved::TreeMerge::ForcedResolution
212+
|| self
213+
.content_merge()
214+
.map_or(false, |merged_blob| content_merge_unresolved(&merged_blob))
215+
}
202216
Resolution::OursModifiedTheirsRenamedAndChangedThenRename {
203217
merged_blob,
204218
final_location,
205219
..
206-
} => final_location.is_some() || merged_blob.as_ref().map_or(false, content_merge_matches),
220+
} => final_location.is_some() || merged_blob.as_ref().map_or(false, content_merge_unresolved),
207221
Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => {
208-
content_merge_matches(merged_blob)
222+
content_merge_unresolved(merged_blob)
209223
}
210224
},
211225
Err(_failure) => true,
@@ -386,7 +400,10 @@ pub struct Options {
386400
pub enum ResolveWith {
387401
/// On irreconcilable conflict, choose neither *our* nor *their* state, but keep the common *ancestor* state instead.
388402
Ancestor,
389-
/// On irreconcilable conflict, choose *our* side
403+
/// On irreconcilable conflict, choose *our* side.
404+
///
405+
/// Note that in order to get something equivalent to *theirs*, put *theirs* into the side of *ours*,
406+
/// swapping the sides essentially.
390407
Ours,
391408
}
392409

gix-merge/src/tree/utils.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use std::collections::HashMap;
2323
/// over a directory rewrite in *our* tree. If so, rewrite it so that we get the path
2424
/// it would have had if it had been renamed along with *our* directory.
2525
pub fn possibly_rewritten_location(
26-
check_tree: &mut TreeNodes,
26+
check_tree: &TreeNodes,
2727
their_location: &BStr,
2828
our_changes: &ChangeListRef,
2929
) -> Option<BString> {
@@ -60,7 +60,7 @@ pub fn rewrite_location_with_renamed_directory(their_location: &BStr, passed_cha
6060
pub fn unique_path_in_tree(
6161
file_path: &BStr,
6262
editor: &tree::Editor<'_>,
63-
tree: &mut TreeNodes,
63+
tree: &TreeNodes,
6464
side_name: &BStr,
6565
) -> Result<BString, Error> {
6666
let mut buf = file_path.to_owned();
@@ -414,12 +414,12 @@ impl TreeNodes {
414414

415415
/// Search the tree with `our` changes for `theirs` by [`source_location()`](Change::source_location())).
416416
/// If there is an entry but both are the same, or if there is no entry, return `None`.
417-
pub fn check_conflict(&mut self, theirs_location: &BStr) -> Option<PossibleConflict> {
417+
pub fn check_conflict(&self, theirs_location: &BStr) -> Option<PossibleConflict> {
418418
if self.0.len() == 1 {
419419
return None;
420420
}
421421
let components = to_components(theirs_location);
422-
let mut cursor = &mut self.0[0];
422+
let mut cursor = &self.0[0];
423423
let mut cursor_idx = 0;
424424
let mut intermediate_change = None;
425425
for component in components {
@@ -436,7 +436,7 @@ impl TreeNodes {
436436
} else {
437437
// a change somewhere else, i.e. `a/c` and we know `a/b` only.
438438
intermediate_change.and_then(|(change, cursor_idx)| {
439-
let cursor = &mut self.0[cursor_idx];
439+
let cursor = &self.0[cursor_idx];
440440
// If this is a destination location of a rename, then the `their_location`
441441
// is already at the right spot, and we can just ignore it.
442442
if matches!(cursor.location, ChangeLocation::CurrentLocation) {
@@ -450,7 +450,7 @@ impl TreeNodes {
450450
}
451451
Some(child_idx) => {
452452
cursor_idx = child_idx;
453-
cursor = &mut self.0[cursor_idx];
453+
cursor = &self.0[cursor_idx];
454454
}
455455
}
456456
}
@@ -504,7 +504,7 @@ impl TreeNodes {
504504
cursor = &mut self.0[existing_idx];
505505
debug_assert!(
506506
cursor.is_leaf_node(),
507-
"BUG: we should really only try to remove leaf nodes"
507+
"BUG: we should really only try to remove leaf nodes: {cursor:?}"
508508
);
509509
cursor.change_idx = None;
510510
} else {
@@ -578,10 +578,7 @@ impl Conflict {
578578
ours: ours.clone(),
579579
theirs: theirs.clone(),
580580
entries,
581-
map: match outer_map {
582-
ConflictMapping::Original => map,
583-
ConflictMapping::Swapped => map.swapped(),
584-
},
581+
map: map.to_global(outer_map),
585582
}
586583
}
587584

Binary file not shown.

0 commit comments

Comments
 (0)