Skip to content

Commit 1732e45

Browse files
committed
Use gitoxide merge-trees for when applying and unapplying branches.
While at it, check make it a thing to check for clean merges easily.
1 parent 650c136 commit 1732e45

File tree

7 files changed

+156
-45
lines changed

7 files changed

+156
-45
lines changed

crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use gitbutler_error::error::Marker;
88
use gitbutler_oplog::SnapshotExt;
99
use gitbutler_project::access::WorktreeWritePermission;
1010
use gitbutler_reference::{Refname, RemoteRefname};
11+
use gitbutler_repo::GixRepositoryExt;
1112
use gitbutler_repo::{
1213
rebase::{cherry_rebase_group, gitbutler_merge_commits},
1314
LogUntil, RepositoryExt,
@@ -301,29 +302,27 @@ impl BranchManager<'_> {
301302
))?;
302303

303304
// Branch is out of date, merge or rebase it
304-
let merge_base_tree = repo
305+
let merge_base_tree_id = repo
305306
.find_commit(merge_base)
306307
.context(format!("failed to find merge base commit {}", merge_base))?
307308
.tree()
308-
.context("failed to find merge base tree")?;
309-
310-
let branch_tree = repo
311-
.find_tree(branch.tree)
312-
.context("failed to find branch tree")?;
309+
.context("failed to find merge base tree")?
310+
.id();
311+
let branch_tree_id = branch.tree;
313312

