From 5b428a9e931b19622ae76c25bfb1fe882744cd1f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 15:51:11 +0100 Subject: [PATCH 01/10] remove a TODO that turned out to be unnecessary. After all, stopping the merge when there is any conflict is highly relevant. --- README.md | 2 +- gix-merge/src/lib.rs | 5 +++++ gix-merge/src/tree/mod.rs | 17 +++++++++-------- gix/src/repository/merge.rs | 5 +++++ 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 8d2b8e2e6e0..1269e889c33 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ What follows is a high-level list of features and those which are planned: * [x] blob-diff * [ ] merge - [x] blobs - - [ ] trees + - [x] trees - [ ] commits * [ ] rebase * [ ] commit diff --git a/gix-merge/src/lib.rs b/gix-merge/src/lib.rs index 18be9a67604..e19ea6074a6 100644 --- a/gix-merge/src/lib.rs +++ b/gix-merge/src/lib.rs @@ -1,3 +1,8 @@ +//! Provide facilities to merge *blobs*, *trees* and *commits*. +//! +//! * [blob-merges](blob) look at file content. +//! * [tree-merges](tree) look at trees and merge them structurally, triggering blob-merges as needed. +//! * [commit-merges](commit) are like tree merges, but compute or create the merge-base on the fly. #![deny(rust_2018_idioms)] #![forbid(unsafe_code)] diff --git a/gix-merge/src/tree/mod.rs b/gix-merge/src/tree/mod.rs index cfc47e6de28..b3c0d9bfaa5 100644 --- a/gix-merge/src/tree/mod.rs +++ b/gix-merge/src/tree/mod.rs @@ -29,7 +29,7 @@ pub enum Error { /// The outcome produced by [`tree()`](crate::tree()). #[derive(Clone)] pub struct Outcome<'a> { - /// The ready-made (but unwritten) which is the *base* tree, including all non-conflicting changes, and the changes that had + /// The ready-made (but unwritten) *base* tree, including all non-conflicting changes, and the changes that had /// conflicts which could be resolved automatically. /// /// This means, if all of their changes were conflicting, this will be equivalent to the *base* tree. @@ -70,7 +70,7 @@ impl Outcome<'_> { #[derive(Debug, Clone)] pub struct Conflict { /// A record on how the conflict resolution succeeded with `Ok(_)` or failed with `Err(_)`. - /// In case of `Err(_)`, no write + /// Note that in case of `Err(_)`, edits may still have been made to the tree to aid resolution. /// On failure, one can examine `ours` and `theirs` to potentially find a custom solution. /// Note that the descriptions of resolutions or resolution failures may be swapped compared /// to the actual changes. This is due to changes like `modification|deletion` being treated the @@ -81,13 +81,17 @@ pub struct Conflict { pub ours: Change, /// The change representing *their* side. pub theirs: Change, + /// Determine how to interpret the `ours` and `theirs` fields. This is used to implement [`Self::changes_in_resolution()`] + /// and [`Self::into_parts_by_resolution()`]. map: ConflictMapping, } -/// A utility to help define which side is what. +/// A utility to help define which side is what in the [`Conflict`] type. #[derive(Debug, Clone, Copy)] enum ConflictMapping { + /// The sides are as described in the field documentation, i.e. `ours` is `ours`. Original, + /// The sides are the opposite of the field documentation. i.e. `ours` is `theirs` and `theirs` is `ours`. Swapped, } @@ -236,11 +240,8 @@ pub struct Options { /// The context to use when invoking merge-drivers. pub blob_merge_command_ctx: gix_command::Context, /// If `Some(what-is-unresolved)`, the first unresolved conflict will cause the entire merge to stop. - /// This is useful to see if there is any conflict, without performing the whole operation. - // TODO: Maybe remove this if the cost of figuring out conflicts is so low - after all, the data structures - // and initial diff is the expensive thing right now, which are always done upfront. - // However, this could change once we know do everything during the traversal, which probably doesn't work - // without caching stuff and is too complicated to actually do. + /// This is useful to see if there is any conflict, without performing the whole operation, something + /// that can be very relevant during merges that would cause a lot of blob-diffs. pub fail_on_conflict: Option, /// This value also affects the size of merge-conflict markers, to allow differentiating /// merge conflicts on each level, for any value greater than 0, with values `N` causing `N*2` diff --git a/gix/src/repository/merge.rs b/gix/src/repository/merge.rs index 7f41f23fd46..e359fcd719f 100644 --- a/gix/src/repository/merge.rs +++ b/gix/src/repository/merge.rs @@ -111,6 +111,11 @@ impl Repository { /// `labels` are typically chosen to identify the refs or names for `our_tree` and `their_tree` and `ancestor_tree` respectively. /// /// `options` should be initialized with [`tree_merge_options()`](Self::tree_merge_options()). + /// + /// ### Performance + /// + /// It's highly recommended to [set an object cache](crate::Repository::compute_object_cache_size_for_tree_diffs) + /// to avoid extracting the same object multiple times. // TODO: Use `crate::merge::Options` here and add niceties such as setting the resolution strategy. pub fn merge_trees( &self, From 9e106c4ab7ceea091cd8baef99a480739bd53c9d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 15:55:41 +0100 Subject: [PATCH 02/10] feat: add `Conflict::is_unresolved()` as utility to see if multiple of them are considered unresolved. --- gix-merge/src/tree/function.rs | 27 ++------------------------- gix-merge/src/tree/mod.rs | 24 +++++++++++++++++++++++- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/gix-merge/src/tree/function.rs b/gix-merge/src/tree/function.rs index 5cffd377cdd..eaa0ea933ba 100644 --- a/gix-merge/src/tree/function.rs +++ b/gix-merge/src/tree/function.rs @@ -3,9 +3,7 @@ use crate::tree::utils::{ to_components, track, unique_path_in_tree, ChangeList, ChangeListRef, PossibleConflict, TrackedChange, TreeNodes, }; use crate::tree::ConflictMapping::{Original, Swapped}; -use crate::tree::{ - Conflict, ConflictMapping, ContentMerge, Error, Options, Outcome, Resolution, ResolutionFailure, UnresolvedConflict, -}; +use crate::tree::{Conflict, ConflictMapping, ContentMerge, Error, Options, Outcome, Resolution, ResolutionFailure}; use bstr::{BString, ByteSlice}; use gix_diff::tree::recorder::Location; use gix_diff::tree_with_rewrites::Change; @@ -129,7 +127,7 @@ where let mut failed_on_first_conflict = false; let mut should_fail_on_conflict = |conflict: Conflict| -> bool { if let Some(how) = options.fail_on_conflict { - if conflict.resolution.is_err() || is_unresolved(std::slice::from_ref(&conflict), how) { + if conflict.resolution.is_err() || conflict.is_unresolved(how) { failed_on_first_conflict = true; } } @@ -1002,27 +1000,6 @@ where }) } -pub(super) fn is_unresolved(conflicts: &[Conflict], how: UnresolvedConflict) -> bool { - match how { - UnresolvedConflict::ConflictMarkers => conflicts.iter().any(|c| { - c.resolution.is_err() - || c.content_merge().map_or(false, |info| { - matches!(info.resolution, crate::blob::Resolution::Conflict) - }) - }), - UnresolvedConflict::Renames => conflicts.iter().any(|c| match &c.resolution { - Ok(success) => match success { - Resolution::SourceLocationAffectedByRename { .. } - | Resolution::OursModifiedTheirsRenamedAndChangedThenRename { .. } => true, - Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => { - matches!(merged_blob.resolution, crate::blob::Resolution::Conflict) - } - }, - Err(_failure) => true, - }), - } -} - 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 b3c0d9bfaa5..f1f52146481 100644 --- a/gix-merge/src/tree/mod.rs +++ b/gix-merge/src/tree/mod.rs @@ -61,7 +61,7 @@ impl Outcome<'_> { /// Return `true` if there is any conflict that would still need to be resolved as they would yield undesirable trees. /// This is based on `how` to determine what should be considered unresolved. pub fn has_unresolved_conflicts(&self, how: UnresolvedConflict) -> bool { - function::is_unresolved(&self.conflicts, how) + self.conflicts.iter().any(|c| c.is_unresolved(how)) } } @@ -108,6 +108,28 @@ impl ConflictMapping { } impl Conflict { + /// Return `true` if this instance is considered unresolved based on the criterion specified by `how`. + pub fn is_unresolved(&self, how: UnresolvedConflict) -> bool { + match how { + UnresolvedConflict::ConflictMarkers => { + self.resolution.is_err() + || self.content_merge().map_or(false, |info| { + matches!(info.resolution, crate::blob::Resolution::Conflict) + }) + } + UnresolvedConflict::Renames => match &self.resolution { + Ok(success) => match success { + Resolution::SourceLocationAffectedByRename { .. } + | Resolution::OursModifiedTheirsRenamedAndChangedThenRename { .. } => true, + Resolution::OursModifiedTheirsModifiedThenBlobContentMerge { merged_blob } => { + matches!(merged_blob.resolution, crate::blob::Resolution::Conflict) + } + }, + Err(_failure) => true, + }, + } + } + /// Returns the changes of fields `ours` and `theirs` so they match their description in the /// [`Resolution`] or [`ResolutionFailure`] respectively. /// Without this, the sides may appear swapped as `ours|theirs` is treated the same as `theirs/ours` From 2fce14ff4724ddc2f1a13c8686a7f221f8d2024e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 11:14:34 +0100 Subject: [PATCH 03/10] feat: add `Object::peel_to_commit()` to assure an object turns into a commit. --- gix/src/object/peel.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/gix/src/object/peel.rs b/gix/src/object/peel.rs index fa1a985a340..d2066a7e63d 100644 --- a/gix/src/object/peel.rs +++ b/gix/src/object/peel.rs @@ -3,7 +3,7 @@ use crate::{ object, object::{peel, Kind}, - Object, Tree, + Commit, Object, Tree, }; /// @@ -69,10 +69,19 @@ impl<'repo> Object<'repo> { } /// Peel this object into a tree and return it, if this is possible. + /// + /// This will follow tag objects and commits until their tree is reached. pub fn peel_to_tree(self) -> Result, peel::to_kind::Error> { Ok(self.peel_to_kind(gix_object::Kind::Tree)?.into_tree()) } + /// Peel this object into a commit and return it, if this is possible. + /// + /// This will follow tag objects until a commit is reached. + pub fn peel_to_commit(self) -> Result, peel::to_kind::Error> { + Ok(self.peel_to_kind(gix_object::Kind::Commit)?.into_commit()) + } + // TODO: tests /// Follow all tag object targets until a commit, tree or blob is reached. /// From 1f9556a7e4b89f00c4de091b50909a8d2081f0d4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 08:31:10 +0100 Subject: [PATCH 04/10] feat: add `Repository::index_or_load_from_head_or_empty()`. It's useful to get a reasonable index in any case, even on unborn repositories. It's for cases where the `HEAD` isn't setup at all, despite content being available, and to avoid unnecessary restrictions on what works. --- gix/src/repository/diff.rs | 2 +- gix/src/repository/index.rs | 29 ++++++++++++++-- gix/src/repository/merge.rs | 2 +- gix/src/repository/mod.rs | 32 +++++++++++++++--- .../generated-archives/make_basic_repo.tar | Bin 342016 -> 384512 bytes gix/tests/fixtures/make_basic_repo.sh | 4 ++- gix/tests/gix/repository/mod.rs | 18 ++++++++++ 7 files changed, 77 insertions(+), 10 deletions(-) diff --git a/gix/src/repository/diff.rs b/gix/src/repository/diff.rs index 8f7390b6f0d..e46eac821a7 100644 --- a/gix/src/repository/diff.rs +++ b/gix/src/repository/diff.rs @@ -19,7 +19,7 @@ impl Repository { mode: gix_diff::blob::pipeline::Mode, worktree_roots: gix_diff::blob::pipeline::WorktreeRoots, ) -> Result { - let index = self.index_or_load_from_head()?; + let index = self.index_or_load_from_head_or_empty()?; Ok(crate::diff::resource_cache( self, mode, diff --git a/gix/src/repository/index.rs b/gix/src/repository/index.rs index 03c7f5553e4..092882d0631 100644 --- a/gix/src/repository/index.rs +++ b/gix/src/repository/index.rs @@ -96,8 +96,10 @@ impl crate::Repository { /// /// ### Note /// - /// The locally stored index is not guaranteed to represent `HEAD^{tree}` if this repository is bare - bare repos - /// don't naturally have an index and if an index is present it must have been generated by hand. + /// * The locally stored index is not guaranteed to represent `HEAD^{tree}` if this repository is bare - bare repos + /// don't naturally have an index and if an index is present it must have been generated by hand. + /// * This method will fail on unborn repositories as `HEAD` doesn't point to a reference yet, which is needed to resolve + /// the revspec. If that is a concern, use [`Self::index_or_load_from_head_or_empty()`] instead. pub fn index_or_load_from_head( &self, ) -> Result { @@ -110,6 +112,29 @@ impl crate::Repository { }) } + /// Open the persisted worktree index or generate it from the current `HEAD^{tree}` to live in-memory only, + /// or resort to an empty index if `HEAD` is unborn. + /// + /// Use this method to get an index in any repository, even bare ones that don't have one naturally, or those + /// that are in a state where `HEAD` is invalid or points to an unborn reference. + pub fn index_or_load_from_head_or_empty( + &self, + ) -> Result { + Ok(match self.try_index()? { + Some(index) => IndexPersistedOrInMemory::Persisted(index), + None => match self.head()?.id() { + Some(id) => { + let head_tree_id = id.object()?.peel_to_commit()?.tree_id()?; + IndexPersistedOrInMemory::InMemory(self.index_from_tree(&head_tree_id)?) + } + None => IndexPersistedOrInMemory::InMemory(gix_index::File::from_state( + gix_index::State::new(self.object_hash()), + self.index_path(), + )), + }, + }) + } + /// Create new index-file, which would live at the correct location, in memory from the given `tree`. /// /// Note that this is an expensive operation as it requires recursively traversing the entire tree to unpack it into the index. diff --git a/gix/src/repository/merge.rs b/gix/src/repository/merge.rs index e359fcd719f..72ac2ae6d09 100644 --- a/gix/src/repository/merge.rs +++ b/gix/src/repository/merge.rs @@ -18,7 +18,7 @@ impl Repository { &self, worktree_roots: gix_merge::blob::pipeline::WorktreeRoots, ) -> Result { - let index = self.index_or_load_from_head()?; + let index = self.index_or_load_from_head_or_empty()?; let mode = { let renormalize = self .config diff --git a/gix/src/repository/mod.rs b/gix/src/repository/mod.rs index 8485b8d7cd5..b19a3a1f504 100644 --- a/gix/src/repository/mod.rs +++ b/gix/src/repository/mod.rs @@ -100,7 +100,7 @@ pub mod merge_resource_cache { #[error(transparent)] PipelineOptions(#[from] crate::config::merge::pipeline_options::Error), #[error(transparent)] - Index(#[from] crate::repository::index_or_load_from_head::Error), + Index(#[from] crate::repository::index_or_load_from_head_or_empty::Error), #[error(transparent)] AttributeStack(#[from] crate::config::attribute_stack::Error), #[error(transparent)] @@ -154,7 +154,7 @@ pub mod diff_resource_cache { #[error("Could not obtain resource cache for diffing")] ResourceCache(#[from] crate::diff::resource_cache::Error), #[error(transparent)] - Index(#[from] crate::repository::index_or_load_from_head::Error), + Index(#[from] crate::repository::index_or_load_from_head_or_empty::Error), #[error(transparent)] AttributeStack(#[from] crate::config::attribute_stack::Error), } @@ -289,7 +289,7 @@ pub mod pathspec_defaults_ignore_case { /// #[cfg(feature = "index")] pub mod index_or_load_from_head { - /// The error returned by [`Repository::index_or_load_from_head()`][crate::Repository::index_or_load_from_head()]. + /// The error returned by [`Repository::index_or_load_from_head()`](crate::Repository::index_or_load_from_head()). #[derive(thiserror::Error, Debug)] #[allow(missing_docs)] pub enum Error { @@ -304,10 +304,32 @@ pub mod index_or_load_from_head { } } +/// +#[cfg(feature = "index")] +pub mod index_or_load_from_head_or_empty { + /// The error returned by [`Repository::index_or_load_from_head_or_empty()`](crate::Repository::index_or_load_from_head_or_empty()). + #[derive(thiserror::Error, Debug)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + ReadHead(#[from] crate::reference::find::existing::Error), + #[error(transparent)] + FindCommit(#[from] crate::object::find::existing::Error), + #[error(transparent)] + PeelToTree(#[from] crate::object::peel::to_kind::Error), + #[error(transparent)] + TreeId(#[from] gix_object::decode::Error), + #[error(transparent)] + TraverseTree(#[from] crate::repository::index_from_tree::Error), + #[error(transparent)] + OpenIndex(#[from] crate::worktree::open_index::Error), + } +} + /// #[cfg(feature = "worktree-stream")] pub mod worktree_stream { - /// The error returned by [`Repository::worktree_stream()`][crate::Repository::worktree_stream()]. + /// The error returned by [`Repository::worktree_stream()`](crate::Repository::worktree_stream()). #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -332,6 +354,6 @@ pub mod worktree_stream { /// #[cfg(feature = "worktree-archive")] pub mod worktree_archive { - /// The error returned by [`Repository::worktree_archive()`][crate::Repository::worktree_archive()]. + /// The error returned by [`Repository::worktree_archive()`](crate::Repository::worktree_archive()). pub type Error = gix_archive::Error; } diff --git a/gix/tests/fixtures/generated-archives/make_basic_repo.tar b/gix/tests/fixtures/generated-archives/make_basic_repo.tar index f65e4ce784510b8f8618bfba98d7434ee0142174..e9669a7aedddb260691f0ef9f2e832742b9be45e 100644 GIT binary patch delta 1083 zcmZvbKWGzC9LMv!f6dUSP0ys14poYO27B-B-rZGPv~IQpTER@lR$7c;QxHm{pbb(& zaI-gbQXDjNa44KK4iy}NxK%{ZrBuX86vVoS!}%`1yO%?6I=JuWeZRlo@BLowhg#dt zZ)Vi|$^B#LsfC?u;TKY=OuqVsxFhP6I`(^E{=>f8ICH0Zai+XmsNHJc@Q%D*hI_es z4(d5!D<^(`(`T3_p{8+qjGS7kT(4@Q-np{^G(U+=WvOy?@n*%H?JuouN^W_B$W*5( zi~X2$0sJ1+wYA>M9et34XwYb$SK@WuMd+JE zWQUqjRV#j)+ccuMV4M!yjLx-qP|SdL;Qr5k4n9Kgm%#4E>5gGS@FmVXK_)8`nK6;o z)7Uh(0>UkKwlHanUZBGObFe9I{9(MOZA69`;;>B*>wOsudBsBS%} x|J>EtT>pP+U(r9SDTbwoH`#)Dp#iPMRcRTTb!p?^KTn!H`aO6soNW?0{TDTeXTtyh delta 135 zcmZqJBi`^qWCO2=IHwyMPoLbsbSTZpz#uHPf!T7hib$YH_^0qh{hIsBcQ)8nKi{%> zpZ%fP2AdU=${Cx)+Qb;!#F(~;F~2<_XkuVsVQ6AuZfc=hnwMFjueaIJfr)Y11LkGR WbXZsLqe%k|xX(CIL1@`ERt^9wvModa diff --git a/gix/tests/fixtures/make_basic_repo.sh b/gix/tests/fixtures/make_basic_repo.sh index e3cd8024ec7..abb30058e09 100755 --- a/gix/tests/fixtures/make_basic_repo.sh +++ b/gix/tests/fixtures/make_basic_repo.sh @@ -41,4 +41,6 @@ git init empty-core-excludes git clone --bare . non-bare-without-worktree (cd non-bare-without-worktree git config core.bare false -) \ No newline at end of file +) + +git init unborn; \ No newline at end of file diff --git a/gix/tests/gix/repository/mod.rs b/gix/tests/gix/repository/mod.rs index 8acf0308529..e9d1df4e4ab 100644 --- a/gix/tests/gix/repository/mod.rs +++ b/gix/tests/gix/repository/mod.rs @@ -17,6 +17,23 @@ mod state; mod submodule; mod worktree; +#[cfg(feature = "index")] +mod index { + #[test] + fn basics() -> crate::Result { + let repo = crate::named_subrepo_opts("make_basic_repo.sh", "unborn", gix::open::Options::isolated())?; + assert!( + repo.index_or_load_from_head().is_err(), + "can't read index if `HEAD^{{tree}}` can't be resolved" + ); + assert!( + repo.index_or_load_from_head_or_empty()?.entries().is_empty(), + "an empty index is created on the fly" + ); + Ok(()) + } +} + #[cfg(feature = "dirwalk")] mod dirwalk { use gix_dir::entry::Kind::*; @@ -44,6 +61,7 @@ mod dirwalk { ("non-bare-repo-without-index".into(), Repository), ("non-bare-without-worktree".into(), Directory), ("some".into(), Directory), + ("unborn".into(), Repository), ]; assert_eq!( collect From 27b663e4116c5aaa7b898283379d4d7d49a30865 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 16:31:50 +0100 Subject: [PATCH 05/10] feat: add `objects::tree::Editor::detach()` to get the underlying editor back. This can be useful to have more control over what gets written, or how. --- gix/src/object/tree/editor.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gix/src/object/tree/editor.rs b/gix/src/object/tree/editor.rs index fdcdcf87d40..d7a42fcd9bb 100644 --- a/gix/src/object/tree/editor.rs +++ b/gix/src/object/tree/editor.rs @@ -63,6 +63,12 @@ impl<'repo> super::Editor<'repo> { repo, }) } + + /// Detach all extras and return the underlying plumbing editor, which won't perform validation + /// when writing the tree. + pub fn detach(self) -> gix_object::tree::Editor<'repo> { + self.inner + } } /// Tree editing From 1d2262f2ca416e3c22f9601e7eab11f3372b2128 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 15:12:12 +0100 Subject: [PATCH 06/10] feat!: `Repository::merge_trees()` now has a fully-wrapped outcome. That way, more attached types are used for greater convenience. --- gix-merge/src/lib.rs | 4 ++-- gix/Cargo.toml | 6 +++--- gix/src/lib.rs | 5 +++-- gix/src/merge.rs | 35 +++++++++++++++++++++++++++++++++++ gix/src/object/tree/mod.rs | 7 ++++--- gix/src/repository/diff.rs | 2 +- gix/src/repository/merge.rs | 21 ++++++++++++++++++--- gix/src/repository/mod.rs | 2 ++ 8 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 gix/src/merge.rs diff --git a/gix-merge/src/lib.rs b/gix-merge/src/lib.rs index e19ea6074a6..e2e9d0ee625 100644 --- a/gix-merge/src/lib.rs +++ b/gix-merge/src/lib.rs @@ -1,8 +1,8 @@ //! Provide facilities to merge *blobs*, *trees* and *commits*. //! //! * [blob-merges](blob) look at file content. -//! * [tree-merges](tree) look at trees and merge them structurally, triggering blob-merges as needed. -//! * [commit-merges](commit) are like tree merges, but compute or create the merge-base on the fly. +//! * [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)] #![forbid(unsafe_code)] diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 2afbf8d2b59..30491fa516b 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -65,11 +65,11 @@ extras = [ "interrupt", "status", "dirwalk", - "blob-merge" ] ## A collection of features that need a larger MSRV, and thus are disabled by default. -need-more-recent-msrv = ["tree-editor"] +## * `blob-merge` should be in extras, but needs `tree-editor` for convenience. +need-more-recent-msrv = ["blob-merge", "tree-editor"] ## Various progress-related features that improve the look of progress message units. comfort = [ @@ -139,7 +139,7 @@ revparse-regex = ["regex", "revision"] blob-diff = ["gix-diff/blob", "attributes"] ## Add functions to specifically merge files, using the standard three-way merge that git offers. -blob-merge = ["blob-diff", "dep:gix-merge", "attributes"] +blob-merge = ["tree-editor", "blob-diff", "dep:gix-merge", "attributes"] ## Make it possible to turn a tree into a stream of bytes, which can be decoded to entries and turned into various other formats. worktree-stream = ["gix-worktree-stream", "attributes"] diff --git a/gix/src/lib.rs b/gix/src/lib.rs index 0c2d376b4f4..4d849ba34e5 100644 --- a/gix/src/lib.rs +++ b/gix/src/lib.rs @@ -120,8 +120,6 @@ pub use gix_ignore as ignore; #[cfg(feature = "index")] pub use gix_index as index; pub use gix_lock as lock; -#[cfg(feature = "blob-merge")] -pub use gix_merge as merge; #[cfg(feature = "credentials")] pub use gix_negotiate as negotiate; pub use gix_object as objs; @@ -204,6 +202,9 @@ pub mod push; /// pub mod diff; +/// +pub mod merge; + /// See [`ThreadSafeRepository::discover()`], but returns a [`Repository`] instead. /// /// # Note diff --git a/gix/src/merge.rs b/gix/src/merge.rs new file mode 100644 index 00000000000..c27c126e786 --- /dev/null +++ b/gix/src/merge.rs @@ -0,0 +1,35 @@ +#[cfg(feature = "blob-merge")] +pub use gix_merge as plumbing; + +pub use gix_merge::blob; + +/// +pub mod tree { + pub use gix_merge::tree::{Conflict, ContentMerge, Resolution, ResolutionFailure, UnresolvedConflict}; + + /// The outcome produced by [`Repository::merge_trees()`](crate::Repository::merge_trees()). + #[derive(Clone)] + pub struct Outcome<'repo> { + /// The ready-made (but unwritten) *base* tree, including all non-conflicting changes, and the changes that had + /// conflicts which could be resolved automatically. + /// + /// This means, if all of their changes were conflicting, this will be equivalent to the *base* tree. + pub tree: crate::object::tree::Editor<'repo>, + /// The set of conflicts we encountered. Can be empty to indicate there was no conflict. + /// Note that conflicts might have been auto-resolved, but they are listed here for completeness. + /// 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. + /// This also makes this outcome a very partial merge that cannot be completed. + pub failed_on_first_unresolved_conflict: bool, + } + + impl Outcome<'_> { + /// Return `true` if there is any conflict that would still need to be resolved as they would yield undesirable trees. + /// This is based on `how` to determine what should be considered unresolved. + pub fn has_unresolved_conflicts(&self, how: UnresolvedConflict) -> bool { + self.conflicts.iter().any(|c| c.is_unresolved(how)) + } + } +} diff --git a/gix/src/object/tree/mod.rs b/gix/src/object/tree/mod.rs index 29f9fe07592..694326edc1a 100644 --- a/gix/src/object/tree/mod.rs +++ b/gix/src/object/tree/mod.rs @@ -6,10 +6,11 @@ use crate::{object::find, Id, ObjectDetached, Tree}; /// All state needed to conveniently edit a tree, using only [update-or-insert](Editor::upsert()) and [removals](Editor::remove()). #[cfg(feature = "tree-editor")] +#[derive(Clone)] pub struct Editor<'repo> { - inner: gix_object::tree::Editor<'repo>, - validate: gix_validate::path::component::Options, - repo: &'repo crate::Repository, + pub(crate) inner: gix_object::tree::Editor<'repo>, + pub(crate) validate: gix_validate::path::component::Options, + pub(crate) repo: &'repo crate::Repository, } /// Initialization diff --git a/gix/src/repository/diff.rs b/gix/src/repository/diff.rs index e46eac821a7..5d504187efa 100644 --- a/gix/src/repository/diff.rs +++ b/gix/src/repository/diff.rs @@ -49,7 +49,7 @@ impl Repository { old_tree: impl Into>>, new_tree: impl Into>>, options: impl Into>, - ) -> Result, diff_tree_to_tree::Error> { + ) -> Result, diff_tree_to_tree::Error> { let mut cache = self.diff_resource_cache(gix_diff::blob::pipeline::Mode::ToGit, Default::default())?; let opts = options .into() diff --git a/gix/src/repository/merge.rs b/gix/src/repository/merge.rs index 72ac2ae6d09..3e33b021d55 100644 --- a/gix/src/repository/merge.rs +++ b/gix/src/repository/merge.rs @@ -124,10 +124,14 @@ impl Repository { their_tree: impl AsRef, labels: gix_merge::blob::builtin_driver::text::Labels<'_>, options: gix_merge::tree::Options, - ) -> Result, merge_trees::Error> { + ) -> Result, merge_trees::Error> { let mut diff_cache = self.diff_resource_cache_for_tree_diff()?; let mut blob_merge = self.merge_resource_cache(Default::default())?; - Ok(gix_merge::tree( + let gix_merge::tree::Outcome { + tree, + conflicts, + failed_on_first_unresolved_conflict, + } = gix_merge::tree( ancestor_tree.as_ref(), our_tree.as_ref(), their_tree.as_ref(), @@ -138,6 +142,17 @@ impl Repository { &mut diff_cache, &mut blob_merge, options, - )?) + )?; + + let validate = self.config.protect_options()?; + Ok(crate::merge::tree::Outcome { + tree: crate::object::tree::Editor { + inner: tree, + validate, + repo: self, + }, + conflicts, + failed_on_first_unresolved_conflict, + }) } } diff --git a/gix/src/repository/mod.rs b/gix/src/repository/mod.rs index b19a3a1f504..b47a81d97b6 100644 --- a/gix/src/repository/mod.rs +++ b/gix/src/repository/mod.rs @@ -125,6 +125,8 @@ pub mod merge_trees { DiffResourceCache(#[from] super::diff_resource_cache::Error), #[error(transparent)] TreeMerge(#[from] gix_merge::tree::Error), + #[error(transparent)] + ValidationOptions(#[from] crate::config::boolean::Error), } } From 254793581a135553e555f0bcc815154bb0951324 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 17:00:49 +0100 Subject: [PATCH 07/10] fix!: rename `blob-merge` feature to `tree-merge`. By now, `blob-merge` is the lowest-level of features which is required for both tree-merges and commit based merges. Hence it's better to just call it `merge`. --- gix/Cargo.toml | 4 ++-- gix/src/config/cache/access.rs | 4 ++-- gix/src/config/tree/sections/merge.rs | 14 +++++++------- gix/src/lib.rs | 1 + gix/src/merge.rs | 1 - gix/src/repository/mod.rs | 10 +++++----- gix/tests/gix/config/tree.rs | 2 +- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 30491fa516b..121b537098b 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -69,7 +69,7 @@ extras = [ ## A collection of features that need a larger MSRV, and thus are disabled by default. ## * `blob-merge` should be in extras, but needs `tree-editor` for convenience. -need-more-recent-msrv = ["blob-merge", "tree-editor"] +need-more-recent-msrv = ["merge", "tree-editor"] ## Various progress-related features that improve the look of progress message units. comfort = [ @@ -139,7 +139,7 @@ revparse-regex = ["regex", "revision"] blob-diff = ["gix-diff/blob", "attributes"] ## Add functions to specifically merge files, using the standard three-way merge that git offers. -blob-merge = ["tree-editor", "blob-diff", "dep:gix-merge", "attributes"] +merge = ["tree-editor", "blob-diff", "dep:gix-merge", "attributes"] ## Make it possible to turn a tree into a stream of bytes, which can be decoded to entries and turned into various other formats. worktree-stream = ["gix-worktree-stream", "attributes"] diff --git a/gix/src/config/cache/access.rs b/gix/src/config/cache/access.rs index 4e70fd5f178..2df060f2657 100644 --- a/gix/src/config/cache/access.rs +++ b/gix/src/config/cache/access.rs @@ -100,7 +100,7 @@ impl Cache { Ok(out) } - #[cfg(feature = "blob-merge")] + #[cfg(feature = "merge")] pub(crate) fn merge_drivers(&self) -> Result, config::merge::drivers::Error> { let mut out = Vec::::new(); for section in self @@ -136,7 +136,7 @@ impl Cache { Ok(out) } - #[cfg(feature = "blob-merge")] + #[cfg(feature = "merge")] pub(crate) fn merge_pipeline_options( &self, ) -> Result { diff --git a/gix/src/config/tree/sections/merge.rs b/gix/src/config/tree/sections/merge.rs index 966dda16083..78852078225 100644 --- a/gix/src/config/tree/sections/merge.rs +++ b/gix/src/config/tree/sections/merge.rs @@ -15,7 +15,7 @@ impl Merge { "The limit is actually squared, so 1000 stands for up to 1 million diffs if fuzzy rename tracking is enabled", ); /// The `merge.renames` key. - #[cfg(feature = "blob-merge")] + #[cfg(feature = "merge")] pub const RENAMES: super::diff::Renames = super::diff::Renames::new_renames("renames", &config::Tree::MERGE); /// The `merge.renormalize` key pub const RENORMALIZE: keys::Boolean = keys::Boolean::new_boolean("renormalize", &Tree::MERGE); @@ -31,7 +31,7 @@ impl Merge { pub const DRIVER_RECURSIVE: keys::String = keys::String::new_string("recursive", &config::Tree::MERGE) .with_subsection_requirement(Some(SubSectionRequirement::Parameter("driver"))); /// The `merge.conflictStyle` key. - #[cfg(feature = "blob-merge")] + #[cfg(feature = "merge")] pub const CONFLICT_STYLE: ConflictStyle = ConflictStyle::new_with_validate("conflictStyle", &config::Tree::MERGE, validate::ConflictStyle); } @@ -44,24 +44,24 @@ impl Section for Merge { fn keys(&self) -> &[&dyn Key] { &[ &Self::RENAME_LIMIT, - #[cfg(feature = "blob-merge")] + #[cfg(feature = "merge")] &Self::RENAMES, &Self::RENORMALIZE, &Self::DEFAULT, &Self::DRIVER_NAME, &Self::DRIVER_COMMAND, &Self::DRIVER_RECURSIVE, - #[cfg(feature = "blob-merge")] + #[cfg(feature = "merge")] &Self::CONFLICT_STYLE, ] } } /// The `merge.conflictStyle` key. -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] pub type ConflictStyle = keys::Any; -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] mod conflict_style { use crate::{bstr::BStr, config, config::tree::sections::merge::ConflictStyle}; use gix_merge::blob::builtin_driver::text; @@ -87,7 +87,7 @@ mod conflict_style { } } -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] mod validate { use crate::{ bstr::BStr, diff --git a/gix/src/lib.rs b/gix/src/lib.rs index 4d849ba34e5..8764b7163ce 100644 --- a/gix/src/lib.rs +++ b/gix/src/lib.rs @@ -203,6 +203,7 @@ pub mod push; pub mod diff; /// +#[cfg(feature = "merge")] pub mod merge; /// See [`ThreadSafeRepository::discover()`], but returns a [`Repository`] instead. diff --git a/gix/src/merge.rs b/gix/src/merge.rs index c27c126e786..b04b70e6571 100644 --- a/gix/src/merge.rs +++ b/gix/src/merge.rs @@ -1,4 +1,3 @@ -#[cfg(feature = "blob-merge")] pub use gix_merge as plumbing; pub use gix_merge::blob; diff --git a/gix/src/repository/mod.rs b/gix/src/repository/mod.rs index b47a81d97b6..1eeb0b36226 100644 --- a/gix/src/repository/mod.rs +++ b/gix/src/repository/mod.rs @@ -43,7 +43,7 @@ mod location; #[cfg(feature = "mailmap")] mod mailmap; /// -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] mod merge; mod object; #[cfg(feature = "attributes")] @@ -75,7 +75,7 @@ pub mod diff_tree_to_tree { } /// -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] pub mod blob_merge_options { /// The error returned by [Repository::blob_merge_options()](crate::Repository::blob_merge_options()). #[derive(Debug, thiserror::Error)] @@ -89,7 +89,7 @@ pub mod blob_merge_options { } /// -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] pub mod merge_resource_cache { /// The error returned by [Repository::merge_resource_cache()](crate::Repository::merge_resource_cache()). #[derive(Debug, thiserror::Error)] @@ -113,7 +113,7 @@ pub mod merge_resource_cache { } /// -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] pub mod merge_trees { /// The error returned by [Repository::merge_trees()](crate::Repository::merge_trees()). #[derive(Debug, thiserror::Error)] @@ -131,7 +131,7 @@ pub mod merge_trees { } /// -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] pub mod tree_merge_options { /// The error returned by [Repository::tree_merge_options()](crate::Repository::tree_merge_options()). #[derive(Debug, thiserror::Error)] diff --git a/gix/tests/gix/config/tree.rs b/gix/tests/gix/config/tree.rs index d89a2a0a1a9..6605d1e4f08 100644 --- a/gix/tests/gix/config/tree.rs +++ b/gix/tests/gix/config/tree.rs @@ -365,7 +365,7 @@ mod diff { } } -#[cfg(feature = "blob-merge")] +#[cfg(feature = "merge")] mod merge { use crate::config::tree::bcow; use gix::config::tree::{Key, Merge}; From 589ee8e3338ab66fe405d85a9b0c2fd8a797212a Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 16:23:35 +0100 Subject: [PATCH 08/10] adapt to changes in `gix` --- gitoxide-core/Cargo.toml | 2 +- gitoxide-core/src/repository/merge/tree.rs | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/gitoxide-core/Cargo.toml b/gitoxide-core/Cargo.toml index 83fe9d05957..5b8ac9fc422 100644 --- a/gitoxide-core/Cargo.toml +++ b/gitoxide-core/Cargo.toml @@ -49,7 +49,7 @@ serde = ["gix/serde", "dep:serde_json", "dep:serde", "bytesize/serde"] [dependencies] # deselect everything else (like "performance") as this should be controllable by the parent application. -gix = { version = "^0.67.0", path = "../gix", default-features = false, features = ["blob-merge", "blob-diff", "revision", "mailmap", "excludes", "attributes", "worktree-mutation", "credentials", "interrupt", "status", "dirwalk"] } +gix = { version = "^0.67.0", path = "../gix", default-features = false, features = ["merge", "blob-diff", "revision", "mailmap", "excludes", "attributes", "worktree-mutation", "credentials", "interrupt", "status", "dirwalk"] } gix-pack-for-configuration-only = { package = "gix-pack", version = "^0.54.0", path = "../gix-pack", default-features = false, features = ["pack-cache-lru-dynamic", "pack-cache-lru-static", "generate", "streaming-input"] } gix-transport-configuration-only = { package = "gix-transport", version = "^0.43.0", path = "../gix-transport", default-features = false } gix-archive-for-configuration-only = { package = "gix-archive", version = "^0.16.0", path = "../gix-archive", optional = true, features = ["tar", "tar_gz"] } diff --git a/gitoxide-core/src/repository/merge/tree.rs b/gitoxide-core/src/repository/merge/tree.rs index 3f497f08a37..9430a0938ea 100644 --- a/gitoxide-core/src/repository/merge/tree.rs +++ b/gitoxide-core/src/repository/merge/tree.rs @@ -72,12 +72,15 @@ pub(super) mod function { .map_or(theirs_id_str.as_str().into(), |n| n.as_bstr()) .into(), }; - let mut res = repo.merge_trees(base_id, ours_id, theirs_id, labels, options)?; + 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(UnresolvedConflict::Renames); { let _span = gix::trace::detail!("Writing merged tree"); let mut written = 0; let tree_id = res .tree + .detach() .write(|tree| { written += 1; repo.write(tree) @@ -86,10 +89,10 @@ pub(super) mod function { writeln!(out, "{tree_id} (wrote {written} trees)")?; } - if !res.conflicts.is_empty() { + if !has_conflicts { writeln!(err, "{} possibly resolved conflicts", res.conflicts.len())?; } - if res.has_unresolved_conflicts(UnresolvedConflict::Renames) { + if has_unresolved_conflicts { bail!("Tree conflicted") } Ok(()) From a43e56314189c91f67b69de4001c6c6fad8184b6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 17:06:45 +0100 Subject: [PATCH 09/10] feat!: `Repository::merge_trees()` now takes portable version of `Options`. --- gix/Cargo.toml | 2 +- gix/src/merge.rs | 86 +++++++++++++++++++++++++++++++++++++ gix/src/repository/merge.rs | 10 ++--- 3 files changed, 92 insertions(+), 6 deletions(-) diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 121b537098b..3d7501515da 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -399,7 +399,7 @@ document-features = { version = "0.2.0", optional = true } [dev-dependencies] # For additional features that aren't enabled by default due to MSRV -gix = { path = ".", default-features = false, features = ["tree-editor"] } +gix = { path = ".", default-features = false, features = ["need-more-recent-msrv"] } pretty_assertions = "1.4.0" gix-testtools = { path = "../tests/tools" } is_ci = "1.1.1" diff --git a/gix/src/merge.rs b/gix/src/merge.rs index b04b70e6571..2f634fc1b77 100644 --- a/gix/src/merge.rs +++ b/gix/src/merge.rs @@ -4,6 +4,7 @@ pub use gix_merge::blob; /// pub mod tree { + use gix_merge::blob::builtin_driver; pub use gix_merge::tree::{Conflict, ContentMerge, Resolution, ResolutionFailure, UnresolvedConflict}; /// The outcome produced by [`Repository::merge_trees()`](crate::Repository::merge_trees()). @@ -31,4 +32,89 @@ pub mod tree { self.conflicts.iter().any(|c| c.is_unresolved(how)) } } + + /// A way to configure [`Repository::merge_trees()`](crate::Repository::merge_trees()). + #[derive(Default, Debug, Clone)] + pub struct Options { + inner: gix_merge::tree::Options, + file_favor: Option, + } + + impl From for Options { + fn from(opts: gix_merge::tree::Options) -> Self { + Options { + inner: opts, + file_favor: None, + } + } + } + + impl From for gix_merge::tree::Options { + fn from(value: Options) -> Self { + let mut opts = value.inner; + if let Some(file_favor) = value.file_favor { + let (resolve_binary, resolve_text) = match file_favor { + FileFavor::Ours => ( + builtin_driver::binary::ResolveWith::Ours, + builtin_driver::text::Conflict::ResolveWithOurs, + ), + FileFavor::Theirs => ( + builtin_driver::binary::ResolveWith::Theirs, + builtin_driver::text::Conflict::ResolveWithTheirs, + ), + }; + + opts.symlink_conflicts = Some(resolve_binary); + opts.blob_merge.resolve_binary_with = Some(resolve_binary); + opts.blob_merge.text.conflict = resolve_text; + } + opts + } + } + + /// Identify how files should be resolved in case of conflicts. + /// + /// This works for… + /// + /// * content merges + /// * binary files + /// * symlinks (a form of file after all) + /// + /// Note that that union merges aren't available as they aren't available for binaries or symlinks. + #[derive(Debug, Copy, Clone)] + pub enum FileFavor { + /// Choose *our* side in case of a conflict. + /// Note that this choice is precise, so *ours* hunk will only be chosen if they conflict with *theirs*, + /// so *their* hunks may still show up in the merged result. + Ours, + /// Choose *their* side in case of a conflict. + /// Note that this choice is precise, so *ours* hunk will only be chosen if they conflict with *theirs*, + /// so *their* hunks may still show up in the merged result. + Theirs, + } + + /// Builder + impl Options { + /// If *not* `None`, rename tracking will be performed when determining the changes of each side of the merge. + pub fn with_rewrites(mut self, rewrites: Option) -> Self { + self.inner.rewrites = rewrites; + self + } + + /// If `Some(what-is-unresolved)`, the first unresolved conflict will cause the entire merge to stop. + /// This is useful to see if there is any conflict, without performing the whole operation, something + /// that can be very relevant during merges that would cause a lot of blob-diffs. + pub fn with_fail_on_conflict(mut self, fail_on_conflict: Option) -> Self { + self.inner.fail_on_conflict = fail_on_conflict; + self + } + + /// When `None`, the default, both sides will be treated equally, and in case of conflict an unbiased representation + /// is chosen both for content and for trees, causing a conflict. + /// When `Some(favor)` one can choose a side to prefer in order to automatically resolve a conflict meaningfully. + pub fn with_file_favor(mut self, file_favor: Option) -> Self { + self.file_favor = file_favor; + self + } + } } diff --git a/gix/src/repository/merge.rs b/gix/src/repository/merge.rs index 3e33b021d55..7b5e18297a2 100644 --- a/gix/src/repository/merge.rs +++ b/gix/src/repository/merge.rs @@ -83,7 +83,7 @@ impl Repository { } /// Read all relevant configuration options to instantiate options for use in [`merge_trees()`](Self::merge_trees). - pub fn tree_merge_options(&self) -> Result { + pub fn tree_merge_options(&self) -> Result { Ok(gix_merge::tree::Options { rewrites: crate::diff::utils::new_rewrites_inner( &self.config.resolved, @@ -97,7 +97,8 @@ impl Repository { marker_size_multiplier: 0, symlink_conflicts: None, allow_lossy_resolution: false, - }) + } + .into()) } /// Merge `our_tree` and `their_tree` together, assuming they have the same `ancestor_tree`, to yield a new tree @@ -116,14 +117,13 @@ impl Repository { /// /// It's highly recommended to [set an object cache](crate::Repository::compute_object_cache_size_for_tree_diffs) /// to avoid extracting the same object multiple times. - // TODO: Use `crate::merge::Options` here and add niceties such as setting the resolution strategy. pub fn merge_trees( &self, ancestor_tree: impl AsRef, our_tree: impl AsRef, their_tree: impl AsRef, labels: gix_merge::blob::builtin_driver::text::Labels<'_>, - options: gix_merge::tree::Options, + options: crate::merge::tree::Options, ) -> Result, merge_trees::Error> { let mut diff_cache = self.diff_resource_cache_for_tree_diff()?; let mut blob_merge = self.merge_resource_cache(Default::default())?; @@ -141,7 +141,7 @@ impl Repository { &mut Default::default(), &mut diff_cache, &mut blob_merge, - options, + options.into(), )?; let validate = self.config.protect_options()?; From 8d590f33f49b556de1748818e0bbec610566842f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 5 Nov 2024 19:13:11 +0100 Subject: [PATCH 10/10] adapt to changes in `gix` --- gitoxide-core/src/repository/merge/tree.rs | 18 +++-------------- justfile | 8 ++++---- src/plumbing/main.rs | 4 ++-- src/plumbing/options/mod.rs | 23 +++++++++++++++++++--- 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/gitoxide-core/src/repository/merge/tree.rs b/gitoxide-core/src/repository/merge/tree.rs index 9430a0938ea..edc7820819f 100644 --- a/gitoxide-core/src/repository/merge/tree.rs +++ b/gitoxide-core/src/repository/merge/tree.rs @@ -2,7 +2,7 @@ use crate::OutputFormat; pub struct Options { pub format: OutputFormat, - pub resolve_content_merge: Option, + pub file_favor: Option, pub in_memory: bool, } @@ -12,8 +12,6 @@ pub(super) mod function { use anyhow::{anyhow, bail, Context}; use gix::bstr::BString; use gix::bstr::ByteSlice; - use gix::merge::blob::builtin_driver::binary; - use gix::merge::blob::builtin_driver::text::Conflict; use gix::merge::tree::UnresolvedConflict; use gix::prelude::Write; @@ -29,7 +27,7 @@ pub(super) mod function { theirs: BString, Options { format, - resolve_content_merge, + file_favor, in_memory, }: Options, ) -> anyhow::Result<()> { @@ -44,17 +42,7 @@ pub(super) mod function { let (ours_ref, ours_id) = refname_and_tree(&repo, ours)?; let (theirs_ref, theirs_id) = refname_and_tree(&repo, theirs)?; - let mut options = repo.tree_merge_options()?; - if let Some(resolve) = resolve_content_merge { - options.blob_merge.text.conflict = resolve; - options.blob_merge.resolve_binary_with = match resolve { - Conflict::Keep { .. } => None, - Conflict::ResolveWithOurs => Some(binary::ResolveWith::Ours), - Conflict::ResolveWithTheirs => Some(binary::ResolveWith::Theirs), - Conflict::ResolveWithUnion => None, - }; - } - + let options = repo.tree_merge_options()?.with_file_favor(file_favor); let base_id_str = base_id.to_string(); let ours_id_str = ours_id.to_string(); let theirs_id_str = theirs_id.to_string(); diff --git a/justfile b/justfile index 758646d0f2c..b0b0c0f4060 100755 --- a/justfile +++ b/justfile @@ -46,11 +46,11 @@ check: if cargo check -p gix-packetline --all-features 2>/dev/null; then false; else true; fi if cargo check -p gix-transport --all-features 2>/dev/null; then false; else true; fi if cargo check -p gix-protocol --all-features 2>/dev/null; then false; else true; fi - if cargo tree -p gix --no-default-features -i imara-diff 2>/dev/null; then false; else true; fi + cargo tree -p gix --no-default-features -e normal -i imara-diff 2>&1 | grep warning # warning happens if nothing found, no exit code :/ if cargo tree -p gix --no-default-features -i gix-protocol 2>/dev/null; then false; else true; fi - if cargo tree -p gix --no-default-features -i gix-submodule 2>/dev/null; then false; else true; fi - if cargo tree -p gix --no-default-features -i gix-pathspec 2>/dev/null; then false; else true; fi - if cargo tree -p gix --no-default-features -i gix-filter 2>/dev/null; then false; else true; fi + cargo tree -p gix --no-default-features -e normal -i gix-submodule 2>&1 | grep warning + cargo tree -p gix --no-default-features -e normal -i gix-pathspec 2>&1 | grep warning + cargo tree -p gix --no-default-features -e normal -i gix-filter 2>&1 | grep warning if cargo tree -p gix --no-default-features -i gix-credentials 2>/dev/null; then false; else true; fi cargo check --no-default-features --features lean cargo check --no-default-features --features lean-async diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index c7859a0938f..daaa0677197 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -173,7 +173,7 @@ pub fn main() -> Result<()> { ), merge::SubCommands::Tree { in_memory, - resolve_content_with, + file_favor, ours, base, theirs, @@ -194,7 +194,7 @@ pub fn main() -> Result<()> { theirs, core::repository::merge::tree::Options { format, - resolve_content_merge: resolve_content_with.map(Into::into), + file_favor: file_favor.map(Into::into), in_memory, }, ) diff --git a/src/plumbing/options/mod.rs b/src/plumbing/options/mod.rs index 47f92b4034f..6d46d546cc2 100644 --- a/src/plumbing/options/mod.rs +++ b/src/plumbing/options/mod.rs @@ -367,6 +367,23 @@ pub mod merge { } } + #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum)] + pub enum FileFavor { + /// Use only ours in case of conflict. + Ours, + /// Use only theirs in case of conflict. + Theirs, + } + + impl From for gix::merge::tree::FileFavor { + fn from(value: FileFavor) -> Self { + match value { + FileFavor::Ours => gix::merge::tree::FileFavor::Ours, + FileFavor::Theirs => gix::merge::tree::FileFavor::Theirs, + } + } + } + #[derive(Debug, clap::Parser)] #[command(about = "perform merges of various kinds")] pub struct Platform { @@ -400,9 +417,9 @@ pub mod merge { /// Note that the resulting tree won't be available or inspectable. #[clap(long, short = 'm')] in_memory: bool, - /// Decide how to resolve content conflicts. If unset, write conflict markers and fail. - #[clap(long, short = 'c')] - resolve_content_with: Option, + /// Decide how to resolve content conflicts in files. If unset, write conflict markers and fail. + #[clap(long, short = 'f')] + file_favor: Option, /// A revspec to our treeish. #[clap(value_name = "OURS", value_parser = crate::shared::AsBString)]