From a57192c0418aab2e3cd2ddb7b7a951cd3aaeb58f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 25 Nov 2024 16:54:25 +0100 Subject: [PATCH 01/20] fix: when binary merges are performed, adjust the returned resolution to indicate auto-resolution. Previously it wasn't possible to detect auto-resolution, even though it can be assumed if a resolution mode was provided. --- gix-merge/src/blob/builtin_driver/binary.rs | 6 ++- gix-merge/src/blob/platform/merge.rs | 22 +++++--- gix-merge/tests/merge/blob/builtin_driver.rs | 6 +-- gix-merge/tests/merge/blob/platform.rs | 53 +++++++++++++++++++- 4 files changed, 73 insertions(+), 14 deletions(-) diff --git a/gix-merge/src/blob/builtin_driver/binary.rs b/gix-merge/src/blob/builtin_driver/binary.rs index 6d4a9696584..1314a91be80 100644 --- a/gix-merge/src/blob/builtin_driver/binary.rs +++ b/gix-merge/src/blob/builtin_driver/binary.rs @@ -25,8 +25,10 @@ pub(super) mod function { use crate::blob::Resolution; /// As this algorithm doesn't look at the actual data, it returns a choice solely based on logic. + /// This also means that the caller has to assure this only gets called if the input *doesn't* match. /// - /// It always results in a conflict with `current` being picked unless `on_conflict` is not `None`. + /// It always results in a conflict with `current` being picked unless `on_conflict` is not `None`, + /// which is when we always return [`Resolution::CompleteWithAutoResolvedConflict`]. pub fn merge(on_conflict: Option) -> (Pick, Resolution) { match on_conflict { None => (Pick::Ours, Resolution::Conflict), @@ -36,7 +38,7 @@ pub(super) mod function { ResolveWith::Theirs => Pick::Theirs, ResolveWith::Ancestor => Pick::Ancestor, }, - Resolution::Complete, + Resolution::CompleteWithAutoResolvedConflict, ), } } diff --git a/gix-merge/src/blob/platform/merge.rs b/gix-merge/src/blob/platform/merge.rs index 460c24ad85b..45e6d052251 100644 --- a/gix-merge/src/blob/platform/merge.rs +++ b/gix-merge/src/blob/platform/merge.rs @@ -329,13 +329,21 @@ pub(super) mod inner { (Pick::Buffer, resolution) } BuiltinDriver::Binary => { - let (pick, resolution) = builtin_driver::binary(self.options.resolve_binary_with); - let pick = match pick { - builtin_driver::binary::Pick::Ours => Pick::Ours, - builtin_driver::binary::Pick::Theirs => Pick::Theirs, - builtin_driver::binary::Pick::Ancestor => Pick::Ancestor, - }; - (pick, resolution) + // easier to reason about the 'split' compared to merging both conditions + #[allow(clippy::if_same_then_else)] + if !(self.current.id.is_null() || self.other.id.is_null()) && self.current.id == self.other.id { + (Pick::Ours, Resolution::Complete) + } else if (self.current.id.is_null() || self.other.id.is_null()) && ours == theirs { + (Pick::Ours, Resolution::Complete) + } else { + let (pick, resolution) = builtin_driver::binary(self.options.resolve_binary_with); + let pick = match pick { + builtin_driver::binary::Pick::Ours => Pick::Ours, + builtin_driver::binary::Pick::Theirs => Pick::Theirs, + builtin_driver::binary::Pick::Ancestor => Pick::Ancestor, + }; + (pick, resolution) + } } BuiltinDriver::Union => { let resolution = builtin_driver::text( diff --git a/gix-merge/tests/merge/blob/builtin_driver.rs b/gix-merge/tests/merge/blob/builtin_driver.rs index 07b357c6a84..f34fb190205 100644 --- a/gix-merge/tests/merge/blob/builtin_driver.rs +++ b/gix-merge/tests/merge/blob/builtin_driver.rs @@ -10,16 +10,16 @@ fn binary() { ); assert_eq!( builtin_driver::binary(Some(ResolveWith::Ancestor)), - (Pick::Ancestor, Resolution::Complete), + (Pick::Ancestor, Resolution::CompleteWithAutoResolvedConflict), "Otherwise we can pick anything and it will mark it as complete" ); assert_eq!( builtin_driver::binary(Some(ResolveWith::Ours)), - (Pick::Ours, Resolution::Complete) + (Pick::Ours, Resolution::CompleteWithAutoResolvedConflict) ); assert_eq!( builtin_driver::binary(Some(ResolveWith::Theirs)), - (Pick::Theirs, Resolution::Complete) + (Pick::Theirs, Resolution::CompleteWithAutoResolvedConflict) ); } diff --git a/gix-merge/tests/merge/blob/platform.rs b/gix-merge/tests/merge/blob/platform.rs index c0d5050240c..6230377b821 100644 --- a/gix-merge/tests/merge/blob/platform.rs +++ b/gix-merge/tests/merge/blob/platform.rs @@ -59,12 +59,61 @@ mod merge { let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?; assert_eq!( res, - (Pick::Theirs, Resolution::Complete), + (Pick::Theirs, Resolution::CompleteWithAutoResolvedConflict), "the auto-binary driver respects its own options" ); Ok(()) } + #[test] + fn same_binaries_do_not_count_as_conflicted() -> crate::Result { + let mut platform = new_platform(None, pipeline::Mode::ToGit); + platform.set_resource( + gix_hash::Kind::Sha1.null(), + EntryKind::Blob, + "a".into(), + ResourceKind::CommonAncestorOrBase, + &gix_object::find::Never, + )?; + + let mut db = ObjectDb::default(); + for (content, kind) in [ + ("any\0", ResourceKind::CurrentOrOurs), + ("any\0", ResourceKind::OtherOrTheirs), + ] { + let id = db.insert(content); + platform.set_resource( + id, + EntryKind::Blob, + "path matters only for attribute lookup".into(), + kind, + &db, + )?; + } + let platform_ref = platform.prepare_merge(&db, Default::default())?; + assert_eq!( + platform_ref.driver, + DriverChoice::BuiltIn(BuiltinDriver::Text), + "it starts out at the default text driver" + ); + + let mut buf = Vec::new(); + let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?; + assert_eq!( + res, + (Pick::Ours, Resolution::Complete), + "as both are the same, it just picks ours, declaring it non-conflicting" + ); + + let mut input = imara_diff::intern::InternedInput::new(&[][..], &[]); + assert_eq!( + platform_ref.builtin_merge(BuiltinDriver::Binary, &mut buf, &mut input, default_labels()), + res, + "no way to work around it - calling the built-in binary merge is the same" + ); + Ok(()) + } + #[test] fn builtin_with_conflict() -> crate::Result { let mut platform = new_platform(None, pipeline::Mode::ToGit); @@ -192,7 +241,7 @@ theirs ] { platform_ref.options.resolve_binary_with = Some(resolve); let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?; - assert_eq!(res, (expected_pick, Resolution::Complete)); + assert_eq!(res, (expected_pick, Resolution::CompleteWithAutoResolvedConflict)); assert_eq!(platform_ref.buffer_by_pick(res.0).unwrap().unwrap().as_bstr(), expected); assert_eq!( From 1c3ba812bd3df5991a457b68a962aa1fd87fa915 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 25 Nov 2024 16:26:38 +0100 Subject: [PATCH 02/20] feat!: replace `tree::Options::allow_lossy_resolution` with `*::tree_conflicts`. That way it's possible to steer how to resolve tree-related conflicts while making it possible to detect that a conflict happened. --- gix-merge/src/blob/builtin_driver/text/mod.rs | 3 + gix-merge/src/commit/mod.rs | 1 + gix-merge/src/commit/virtual_merge_base.rs | 12 +- gix-merge/src/lib.rs | 1 + gix-merge/src/tree/function.rs | 3 +- gix-merge/src/tree/mod.rs | 145 +++++++++++------- .../generated-archives/tree-baseline.tar | Bin 2840576 -> 2910208 bytes gix-merge/tests/fixtures/tree-baseline.sh | 22 ++- gix-merge/tests/merge/tree/mod.rs | 33 ++-- 9 files changed, 133 insertions(+), 87 deletions(-) diff --git a/gix-merge/src/blob/builtin_driver/text/mod.rs b/gix-merge/src/blob/builtin_driver/text/mod.rs index 0252f423c40..198e0ba94d1 100644 --- a/gix-merge/src/blob/builtin_driver/text/mod.rs +++ b/gix-merge/src/blob/builtin_driver/text/mod.rs @@ -57,8 +57,11 @@ pub enum ConflictStyle { /// That way it becomes clearer where the content of conflicts are originating from. #[derive(Default, Copy, Clone, Debug, Eq, PartialEq)] pub struct Labels<'a> { + /// The label for the common *ancestor*. pub ancestor: Option<&'a BStr>, + /// The label for the *current* (or *our*) side. pub current: Option<&'a BStr>, + /// The label for the *other* (or *their*) side. pub other: Option<&'a BStr>, } diff --git a/gix-merge/src/commit/mod.rs b/gix-merge/src/commit/mod.rs index 4039f00d93c..801193c8b74 100644 --- a/gix-merge/src/commit/mod.rs +++ b/gix-merge/src/commit/mod.rs @@ -55,5 +55,6 @@ pub struct Outcome<'a> { pub(super) mod function; +/// pub mod virtual_merge_base; pub use virtual_merge_base::function::virtual_merge_base; diff --git a/gix-merge/src/commit/virtual_merge_base.rs b/gix-merge/src/commit/virtual_merge_base.rs index 86ca96a3f50..e67c5d2764d 100644 --- a/gix-merge/src/commit/virtual_merge_base.rs +++ b/gix-merge/src/commit/virtual_merge_base.rs @@ -30,7 +30,7 @@ pub enum Error { pub(super) mod function { use super::Error; use crate::blob::builtin_driver; - use crate::tree::TreatAsUnresolved; + use crate::tree::{treat_as_unresolved, TreatAsUnresolved}; use gix_object::FindExt; /// Create a single virtual merge-base by merging `first_commit`, `second_commit` and `others` into one. @@ -55,7 +55,7 @@ pub(super) mod function { let mut merged_commit_id = first_commit; others.push(second_commit); - options.allow_lossy_resolution = true; + options.tree_conflicts = Some(crate::tree::ResolveWith::Ancestor); options.blob_merge.is_virtual_ancestor = true; options.blob_merge.text.conflict = builtin_driver::text::Conflict::ResolveWithOurs; let favor_ancestor = Some(builtin_driver::binary::ResolveWith::Ancestor); @@ -86,10 +86,10 @@ pub(super) mod function { }, )?; // This shouldn't happen, but if for some buggy reason it does, we rather bail. - if out - .tree_merge - .has_unresolved_conflicts(TreatAsUnresolved::ConflictMarkers) - { + if out.tree_merge.has_unresolved_conflicts(TreatAsUnresolved { + content_merge: treat_as_unresolved::ContentMerge::Markers, + tree_merge: treat_as_unresolved::TreeMerge::Undecidable, + }) { return Err(Error::VirtualMergeBaseConflict.into()); } let merged_tree_id = out diff --git a/gix-merge/src/lib.rs b/gix-merge/src/lib.rs index e2e9d0ee625..ba49194046c 100644 --- a/gix-merge/src/lib.rs +++ b/gix-merge/src/lib.rs @@ -4,6 +4,7 @@ //! * [tree-merges](mod@tree) look at trees and merge them structurally, triggering blob-merges as needed. //! * [commit-merges](mod@commit) are like tree merges, but compute or create the merge-base on the fly. #![deny(rust_2018_idioms)] +#![deny(missing_docs)] #![forbid(unsafe_code)] /// diff --git a/gix-merge/src/tree/function.rs b/gix-merge/src/tree/function.rs index ca131db27a7..b91d56fe4dc 100644 --- a/gix-merge/src/tree/function.rs +++ b/gix-merge/src/tree/function.rs @@ -71,7 +71,8 @@ where let _span = gix_trace::coarse!("gix_merge::tree", ?base_tree, ?our_tree, ?their_tree, ?labels); let (mut base_buf, mut side_buf) = (Vec::new(), Vec::new()); let ancestor_tree = objects.find_tree(base_tree, &mut base_buf)?; - let allow_resolution_failure = !options.allow_lossy_resolution; + // TODO: properly handle 'Ours' as well. + let allow_resolution_failure = !matches!(options.tree_conflicts, Some(crate::tree::ResolveWith::Ancestor)); let mut editor = tree::Editor::new(ancestor_tree.to_owned(), objects, base_tree.kind()); let ancestor_tree = gix_object::TreeRefIter::from_bytes(&base_buf); diff --git a/gix-merge/src/tree/mod.rs b/gix-merge/src/tree/mod.rs index f6a534e916e..d025c7fa8b3 100644 --- a/gix-merge/src/tree/mod.rs +++ b/gix-merge/src/tree/mod.rs @@ -39,40 +39,47 @@ pub struct Outcome<'a> { /// Use [`has_unresolved_conflicts()`](Outcome::has_unresolved_conflicts()) to see if any action is needed /// before using [`tree`](Outcome::tree). pub conflicts: Vec, - /// `true` if `conflicts` contains only a single *unresolved* conflict in the last slot, but possibly more resolved ones. + /// `true` if `conflicts` contains only a single [*unresolved* conflict](ResolutionFailure) in the last slot, but + /// possibly more [resolved ones](Resolution) before that. /// This also makes this outcome a very partial merge that cannot be completed. /// Only set if [`fail_on_conflict`](Options::fail_on_conflict) is `true`. pub failed_on_first_unresolved_conflict: bool, } /// Determine what should be considered an unresolved conflict. +#[derive(Default, Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub struct TreatAsUnresolved { + /// Determine which content merges should be considered unresolved. + pub content_merge: treat_as_unresolved::ContentMerge, + /// Determine which tree merges should be considered unresolved. + pub tree_merge: treat_as_unresolved::TreeMerge, +} + /// -/// Note that no matter which variant, [conflicts](Conflict) with -/// [resolution failure](`ResolutionFailure`) will always be unresolved. -/// -/// Also, when one side was modified but the other side renamed it, this will not -/// be considered a conflict, even if a non-conflicting merge happened. -#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub enum TreatAsUnresolved { - /// Only consider content merges with conflict markers as unresolved. - /// - /// Auto-resolved tree conflicts will *not* be considered unresolved. - ConflictMarkers, - /// Consider content merges with conflict markers as unresolved, and content - /// merges where conflicts where auto-resolved in any way, like choosing - /// *ours*, *theirs* or by their *union*. - /// - /// Auto-resolved tree conflicts will *not* be considered unresolved. - ConflictMarkersAndAutoResolved, - /// Whenever there were conflicting renames, or conflict markers, it is unresolved. - /// Note that auto-resolved content merges will *not* be considered unresolved. - /// - /// Also note that files that were changed in one and renamed in another will - /// be moved into place, which will be considered resolved. - Renames, - /// Similar to [`Self::Renames`], but auto-resolved content-merges will - /// also be considered unresolved. - RenamesAndAutoResolvedContent, +pub mod treat_as_unresolved { + /// Which kind of content merges should be considered unresolved? + #[derive(Default, Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] + pub enum ContentMerge { + /// Content merges that still show conflict markers. + #[default] + Markers, + /// Content merges who would have conflicted if it wasn't for a + /// [resolution strategy](crate::blob::builtin_driver::text::Conflict::ResolveWithOurs). + ForcedResolution, + } + + /// Which kind of tree merges should be considered unresolved? + #[derive(Default, Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] + pub enum TreeMerge { + /// All failed renames. + Undecidable, + /// All failed renames, and the ones where a tree item was renamed to avoid a clash. + #[default] + EvasiveRenames, + /// All of `EvasiveRenames`, and tree merges that would have conflicted but which were resolved + /// with a [resolution strategy](super::ResolveWith). + ForcedResolution, + } } impl Outcome<'_> { @@ -174,35 +181,36 @@ impl Conflict { /// Return `true` if this instance is considered unresolved based on the criterion specified by `how`. pub fn is_unresolved(&self, how: TreatAsUnresolved) -> bool { use crate::blob; - let content_merge_matches = |info: &ContentMerge| match how { - TreatAsUnresolved::ConflictMarkers | TreatAsUnresolved::Renames => { - matches!(info.resolution, blob::Resolution::Conflict) - } - TreatAsUnresolved::RenamesAndAutoResolvedContent | TreatAsUnresolved::ConflictMarkersAndAutoResolved => { + let content_merge_matches = |info: &ContentMerge| match how.content_merge { + treat_as_unresolved::ContentMerge::Markers => matches!(info.resolution, blob::Resolution::Conflict), + treat_as_unresolved::ContentMerge::ForcedResolution => { matches!( info.resolution, blob::Resolution::Conflict | blob::Resolution::CompleteWithAutoResolvedConflict ) } }; - match how { - TreatAsUnresolved::ConflictMarkers | TreatAsUnresolved::ConflictMarkersAndAutoResolved => { + match how.tree_merge { + treat_as_unresolved::TreeMerge::Undecidable => { self.resolution.is_err() || self.content_merge().map_or(false, |info| content_merge_matches(&info)) } - TreatAsUnresolved::Renames | TreatAsUnresolved::RenamesAndAutoResolvedContent => match &self.resolution { - Ok(success) => match success { - Resolution::SourceLocationAffectedByRename { .. } => false, - Resolution::OursModifiedTheirsRenamedAndChangedThenRename { - merged_blob, - final_location, - .. - } => final_location.is_some() || merged_blob.as_ref().map_or(false, content_merge_matches), - Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => { - content_merge_matches(merged_blob) - } - }, - Err(_failure) => true, - }, + treat_as_unresolved::TreeMerge::EvasiveRenames | treat_as_unresolved::TreeMerge::ForcedResolution => { + match &self.resolution { + Ok(success) => match success { + Resolution::SourceLocationAffectedByRename { .. } => false, + Resolution::Forced(_) => how.tree_merge == treat_as_unresolved::TreeMerge::ForcedResolution, + Resolution::OursModifiedTheirsRenamedAndChangedThenRename { + merged_blob, + final_location, + .. + } => final_location.is_some() || merged_blob.as_ref().map_or(false, content_merge_matches), + Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => { + content_merge_matches(merged_blob) + } + }, + Err(_failure) => true, + } + } } } @@ -237,13 +245,8 @@ impl Conflict { /// Return information about the content merge if it was performed. pub fn content_merge(&self) -> Option { - match &self.resolution { - Ok(success) => match success { - Resolution::SourceLocationAffectedByRename { .. } => None, - Resolution::OursModifiedTheirsRenamedAndChangedThenRename { merged_blob, .. } => *merged_blob, - Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => Some(*merged_blob), - }, - Err(failure) => match failure { + fn failure_merged_blob(failure: &ResolutionFailure) -> Option { + match failure { ResolutionFailure::OursRenamedTheirsRenamedDifferently { merged_blob } => *merged_blob, ResolutionFailure::Unknown | ResolutionFailure::OursModifiedTheirsDeleted @@ -253,7 +256,16 @@ impl Conflict { } | ResolutionFailure::OursAddedTheirsAddedTypeMismatch { .. } | ResolutionFailure::OursDeletedTheirsRenamed => None, + } + } + match &self.resolution { + Ok(success) => match success { + Resolution::Forced(failure) => failure_merged_blob(failure), + Resolution::SourceLocationAffectedByRename { .. } => None, + Resolution::OursModifiedTheirsRenamedAndChangedThenRename { merged_blob, .. } => *merged_blob, + Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => Some(*merged_blob), }, + Err(failure) => failure_merged_blob(failure), } } } @@ -291,6 +303,9 @@ pub enum Resolution { /// The outcome of the content merge. merged_blob: ContentMerge, }, + /// This is a resolution failure was forcefully turned into a usable resolution, i.e. [making a choice](ResolveWith) + /// is turned into a valid resolution. + Forced(ResolutionFailure), } /// Describes of a conflict involving *our* change and *their* failed to be resolved. @@ -358,13 +373,26 @@ pub struct Options { /// If `None`, when symlinks clash *ours* will be chosen and a conflict will occur. /// Otherwise, the same logic applies as for the merge of binary resources. pub symlink_conflicts: Option, - /// If `true`, instead of issuing a conflict with [`ResolutionFailure`], do nothing and keep the base/ancestor - /// version. This is useful when one wants to avoid any kind of merge-conflict to have *some*, *lossy* resolution. - pub allow_lossy_resolution: bool, + /// If `None`, tree irreconcilable tree conflicts will result in [resolution failures](ResolutionFailure). + /// Otherwise, one can choose a side. Note that it's still possible to determine that auto-resolution happened + /// despite this choice, which allows carry forward the conflicting information, possibly for later resolution. + /// If `Some(…)`, irreconcilable conflicts are reconciled by making a choice. This mlso means that [`Conflict::entries()`] + /// won't be set as the conflict was officially resolved. + pub tree_conflicts: Option, +} + +/// Decide how to resolve tree-related conflicts, but only those that have [no way of being correct](ResolutionFailure). +#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub enum ResolveWith { + /// On irreconcilable conflict, choose neither *our* nor *their* state, but keep the common *ancestor* state instead. + Ancestor, + /// On irreconcilable conflict, choose *our* side + Ours, } pub(super) mod function; mod utils; +/// pub mod apply_index_entries { pub(super) mod function { @@ -398,6 +426,7 @@ pub mod apply_index_entries { for conflict in conflicts.iter().filter(|c| c.is_unresolved(how)) { let (renamed_path, current_path): (Option<&BStr>, &BStr) = match &conflict.resolution { Ok(success) => match success { + Resolution::Forced(_) => continue, Resolution::SourceLocationAffectedByRename { final_location } => { (Some(final_location.as_bstr()), final_location.as_bstr()) } diff --git a/gix-merge/tests/fixtures/generated-archives/tree-baseline.tar b/gix-merge/tests/fixtures/generated-archives/tree-baseline.tar index 325533a59a83b71145d73e245f284b601995841b..4c6bfb8f4a25c4f50895864d061f00d0cfd149ae 100644 GIT binary patch delta 8760 zcmd5?2~<HiL~sD7)&aDjI8;D$ zu~tPZPOmCe%dHk?Yb#dmz_a?Q{Lb@;6ZYrnJLldDM4#CY(>c+}5Nog9pX$2r z*x7CVV( zwW#s5g5ZZ`8!7&>jWnO|v_i)(J{SP`kkb4+_`2DgB%$qxwVMZ}&*?gM-`M%@Jk&bA zIf6ZgiasqW$c>=J!-Mew}Z16ZqsmHIb#Fb0A7Rcn4RgZ&ANNMKJc-~U(SmEcZ zGh^zS3Jj8(eu~8@50>iJ7c7A4@ep?fS5d5RbHT@(P`EqX`6je)RchCb8y^9rR6TMl zm_j-`l))&tgWtCy5%|CqpU9x<=IQtEb^6|LSg^llqkVku!^1)d{wmFE#9iqVj*C;g z`jmai#xu6dwZ+TRe2C>~zIh1Pq*Xe;HV#0Ol!88g16SI7dy~x3IeSSapfvL){sGMl zN%7J_CZsg;7&L7Z{>>;k|frGPIT!<7yE1YumS8FO}(YQVQjIXIyzxFjjcKyvwdAY16Hv_X_VNj@VMU`=|K< z5M>PFejht}pa z+3il7(~O~divhFPQI}QGTVZ#ZoG7M69TrUMbXaX>n;xv^G+9g*tKMmMnXOLLskiEI zYl1KmAKz8z=lu8e+6$t1qupD>OASIjKGY&)6TT6`!~NYenF3sxCmUM+oZ#RY?}h+Z zW(kRkEK&YPzyWF4XFtFbilmeS8XkZc7QwIsKl1xhOnzUQ*@$PiGAHIz%!#=)^C7L$4kgI}Y!dJSa}HZOhTWMNY_n4-Zck^}*8)GOOH8B3?1CS+s%I9K06>Qm4SV z7{DPZg>$)?Ds2h7#ePtt%YZfWp+xM#b5kjaD<(X9w|J5;e7kp_Qm?v2tG2Jl7FE3y zo4ri33HrNh!!* zl(-TNArn}(uz3i{1e9i~@az_5<)>FjCZIGk3g3x_6bO%d?;nsZdOFf^U@JdzQ%23j zrjbA4KtIB53<)d_)P;agC=sZWHj7vwym0?)&WM(jWn)?x_7$SGu;~ z4CFER5`qs$cEwrV&Fg7m{)>;DifyT@OOoSc3^rUFaKRt>{Cd;Q7E0s1_4_!{G>~_T z5)_c1f?EKAAfcYx83vZ6lC4L`6?o9zwZyF8y<&8u($I zAQR!`z^c~fj3Qbfg^RKcWL&*I{^rqgbgoY2%sFVp*Kqy^G~KyhhMW+Co*h=$OdkY46et2DQ*!c>D*VkG&< z4*#-eK_06JxKvRd*a3JTQ6gf!WQxI(7wk3#CA4h!$T%;f(PZ$j!~%a$H%Mtgkf9*% zbwKX*nI@;@K+3X=u+YC=OhJo*V~wad<)7gZxe+w0St<|Ky?VJaVR}z*dHHHUg0w5^ z@FidHqVT|etU!G3XYA0S?RvqMtE+&D<9qkX{d+CiY z<*hoDi}=|!2{8kQ#3Bs&yKG|3P<1K-Uuzb%87hY>#ub5`d$#O}MV6Ji>-?lko$9NEcQhBbt(&l5ZDv*9 zM-O??kIzf5@>s0$4Z}`|q!%`y*!(?L*m%A6qq?Py@k{4!I+?}2t^a%;@n-Q=w@cPp zc``(p(8*5_)jzXmSf)fZ1gY~u$UfG<|b9{w6eySYSF ziL||}45uT-6%HcB$P}=RI?qZC-9mVG@ntdD@SPs_&jTI^PZPCyfP52qy$67-${5$) z_ZTqbsu8&-~yQdyM^VO(HW2V(F>N8||p_<9S ztYJ8Rfb?znW;N2oG%(cTc6Y=|6+Yx9$SZj5zeT$RjdQrPE`wEPa_Q}$m4RUfD`-g= zv{r-7Vz9a_HVcM1b$EC!9ZoxkdMU{!ik;=zWwF8sBg-bw`Fee9_}bOE@te5U(1#L9h4uKf&BMmd z*!baliw{Uv#AFSR>~w_h*UujLcKxP7>zDgV60+CaALgw7cJYDg3G=&88*)5I^I?gh z>rnG#gcEonDR*q?|6K1i?aIh+fBEj3^|<)Vnn`onijTfmCd<$49nq!n&3`VQHR|57 z2EwM1MjxA&e0Rf#t1t3qC-ld|%S3%(D1~}h9@Lte-P!+M)e++ffTPZiabh z-Df|1X7kGM*7g6YWK#Tq$af393fSVj#oVMv*~GBN;)liJnMpIVKPZV>jTJr(N<4D? z(;G=MZl^B#+w(7`ZJU+jJ^A#Q5m~#}cN=$;SI#s232b^d*q9=-v2;N6} zQ1!ro+WwEci7BhiNy)NI#yA|8O63zZzu%2bQMh(0OYFs%ai?=WRHDj5iRpA2?3bWy zqqgn6ryN0E)VO4R^f@_`7JJe`+W|8&bYxcm=lcb`U8?Y-dY__KdY^HvGH;UMe%~L2 z`}qI?+eF%_CD>uxRQ&3rbaw&||JJV$)nEiK{yqi+_S%)14hQT|y+ieFK`WA|7$D8( zgXUS9_kcHU!NvxpRAb}pWn7soO$r*4TEZAPst?xNarSsjdi4-qxP@3VoRf-_Dmnrm zgHCWEhwcP}?ojda(NbdhXjxu3K(+Ekg|PsYq!j9TbL+f?BInSjzvOY6MF|GFg; zG9jgzAurEc_`=eaKmUGRBl@}uos-SER=zH%WT;L zauoE}+FE}(*xKB`4c*7?gxf654!n>2w*Vi z2m^x-MLwL%Ied2Zg56n?Qjj;_f}zkd`}Nsb&!qVEOqxH0;#0HVz=a6hk^@qjUqDSg zi;Ca%`Q%uS=xslzulc+F(R;HtNV?--OHbV?iVz%(cl%SB62bvnN>&gR$>C(S*rSG4 z^#_n7r68|6abr@}s3nO#a?U={jISIFwVZ;u$A^o_smV)Iot$u@ zA}1mr1B#}_43?MyDoH8Sh7VwmItou%7ZD?zae}uXr_K^M`zOPmpp^xx)TN@mH1yht za5yl6z4qda^$|qI`pBwC0GS&1V-;+Cl9YlxD{Q%YGw=c4kLcKJmWkMU?G7z=# zCLKBir68rHQt-TpsP~1B;~(G3O7TiBoU&(h-@BG4gBmU5Z@y2db7Pide2u<@NsWt}BQNB|@`RjU8cJGY-##{}4b@%04$XlB2hva@qF5 zlgF4fmCeqq8$4dv8E!X%U#Lkz+?!RhV%aAlKf+m7l1` z3iBT~ZAlK8G*2+~lGo(Zg$WsxwZ+iEV1Bn&4W@vr)Zn8bq^mk*EyRmXd4u=AmrpgG z&SEmdi@82FKPMCCH;KqQaV1jIC7Kru16EnR0#1%dN-f?1qN{IolJMtCUj?Ka_UR6H npOc#(es|=F?o4|A>K+gsN(~QR5MFo&nOgm$iHiQ6Gx+}kNOqsM delta 3757 zcmd6q`CC(08ppX95+n&p$PHkCn2?*r;*zimE`W>(ieg+^AGK5{35) zMe&|QJED)E)>;+8T(#DsovIymw44L7j8Xy*QmepwHtE_xV0gKKFdj zd)|A_d)|~>Y#>Swd_x=%lRgv1P^y-UU74H`E-qr1ikSB|Qly2PCroJ$ZyxR~P(Hu@ zNc+>Wz9o){jM;VY5@N#)b-CJ`QF{{|Q=o+*>S|d%w7|+*B&`7LX@p*x>Vki7aep+R z94}#c8sbLYi27#(%JIXXE{!l_^OoW|c5rbW2TRf5L~;``yt?e>`8D+!3SC%PZcyH( zZy$pOqky5a36)KiPQb^NQKb*+tc6B;)uD1U8&=k6k`7Qj9-2k3&p7xVg8GD5EeG>} zmB-5=t7=VGXfE@9v-3W9d$jR5L4S;U6JJdjl~$YitgdGK*# z?0kOVC(za?I_(TZD?`fKPM!6;%os&y;uM%e<@oI1XGSgzOGO+h$16cy3Ex7-{qkjs zjaQ*LkZ%9`ze6r=+F@KY9WB1QqKxk>6NtS~?%Xx!yWnuF?C`{RK-CB$5frWNoPi)x z4wis|8bJ=3yzxEo*{Pj1j`qq^M^d7mntHacMN1YFv|XUKsc8Xx+!-}JxZ|`R+&YpC zgMWff)ZnQ*TUe*)4_sVIJHV4PA`nbY@zBbjqzBoCjTA&|IwW@2l`bn9dw4(4*cEY1 zvr#aPpwm5cz%Ruu8fUW4BiOcifd{jBfiRzdk?I1N>{95DmGzIK0hK0<#c;=_s~AG% zAOOW2^sAjUA3>xXT#E9m1HEZNwEcAf&Mz+D;ZV6Z?11`ESSw#R$A0Ge*dRK0?^iLi ze|Qx*K6?$2%}~8VFBAGR{^`Q5@7~tfC{fpzw=cGelOpFyv3N9$%;5S;8V$3KmCbs> z9zgYpE#$S#EygW*V_f8SP8}PW&|Z3Q$M-+8Rj0;Ni}pw?-XmXnnQe}KG2`g>EbxJj z+Nh#E7U+nTbsT>PP^1K%Zf(?(Yd0{2%0VZv4#3sRjTl1ZU?XTECFqc7qn7sOVF;Cj z4B#mwi^*pvzgdyicbgNo>CqB7@LfYDRjb`_te4$kW$>23 zN6Of}Wre4qv0iaJO#qFtvc~a6fNGT`lCmo~{PwjC4ZkgX(YE=-$(I22&?|@Z-;io49&*NVbm#J(qr3xy{+awE&f)t+ zMZ1@?UH*BfugM(jVm5__n@z!{;1H9G$z(Q1LSbY6 zF9&OZ^$lGAfQ9QHaBv%F>Qh)qU14J6mj15;^%?fot(D8_8mIOgLq+Z$-J=lO#Qln0 z;(qVrD4oVtsYTEOE9)_77kkhXkNSwv8zauWLbK>cBsL~Xr!?{5s*m|F_G7+C=bjr~itQ$FrqmD1iC>e?_VzFKs&6g zUG#H6Wd~Tu3js%J`}U8zKC5+Fe)#=}q;8|el|88Rwg6ucFyDXDzyVn72A5|IU>sIe zfd;zjyh^tZhpt#z*9mvt6m+j_PY(>Ca`5h(f-aqDnSdZt4xax1g6>ta8#N<3q}&_# za6u=xew^uaGoR{DaX%Z96EJbkDpgHUwQLx@RR!z$dQ}&`Yu>idl#H@Q_3W(-4fa@e z6{E5b?W}|!RqtO_V1i%E#eo+2(hXyZn)cqQQa_%u zPFsFlU;K&6N;~7Odj=k?5~F?zpwLW9`@wUVV_omE+V#cO%uUG=+rU}Csm+DzI?|H zR5G!$zR?DJwB;U`v%6ylooK$@e|X8x+{khDCrW`#B*CRiN6?QcST1f-jP$1N*@Zz1L05`uzY{ikHA@O;;~McujG;hq^E zeQshV{E7`vhoJov z87&(F^MjSmPZDg_(S2V1`q+xQ7N@4GQ7~!$-`1}FlY#Ids0eqJ^39foo @@ -993,7 +1013,7 @@ git init type-change-to-symlink -# TODO: Git does not detect the conflict (one turns exe off, the other turns it on), and we do exactly the same. +baseline rename-add-same-symlink A-B A B baseline rename-add-exe-bit-conflict A-B A B baseline remove-executable-mode A-B A B baseline simple side-1-3-without-conflict side1 side3 diff --git a/gix-merge/tests/merge/tree/mod.rs b/gix-merge/tests/merge/tree/mod.rs index 662bcc67998..fc11087ae82 100644 --- a/gix-merge/tests/merge/tree/mod.rs +++ b/gix-merge/tests/merge/tree/mod.rs @@ -1,7 +1,7 @@ use crate::tree::baseline::Deviation; use gix_diff::Rewrites; use gix_merge::commit::Options; -use gix_merge::tree::TreatAsUnresolved; +use gix_merge::tree::{treat_as_unresolved, TreatAsUnresolved}; use gix_object::Write; use gix_worktree::stack::state::attributes; use std::path::Path; @@ -119,7 +119,10 @@ fn run_baseline() -> crate::Result { index } }; - let conflicts_like_in_git = TreatAsUnresolved::Renames; + let conflicts_like_in_git = TreatAsUnresolved { + content_merge: treat_as_unresolved::ContentMerge::Markers, + tree_merge: treat_as_unresolved::TreeMerge::EvasiveRenames, + }; let did_change = actual.index_changed_after_applying_conflicts(&mut actual_index, conflicts_like_in_git); actual_index.remove_entries(|_, _, e| e.flags.contains(gix_index::entry::Flags::REMOVE)); @@ -130,27 +133,15 @@ fn run_baseline() -> crate::Result { actual.conflicts, merge_info.conflicts ); - // if case_name.starts_with("submodule-both-modify-A-B") { - if false { - assert!( - !did_change, - "{case_name}: We can't handle submodules, so there is no index change" - ); - assert!( - actual.has_unresolved_conflicts(conflicts_like_in_git), - "{case_name}: submodules currently result in an unresolved (unknown) conflict" - ); - } else { - assert_eq!( - did_change, - actual.has_unresolved_conflicts(conflicts_like_in_git), - "{case_name}: If there is any kind of conflict, the index should have been changed" - ); - } + assert_eq!( + did_change, + actual.has_unresolved_conflicts(conflicts_like_in_git), + "{case_name}: If there is any kind of conflict, the index should have been changed" + ); } assert_eq!( - actual_cases, 107, + actual_cases, 109, "BUG: update this number, and don't forget to remove a filter in the end" ); @@ -163,6 +154,7 @@ fn basic_merge_options() -> Options { use_first_merge_base: false, tree_merge: gix_merge::tree::Options { symlink_conflicts: None, + tree_conflicts: None, rewrites: Some(Rewrites { copies: None, percentage: Some(0.5), @@ -172,7 +164,6 @@ fn basic_merge_options() -> Options { blob_merge_command_ctx: Default::default(), fail_on_conflict: None, marker_size_multiplier: 0, - allow_lossy_resolution: false, }, } } From 3228de627fd059db8abbad7f465023fa559b9b0e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 25 Nov 2024 20:40:54 +0100 Subject: [PATCH 03/20] adapt to changes in `gix-merge` --- gitoxide-core/src/repository/merge/commit.rs | 2 +- gitoxide-core/src/repository/merge/tree.rs | 2 +- gix/src/repository/merge.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gitoxide-core/src/repository/merge/commit.rs b/gitoxide-core/src/repository/merge/commit.rs index e95d4803a3c..ace09d5879f 100644 --- a/gitoxide-core/src/repository/merge/commit.rs +++ b/gitoxide-core/src/repository/merge/commit.rs @@ -49,7 +49,7 @@ pub fn commit( .merge_commits(ours_id, theirs_id, labels, options.into())? .tree_merge; let has_conflicts = res.conflicts.is_empty(); - let has_unresolved_conflicts = res.has_unresolved_conflicts(TreatAsUnresolved::Renames); + let has_unresolved_conflicts = res.has_unresolved_conflicts(TreatAsUnresolved::default()); { let _span = gix::trace::detail!("Writing merged tree"); let mut written = 0; diff --git a/gitoxide-core/src/repository/merge/tree.rs b/gitoxide-core/src/repository/merge/tree.rs index 924b581048b..425ab578c14 100644 --- a/gitoxide-core/src/repository/merge/tree.rs +++ b/gitoxide-core/src/repository/merge/tree.rs @@ -64,7 +64,7 @@ pub(super) mod function { }; let res = repo.merge_trees(base_id, ours_id, theirs_id, labels, options)?; let has_conflicts = res.conflicts.is_empty(); - let has_unresolved_conflicts = res.has_unresolved_conflicts(TreatAsUnresolved::Renames); + let has_unresolved_conflicts = res.has_unresolved_conflicts(TreatAsUnresolved::default()); { let _span = gix::trace::detail!("Writing merged tree"); let mut written = 0; diff --git a/gix/src/repository/merge.rs b/gix/src/repository/merge.rs index 4756ac2433b..1eef30283a2 100644 --- a/gix/src/repository/merge.rs +++ b/gix/src/repository/merge.rs @@ -108,7 +108,7 @@ impl Repository { fail_on_conflict: None, marker_size_multiplier: 0, symlink_conflicts: None, - allow_lossy_resolution: false, + tree_conflicts: None, } .into()) } From e487cca78d8e6c5b51d2614daf05c98e1469ee69 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 2 Dec 2024 16:10:55 +0100 Subject: [PATCH 04/20] implement support for resolving irreconcilable tree conflicts with 'ours' or 'ancestor' --- crate-status.md | 3 + gix-merge/src/tree/function.rs | 518 +++++++++---- gix-merge/src/tree/mod.rs | 82 +- gix-merge/src/tree/utils.rs | 31 +- .../generated-archives/tree-baseline.tar | Bin 2910208 -> 3367424 bytes gix-merge/tests/fixtures/tree-baseline.sh | 705 +++++++++++++++++- gix-merge/tests/merge/tree/baseline.rs | 25 +- gix-merge/tests/merge/tree/mod.rs | 106 ++- 8 files changed, 1268 insertions(+), 202 deletions(-) diff --git a/crate-status.md b/crate-status.md index 9027fb8240d..730982b2961 100644 --- a/crate-status.md +++ b/crate-status.md @@ -352,8 +352,11 @@ Check out the [performance discussion][gix-diff-performance] as well. - [ ] a way to control inter-hunk merging based on proximity (maybe via `gix-diff` feature which could use the same) * [x] **tree**-diff-heuristics match Git for its test-cases - [x] a way to generate an index with stages, mostly conforming with Git. + - [ ] resolve to be *ours* or the *ancestors* version of the tree. - [ ] submodule merges (*right now they count as conflicts if they differ*) - [ ] assure sparse indices are handled correctly during application - right now we refuse. + - [ ] rewrite so that the whole logic can be proven to be correct - it's too insane now and probably has way + more possible states than are tested, despite best attempts. * [x] **commits** - with handling of multiple merge bases by recursive merge-base merge * [x] API documentation * [ ] Examples diff --git a/gix-merge/src/tree/function.rs b/gix-merge/src/tree/function.rs index b91d56fe4dc..782f34b99c5 100644 --- a/gix-merge/src/tree/function.rs +++ b/gix-merge/src/tree/function.rs @@ -5,7 +5,7 @@ use crate::tree::utils::{ use crate::tree::ConflictMapping::{Original, Swapped}; use crate::tree::{ Conflict, ConflictIndexEntry, ConflictIndexEntryPathHint, ConflictMapping, ContentMerge, Error, Options, Outcome, - Resolution, ResolutionFailure, + Resolution, ResolutionFailure, ResolveWith, }; use bstr::{BString, ByteSlice}; use gix_diff::tree::recorder::Location; @@ -71,11 +71,9 @@ where let _span = gix_trace::coarse!("gix_merge::tree", ?base_tree, ?our_tree, ?their_tree, ?labels); let (mut base_buf, mut side_buf) = (Vec::new(), Vec::new()); let ancestor_tree = objects.find_tree(base_tree, &mut base_buf)?; - // TODO: properly handle 'Ours' as well. - let allow_resolution_failure = !matches!(options.tree_conflicts, Some(crate::tree::ResolveWith::Ancestor)); - let mut editor = tree::Editor::new(ancestor_tree.to_owned(), objects, base_tree.kind()); let ancestor_tree = gix_object::TreeRefIter::from_bytes(&base_buf); + let tree_conflicts = options.tree_conflicts; let mut our_changes = Vec::new(); if ours_needs_diff { @@ -129,7 +127,12 @@ where let mut conflicts = Vec::new(); let mut failed_on_first_conflict = false; - let mut should_fail_on_conflict = |conflict: Conflict| -> bool { + let mut should_fail_on_conflict = |mut conflict: Conflict| -> bool { + if tree_conflicts.is_some() { + if let Err(failure) = conflict.resolution { + conflict.resolution = Ok(Resolution::Forced(failure)); + } + } if let Some(how) = options.fail_on_conflict { if conflict.resolution.is_err() || conflict.is_unresolved(how) { failed_on_first_conflict = true; @@ -239,7 +242,7 @@ where PossibleConflict::TreeToNonTree { change_idx: Some(idx) } if matches!( our_changes[idx].inner, - Change::Deletion { .. } | Change::Addition { .. } + Change::Deletion { .. } | Change::Addition { .. } | Change::Rewrite { .. } ) => { (Some(idx), Some(MatchKind::EraseTree)) @@ -256,34 +259,76 @@ where PossibleConflict::Match { change_idx } | PossibleConflict::PassedRewrittenDirectory { change_idx } => Some(change_idx), } - .map(|idx| &our_changes[idx]); + .map(|idx| &mut our_changes[idx]); if let Some(ours) = ours { gix_trace::debug!("Turning a case we could probably handle into a conflict for now. theirs: {theirs:#?} ours: {ours:#?} kind: {match_kind:?}"); - if allow_resolution_failure - && should_fail_on_conflict(Conflict::unknown(( - &ours.inner, - theirs, - Original, - outer_side, - ))) - { + let conflict = Conflict::unknown((&ours.inner, theirs, Original, outer_side)); + if let Some(ResolveWith::Ours) = tree_conflicts { + apply_our_resolution(&ours.inner, theirs, outer_side, &mut editor)?; + *match outer_side { + Original => &mut ours.was_written, + Swapped => &mut their_changes[theirs_idx].was_written, + } = true; + } + if should_fail_on_conflict(conflict) { break 'outer; }; - continue; + } else if matches!(candidate, PossibleConflict::TreeToNonTree { .. }) { + let (mode, id) = theirs.entry_mode_and_id(); + let location = theirs.location(); + let renamed_location = unique_path_in_tree( + location.as_bstr(), + &editor, + their_tree, + labels.other.unwrap_or_default(), + )?; + match tree_conflicts { + None => { + editor.upsert(toc(&renamed_location), mode.kind(), id.to_owned())?; + } + Some(ResolveWith::Ours) => { + if outer_side.is_swapped() { + editor.upsert(to_components(location), mode.kind(), id.to_owned())?; + } + } + Some(ResolveWith::Ancestor) => { + // we found no matching node of 'ours', so nothing to apply here. + } + } + + let conflict = Conflict::without_resolution( + ResolutionFailure::OursDirectoryTheirsNonDirectoryTheirsRenamed { + renamed_unique_path_of_theirs: renamed_location, + }, + (theirs, theirs, Original, outer_side), + [ + None, + None, + index_entry_at_path( + &mode.kind().into(), + &id.to_owned(), + ConflictIndexEntryPathHint::RenamedOrTheirs, + ), + ], + ); + their_changes[theirs_idx].was_written = true; + if should_fail_on_conflict(conflict) { + break 'outer; + }; + } else if matches!(candidate, PossibleConflict::NonTreeToTree { .. }) { + // We are writing on top of what was a file, a conflict we probably already saw and dealt with. + let location = theirs.location(); + let (mode, id) = theirs.entry_mode_and_id(); + editor.upsert(to_components(location), mode.kind(), id.to_owned())?; + their_changes[theirs_idx].was_written = true; } else { gix_trace::debug!("Couldn't figure out how to handle {match_kind:?} theirs: {theirs:#?} candidate: {candidate:#?}"); - continue; } + continue; }; let ours = &our_changes[ours_idx].inner; - debug_assert!( - match_kind.is_none() - || (ours.location() == theirs.location() - || ours.source_location() == theirs.source_location()), - "BUG: right now it's not known to be possible to match changes from different paths: {match_kind:?} {candidate:?}" - ); match (ours, theirs) { ( Change::Modification { @@ -327,7 +372,7 @@ where Swapped }; if let Some(merged_mode) = merge_modes(*our_mode, *their_mode) { - assert_eq!( + debug_assert_eq!( previous_id, their_source_id, "both refer to the same base, so should always match" ); @@ -408,9 +453,23 @@ where (new_change, None), pick_our_changes_mut(side, their_changes, our_changes), ); - } else if allow_resolution_failure { - editor.upsert(toc(our_location), our_mode.kind(), *our_id)?; - editor.upsert(toc(their_location), their_mode.kind(), *their_id)?; + } else { + match tree_conflicts { + None => { + // keep both states - 'our_location' is the previous location as well. + editor.upsert(toc(our_location), our_mode.kind(), *our_id)?; + editor.upsert(toc(their_location), their_mode.kind(), *their_id)?; + } + Some(ResolveWith::Ours) => { + editor.remove(toc(source_location))?; + if side.to_global(outer_side).is_swapped() { + editor.upsert(toc(their_location), their_mode.kind(), *their_id)?; + } else { + editor.upsert(toc(our_location), our_mode.kind(), *our_id)?; + } + } + Some(ResolveWith::Ancestor) => {} + } if should_fail_on_conflict(Conflict::without_resolution( ResolutionFailure::OursModifiedTheirsRenamedTypeMismatch, @@ -517,7 +576,7 @@ where &options, )?; editor.upsert(toc(location), merged_mode.kind(), merged_blob_id)?; - Some(Conflict::with_resolution( + Conflict::with_resolution( Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob: ContentMerge { resolution, @@ -526,8 +585,8 @@ where }, (ours, theirs, Original, outer_side), [None, index_entry(our_mode, our_id), index_entry(their_mode, their_id)], - )) - } else if allow_resolution_failure { + ) + } else { // Actually this has a preference, as symlinks are always left in place with the other side renamed. let ( logical_side, @@ -556,8 +615,7 @@ where tree_with_rename, label_of_side_to_be_moved, )?; - editor.upsert(toc(location), our_mode.kind(), our_id)?; - let conflict = Conflict::without_resolution( + let mut conflict = Conflict::without_resolution( ResolutionFailure::OursAddedTheirsAddedTypeMismatch { their_unique_location: renamed_location.clone(), }, @@ -568,27 +626,45 @@ where index_entry_at_path(&their_mode, &their_id, their_path_hint), ], ); - - let new_change = Change::Addition { - location: renamed_location, - entry_mode: their_mode, - id: their_id, - relation: None, - }; - tree_with_rename.remove_existing_leaf(location.as_bstr()); - push_deferred( - (new_change, None), - pick_our_changes_mut(logical_side, their_changes, our_changes), - ); - Some(conflict) - } else { - None + match tree_conflicts { + None => { + let new_change = Change::Addition { + location: renamed_location, + entry_mode: their_mode, + id: their_id, + relation: None, + }; + editor.upsert(toc(location), our_mode.kind(), our_id)?; + tree_with_rename.remove_existing_leaf(location.as_bstr()); + push_deferred( + (new_change, None), + pick_our_changes_mut(logical_side, their_changes, our_changes), + ); + } + Some(resolve) => { + conflict.entries = Default::default(); + match resolve { + ResolveWith::Ours => match outer_side { + Original => { + editor.upsert(toc(location), our_mode.kind(), our_id)?; + } + Swapped => { + editor.upsert(toc(location), their_mode.kind(), their_id)?; + } + }, + ResolveWith::Ancestor => { + // Do nothing - this discards both sides. + // Note that one of these adds might be the result of a rename, which + // means we effectively loose the original and can't get it back as that information is degenerated. + } + } + } + } + conflict }; - if let Some(conflict) = conflict { - if should_fail_on_conflict(conflict) { - break 'outer; - }; + if should_fail_on_conflict(conflict) { + break 'outer; } } ( @@ -610,7 +686,7 @@ where previous_entry_mode, previous_id, }, - ) if allow_resolution_failure => { + ) => { let (label_of_side_to_be_moved, side) = if matches!(ours, Change::Modification { .. }) { (labels.current.unwrap_or_default(), Original) } else { @@ -623,62 +699,135 @@ where }; change_on_right .map(|change| { - change.inner.entry_mode().is_tree() && change.inner.location() == location + change.inner.entry_mode().is_tree() + && change.inner.location() == location + && matches!(change.inner, Change::Addition { .. }) }) .unwrap_or_default() }; - if deletion_prefaces_addition_of_directory { - let our_tree = pick_our_tree(side, our_tree, their_tree); - let renamed_path = unique_path_in_tree( - location.as_bstr(), - &editor, - our_tree, - label_of_side_to_be_moved, - )?; - editor.remove(toc(location))?; - our_tree.remove_existing_leaf(location.as_bstr()); + let should_break = if deletion_prefaces_addition_of_directory { + let entries = [ + index_entry(previous_entry_mode, previous_id), + index_entry(entry_mode, id), + None, + ]; + match tree_conflicts { + None => { + let our_tree = pick_our_tree(side, our_tree, their_tree); + let renamed_path = unique_path_in_tree( + location.as_bstr(), + &editor, + our_tree, + label_of_side_to_be_moved, + )?; + editor.remove(toc(location))?; + our_tree.remove_existing_leaf(location.as_bstr()); - let new_change = Change::Addition { - location: renamed_path.clone(), - relation: None, - entry_mode: *entry_mode, - id: *id, - }; - let should_break = should_fail_on_conflict(Conflict::without_resolution( - ResolutionFailure::OursModifiedTheirsDirectoryThenOursRenamed { - renamed_unique_path_to_modified_blob: renamed_path, - }, - (ours, theirs, side, outer_side), - [ - index_entry(previous_entry_mode, previous_id), - index_entry(entry_mode, id), - None, - ], - )); + let new_change = Change::Addition { + location: renamed_path.clone(), + relation: None, + entry_mode: *entry_mode, + id: *id, + }; + let should_break = should_fail_on_conflict(Conflict::without_resolution( + ResolutionFailure::OursModifiedTheirsDirectoryThenOursRenamed { + renamed_unique_path_to_modified_blob: renamed_path, + }, + (ours, theirs, side, outer_side), + entries, + )); - // Since we move *our* side, our tree needs to be modified. - push_deferred( - (new_change, None), - pick_our_changes_mut(side, our_changes, their_changes), - ); + // Since we move *our* side, our tree needs to be modified. + push_deferred( + (new_change, None), + pick_our_changes_mut(side, our_changes, their_changes), + ); + should_break + } + Some(ResolveWith::Ours) => { + match side.to_global(outer_side) { + Original => { + // ours is modification + editor.upsert(toc(location), entry_mode.kind(), *id)?; + } + Swapped => { + // ours is deletion + editor.remove(toc(location))?; + } + } + should_fail_on_conflict(Conflict::without_resolution( + ResolutionFailure::OursModifiedTheirsDeleted, + (ours, theirs, side, outer_side), + entries, + )) + } + Some(ResolveWith::Ancestor) => { + should_fail_on_conflict(Conflict::without_resolution( + ResolutionFailure::OursModifiedTheirsDeleted, + (ours, theirs, side, outer_side), + entries, + )) + } + } + } else { + let entries = [ + index_entry(previous_entry_mode, previous_id), + index_entry(entry_mode, id), + None, + ]; + match tree_conflicts { + None => { + editor.upsert(toc(location), entry_mode.kind(), *id)?; + } + Some(ResolveWith::Ours) => { + let ours = match outer_side { + Original => ours, + Swapped => theirs, + }; - if should_break { - break 'outer; + match ours { + Change::Modification { .. } => { + editor.upsert(toc(location), entry_mode.kind(), *id)?; + } + Change::Deletion { .. } => { + editor.remove(toc(location))?; + } + _ => unreachable!("parent-match assures this"), + }; + } + Some(ResolveWith::Ancestor) => {} }; - } else { - let should_break = should_fail_on_conflict(Conflict::without_resolution( + should_fail_on_conflict(Conflict::without_resolution( ResolutionFailure::OursModifiedTheirsDeleted, (ours, theirs, side, outer_side), - [ - index_entry(previous_entry_mode, previous_id), - index_entry(entry_mode, id), - None, - ], - )); - editor.upsert(toc(location), entry_mode.kind(), *id)?; - if should_break { - break 'outer; + entries, + )) + }; + if should_break { + break 'outer; + }; + } + ( + Change::Modification { .. }, + Change::Addition { + location, + entry_mode, + id, + .. + }, + ) if ours.location() != theirs.location() => { + match tree_conflicts { + None => { + unreachable!("modification/deletion pair should prevent modification/addition from happening") + } + Some(ResolveWith::Ancestor) => {} + Some(ResolveWith::Ours) => { + if outer_side.is_swapped() { + editor.upsert(toc(location), entry_mode.kind(), *id)?; + } + // we have already taken care of the 'root' of this - + // everything that follows can safely be ignored } } } @@ -721,9 +870,11 @@ where let merged_mode = merge_modes(*our_mode, *their_mode).expect("this case was assured earlier"); - editor.remove(toc(source_location))?; - our_tree.remove_existing_leaf(source_location.as_bstr()); - their_tree.remove_existing_leaf(source_location.as_bstr()); + if matches!(tree_conflicts, None | Some(ResolveWith::Ours)) { + editor.remove(toc(source_location))?; + our_tree.remove_existing_leaf(source_location.as_bstr()); + their_tree.remove_existing_leaf(source_location.as_bstr()); + } let their_rewritten_location = possibly_rewritten_location(our_tree, their_location.as_bstr(), our_changes); @@ -755,11 +906,12 @@ where ); // Pretend this is the end of the loop and keep this as conflict. // If this happens in the wild, we'd want to reproduce it. - if allow_resolution_failure - && should_fail_on_conflict(Conflict::unknown(( - ours, theirs, Original, outer_side, - ))) - { + if let Some(ResolveWith::Ours) = tree_conflicts { + apply_our_resolution(ours, theirs, outer_side, &mut editor)?; + } + if should_fail_on_conflict(Conflict::unknown(( + ours, theirs, Original, outer_side, + ))) { break 'outer; }; their_changes[theirs_idx].was_written = true; @@ -778,11 +930,6 @@ where }), ) } else { - if !allow_resolution_failure { - their_changes[theirs_idx].was_written = true; - our_changes[ours_idx].was_written = true; - continue; - } if should_fail_on_conflict(Conflict::without_resolution( ResolutionFailure::OursRenamedTheirsRenamedDifferently { merged_blob: resolution.take().map(|resolution| ContentMerge { @@ -811,19 +958,37 @@ where )) { break 'outer; }; - let our_addition = Change::Addition { - location: our_location.to_owned(), - relation: None, - entry_mode: merged_mode, - id: merged_blob_id, - }; - let their_addition = Change::Addition { - location: their_location.to_owned(), - relation: None, - entry_mode: merged_mode, - id: merged_blob_id, - }; - (Some(our_addition), Some(their_addition)) + match tree_conflicts { + None => { + let our_addition = Change::Addition { + location: our_location.to_owned(), + relation: None, + entry_mode: merged_mode, + id: merged_blob_id, + }; + let their_addition = Change::Addition { + location: their_location.to_owned(), + relation: None, + entry_mode: merged_mode, + id: merged_blob_id, + }; + (Some(our_addition), Some(their_addition)) + } + Some(ResolveWith::Ancestor) => (None, None), + Some(ResolveWith::Ours) => { + let our_addition = Change::Addition { + location: match outer_side { + Original => our_location, + Swapped => their_location, + } + .to_owned(), + relation: None, + entry_mode: merged_mode, + id: merged_blob_id, + }; + (Some(our_addition), None) + } + } } } }; @@ -884,16 +1049,21 @@ where .. }, Change::Deletion { .. }, - ) if !rewritten_mode.is_commit() && allow_resolution_failure => { + ) if !rewritten_mode.is_commit() => { let side = if matches!(ours, Change::Deletion { .. }) { Original } else { Swapped }; - editor.remove(toc(source_location))?; - pick_our_tree(side, our_tree, their_tree) - .remove_existing_leaf(source_location.as_bstr()); + match tree_conflicts { + None | Some(ResolveWith::Ours) => { + editor.remove(toc(source_location))?; + pick_our_tree(side, our_tree, their_tree) + .remove_existing_leaf(source_location.as_bstr()); + } + Some(ResolveWith::Ancestor) => {} + } let their_rewritten_location = possibly_rewritten_location( pick_our_tree(side, our_tree, their_tree), @@ -924,10 +1094,15 @@ where break 'outer; }; - push_deferred( - (our_addition, None), - pick_our_changes_mut(side, their_changes, our_changes), - ); + let ours_is_rewrite = side.is_swapped(); + if tree_conflicts.is_none() + || (matches!(tree_conflicts, Some(ResolveWith::Ours)) && ours_is_rewrite) + { + push_deferred( + (our_addition, None), + pick_our_changes_mut(side, their_changes, our_changes), + ); + } } ( Change::Rewrite { @@ -942,6 +1117,7 @@ where Change::Addition { id: their_id, entry_mode: their_mode, + location: add_location, .. }, ) @@ -949,6 +1125,7 @@ where Change::Addition { id: their_id, entry_mode: their_mode, + location: add_location, .. }, Change::Rewrite { @@ -1006,9 +1183,17 @@ where // Because this constellation can only be found by the lookup tree, there is // no need to put it as addition, we know it's not going to intersect on the other side. editor.upsert(toc(location), merged_mode.kind(), merged_blob_id)?; - } else if allow_resolution_failure { - editor.remove(toc(source_location))?; - pick_our_tree(side, our_tree, their_tree).remove_leaf(source_location.as_bstr()); + } else { + // We always remove the source from the tree - it might be re-added later. + let ours_is_rename = + tree_conflicts == Some(ResolveWith::Ours) && side == outer_side; + let remove_rename_source = + tree_conflicts.is_none() || ours_is_rename || add_location != source_location; + if remove_rename_source { + editor.remove(toc(source_location))?; + pick_our_tree(side, our_tree, their_tree) + .remove_leaf(source_location.as_bstr()); + } let ( logical_side, @@ -1037,7 +1222,13 @@ where tree_with_rename, label_of_side_to_be_moved, )?; - editor.upsert(toc(location), our_mode.kind(), our_id)?; + + let upsert_rename_destination = tree_conflicts.is_none() || ours_is_rename; + if upsert_rename_destination { + editor.upsert(toc(location), our_mode.kind(), our_id)?; + tree_with_rename.remove_existing_leaf(location.as_bstr()); + } + let conflict = Conflict::without_resolution( ResolutionFailure::OursAddedTheirsAddedTypeMismatch { their_unique_location: renamed_location.clone(), @@ -1050,20 +1241,21 @@ where ], ); - let new_change_with_rename = Change::Addition { - location: renamed_location, - entry_mode: their_mode, - id: their_id, - relation: None, - }; - tree_with_rename.remove_existing_leaf(location.as_bstr()); - push_deferred( - ( - new_change_with_rename, - Some(pick_idx(logical_side, theirs_idx, ours_idx)), - ), - pick_our_changes_mut(logical_side, their_changes, our_changes), - ); + if tree_conflicts.is_none() { + let new_change_with_rename = Change::Addition { + location: renamed_location, + entry_mode: their_mode, + id: their_id, + relation: None, + }; + push_deferred( + ( + new_change_with_rename, + Some(pick_idx(logical_side, theirs_idx, ours_idx)), + ), + pick_our_changes_mut(logical_side, their_changes, our_changes), + ); + } if should_fail_on_conflict(conflict) { break 'outer; @@ -1071,9 +1263,16 @@ where } } _unknown => { - if allow_resolution_failure - && should_fail_on_conflict(Conflict::unknown((ours, theirs, Original, outer_side))) - { + debug_assert!( + match_kind.is_none() + || (ours.location() == theirs.location() + || ours.source_location() == theirs.source_location()), + "BUG: right now it's not known to be possible to match changes from different paths: {match_kind:?} {candidate:?}" + ); + if let Some(ResolveWith::Ours) = tree_conflicts { + apply_our_resolution(ours, theirs, outer_side, &mut editor)?; + } + if should_fail_on_conflict(Conflict::unknown((ours, theirs, Original, outer_side))) { break 'outer; }; } @@ -1099,6 +1298,19 @@ where }) } +fn apply_our_resolution( + local_ours: &Change, + local_theirs: &Change, + outer_side: ConflictMapping, + editor: &mut gix_object::tree::Editor<'_>, +) -> Result<(), Error> { + let ours = match outer_side { + Original => local_ours, + Swapped => local_theirs, + }; + Ok(apply_change(editor, ours, None)?) +} + fn involves_submodule(a: &EntryMode, b: &EntryMode) -> bool { a.is_commit() || b.is_commit() } diff --git a/gix-merge/src/tree/mod.rs b/gix-merge/src/tree/mod.rs index d025c7fa8b3..8779b55be00 100644 --- a/gix-merge/src/tree/mod.rs +++ b/gix-merge/src/tree/mod.rs @@ -57,6 +57,8 @@ pub struct TreatAsUnresolved { /// pub mod treat_as_unresolved { + use crate::tree::TreatAsUnresolved; + /// Which kind of content merges should be considered unresolved? #[derive(Default, Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub enum ContentMerge { @@ -80,6 +82,34 @@ pub mod treat_as_unresolved { /// with a [resolution strategy](super::ResolveWith). ForcedResolution, } + + /// Instantiation/Presets + impl TreatAsUnresolved { + /// Return an instance with the highest sensitivity to what should be considered unresolved as it + /// includes entries which have been resolved using a [merge strategy](super::ResolveWith). + pub fn forced_resolution() -> Self { + Self { + content_merge: ContentMerge::ForcedResolution, + tree_merge: TreeMerge::ForcedResolution, + } + } + + /// Return an instance that considers unresolved any conflict that Git would also consider unresolved. + /// This is the same as the `default()` implementation. + pub fn git() -> Self { + Self::default() + } + + /// Only undecidable tree merges and conflict markers are considered unresolved. + /// This also means that renamed entries to make space for a conflicting one is considered acceptable, + /// making this preset the most lenient. + pub fn undecidable() -> Self { + Self { + content_merge: ContentMerge::Markers, + tree_merge: TreeMerge::Undecidable, + } + } + } } impl Outcome<'_> { @@ -157,7 +187,7 @@ enum ConflictIndexEntryPathHint { } /// A utility to help define which side is what in the [`Conflict`] type. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] enum ConflictMapping { /// The sides are as described in the field documentation, i.e. `ours` is `ours`. Original, @@ -175,13 +205,19 @@ impl ConflictMapping { ConflictMapping::Swapped => ConflictMapping::Original, } } + fn to_global(self, global: ConflictMapping) -> ConflictMapping { + match global { + ConflictMapping::Original => self, + ConflictMapping::Swapped => self.swapped(), + } + } } impl Conflict { /// Return `true` if this instance is considered unresolved based on the criterion specified by `how`. pub fn is_unresolved(&self, how: TreatAsUnresolved) -> bool { use crate::blob; - let content_merge_matches = |info: &ContentMerge| match how.content_merge { + let content_merge_unresolved = |info: &ContentMerge| match how.content_merge { treat_as_unresolved::ContentMerge::Markers => matches!(info.resolution, blob::Resolution::Conflict), treat_as_unresolved::ContentMerge::ForcedResolution => { matches!( @@ -192,20 +228,28 @@ impl Conflict { }; match how.tree_merge { treat_as_unresolved::TreeMerge::Undecidable => { - self.resolution.is_err() || self.content_merge().map_or(false, |info| content_merge_matches(&info)) + self.resolution.is_err() + || self + .content_merge() + .map_or(false, |info| content_merge_unresolved(&info)) } treat_as_unresolved::TreeMerge::EvasiveRenames | treat_as_unresolved::TreeMerge::ForcedResolution => { match &self.resolution { Ok(success) => match success { Resolution::SourceLocationAffectedByRename { .. } => false, - Resolution::Forced(_) => how.tree_merge == treat_as_unresolved::TreeMerge::ForcedResolution, + Resolution::Forced(_) => { + how.tree_merge == treat_as_unresolved::TreeMerge::ForcedResolution + || self + .content_merge() + .map_or(false, |merged_blob| content_merge_unresolved(&merged_blob)) + } Resolution::OursModifiedTheirsRenamedAndChangedThenRename { merged_blob, final_location, .. - } => final_location.is_some() || merged_blob.as_ref().map_or(false, content_merge_matches), + } => final_location.is_some() || merged_blob.as_ref().map_or(false, content_merge_unresolved), Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => { - content_merge_matches(merged_blob) + content_merge_unresolved(merged_blob) } }, Err(_failure) => true, @@ -249,6 +293,7 @@ impl Conflict { match failure { ResolutionFailure::OursRenamedTheirsRenamedDifferently { merged_blob } => *merged_blob, ResolutionFailure::Unknown + | ResolutionFailure::OursDirectoryTheirsNonDirectoryTheirsRenamed { .. } | ResolutionFailure::OursModifiedTheirsDeleted | ResolutionFailure::OursModifiedTheirsRenamedTypeMismatch | ResolutionFailure::OursModifiedTheirsDirectoryThenOursRenamed { @@ -321,6 +366,17 @@ pub enum ResolutionFailure { /// The path at which `ours` can be found in the tree - it's in the same directory that it was in before. renamed_unique_path_to_modified_blob: BString, }, + /// *ours* is a directory, but *theirs* is a non-directory (i.e. file), which wants to be in its place, even though + /// *ours* has a modification in that subtree. + /// Rename *theirs* to retain that modification. + /// + /// Important: there is no actual modification on *ours* side, so *ours* is filled in with *theirs* as the data structure + /// cannot represent this case. + // TODO: Can we have a better data-structure? This would be for a rewrite though. + OursDirectoryTheirsNonDirectoryTheirsRenamed { + /// The non-conflicting path of *their* non-tree entry. + renamed_unique_path_of_theirs: BString, + }, /// *ours* was added (or renamed into place) with a different mode than theirs, e.g. blob and symlink, and we kept /// the symlink in its original location, renaming the other side to `their_unique_location`. OursAddedTheirsAddedTypeMismatch { @@ -376,8 +432,10 @@ pub struct Options { /// If `None`, tree irreconcilable tree conflicts will result in [resolution failures](ResolutionFailure). /// Otherwise, one can choose a side. Note that it's still possible to determine that auto-resolution happened /// despite this choice, which allows carry forward the conflicting information, possibly for later resolution. - /// If `Some(…)`, irreconcilable conflicts are reconciled by making a choice. This mlso means that [`Conflict::entries()`] - /// won't be set as the conflict was officially resolved. + /// If `Some(…)`, irreconcilable conflicts are reconciled by making a choice. + /// Note that [`Conflict::entries()`] will still be set, to not degenerate information, even though they then represent + /// the entries what would fit the index if no forced resolution was performed. + /// It's up to the caller to handle that information mindfully. pub tree_conflicts: Option, } @@ -386,7 +444,10 @@ pub struct Options { pub enum ResolveWith { /// On irreconcilable conflict, choose neither *our* nor *their* state, but keep the common *ancestor* state instead. Ancestor, - /// On irreconcilable conflict, choose *our* side + /// On irreconcilable conflict, choose *our* side. + /// + /// Note that in order to get something equivalent to *theirs*, put *theirs* into the side of *ours*, + /// swapping the sides essentially. Ours, } @@ -439,6 +500,9 @@ pub mod apply_index_entries { } }, Err(failure) => match failure { + ResolutionFailure::OursDirectoryTheirsNonDirectoryTheirsRenamed { + renamed_unique_path_of_theirs, + } => (Some(renamed_unique_path_of_theirs.as_bstr()), conflict.ours.location()), ResolutionFailure::OursRenamedTheirsRenamedDifferently { .. } => { (Some(conflict.theirs.location()), conflict.ours.location()) } diff --git a/gix-merge/src/tree/utils.rs b/gix-merge/src/tree/utils.rs index 318916ff981..db624605c49 100644 --- a/gix-merge/src/tree/utils.rs +++ b/gix-merge/src/tree/utils.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; /// over a directory rewrite in *our* tree. If so, rewrite it so that we get the path /// it would have had if it had been renamed along with *our* directory. pub fn possibly_rewritten_location( - check_tree: &mut TreeNodes, + check_tree: &TreeNodes, their_location: &BStr, our_changes: &ChangeListRef, ) -> Option { @@ -60,7 +60,7 @@ pub fn rewrite_location_with_renamed_directory(their_location: &BStr, passed_cha pub fn unique_path_in_tree( file_path: &BStr, editor: &tree::Editor<'_>, - tree: &mut TreeNodes, + tree: &TreeNodes, side_name: &BStr, ) -> Result { let mut buf = file_path.to_owned(); @@ -400,11 +400,11 @@ impl TreeNodes { Some(index) => { cursor = &mut self.0[index]; if is_last && !cursor.is_leaf_node() { - assert_eq!( - cursor.change_idx, None, - "BUG: each node should only see a single change when tracking initially: {path} {change_idx}" - ); - cursor.change_idx = Some(change_idx); + // NOTE: we might encounter the same path multiple times in rare conditions. + // At least we avoid overwriting existing intermediate changes, for good measure. + if cursor.change_idx.is_none() { + cursor.change_idx = Some(change_idx); + } } } } @@ -414,12 +414,12 @@ impl TreeNodes { /// Search the tree with `our` changes for `theirs` by [`source_location()`](Change::source_location())). /// If there is an entry but both are the same, or if there is no entry, return `None`. - pub fn check_conflict(&mut self, theirs_location: &BStr) -> Option { + pub fn check_conflict(&self, theirs_location: &BStr) -> Option { if self.0.len() == 1 { return None; } let components = to_components(theirs_location); - let mut cursor = &mut self.0[0]; + let mut cursor = &self.0[0]; let mut cursor_idx = 0; let mut intermediate_change = None; for component in components { @@ -436,7 +436,7 @@ impl TreeNodes { } else { // a change somewhere else, i.e. `a/c` and we know `a/b` only. intermediate_change.and_then(|(change, cursor_idx)| { - let cursor = &mut self.0[cursor_idx]; + let cursor = &self.0[cursor_idx]; // If this is a destination location of a rename, then the `their_location` // is already at the right spot, and we can just ignore it. if matches!(cursor.location, ChangeLocation::CurrentLocation) { @@ -450,7 +450,7 @@ impl TreeNodes { } Some(child_idx) => { cursor_idx = child_idx; - cursor = &mut self.0[cursor_idx]; + cursor = &self.0[cursor_idx]; } } } @@ -496,7 +496,7 @@ impl TreeNodes { let mut cursor = &mut self.0[0]; while let Some(component) = components.next() { match cursor.children.get(component).copied() { - None => assert!(!must_exist, "didn't find '{location}' for removal"), + None => debug_assert!(!must_exist, "didn't find '{location}' for removal"), Some(existing_idx) => { let is_last = components.peek().is_none(); if is_last { @@ -504,7 +504,7 @@ impl TreeNodes { cursor = &mut self.0[existing_idx]; debug_assert!( cursor.is_leaf_node(), - "BUG: we should really only try to remove leaf nodes" + "BUG: we should really only try to remove leaf nodes: {cursor:?}" ); cursor.change_idx = None; } else { @@ -578,10 +578,7 @@ impl Conflict { ours: ours.clone(), theirs: theirs.clone(), entries, - map: match outer_map { - ConflictMapping::Original => map, - ConflictMapping::Swapped => map.swapped(), - }, + map: map.to_global(outer_map), } } diff --git a/gix-merge/tests/fixtures/generated-archives/tree-baseline.tar b/gix-merge/tests/fixtures/generated-archives/tree-baseline.tar index 4c6bfb8f4a25c4f50895864d061f00d0cfd149ae..c2ab2fe2918c100d0bf58c24937e5434d567d1bd 100644 GIT binary patch delta 48081 zcmeHwd3+P))_*2x(tSyrbV=GY-S;M$Og2D-vV-i%CLmd9Sz0V*5fG3aM0Pr5Qv?J= zL_}Z!Q4vuT0Rh=XHbqbn*+c}D-+5*xOS807-+SNJKYsUq?u~ZlnP)rCIp;a&d(N43 z^gV3S{=?W7Wo$~NeMH63QTC$dMI}X}%c@F?%;i@5h^mUp;;KqJso8+#EZiAHmP&$D z6xk)a4CCt!WI-{OOG6{E;z4CqDxYuuquIEI=2{f4L$m40f-v{ctOB zXPc-+>LA=eaB4G?ZG)#O$mub{7?H+6Xlw6!`_Xo#&0b}QON_)9^m|a;_YOYek z-z&{krS?kYh|-FYLv7_bRfX`RvSM^u`5>izl)duBs#5fi6%L2eiZ+roCq%rCNH+m{ z+V8xJnD47^n4e>R(6OUg>*^uW$TW;*msjJSSp-NMSL%N9V;Vp?1~hARWkPB$-29&VY}>|i1WD--!itv zB!N0eV?dixKf$hWx1nl|M3e?xf!Ej7;+Zz?iU?4l7))Ay;89&!WQ@DXXcW(CgAMA8BJ0VFB z996}H!$N7UWLM%OGq$*k_Sc9fgazE9&2&GD zO21GC5gG$0(%M2dSx_n`r>x~mgFFXEVIzc3A_6VjN9>$thod>^-E*R_%lsUS%3UWq z8y$cFA7$GfRX|%p=U#IP`#$%HC{UMY*k*_L>L9%b^Iebqkst`r)p@p!I!-0js%v$1 zpkM$-J-=VHI*^>WM7Lc~n!IRgUkOz+Hlq|g|2Xm~%cT^$rbR|M`!tWPom&*~=I zY*VsIh-=iIi*cF%Bq&gCciCp`qu{9b2rk7|mrDY@8@s%CB2eAw6G#IwNx7CWAPpDa~ps)Ib40);JzeiCqWg>0)$$*|DtBteE+ zyL2|C1-7X5KqY091g{h{@+Z;#3PZm-Tb`R~;cE9PcniWn>xi=>#Zy&6gI0lb!CXAfmc9?^i@G_KW|cBKTWPl0l=fl6t6o&*m~)i5 zqf5)IrOL7q%AsXL>~3AlQCTreX)b5qwAqJ(E@o3Y%7)s>v}Ogf0&QyB)GSZAmO+yU zUj%9%9xfJY4QkJWoDrOh<#L_LqOLpJVkPf4Z|exiv750w=IT#_tI=LN^(VSN6UQYc zyB3SvQELXu`eO@}p@HVxlI?P10!(pIYz$THNErNI+;@Vb2&MNuE~j~p(pqXRA7mc^ zo2?o#(xM`p1@reN@_4)|e}|upZ0jQ71Q0_Skoy`nT*Y4s}JXe)d4u-mv3 z$^YdcdYv3TPcS{uN$SC#BoN`$(t#m^0F^kdbgj&PGS%fZ1c6He18^$@|!M7&g(~`IAmTX_9+~dfnglwq~>7EA{-Lb1q&= zZE|TrT2$jW+nT>W%-lS73#sq8e4Du_%aGgn!nT7EzdS?q?el9%<-tBlQ@$zh(QT_a z@8tYF?G7$Fv*GTpPqHeHZ;c*sFLO=On~R&4*{>WMrz;&1QJtLBoYcQ7?bYa|d)kRz z&c-*Ly1hx4Ps*&Dc6>VN@f$6w-me(a@6P&8AK!g`^s+~LUf;Yy7_ADuKl8gNNyg5h zzr>9@_x*#Hj+k3d_;lg(NmKux8kTW#`=leMZ9hztw^_UA_R5F+y|K^bp^a13xgli4 zxoBobXswi{Z4c{(ypAm)<;w83^k$xjRS5|#?n!(XLqjksG(G%xEV&MHu#=MZV&)ru zyXQ_3MxDxGGT6X`XM%0Escj}bPFTPkV$~ZB4x`0k(`g8^#$sY9qBJ5U0kkqNihwzI zYHD|aLa_9KYC;Vbwyte0XVTM;o_@Hv``@2bsbQ1KatQZYZ0kFBS;YPZpmex zkY#CCz=KlB@&lUoxvBqEY?I>=pxIdju|c|8H$4OY3BT_3l^%^`*4JOB+?Us4Qt8!3 zLT9kzdI#>XX!K@>O>ffLEMQ2{X>B^2-9%W`Y6H8xQXmMT7B{*W8jZ=Ox5CFs`^eAg zGZQvjcW*vN&6=Tk5Xt*kA5NvU%s7P=)I}?dWz@+6%w*a?smaYtyzWNar)JixbYQ7b zn+XSw{$o%RX814A3~qNAv__52W&n!@;ieqE+Ca_N9W@TiFUV`(@mckDH;oK8hYx3TVYCSgHdO(n=}rc)k+Wsi%p}|7zqM58nq^N(;A`M!|-6Skih8+t&1^=u!t>N zTyr#&>iL4ito)ws;snYrxy2nVrGND)xRrP5tHvGekFQvNdHTWg(mp%mc#Gf2M|;$Q zYQ_&87VuDvVJRG)e=U9g_L!^0ANwIA$EGG`>vU?|sL>L7LSr_mwO6BuzwUg~{gv~^CEKTT{%qnPKJUIc z>M)g7oOUAMsONf&TCexI{>%K0nvr8<`u(7i1k^lA%zig+lKryj-Is2wc0{Z0hyC)xn(jaU za_Ga>Rp+qV8Jo7~Hox0$%9C?DPCtFRZ`x?`yoHN9pSF091Rdv_|Kp zF$AEG8n(?oun-b>E&JQ2ae?)<-R`can#{D)x&>!UQ<29ZMYQ#gwvn)?xY?tbZCsHE zE>V8Y5YScT9uC{5j+y$@wY&$XrmSkWyF51LSevsQsr)mkgTtDBBz#k`;?qM1_U`@R zjT^Uwd*&_2-{SXqea-$xT}lr3{49=YAKbBPLU`i*EoCR>G%0&txqU^y9$!wD^=lD) zZN;|hZ@txOPpge1e%NmtQ%spBS0q3E;;Dqu-?te3;`L7R=jU3s<;Cw9wV=9PKA8(y&I=`kYRUUef)OQH?I6OqWZ%gKSus8Z#xVzyq@)_0Wn?D-D-eVRx+c@QtyOX7i$tFHJk};;HM0Z7X$6V}9H}t9@wiqq`@aiXCDY zGIHIK<;`*>bFS5_zWSNXAh%lHyM9-aG5E&1AM)mR-_~JR%7n|er&Rpe{Kg;qRd2oX zZp@|*BQA`6=W@GwU0Wo7H?{Gf?|r)R?|a3seDXoB?><)*t_TWg-Mw$p+J$?MHg1>F zMz#Hy^?RN=c<;>pQH{%g-P`Nd>)k#sE3p4?>#u!3hqvj}pG@l!(}Dyi$(;sh2Np7I zA*&rQf(6~#JkDpAz%1HcThFjqRThg?=fJHd++;D>wRWA=Yz85tHsM;_qSIQAZH3`;pYERk+n?F#;aIMA)rrAWgWA*!4QQ$!LfFGZ0_{C2VBc zT>*0>OVM>ir}ngqBK=Q3T^halGT?UH zWw&E%hB`fx<>Ap70xh?{K-a{vp1jas3x2CpyUmCGmt6<Ez`P|SU_j1NXY=1 zng~)7z<1$gZGcOP`%~+&i6K}H!HrjuTSAm3%t8+ABGd_yKgB{8zQ`vcCzSAb8XjZa zvyfdY`AOtnTa4Uk-6Icn5ULdHW93l0qe@v;u0#}69LB?WW@rw#YMHE4hFM8_-bxxP zhgFP1Fs8C9WmSa|-hdV(ssJESyr_BG{6L1)2=~1CS~WG}uIxoD*V53*wRcbMh5v)e zRZrl{DA`cHD6s)>MImnRFeWYrK-h4#U2P|{7Q0zZXpIJ&+F^kI!ZqAt z)~j(!+&AnjR^48ZPA2sgme3S8Tm+U~b#@$8vo&J`hVh+PNAmh~zAe2+w^ps-`qY?* zD>Y#=f4Hf&H6!MJ^YzX$(Zz4#J{>K3-At~Yr$~?8G`7#2pu{(=L*AJ`rRjqUt9s^; z?=?#MyLv7^lavn-Hlw@h6am)Tnfw_mT`)u!Yx3k5o(M^UC-p9oVA)1HaAU1j4^SE{ z0SFka)nPH}LHD*fY+8-hsx=tYdXop}1%{xcl-Yui9yvx(bvd*qHTQ$0^nkYtG=f@y zjE<=0XI#{_3yZ4rmtVAudlGH6U${4U94y1w>mHhh|Nv;Q$xB8|alT6r<;6aL41XXE(GRzJgxfX#; z>0btAm~@TI3<^RB)NFCvl-B5>IHz|r=AleW4h;$#A~`*HR)^EoCr#Gd@4nrAdDrOI z_K#bGjNwwOIwYuwR9+Gr*v(hZs1XHGH6r1}?)9H@HQfjy*WgIk5(9jzo-CH7vH0L> zn=pY)oFI8-(GyBYmV&jJn$}miaA_Tf3j`eD`Z}oJuCLQb7GU;S3&rf$*WDM6fgY0N z%<8)is<#f&fYBxe+{u9BqEAPAA?WM~5xrxmC{lt@!MfdLs}L#}{W5!^jQ4~b>vmt+ z$vnTZlYO4x0Xfd)^%^@Fp3}eV^N!@&NusWjD~8>lZeP$g`my1UaeG!Sdimw7Vb7DW zr@v31EZQ+SI5HNIglg2%1BfKhFVj3ZjLcjdY(fu-u(sRLL-fl&Y)Muv4jv-O=jHY4 zy>DPt%bMF)Uj03N=8@lCZ;$9D605EWR?bjX2UE)GkVpONClSUchcqe;;RPqkwd=s@ zid0se-2paps|k>db`82k8jvyFtT*X!o84h!mbCg&h>T1g7~Xc#-;{_fy`G(t06xUp zN`!_mc(7pviVefa@ag8=KkK$v5{!;?%O8p)&#d<521Cpz7KzxLNNoz$QU&8iT=X z*XkWsz1gBM<9dxvgET=IJuq~i&}#H<4#3!x<|T6TbeE zj(2!T_#@^%QN2Gjoy^onG-I~3Fbu}VffltOn;5MU?6n9gwV~haZ;fK&T0KmHSdJsUt7#Z?oXgsNi z4U_VlZ6^!I2-3)B%B2ZGNfQfOBOx;n+W}oFNNH>WGbRAlQ#DsYXJce$sdMP15pZuc zDu!cJ@Zg9)uW)Q&mI?#KamKS<9@M4Zyr6n{byGgGz z1J2iBve|G80e^zO!EW00Mze!3IsoG}!MWgQGP@(BeD_rgrXs?~;4^|&@-3}eu6p-Q z-ugeM{4^j|{9JLr3kj#b5pyvejN&h?Oj1gZrnKtqr{wKAn+_q!750Lilz8$vED zc=xBOH7UF==SO_Ok{=p_vY*H~2B1Dk*ti^yn!|sTvn2-_Pi?4&g6~uan?=^e^+h_+DhDCA>NP zu-oZf^h5Y%AND54vkwJ9)CBYp{jv{DNb+Toe^|ni?1XPdM={@yj$+^FLuOjTUzg0e zc-ZmO@B`m>-1?WpzQ|PGBx=U21>(HwvEj*7vMv11hyT){Wk%dt{;wO_j(rhwWBro6 zkQG9}=*D1W5LMF)lvX?|`v`;BujWU}N%^CQGIHuRu~gu}W!^t;B_+=kPkVBmhZ%nyKKZIZQVJNvx5{(qg4Dp#ad(#i$mwgyX21%ob zOP03DIL3Q@R^!)-l8SonFF2NQ`1kIld7fZQwJ|!5Oq?xw#%UHi5dv3{_-LvoK4x;C z`l-{!jVZEeBt%FW4P@4A=M7=U@EF0d-t9cEpOM8#xk5onxx5is8oe zYYm7dTU(6tma#`1nT{xfhqt!Eg44guf+rct%nRanaSK-FV;zQ7O}g>yU%w|mH1*8; z?Mv#79-`*~Kb=;6Mw~n&`I4AQz7%^0QCLp+!(V=f@6f-@cT%d!%%5Y2NG9G~n){*S zK+5-P2V5P~^0S4jjw!2TO(QKxEWp6RE7@KoL7;mKOc;b^gV~9r9lh>PM+OSN ztYgxV&hX3OVlb;3JZ+(9&$Yuw(UAdQb!0K)mYhQ6@QUFhhnlPG=x;enwBBrJ0162h zfyDn|>}kLI;bOtw~n;p3Ayl!>?Y;hadJa2YzjY}jo zZBx@daGfXuI)uj>deKEifgWZ|WdC9X@xzLq&h+(eVBsrMMlHS>wRP~{_MhesAltO= zc!9b2ZMby$=I~}?p8F}a-~Kiq#wE7QokAk{g|2(k{&*>&T};Qw>P%|Q=QAFDd{Xkp zu1#SH?cbc;@<2$`0_#^ZzZobpwWU!Vw7??=&l3y`K#!6B11;+!nLvS-b+z#gv<$Et zK7p3GI*r<7Aq?Q`CE#jqQ$v)MLt}xz5a1;>gEMsNeuGHPd_X$FdST#sc0gK^k| z;}#8$o9u9%wHh2c9T?ZG06Ei{^Z;x!0d^5rn<;Ti;n_e3J2G&%BlM7iTfxL&(Axk8 zW^kC*7Q0nr1_&5#F%U+z75ut(xJ>Jv;f?_oCqNitT?=1-7-E3F>J39&oqjIbFC&et zFA6b0kAb2PDTqP@xKMyr^~NFYO<&`m;YS7vLk!SUePM_Jn$;JE7@(QoFvI}w)*FTx zpq-yE#P@S&o(T@L*7bxT2Iz%r7-CGuYXbiSRd;w`dXnz`D*$py&r@RJe0f`C3@ABaANLsfLc`TC@HdeF#lr| zR2X0`&N#E#NsxNMpz{oRp@Yh}4w7EEZed(sS8{!k4k~+Ym>IdSE0oO}&_P2M&0KrY za>BSSdBf&GqM)*W)2aF_5Lx&JO$DHz`huncG;;<`b;Dd?o2t{K;cG~#5MUcrP;Fi0#nII@8YfNNENzk=`x0|ZLw zbbuiuASH4XAfWeprBFg}= zHFZfsF0nOrEh-r(wx)rL_`elfQ`cN-6G-6Lng&ko|0uSmj+q9Gt!e1Y{!Otpb#uG4 zy-6lpevssoVtlS5ca)+l>P6)z6V&A0ND4sHXs1RT<*6AM!z8= z@EDJ1U^6d+dpL_D-s_LywYSY#h_m4!xRFvaR?xO=)V}+i|homh)e* z#(&vJx+OX0u}rh!#_}%&3);_KTvPJgJUJ|Q?Eq!>wiQ(I*p2(>D3kZlmL-iwGHSvWQ}t>ww%QD z)!lTVizI^w(CS6nzJNNv7i>K4l`rj`rFF7moF7+gzq5Y%XXre9yi87eCrI?V`0Pwb{95iR)J`zMcR3%>K77HhTB=>A7D& zsu=RihS#3!+SGhoaDVsr!@t{?7Ey6@@VD_7|M*e#aM|Oeo!kBvyg1-t{se6H)~*i` zB<<;d;;^2mggVYW8}sB06$pqZ448HDANfV<=InKa3^1o9C9r^3)fqCtsd?JkycP^h ziw!w&b0(&JYMtHt2{~YzrPb5SGvuIeF!03^p;+iA>_EuRsB=f1Nj{vABbWt6?#V5~ zs+E!{N#I@5dL0-*mDLK7M*hmu+JjXNvsVtXb2d9En=2t{p+m3NSu8q})nNdbqh15h zE~GaC+RkB6+s!5|Bn~iG17o1=G_bY`Q~Y;@=hMz!5$HCdSX1?Z3KM__(ja~jd}o9Nvilk%8N zWXtR&*AJq(a!WjFhhd^%Bb%Mv*j5rdk!u{0vX>oFyOrDA>EGy7yKNx$#8v^h!^$hF z=;wuy5e{CetSlR4M*%Emr3;j*96r=+WmEK`WO8Qfkg|yozG7C6EH8Uuq@A_mgmGAx z228f@C5&{Z6ooY;Rr3|`9744ZWng|q_hMF-!GSv&F zJxM~tv?obA16unI^V9rgNnCBb_Inp5rS4qO$=3BzpT_B*ojh=&*@7Wv4@@?~bnhPc z%c!3&b(nW#>AjaD-kI|8hdai6>vZ7z!)y0>W5#E9_ugA^hW~O&;oY;%hJ}0-Mmz86 z|6BA-+oI~$(qgK3s^sV8bwi2_C}TS;VSFlrY-?OfS z>#PPFLZHQL384X4=#8_F(5MV{7?U5$a8#Nr#M?-!hHxQb_T;W1w($-X|P{ezF zMjR=Qh^FIq$70oer4X7lP>PAjj8mb_ncagS2xvzLqtw#aq@mKZ9F2<8ToEpV8P8S$ z$o2CO9)#;X;QK5(dxbQXl+BmMi$uIhS(2^@baW7_9x6>I^Bsal3zQ-BlxR@4LpsjR z5u4Qzij~y`<@4NS02grE33sC7>Qhn$r8p%0S-d{viZn#vC>urJpBQBJqS6P7Mbo3i zLXFldz^3|=R8GYz_*b#&htd!#7o+$)<0Wd^dbdCsPLKOhJUY7h!!S~G zCO#%dui`X#_%DjAZmX>1GXX4{YrLwT) z0%aIGK0v}>k019OnS3a&tusPoQUd(-sB91*f14&q0dWR2X)zg1IvbciAz;A@t_`&g z{2mrF=(Ge%1fkYC(8N7PElZF9*e(%({vSc7@nH#a{rr6!j{nj1%C;$&W`&*`@Z}xh zXnnF#NDqC?&{V@`D^E9?@N(=EYRURhy$b){{Ba_0@y^^9?9hz~fNgJ?aJK`OW8csN zfQZq>;(g=A{m`Mt%xO{BeC_c4ko6&p6GHeEpFuE=`xTxv={R)wMh!3J4-0 z4I!0V(7n>^d{4r(C9d^%UBW$sDvO;%q{v%vmE-FjWbPiWfd6uyD-1KW zE^**JBDI;!o$DM?^~QumDsg|}ZwP#y6ZGYs8whNRep#?>7MXcIafoC{f#twP`%>xZ z9Va%=ZnkjJf|(IC!S~0ie@HAKA^gsj>21c+3yIXy3rQn7q&dwMBg$-cya@fj#;c|a zP%IPYi6j6d1gy2Exmt}*V}b-AxDl8q+)=F>s{s-Pnh2X6{#C8lm;l`e`GVN661^;9 zeMG)2DBIa@)o8nUh=%SMXFB$dz{J_@0`}Ke4s6@3B`grz1mV$E6Qt3D*l4R+57{fg zePV(zD0pbISYSZz9?6Je*^7KVPR;l^59y5eBjmKGNeL}@y!>L-cXR$qzaA@%efie1 zb+=`_kUSXw7(d&o0?0f&4l4*Y}eRw_o$iAnGou8zm z7@CjAzId^(Zs&EKxY4vX_kMC<;-JTazN!9t&hX4yLl!~^4TNG2Mh&>9 ztl*WxJpj1SYYY{G%l^!Qgbz&bzI*37W_9!q#Z@p9nXp;r)b9JC5BHY+S&-%YFhhpS z(}YuhLb$73yTM{2basQysa}ypEKo8LX#DmMreN)^h@_$pStQ5H|8oQsBw z6JpwA>A)C}j~iq!Wn^)h1A;8q?k~ub-LuN?G8rhp9{~w=S|ALd8_ashcBqA_8+MD)U^bh; z_F~hSj2Z`oa1xMu0f)>`dId8M+UxMQ3{v9Jne|=y4pE5Y7Br47a_Pj>jq93ZyqCGX zaQn>W+kY&bedKdtTI$$NXKr+QFZ}Tksv^6`;a#(?J@+-W;<+`$%fH_B*R|)QN2rzG zjw%0Z*N4|0y#8&tm}_4{`9zcz|5JoVbQqP3$}e*4AkDK9L&^}+Ipr+eO>I9I!) z>i6A_jk+XW+4#RY?v~-wO=ykklx$o(GILT$vjxf+c1K)zN_Lo!rNm`2F)1SuMTq3#@(rhwJ03f`Q~5)SuS3Uump95h@s|gOgLr#Oi!m2PNFmv# z2$VZU`J~mj@$zwEwDI5wk8m&5VEH&K$E*c)w0@>cYGgttTaJ@#sG6bC&f~HL4RXI+ zn$i5^;z60!YpdUk*Z3Xk)Qu)`R2MlRFCl8Gzuq(YyNNH)d0hNgr;}}pUeTsvJX?Av z37W|BSoM5)JejY=8j)QdQ%!+@3T z2JD+KNXOg!TjyvrXb{(1e;)Un}5)Vap7QuAezW*&Z(BYjR2JsLVj3&07d~D+X4a6Yp}~=T6BZr=qws z-H2}cP}~{%Wi+Z-iX7TEIg4!9H#vfSW9`NF=r`b(ePb7TgARC8%c3jLL-fl&oX$j~ zp6#1lDEV~6q?4mRyF8$*=KNFNexwOttPMML?FPTc|m-0Bgo?AH3n}_W| zV?goXPHP#Gr=FQDs#m2Xk}18DLB=q-qG-|7R~QFzmJ1duhY3W79rQpH zg~9ZA*WF{wJopY@F~da4Z5b_T?$SONlGC68JH4N2DJU0vGlewUsDk^V&0#i}O?q&K z;1*E)%r?;b%w|3GYc^}`YKw+c)<{$6Po0Q`GFi2v%{z+Fw@MVrqG;Z^(8x{8n~Gecpq7>@W?*D$GxkqR9%*SDwRB->ImGUlh#+>KZrM)}OQy@hO5Bz% z!kuT1!k7uD71AuoS6csKkliI!M$nplc^xC#B^q)|h$NTwc-#8Gi$6py5E_A-et}YE^sxmi0!6--S_Y;%95P^`T(pGUW-!~dkWU1x;#Rn+s5Owq!=i_(b4W*J zrN}vD7D@p2G{Z+i6x8%|<#bn-Q?m3{NQ8f5L#9$!(yQPPll4ZjxzF#ryI(Gxu&VdB z2Qtj`EloRT^5OHZH9IL%^u75_ztFPZ+ip8@I_{I%eKY#?>T&F%I_uV`tI~uvpR!4B z6v_-Tfr+A~O0$(8)Wb~~g@T^5O#yS(8E}gOajnG z5n#dwoE2x)bZ__BnC$^M$WN_YGKf?*D>Tx}@PiWPyD`e|W0(*ui9af{b8!{7wOw?Z zl8~kFWuXrDxK4tugYt0|LS?9&wjza~r&vy=4Ns$kT}NWoL1_hKYI~6j(SnjfFAgV@ zWiiaK5O^ALq`KHVJFo{ZFq;Ze$!H9AyTb^z&m3k4q>_QG!&NF6UY1=ZH zfcNAejeyR@$_O6UQf1}o3@UDW;xQxwltX}ayCp9(Bm){!D~eHkzZNQSY?tCdUAyr07f-WMQ>ykQ*{UT<_1k zgH(wOIT=$9`%C{a`zxM8W)4p2DlrT^Lw+%b_vFW_v}+Z+zqc}DX2hJ& zq+-rz-EI->g?XUu=!~-0Tc#0cP-Z2DY0wE(qiSbHCB;J}-JLFMKgmorkPq9Pn+Yv> zt{DQC4R*a9P8)b=tvaYQ0V$X@kb+sQwm_OQkOepxrp%->nJMJ^Yz>ZTl?A+1V1Fl+ z4l{u)uqbn})&#y?oly<7`*dmtZZ_x~1_JVP88mhiuGEI2+@=4ksZV{Ev(jUM=&Yw> zMUeHh!K(l%UH{=K;QxkI7~@<8U?xFeSg41p#xk!XPYyYKU2bd;?qQ@~gJD>*chN`$ zC2W*0ZoN~QYi$acmpzpq;NVY zBJ9xEKyVw8Mb>6>u&@PRDwyD$KdM+wXz0=)%x%}56$o*`j*r@H8BJy^c&E2uc$d5D zpRf9UkYXVZGdt5XFd2f{KUQ}sh`gQkl&-Vl#ND6zj=VS&@7Mdv+=ayxkFT74-grB4 zRqs}F*Y-sRej?^On%U0Lpq$H)yuF^?nB)1n!6PcdKTDYFd|fSq^$9m9Af-CZk5yrj~T8I#M`e?LP)*wR>ov}=GlSQ;vneq)DUL9~K!ac5 z{Cs5wXl-r<*Z*q2#=K3}bt%CPlVjVQv0S8sBDcy>7CR)OGM&+E6ll(SE} zkDTqUK=36_Yqn0>MMa!_7wB_mgF}1}qRZrE*p9s-jPLW|gU@u6oGmsb7gN=* z#VmxX$R|W+1wNm?PLZa=r_t zyN?K^Q0&XCdQ;eef+r!q-w%?qa#@{;^62R3L+)H$RHNCqiOksq$}^KwZwVz}U2~H* zh1IUlTwGfd@M5gKUTSck>1fSxR5+9@b}I$Ukuf_!0f5p?7PZZx21J%kt0Pb~dnmpI zrO`pI+Z=kUmUV#o4qfRSddoamevg|8;wE&tW}h9)bT*-RAnuMy@PEv*Jzl43?najS zVQRU|J5=eTuo^#)^O?t}0y83k4$ z9)$)J=jDptM+MC5KQE9@Bi@&Yn>cr#QNrqz^G%(vqiVAt0X#(vsy6G}L5Z`);w&(> z>uSMVQPKq87SA|aOb7*_Pv6ji5;QRnPh7JYm$nS#cP*6A(U5=_I%Re?&Qg39cHeg zs8!eZogM$_-r%@v-8M(Atz6!7Rx@kVYa#VX4U%nj8~+Rk^1Bim$|bc8ofa zQG9L7)gfmZZN4ab_TlzVslRs}3H@VapCa`N?Ip$TD0NoNk7cX2Zp*oU=KX|uZQB2Y zY^YtZXs=~KStq*C1{|^!QG&s{^k3cU$TimlN}7y3FLc&XoOabIMWDWR&i1b$h5xf+#s-=BD)n1!3voYoLk)y?IET*MbQK~%I5Tx4of{D> z3&Enl=up(F?N^7|UFGu%TM8r5crQZY%21yniw8PSFnuEH<0%&B+qD0$nJ6}>cC}tq zN4B+>$YK>e=Q+0PA!#=hK(pwrHi-1MLB4f6Bt|EoniLfH0As!Z{$n#+P@QRp{@lg& zjEYJu(_LHdkD=H4>n!g#k^d^|S6ptre(=zexxu{4e@0Mf!rx#q-XkFsrYClE!j5S2 z^18{^eRQmZ%7&X9TDaFkMOU){ph1u|$!gH)0nQ7MX*;flYEn7^0YuoNBZV-AgCan8 z{2U~f`K|lKyEr@YS2W&}e#Q%#K{`9Fs{S^p?(uR>2!bFtuJH!pw7=F#PMpf^rg|^5 zpEMwB3}nMsoonuo`WO~~#N3w%REaGF%WxH=wfN5;;iurP&k=$myU)^mlAfBE3~cGe zyuB92sN!PVT%r-C(mlAU!N5hp^P+{S$~E#!UGA!K5g#`(EbiRvwVnFOyOfu{x%+V8 zfuAZD@fy7lzt+|-@+{y?l9Srab_d+;J2M@}A|LqawLwE1G zeL4NfheJotKKNC|F9O~#Yom7|u5iNlUdWXGWfG&%Ou>tZ)lG9y?pLk1ItU$LU2S@> zj2ksJn^tYpLgppC8tmwLlisGWkjgz$hUif46)=5x3j5l*R|1?J-i1R@vY?ydoBAk= zYXM>u5y7x2HvI{dTOF=I6>R?vb%%WkveHSApruhYW3&8O0IHnt>;r?A!2>==VW`5e z??-1l8i8~^(9!oE5KloXyx)kq3U>!V|?HG|!B?9^=FeBthOV`2(D^ql%?2>Xs1vf0$q z8)#MCgHq~d5e-OU!>|aN(jFX9+_h1C7KKR$ z!$<>~(Nx8hVy)^{$QqFKh1uXzs1t*GATFJCit4L->N8sL7tST#fef!6IqI2`r}ens zTvp=$tou#nf4Q8Y7%r!uC&t*XZJPE)S)oKDhg$**TJk--Ou`aIHKFfWF)|47CFp|8 z3};=BY5?%??-FVP}HF&XI)J$KiVr(L12^wlguTuR(=!@l*ILH zMZDfS+?LR5+@3g^=5!9bNe$2j69G_js~xJ0YjL9$;HHEF0PhCe3C3C($Qz1HSzz1DoTUTbrodacb}^;*LaJOV!8stPMW|As2U(*K}&Ct~Y^>ItRjY~R4XFWmR?R9|_paKOSH&8QH#eq*^I-fwL0)1*spkUWv?jVU`x z|F8Q&K6q^@DGrQ$U_7B56C<(U0iVwYL_RdUu7vOE)dhTLa7_u{&j$iN_+M2cO0F#XYRsEAk^e>cu^v5)s~`;R1WupMKn@UA?FWpK16* zJseFxC-5FoeV_-Q4+j7}{LR0M`BDQ1)Pqk4^`jnqns7%wpmGpJwFJ8tgLbL2+!RYl z@}h-katJAhoE?y>!)UWQ91y+?!H^CeSaTqmuE}gMu!^Pc0@V$AKt(9LH!L9L!T$;K zgL&{-zS@`vpT};@0|5C}hXO1BuNAS?pppTo#02*u2wbzn(N6n+BD7JF3Ey4ehJ@f; z&7f;7B!tiUx{wfTX$i2QwFc1jzq_=A@1nU-5U8pIq0@Rgw;Mzv|G%n4odu(G9%u-z zB*;BiBSJ&`!-MphFHHo*NBCz}_2MIZ&Q87f2%i>#@Da?2yz-9L_6^a+6AuGuAK}%6yJmPsQiNg6B-q# z44-Z{U;B_0KiPV5P3EJ$_$|==#V5mv?{G zBKg9qlRJvuxOX`6w(fWB@Up*x@9w$%?4Nw)$6a4RIFP6PC_I6D_x|afi|^idOPQqq ze*I)jH!bR-!UiPzcR8CVvM`7t!sNEx()#2$)!n{x4y$g!Rd&J( zq3>#)-DcOpe@%KbKuVx0Aw)z%a1dcpo1rYJn_sa0Xoy+gBrU_lG~j2;78eA-OseM# z60`Grwu=)eyW|#kv}_o++j8&b`Wrhxdj8_U$IUW-o6x@p8N{QM1)Wf!#JiZ!aakn0 z1rWC|Cl6%2eyzRc?FERz^(szky_-`bYdmD>b2-|a*7(&q2afl?*z~1G_>H0SP83^3 z6~!w%@qXG6`8I?%7XC$?x!7WjowREGgF9q80HyBdPH z0Df&ikIkfV4(mtWDgdaJQSky6zTLIv7JAJ?@Sg2>)=Ap_44BP`~mZ~v|!7b?Zir4;fEn(pHq_R~EKsC3CkbAq92ld}xxxk0W z#DTnO91gk8q2grMcC%I|M1~^E^M+vU7*yYO8$`wMr?s5!_k^?%2()o=)Al!NGrcIC z|I(|SO;=N&`F-KcGnJPY?r8QB#>-!x_$eaI?S4qJK7W>f+HiW7Tn2#utc6xpcdstH zFh{e4mRoHu#=_{s6G=~hRz$Q$p!Jrn;@a|<&AH!|=AtNPJ`PxI4`zq*$@^)7^6W2A zMtQb!J4qN?^C~a{9YJ<|R}Lh|g(aZf@SaMx#Aj?^VJ2gGrgd_4q$NCrIMb2TUn>=v zwonQ~WW)oSm=sie!}-RPTrE-=Kkpgu|>Velm%a|T(+Y> zzr2ee`q%idfq$V4fn%y+MC&7ON?FV4qlllO7|Wj9WrjLN#Xr0c}8s{MT(Q|3G- zZ^TXGrCy1C8Oi&jSoM~CDVbUgN*1TRMVbAW6tuMJv@;)NxO1OG z&{t8`iwn`&Wc2m?l%O6liH8vxZeco>$H%l|k};E|o1;MX-^_pEX)dwx|6#g}4w!Wm zI`X^y$SH>swI=?U@61B5KMztx7ZyB>js%bvjw6tA5tITz332E|->a}4O$!nv^_)0> zG^=TPgY@Qr+xI+iknf$uS{LLnOtdd4uk4!r<%dIf0*kql^=Gliii!TcdvVhTC&QBORSjIQ&19WnIk9<@W%SR} zqf2(5DBtr*`ne8yUp8yrDKr^v9;9>VRFFU>&XzpG^xYTJ%vjp90OE2BUkp{$%7W%% kE1n06a7|F7abucjmI6uO(n|`ROWLU*mWmx%cq{n-0j$({lK=n! delta 5586 zcmd5=dsNd$_RlvYAwU8o1oBS4iQy%$JV1H5LO}%te5@6pAP5qa1w{~4R6t5lyH-## zijSg#*sfYuDYLS^TCH1YwXL<5w#c#gMlHTjL~MUE-)~TV?%#V(a}MXjna};+J9F=y zJ9nn+%z2{h+kX-*r0@voagUH>$2QHA-)#SN7zqZ0&)9UO1?Up_o}1$({-VK6>-`3< zxPK!{AKSjy7c?SkYn7Izv@-?WdcJ;mB=?lHh^BK8L}LC`K`NDXO^27!5qg+>e6 z6?`aP&wivqlgm&&$@(2ljzW5J*H-}D0XCj_hS%qeJDBupRF&Dtvmo~Xdy(k&AL z@18m8qi1~ra?ToUX^A~{uscz8h~0LGBc28%m&J*j|2WA8X3|j!-{aWVZUd4P65Cjh z5lyAhKD*fJ#q|JUA2NxSt(NKGRvov<4SP=b9goDG!)SAB+nZq#epk1j=Ii#`c=%>m zq@R|?VIHH6zYiKixl=?L-|hK$V)okb%9D%MO@D02{Lk5te(d>Y)>tlSA;Y0%u@@rfEhDnSz@<&jZ=&(ri&ku9_I|j;wC2ix#jj0c} znD&)#z#ifg))@}SM6v@UlN|@=0!0EpZu9RaIl#QfF%Vj(c`qc|WIP26sFL~PX~JKK zoYLx8Q3n*ahlP*Hf*?qC+7JMAq;nK4_p=jnYK7A}HVBHuQRh=p&t#&nH#)6XHfs+% zae%tp{vB$3h8>bV;k3>VsCis^h*I#>sni>~d+`P|^4O-d@csLrpcST>h`S#23cDsV z80GiDg91Ve65@Cx+**cKJm~ZNHu#M2RNMafi=)p>l|ez3zKp0!;I$?2`Ap=gbj9Lh zK}ZjUw4SSX@P>`1$T@i@C=#KK>;S}@f562cU4hUh zaTn;4_#cWS+>*juvnuXha$j6Kt|)2kK*X*oH3DDj0=@z&lKBsh|4FCj?EG;1pLA*m zoc|aNo!1@T=!o%N0F5bjI@<&Ujf~1w|G&| z*i~zf*5(JdvA)=!xBDa2){rsVu?yA^)&VzEGkV=1*(-AQTILc}PeuElx(8eH#A~Id;L#?AK_5KIpf~D` zCOt#1HyHK$kT*j4l7~Y1OOd#L0y=?&KvbojOxGD#UsO>}h|Ao7%&JWoE?x>y`LkFE zt(V0`CvXA{IZ6)&fy`?}TcVW9h1?lZwlm}} zl>R~ZAQ#OGIq)4mr75Ic)%_5auto-h=w~n(BJ`&M!MRd$4J{>z9Y&klA@VF(<+Gp zQK`x%Yl&0<XA}D>&W>F|-u*bDLt3YX)QxJuA!M#_Z}s2nfvqq@foU)RbI z6dMZZ*jyi?kCga0xf4#BjKH|v>o5jP)Y(q@X>tt?ObnWKotcXqlOV6GgUajj7fYZdRnd=&*?j4_Z%oJ% zg|wwxM}cmcVv(qMs=jXJ&ZvmS)5l|XEg5Lqd8xyP-pSTdg&#mYL>T3ePOi~XU!s8Z zCCY+{&Acy_EWrC#xg5N=m=Lm!rjXZurt9+>J7h0SxjyZuVWjQ;qI)Yr?ZzEldtwxf zw6E^kaSZ1EyKHD>OA3RVU67Skn3I=HC3>3j{h>*e9)vbMUT^7PrFeAAw0!k^)uK=1 z$rJmM4|eMUEH@e4Vsm<7*0QVuo1%ujHz-92ZHheKQY6SawoocewDu@np=+md$g`QpRQW@mQRScmic5kxI$fc;TDFaVM5@X|3&oYHmkvO_ z%o}Yp+)hJtt?EF0v#3Wk2dFvRaW$k96^u}|5!T15R|1~J80jg=2W7Dm?vEFZZz@(g|r{8B!b*<(i6molRQdd-{@9K z0-;SJ8B5^vZyt}dFUKTC8!4xg`=@ZyUu4QYAO5#<^@(@=1yviaf7ZFEVu*{qamyM- zY5;0|akc{U*8v90wn!3gi`1-f0?JTw!te=~APiy$hnPmaDhN2Kcmy`hjeF-VHjR-s z-E|$E%f3_GCZ(-sq|A;9iO%dg@1NZ=iLVKRyII^cqK1@OXK57Ja4eWt=EenPe<9i_ zlQnDVfb!yNvpuLVZ0*%lhc%NUwPZ6nSu2F3S^L-lx~`EAmd(0Lsy0z76A7jt_8K-9bvrf$-tJgwV{RoHkK+Lg^rqc0%iwaix=&I=Y zt@tEDhXVG&tQBAXDCMoKc@tDCiU6`$NqN}AE^s@r)F>D zgA)TAEZ-j+VhCHa5v-9JCLm zq1nZps>Pmta+Fk=j;&*H!K9F0 zU2ihZ+Tmvl$JR?sn!m)f@sntteztIZpTOS9#AxHo>HBe&WrpL9uf~l0#5J;h9vB>Vxnq~SKA(MO)!2lpsNqH*q^@I!lU|@mZF7Ig3stf!Lc^zS{W#0!MciUO+mA1wG1Z~5r_0ix)oz~y!f;`^sZK=ag z67w#-_Ylrq!ZzuNB0^s60N zzuWAnd_}fq44)Dfv#T#OyN=u%W9{@q^`PDFu>f7GGkASRPa)MAyptnpXw}!|q9sw+ zYR&M)HiN(P^l2=G(I%A&3cvPSDe8~vzq>G%on29Ud`|3dVNWMM3Oj@Y^b(mNA$h^? zKJmXcqXXzd5=A9&CKDYN7yaP2i~fbt|7*K)DEA0>B*hzmai$tSP*F(x=gxlW66Cg7 zc{pj=+)$r}p{#{>_JjJ$oqFYJHIylV%2hbNhU8FQY-q9JPc8;CZ&Q7n4neT~{IamFHoy1(dJw&UJ*9!7dC z-IBL9i*w1KOZ2Rs ".git/${filename}.tree" +} + function baseline () ( local dir=${1:?the directory to enter} local output_name=${2:?the basename of the output of the merge} @@ -58,8 +67,13 @@ function baseline () ( our_commit_id="$(git rev-parse "$our_committish")" their_commit_id="$(git rev-parse "$their_committish")" local maybe_expected_tree="$(git rev-parse expected^{tree})" + local maybe_expected_reversed_tree="$(git rev-parse expected-reversed^{tree})" + if [ "$maybe_expected_reversed_tree" == "expected-reversed^{tree}" ]; then + maybe_expected_reversed_tree="$(git rev-parse expected^{tree} || :)" + fi if [ -z "$opt_deviation_message" ]; then maybe_expected_tree="expected^{tree}" + maybe_expected_reversed_tree="expected^{tree}" fi local merge_info="${output_name}.merge-info" @@ -69,10 +83,107 @@ function baseline () ( if [[ "$one_side" != "no-reverse" ]]; then local merge_info="${output_name}-reversed.merge-info" git -c merge.conflictStyle=$conflict_style merge-tree -z --write-tree --allow-unrelated-histories "$their_committish" "$our_committish" > "$merge_info" || : - echo "$dir" "$conflict_style" "$their_commit_id" "$their_committish" "$our_commit_id" "$our_committish" "$merge_info" "$maybe_expected_tree" "$opt_deviation_message" >> ../baseline.cases + echo "$dir" "$conflict_style" "$their_commit_id" "$their_committish" "$our_commit_id" "$our_committish" "$merge_info" "$maybe_expected_reversed_tree" "$opt_deviation_message" >> ../baseline.cases fi ) + +git init non-tree-to-tree +(cd non-tree-to-tree + write_lines original 1 2 3 4 5 >a + git add a && git commit -m "init" + + git branch A + git branch B + + git checkout A + write_lines 1 2 3 4 5 6 >a + git commit -am "'A' changes 'a'" + + git checkout B + rm a + mkdir -p a/sub + touch a/sub/b a/sub/c a/d a/e + git add a && git commit -m "mv 'a' to 'a/sub/b', populate 'a/' with empty files" +) + +git init tree-to-non-tree +(cd tree-to-non-tree + mkdir -p a/sub + write_lines original 1 2 3 4 5 >a/sub/b + touch a/sub/c a/d a/e + git add a && git commit -m "init" + + git branch A + git branch B + + git checkout A + write_lines 1 2 3 4 5 6 >a/sub/b + git commit -am "'A' changes 'a/sub/b'" + + git checkout B + rm -Rf a + echo "new file" > a + git add a && git commit -m "rm -Rf a/ && add non-empty 'a'" +) + +git init non-tree-to-tree-with-rename +(cd non-tree-to-tree-with-rename + write_lines original 1 2 3 4 5 >a + git add a && git commit -m "init" + + git branch A + git branch B + + git checkout A + write_lines 1 2 3 4 5 6 >a + git commit -am "'A' changes 'a'" + + git checkout B + mv a tmp + mkdir -p a/sub + mv tmp a/sub/b + touch a/sub/c a/d a/e + git add a && git commit -m "mv 'a' to 'a/sub/b', populate 'a/' with empty files" +) + +git init tree-to-non-tree-with-rename +(cd tree-to-non-tree-with-rename + mkdir -p a/sub + write_lines original 1 2 3 4 5 >a/sub/b + touch a/sub/c a/d a/e + git add a && git commit -m "init" + + git branch A + git branch B + git branch expected + + git checkout A + write_lines 1 2 3 4 5 6 >a/sub/b + git commit -am "'A' changes 'a/sub/b'" + + git checkout B + rm -Rf a + touch a + git add a && git commit -m "rm -Rf a/ && add empty 'a' (which is like a rename from an empty deleted file)" + # And because it's so thrown off, it gets a completely different result if reversed. + git branch expected-reversed + make_conflict_index tree-to-non-tree-with-rename-A-B-deviates-reversed + + git checkout expected + write_lines 1 2 3 4 5 6 >a/sub/b + rm a/sub/c a/d + git commit -am "we detect a rename that we rather shouldn't, throwing everything off course" + + rm .git/index + git update-index --index-info <