314313
// We don't support having two branches applied that conflict with each other
315314
{
316-
let uncommited_changes_tree = repo.create_wd_tree()?;
317-
let branch_merged_with_other_applied_branches = repo
318-
.merge_trees(
319-
&merge_base_tree,
320-
&branch_tree,
321-
&uncommited_changes_tree,
322-
None,
315+
let uncommited_changes_tree_id = repo.create_wd_tree()?.id();
316+
let gix_repo = self.ctx.gix_repository_for_merging_non_persisting()?;
317+
let merges_cleanly = gix_repo
318+
.merges_cleanly_compat(
319+
merge_base_tree_id,
320+
branch_tree_id,
321+
uncommited_changes_tree_id,
323322
)
324323
.context("failed to merge trees")?;
325324

326-
if branch_merged_with_other_applied_branches.has_conflicts() {
325+
if !merges_cleanly {
327326
for branch in vb_state
328327
.list_branches_in_workspace()?
329328
.iter()

crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,16 @@ use git2::Commit;
55
use gitbutler_branch::BranchExt;
66
use gitbutler_commit::commit_headers::CommitHeadersV2;
77
use gitbutler_oplog::SnapshotExt;
8+
use gitbutler_oxidize::git2_to_gix_object_id;
9+
use gitbutler_oxidize::gix_to_git2_oid;
810
use gitbutler_project::access::WorktreeWritePermission;
911
use gitbutler_reference::{normalize_branch_name, ReferenceName, Refname};
12+
use gitbutler_repo::GixRepositoryExt;
1013
use gitbutler_repo::RepositoryExt;
1114
use gitbutler_repo::SignaturePurpose;
1215
use gitbutler_repo_actions::RepoActionsExt;
1316
use gitbutler_stack::{Stack, StackId};
17+
use gix::objs::Write;
1418
use tracing::instrument;
1519

1620
use super::BranchManager;
@@ -79,7 +83,10 @@ impl BranchManager<'_> {
7983

8084
let repo = self.ctx.repository();
8185

82-
let base_tree = target_commit.tree().context("failed to get target tree")?;
86+
let base_tree_id = target_commit
87+
.tree()
88+
.context("failed to get target tree")?
89+
.id();
8390

8491
let applied_statuses = get_applied_status(self.ctx, None)
8592
.context("failed to get status by branch")?
@@ -98,13 +105,14 @@ impl BranchManager<'_> {
98105
num_branches = applied_statuses.len() - 1
99106
)
100107
.entered();
101-
applied_statuses
108+
let gix_repo = self.ctx.gix_repository()?;
109+
let merge_options = gix_repo.tree_merge_options()?;
110+
let final_tree_id = applied_statuses
102111
.into_iter()
103112
.filter(|(branch, _)| branch.id != branch_id)
104-
.fold(
105-
target_commit.tree().context("failed to get target tree"),
106-
|final_tree, status| {
107-
let final_tree = final_tree?;
113+
.try_fold(
114+
git2_to_gix_object_id(target_commit.tree_id()),
115+
|final_tree_id, status| -> Result<_> {
108116
let branch = status.0;
109117
let files = status
110118
.1
@@ -113,14 +121,21 @@ impl BranchManager<'_> {
113121
.collect::<Vec<(PathBuf, Vec<VirtualBranchHunk>)>>();
114122
let tree_oid =
115123
gitbutler_diff::write::hunks_onto_oid(self.ctx, branch.head(), files)?;
116-
let branch_tree = repo.find_tree(tree_oid)?;
117-
let mut result =
118-
repo.merge_trees(&base_tree, &final_tree, &branch_tree, None)?;
119-
let final_tree_oid = result.write_tree_to(repo)?;
120-
repo.find_tree(final_tree_oid)
121-
.context("failed to find tree")
124+
let mut merge = gix_repo.merge_trees(
125+
git2_to_gix_object_id(base_tree_id),
126+
final_tree_id,
127+
git2_to_gix_object_id(tree_oid),
128+
gix_repo.default_merge_labels(),
129+
merge_options.clone(),
130+
)?;
131+
let final_tree_id = merge
132+
.tree
133+
.write(|tree| gix_repo.write(tree))
134+
.map_err(|err| anyhow::anyhow!("Could not write merged tree: {err}"))?;
135+
Ok(final_tree_id)
122136
},
123-
)?
137+
)?;
138+
repo.find_tree(gix_to_git2_oid(final_tree_id))?
124139
};
125140

126141
let _span = tracing::debug_span!("checkout final tree").entered();

crates/gitbutler-branch-actions/src/integration.rs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ use gitbutler_command_context::CommandContext;
99
use gitbutler_commit::commit_ext::CommitExt;
1010
use gitbutler_error::error::Marker;
1111
use gitbutler_operating_modes::OPEN_WORKSPACE_REFS;
12+
use gitbutler_oxidize::{git2_to_gix_object_id, gix_to_git2_oid};
1213
use gitbutler_project::access::WorktreeWritePermission;
13-
use gitbutler_repo::SignaturePurpose;
14+
use gitbutler_repo::{GixRepositoryExt, SignaturePurpose};
1415
use gitbutler_repo::{LogUntil, RepositoryExt};
1516
use gitbutler_stack::{Stack, VirtualBranchesHandle};
17+
use gix::objs::Write;
1618
use tracing::instrument;
1719

1820
use crate::{branch_manager::BranchManagerExt, conflicts, VirtualBranchesExt};
@@ -41,6 +43,7 @@ pub(crate) fn get_workspace_head(ctx: &CommandContext) -> Result<git2::Oid> {
4143

4244
let target_commit = repo.find_commit(target.sha)?;
4345
let mut workspace_tree = repo.find_real_tree(&target_commit, Default::default())?;
46+
let mut workspace_tree_id = git2_to_gix_object_id(workspace_tree.id());
4447

4548
if conflicts::is_conflicting(ctx, None)? {
4649
let merge_parent = conflicts::merge_parent(ctx)?.ok_or(anyhow!("No merge parent"))?;
@@ -49,22 +52,37 @@ pub(crate) fn get_workspace_head(ctx: &CommandContext) -> Result<git2::Oid> {
4952
let merge_base = repo.merge_base(first_branch.head(), merge_parent)?;
5053
workspace_tree = repo.find_commit(merge_base)?.tree()?;
5154
} else {
55+
let gix_repo = ctx.gix_repository_for_merging()?;
56+
let mut merge_options_fail_fast = gix_repo.tree_merge_options()?;
57+
let conflict_kind = gix::merge::tree::UnresolvedConflict::Renames;
58+
merge_options_fail_fast.fail_on_conflict = Some(conflict_kind);
59+
let merge_tree_id = git2_to_gix_object_id(repo.find_commit(target.sha)?.tree_id());
5260
for branch in virtual_branches.iter_mut() {
53-
let merge_tree = repo.find_commit(target.sha)?.tree()?;
54-
let branch_tree = repo.find_commit(branch.head())?;
55-
let branch_tree = repo.find_real_tree(&branch_tree, Default::default())?;
56-
57-
let mut index = repo.merge_trees(&merge_tree, &workspace_tree, &branch_tree, None)?;
61+
let branch_head = repo.find_commit(branch.head())?;
62+
let branch_tree_id =
63+
git2_to_gix_object_id(repo.find_real_tree(&branch_head, Default::default())?.id());
64+
65+
let mut merge = gix_repo.merge_trees(
66+
merge_tree_id,
67+
workspace_tree_id,
68+
branch_tree_id,
69+
gix_repo.default_merge_labels(),
70+
merge_options_fail_fast.clone(),
71+
)?;
5872

59-
if !index.has_conflicts() {
60-
workspace_tree = repo.find_tree(index.write_tree_to(repo)?)?;
73+
if !merge.has_unresolved_conflicts(conflict_kind) {
74+
workspace_tree_id = merge
75+
.tree
76+
.write(|tree| gix_repo.write(tree))
77+
.map_err(|err| anyhow!("{err}"))?;
6178
} else {
6279
// This branch should have already been unapplied during the "update" command but for some reason that failed
6380
tracing::warn!("Merge conflict between base and {:?}", branch.name);
6481
branch.in_workspace = false;
6582
vb_state.set_branch(branch.clone())?;
6683
}
6784
}
85+
workspace_tree = repo.find_tree(gix_to_git2_oid(workspace_tree_id))?;
6886
}
6987

7088
let committer = gitbutler_repo::signature(SignaturePurpose::Committer)?;

crates/gitbutler-branch-actions/src/stack.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use gitbutler_oplog::entry::{OperationKind, SnapshotDetails};
77
use gitbutler_oplog::{OplogExt, SnapshotExt};
88
use gitbutler_project::Project;
99
use gitbutler_reference::normalize_branch_name;
10-
use gitbutler_repo::GixRepositoryExt;
1110
use gitbutler_repo_actions::RepoActionsExt;
1211
use gitbutler_stack::{
1312
CommitOrChangeId, ForgeIdentifier, PatchReference, PatchReferenceUpdate, Series,
@@ -192,10 +191,7 @@ pub fn push_stack(project: &Project, branch_id: StackId, with_force: bool) -> Re
192191

193192
// First fetch, because we dont want to push integrated series
194193
ctx.fetch(&default_target.push_remote_name(), None)?;
195-
let gix_repo = ctx
196-
.gix_repository()?
197-
.for_tree_diffing()?
198-
.with_object_memory();
194+
let gix_repo = ctx.gix_repository_for_merging_non_persisting()?;
199195
let cache = gix_repo.commit_graph_if_enabled()?;
200196
let mut graph = gix_repo.revision_graph(cache.as_ref());
201197
let mut check_commit = IsCommitIntegrated::new(ctx, &default_target, &gix_repo, &mut graph)?;

crates/gitbutler-branch-actions/src/virtual.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use gitbutler_project::access::WorktreeWritePermission;
2525
use gitbutler_reference::{normalize_branch_name, Refname, RemoteRefname};
2626
use gitbutler_repo::{
2727
rebase::{cherry_rebase, cherry_rebase_group},
28-
GixRepositoryExt, LogUntil, RepositoryExt,
28+
LogUntil, RepositoryExt,
2929
};
3030
use gitbutler_repo_actions::RepoActionsExt;
3131
use gitbutler_stack::{
@@ -302,10 +302,7 @@ pub fn list_virtual_branches_cached(
302302
let branches_span =
303303
tracing::debug_span!("handle branches", num_branches = status.branches.len()).entered();
304304
let repo = ctx.repository();
305-
let gix_repo = ctx
306-
.gix_repository()?
307-
.for_tree_diffing()?
308-
.with_object_memory();
305+
let gix_repo = ctx.gix_repository_for_merging_non_persisting()?;
309306
// We will perform virtual merges, no need to write them to the ODB.
310307
let cache = gix_repo.commit_graph_if_enabled()?;
311308
let mut graph = gix_repo.revision_graph(cache.as_ref());

crates/gitbutler-command-context/src/lib.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,26 @@ impl CommandContext {
8686
Ok(gix::open(self.repository().path())?)
8787
}
8888

89+
/// Return a newly opened `gitoxide` repository, with all configuration available
90+
/// to correctly figure out author and committer names (i.e. with most global configuration loaded),
91+
/// *and* which will perform diffs quickly thanks to an adequate object cache.
92+
pub fn gix_repository_for_merging(&self) -> Result<gix::Repository> {
93+
let mut repo = gix::open(self.repository().path())?;
94+
let bytes = repo.compute_object_cache_size_for_tree_diffs(&***repo.index_or_empty()?);
95+
repo.object_cache_size_if_unset(bytes);
96+
Ok(repo)
97+
}
98+
99+
/// Return a newly opened `gitoxide` repository, with all configuration available
100+
/// to correctly figure out author and committer names (i.e. with most global configuration loaded),
101+
/// *and* which will perform diffs quickly thanks to an adequate object cache, *and*
102+
/// which **writes all objects into memory**.
103+
///
104+
/// This means *changes are non-persisting*.
105+
pub fn gix_repository_for_merging_non_persisting(&self) -> Result<gix::Repository> {
106+
Ok(self.gix_repository_for_merging()?.with_object_memory())
107+
}
108+
89109
/// Return a newly opened `gitoxide` repository with only the repository-local configuration
90110
/// available. This is a little faster as it has to open less files upon startup.
91111
///

crates/gitbutler-repo/src/repository_ext.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,38 @@ pub trait GixRepositoryExt: Sized {
699699
/// Configure the repository for diff operations between trees.
700700
/// This means it needs an object cache relative to the amount of files in the repository.
701701
fn for_tree_diffing(self) -> Result<Self>;
702+
703+
/// Returns `true` if the merge between `our_tree` and `their_tree` is free of conflicts.
704+
/// Conflicts entail content merges with conflict markers, or anything else that doesn't merge cleanly in the tree.
705+
///
706+
/// # Important
707+
///
708+
/// Make sure the repository is configured [`with_object_memory()`](gix::Repository::with_object_memory()).
709+
fn merges_cleanly_compat(
710+
&self,
711+
ancestor_tree: git2::Oid,
712+
our_tree: git2::Oid,
713+
their_tree: git2::Oid,
714+
) -> Result<bool>;
715+
716+
/// Just like the above, but with `gix` types.
717+
fn merges_cleanly(
718+
&self,
719+
ancestor_tree: gix::ObjectId,
720+
our_tree: gix::ObjectId,
721+
their_tree: gix::ObjectId,
722+
) -> Result<bool>;
723+
724+
/// Return default lable names when merging trees.
725+
///
726+
/// Note that these should probably rather be branch names, but that's for another day.
727+
fn default_merge_labels(&self) -> gix::merge::blob::builtin_driver::text::Labels<'static> {
728+
gix::merge::blob::builtin_driver::text::Labels {
729+
ancestor: Some("base".into()),
730+
current: Some("ours".into()),
731+
other: Some("theirs".into()),
732+
}
733+
}
702734
}
703735

704736
impl GixRepositoryExt for gix::Repository {
@@ -707,6 +739,40 @@ impl GixRepositoryExt for gix::Repository {
707739
self.object_cache_size_if_unset(bytes);
708740
Ok(self)
709741
}
742+
743+
fn merges_cleanly_compat(
744+
&self,
745+
ancestor_tree: git2::Oid,
746+
our_tree: git2::Oid,
747+
their_tree: git2::Oid,
748+
) -> Result<bool> {
749+
self.merges_cleanly(
750+
git2_to_gix_object_id(ancestor_tree),
751+
git2_to_gix_object_id(our_tree),
752+
git2_to_gix_object_id(their_tree),
753+
)
754+
}
755+
756+
fn merges_cleanly(
757+
&self,
758+
ancestor_tree: gix::ObjectId,
759+
our_tree: gix::ObjectId,
760+
their_tree: gix::ObjectId,
761+
) -> Result<bool> {
762+
let mut options = self.tree_merge_options()?;
763+
let conflict_kind = gix::merge::tree::UnresolvedConflict::Renames;
764+
options.fail_on_conflict = Some(conflict_kind);
765+
let merge_outcome = self
766+
.merge_trees(
767+
ancestor_tree,
768+
our_tree,
769+
their_tree,
770+
Default::default(),
771+
options,
772+
)
773+
.context("failed to merge trees")?;
774+
Ok(!merge_outcome.has_unresolved_conflicts(conflict_kind))
775+
}
710776
}
711777

712778
type OidFilter = dyn Fn(&git2::Commit) -> Result<bool>;

0 commit comments

Comments
 (0)