Skip to content

Commit

Permalink
Use gitoxide merge-trees for when applying and unapplying branches.
Browse files Browse the repository at this point in the history
While at it, check make it a thing to check for clean merges easily.
  • Loading branch information
Byron committed Nov 3, 2024
1 parent 650c136 commit 1732e45
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use gitbutler_error::error::Marker;
use gitbutler_oplog::SnapshotExt;
use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_reference::{Refname, RemoteRefname};
use gitbutler_repo::GixRepositoryExt;
use gitbutler_repo::{
rebase::{cherry_rebase_group, gitbutler_merge_commits},
LogUntil, RepositoryExt,
Expand Down Expand Up @@ -301,29 +302,27 @@ impl BranchManager<'_> {
))?;

// Branch is out of date, merge or rebase it
let merge_base_tree = repo
let merge_base_tree_id = repo
.find_commit(merge_base)
.context(format!("failed to find merge base commit {}", merge_base))?
.tree()
.context("failed to find merge base tree")?;

let branch_tree = repo
.find_tree(branch.tree)
.context("failed to find branch tree")?;
.context("failed to find merge base tree")?
.id();
let branch_tree_id = branch.tree;

// We don't support having two branches applied that conflict with each other
{
let uncommited_changes_tree = repo.create_wd_tree()?;
let branch_merged_with_other_applied_branches = repo
.merge_trees(
&merge_base_tree,
&branch_tree,
&uncommited_changes_tree,
None,
let uncommited_changes_tree_id = repo.create_wd_tree()?.id();
let gix_repo = self.ctx.gix_repository_for_merging_non_persisting()?;
let merges_cleanly = gix_repo
.merges_cleanly_compat(
merge_base_tree_id,
branch_tree_id,
uncommited_changes_tree_id,
)
.context("failed to merge trees")?;

if branch_merged_with_other_applied_branches.has_conflicts() {
if !merges_cleanly {
for branch in vb_state
.list_branches_in_workspace()?
.iter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ use git2::Commit;
use gitbutler_branch::BranchExt;
use gitbutler_commit::commit_headers::CommitHeadersV2;
use gitbutler_oplog::SnapshotExt;
use gitbutler_oxidize::git2_to_gix_object_id;
use gitbutler_oxidize::gix_to_git2_oid;
use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_reference::{normalize_branch_name, ReferenceName, Refname};
use gitbutler_repo::GixRepositoryExt;
use gitbutler_repo::RepositoryExt;
use gitbutler_repo::SignaturePurpose;
use gitbutler_repo_actions::RepoActionsExt;
use gitbutler_stack::{Stack, StackId};
use gix::objs::Write;
use tracing::instrument;

use super::BranchManager;
Expand Down Expand Up @@ -79,7 +83,10 @@ impl BranchManager<'_> {

let repo = self.ctx.repository();

let base_tree = target_commit.tree().context("failed to get target tree")?;
let base_tree_id = target_commit
.tree()
.context("failed to get target tree")?
.id();

let applied_statuses = get_applied_status(self.ctx, None)
.context("failed to get status by branch")?
Expand All @@ -98,13 +105,14 @@ impl BranchManager<'_> {
num_branches = applied_statuses.len() - 1
)
.entered();
applied_statuses
let gix_repo = self.ctx.gix_repository()?;
let merge_options = gix_repo.tree_merge_options()?;
let final_tree_id = applied_statuses
.into_iter()
.filter(|(branch, _)| branch.id != branch_id)
.fold(
target_commit.tree().context("failed to get target tree"),
|final_tree, status| {
let final_tree = final_tree?;
.try_fold(
git2_to_gix_object_id(target_commit.tree_id()),
|final_tree_id, status| -> Result<_> {
let branch = status.0;
let files = status
.1
Expand All @@ -113,14 +121,21 @@ impl BranchManager<'_> {
.collect::<Vec<(PathBuf, Vec<VirtualBranchHunk>)>>();
let tree_oid =
gitbutler_diff::write::hunks_onto_oid(self.ctx, branch.head(), files)?;
let branch_tree = repo.find_tree(tree_oid)?;
let mut result =
repo.merge_trees(&base_tree, &final_tree, &branch_tree, None)?;
let final_tree_oid = result.write_tree_to(repo)?;
repo.find_tree(final_tree_oid)
.context("failed to find tree")
let mut merge = gix_repo.merge_trees(
git2_to_gix_object_id(base_tree_id),
final_tree_id,
git2_to_gix_object_id(tree_oid),
gix_repo.default_merge_labels(),
merge_options.clone(),
)?;
let final_tree_id = merge
.tree
.write(|tree| gix_repo.write(tree))
.map_err(|err| anyhow::anyhow!("Could not write merged tree: {err}"))?;
Ok(final_tree_id)
},
)?
)?;
repo.find_tree(gix_to_git2_oid(final_tree_id))?
};

let _span = tracing::debug_span!("checkout final tree").entered();
Expand Down
34 changes: 26 additions & 8 deletions crates/gitbutler-branch-actions/src/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ use gitbutler_command_context::CommandContext;
use gitbutler_commit::commit_ext::CommitExt;
use gitbutler_error::error::Marker;
use gitbutler_operating_modes::OPEN_WORKSPACE_REFS;
use gitbutler_oxidize::{git2_to_gix_object_id, gix_to_git2_oid};
use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_repo::SignaturePurpose;
use gitbutler_repo::{GixRepositoryExt, SignaturePurpose};
use gitbutler_repo::{LogUntil, RepositoryExt};
use gitbutler_stack::{Stack, VirtualBranchesHandle};
use gix::objs::Write;
use tracing::instrument;

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

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

if conflicts::is_conflicting(ctx, None)? {
let merge_parent = conflicts::merge_parent(ctx)?.ok_or(anyhow!("No merge parent"))?;
Expand All @@ -49,22 +52,37 @@ pub(crate) fn get_workspace_head(ctx: &CommandContext) -> Result<git2::Oid> {
let merge_base = repo.merge_base(first_branch.head(), merge_parent)?;
workspace_tree = repo.find_commit(merge_base)?.tree()?;
} else {
let gix_repo = ctx.gix_repository_for_merging()?;
let mut merge_options_fail_fast = gix_repo.tree_merge_options()?;
let conflict_kind = gix::merge::tree::UnresolvedConflict::Renames;
merge_options_fail_fast.fail_on_conflict = Some(conflict_kind);
let merge_tree_id = git2_to_gix_object_id(repo.find_commit(target.sha)?.tree_id());
for branch in virtual_branches.iter_mut() {
let merge_tree = repo.find_commit(target.sha)?.tree()?;
let branch_tree = repo.find_commit(branch.head())?;
let branch_tree = repo.find_real_tree(&branch_tree, Default::default())?;

let mut index = repo.merge_trees(&merge_tree, &workspace_tree, &branch_tree, None)?;
let branch_head = repo.find_commit(branch.head())?;
let branch_tree_id =
git2_to_gix_object_id(repo.find_real_tree(&branch_head, Default::default())?.id());

let mut merge = gix_repo.merge_trees(
merge_tree_id,
workspace_tree_id,
branch_tree_id,
gix_repo.default_merge_labels(),
merge_options_fail_fast.clone(),
)?;

if !index.has_conflicts() {
workspace_tree = repo.find_tree(index.write_tree_to(repo)?)?;
if !merge.has_unresolved_conflicts(conflict_kind) {
workspace_tree_id = merge
.tree
.write(|tree| gix_repo.write(tree))
.map_err(|err| anyhow!("{err}"))?;
} else {
// This branch should have already been unapplied during the "update" command but for some reason that failed
tracing::warn!("Merge conflict between base and {:?}", branch.name);
branch.in_workspace = false;
vb_state.set_branch(branch.clone())?;
}
}
workspace_tree = repo.find_tree(gix_to_git2_oid(workspace_tree_id))?;
}

let committer = gitbutler_repo::signature(SignaturePurpose::Committer)?;
Expand Down
6 changes: 1 addition & 5 deletions crates/gitbutler-branch-actions/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use gitbutler_oplog::entry::{OperationKind, SnapshotDetails};
use gitbutler_oplog::{OplogExt, SnapshotExt};
use gitbutler_project::Project;
use gitbutler_reference::normalize_branch_name;
use gitbutler_repo::GixRepositoryExt;
use gitbutler_repo_actions::RepoActionsExt;
use gitbutler_stack::{
CommitOrChangeId, ForgeIdentifier, PatchReference, PatchReferenceUpdate, Series,
Expand Down Expand Up @@ -192,10 +191,7 @@ pub fn push_stack(project: &Project, branch_id: StackId, with_force: bool) -> Re

// First fetch, because we dont want to push integrated series
ctx.fetch(&default_target.push_remote_name(), None)?;
let gix_repo = ctx
.gix_repository()?
.for_tree_diffing()?
.with_object_memory();
let gix_repo = ctx.gix_repository_for_merging_non_persisting()?;
let cache = gix_repo.commit_graph_if_enabled()?;
let mut graph = gix_repo.revision_graph(cache.as_ref());
let mut check_commit = IsCommitIntegrated::new(ctx, &default_target, &gix_repo, &mut graph)?;
Expand Down
7 changes: 2 additions & 5 deletions crates/gitbutler-branch-actions/src/virtual.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_reference::{normalize_branch_name, Refname, RemoteRefname};
use gitbutler_repo::{
rebase::{cherry_rebase, cherry_rebase_group},
GixRepositoryExt, LogUntil, RepositoryExt,
LogUntil, RepositoryExt,
};
use gitbutler_repo_actions::RepoActionsExt;
use gitbutler_stack::{
Expand Down Expand Up @@ -302,10 +302,7 @@ pub fn list_virtual_branches_cached(
let branches_span =
tracing::debug_span!("handle branches", num_branches = status.branches.len()).entered();
let repo = ctx.repository();
let gix_repo = ctx
.gix_repository()?
.for_tree_diffing()?
.with_object_memory();
let gix_repo = ctx.gix_repository_for_merging_non_persisting()?;
// We will perform virtual merges, no need to write them to the ODB.
let cache = gix_repo.commit_graph_if_enabled()?;
let mut graph = gix_repo.revision_graph(cache.as_ref());
Expand Down
20 changes: 20 additions & 0 deletions crates/gitbutler-command-context/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,26 @@ impl CommandContext {
Ok(gix::open(self.repository().path())?)
}

/// Return a newly opened `gitoxide` repository, with all configuration available
/// to correctly figure out author and committer names (i.e. with most global configuration loaded),
/// *and* which will perform diffs quickly thanks to an adequate object cache.
pub fn gix_repository_for_merging(&self) -> Result<gix::Repository> {
let mut repo = gix::open(self.repository().path())?;
let bytes = repo.compute_object_cache_size_for_tree_diffs(&***repo.index_or_empty()?);
repo.object_cache_size_if_unset(bytes);
Ok(repo)
}

/// Return a newly opened `gitoxide` repository, with all configuration available
/// to correctly figure out author and committer names (i.e. with most global configuration loaded),
/// *and* which will perform diffs quickly thanks to an adequate object cache, *and*
/// which **writes all objects into memory**.
///
/// This means *changes are non-persisting*.
pub fn gix_repository_for_merging_non_persisting(&self) -> Result<gix::Repository> {
Ok(self.gix_repository_for_merging()?.with_object_memory())
}

/// Return a newly opened `gitoxide` repository with only the repository-local configuration
/// available. This is a little faster as it has to open less files upon startup.
///
Expand Down
66 changes: 66 additions & 0 deletions crates/gitbutler-repo/src/repository_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,38 @@ pub trait GixRepositoryExt: Sized {
/// Configure the repository for diff operations between trees.
/// This means it needs an object cache relative to the amount of files in the repository.
fn for_tree_diffing(self) -> Result<Self>;

/// Returns `true` if the merge between `our_tree` and `their_tree` is free of conflicts.
/// Conflicts entail content merges with conflict markers, or anything else that doesn't merge cleanly in the tree.
///
/// # Important
///
/// Make sure the repository is configured [`with_object_memory()`](gix::Repository::with_object_memory()).
fn merges_cleanly_compat(
&self,
ancestor_tree: git2::Oid,
our_tree: git2::Oid,
their_tree: git2::Oid,
) -> Result<bool>;

/// Just like the above, but with `gix` types.
fn merges_cleanly(
&self,
ancestor_tree: gix::ObjectId,
our_tree: gix::ObjectId,
their_tree: gix::ObjectId,
) -> Result<bool>;

/// Return default lable names when merging trees.
///
/// Note that these should probably rather be branch names, but that's for another day.
fn default_merge_labels(&self) -> gix::merge::blob::builtin_driver::text::Labels<'static> {
gix::merge::blob::builtin_driver::text::Labels {
ancestor: Some("base".into()),
current: Some("ours".into()),
other: Some("theirs".into()),
}
}
}

impl GixRepositoryExt for gix::Repository {
Expand All @@ -707,6 +739,40 @@ impl GixRepositoryExt for gix::Repository {
self.object_cache_size_if_unset(bytes);
Ok(self)
}

fn merges_cleanly_compat(
&self,
ancestor_tree: git2::Oid,
our_tree: git2::Oid,
their_tree: git2::Oid,
) -> Result<bool> {
self.merges_cleanly(
git2_to_gix_object_id(ancestor_tree),
git2_to_gix_object_id(our_tree),
git2_to_gix_object_id(their_tree),
)
}

fn merges_cleanly(
&self,
ancestor_tree: gix::ObjectId,
our_tree: gix::ObjectId,
their_tree: gix::ObjectId,
) -> Result<bool> {
let mut options = self.tree_merge_options()?;
let conflict_kind = gix::merge::tree::UnresolvedConflict::Renames;
options.fail_on_conflict = Some(conflict_kind);
let merge_outcome = self
.merge_trees(
ancestor_tree,
our_tree,
their_tree,
Default::default(),
options,
)
.context("failed to merge trees")?;
Ok(!merge_outcome.has_unresolved_conflicts(conflict_kind))
}
}

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

0 comments on commit 1732e45

Please sign in to comment.