From 6b2130474d7e777303534bb6d48c88c9f43471b2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 2 Nov 2024 15:27:39 +0100 Subject: [PATCH] Use `gitoxide` or `is_integrated()`. This is the most expensive call as it's possible to trigger a lot of merges. --- Cargo.lock | 24 +++ crates/gitbutler-branch-actions/Cargo.toml | 2 +- crates/gitbutler-branch-actions/src/stack.rs | 15 +- .../gitbutler-branch-actions/src/virtual.rs | 137 +++++++++++------- 4 files changed, 124 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1d3cab556e..840253f75b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3059,6 +3059,7 @@ dependencies = [ "gix-ignore 0.12.0", "gix-index 0.36.0", "gix-lock 15.0.0", + "gix-merge", "gix-negotiate", "gix-object 0.45.0", "gix-odb", @@ -3603,6 +3604,29 @@ dependencies = [ "thiserror", ] +[[package]] +name = "gix-merge" +version = "0.0.0" +source = "git+https://github.com/Byron/gitoxide?rev=3fb989be21c739bbfeac93953c1685e7c6cd2106#3fb989be21c739bbfeac93953c1685e7c6cd2106" +dependencies = [ + "bstr", + "gix-command", + "gix-diff", + "gix-filter", + "gix-fs 0.12.0", + "gix-hash 0.15.0", + "gix-object 0.45.0", + "gix-path 0.10.12", + "gix-quote 0.4.13", + "gix-revision", + "gix-revwalk 0.16.0", + "gix-tempfile 15.0.0", + "gix-trace 0.1.11", + "gix-worktree 0.37.0", + "imara-diff", + "thiserror", +] + [[package]] name = "gix-negotiate" version = "0.16.0" diff --git a/crates/gitbutler-branch-actions/Cargo.toml b/crates/gitbutler-branch-actions/Cargo.toml index fff381de7c..59c735aec5 100644 --- a/crates/gitbutler-branch-actions/Cargo.toml +++ b/crates/gitbutler-branch-actions/Cargo.toml @@ -9,7 +9,7 @@ publish = false tracing.workspace = true anyhow = "1.0.92" git2.workspace = true -gix = { workspace = true, features = ["blob-diff", "revision"] } +gix = { workspace = true, features = ["blob-diff", "revision", "blob-merge"] } tokio.workspace = true gitbutler-oplog.workspace = true gitbutler-repo.workspace = true diff --git a/crates/gitbutler-branch-actions/src/stack.rs b/crates/gitbutler-branch-actions/src/stack.rs index 9893e97ef0..03f8a5c787 100644 --- a/crates/gitbutler-branch-actions/src/stack.rs +++ b/crates/gitbutler-branch-actions/src/stack.rs @@ -7,6 +7,7 @@ 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, @@ -191,14 +192,20 @@ 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 check_commit = IsCommitIntegrated::new(ctx, &default_target)?; + let gix_repo = ctx + .gix_repository()? + .for_tree_diffing()? + .with_object_memory(); + 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)?; let stack_series = stack.list_series(ctx)?; for series in stack_series { if series.head.target == merge_base { // Nothing to push for this one continue; } - if series_integrated(&check_commit, &series)? { + if series_integrated(&mut check_commit, &series)? { // Already integrated, nothing to push continue; } @@ -214,7 +221,7 @@ pub fn push_stack(project: &Project, branch_id: StackId, with_force: bool) -> Re Ok(()) } -fn series_integrated(check_commit: &IsCommitIntegrated, series: &Series) -> Result { +fn series_integrated(check_commit: &mut IsCommitIntegrated, series: &Series) -> Result { let mut is_integrated = false; for commit in series.clone().local_commits.iter().rev() { if !is_integrated { @@ -230,7 +237,7 @@ pub(crate) fn stack_series( ctx: &CommandContext, branch: &mut Stack, default_target: &Target, - check_commit: &IsCommitIntegrated, + check_commit: &mut IsCommitIntegrated, remote_commit_data: HashMap, ) -> Result<(Vec, bool)> { let mut requires_force = false; diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index b972f9e278..4649024ab3 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -20,12 +20,12 @@ use gitbutler_commit::{commit_ext::CommitExt, commit_headers::HasCommitHeaders}; use gitbutler_diff::{trees, GitHunk, Hunk}; use gitbutler_error::error::Code; use gitbutler_operating_modes::assure_open_workspace_mode; -use gitbutler_oxidize::git2_signature_to_gix_signature; +use gitbutler_oxidize::{git2_signature_to_gix_signature, git2_to_gix_object_id, gix_to_git2_oid}; use gitbutler_project::access::WorktreeWritePermission; use gitbutler_reference::{normalize_branch_name, Refname, RemoteRefname}; use gitbutler_repo::{ rebase::{cherry_rebase, cherry_rebase_group}, - LogUntil, RepositoryExt, + GixRepositoryExt, LogUntil, RepositoryExt, }; use gitbutler_repo_actions::RepoActionsExt; use gitbutler_stack::{ @@ -33,6 +33,7 @@ use gitbutler_stack::{ VirtualBranchesHandle, }; use gitbutler_time::time::now_since_unix_epoch_ms; +use gix::objs::Write; use serde::Serialize; use std::collections::HashSet; use std::{collections::HashMap, path::PathBuf, vec}; @@ -300,8 +301,15 @@ 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(); + // 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()); for (mut branch, mut files) in status.branches { - let repo = ctx.repository(); update_conflict_markers(ctx, files.clone())?; let upstream_branch = match branch.clone().upstream { @@ -323,13 +331,18 @@ pub fn list_virtual_branches_cached( .as_ref() .map( |upstream| -> Result<(HashSet, HashMap)> { - let merge_base = - repo.merge_base(upstream.id(), default_target.sha) - .context(format!( - "failed to find merge base between {} and {}", - upstream.id(), - default_target.sha - ))?; + let merge_base = gix_repo + .merge_base_with_graph( + git2_to_gix_object_id(upstream.id()), + git2_to_gix_object_id(default_target.sha), + &mut graph, + ) + .context(format!( + "failed to find merge base between {} and {}", + upstream.id(), + default_target.sha + ))?; + let merge_base = gitbutler_oxidize::gix_to_git2_oid(merge_base); let remote_commit_ids = HashSet::from_iter(repo.l( upstream.id(), LogUntil::Commit(merge_base), @@ -356,7 +369,8 @@ pub fn list_virtual_branches_cached( // find all commits on head that are not on target.sha let commits = repo.log(branch.head(), LogUntil::Commit(default_target.sha), false)?; - let check_commit = IsCommitIntegrated::new(ctx, &default_target)?; + let mut check_commit = + IsCommitIntegrated::new(ctx, &default_target, &gix_repo, &mut graph)?; let vbranch_commits = { let _span = tracing::debug_span!( "is-commit-integrated", @@ -397,9 +411,14 @@ pub fn list_virtual_branches_cached( .collect::>>()? }; - let merge_base = repo - .merge_base(default_target.sha, branch.head()) + let merge_base = gix_repo + .merge_base_with_graph( + git2_to_gix_object_id(default_target.sha), + git2_to_gix_object_id(branch.head()), + check_commit.graph, + ) .context("failed to find merge base")?; + let merge_base = gix_to_git2_oid(merge_base); let base_current = true; let upstream = upstream_branch.and_then(|upstream_branch| { @@ -436,7 +455,7 @@ pub fn list_virtual_branches_cached( ctx, &mut branch, &default_target, - &check_commit, + &mut check_commit, remote_commit_data, ) { Ok((series, force)) => { @@ -943,40 +962,50 @@ pub(crate) fn push( }) } -pub(crate) struct IsCommitIntegrated<'repo> { - repo: &'repo git2::Repository, - target_commit_id: git2::Oid, - remote_head_id: git2::Oid, +type MergeBaseCommitGraph<'repo, 'cache> = gix::revwalk::Graph< + 'repo, + 'cache, + gix::revision::plumbing::graph::Commit, +>; + +pub(crate) struct IsCommitIntegrated<'repo, 'cache, 'graph> { + gix_repo: &'repo gix::Repository, + graph: &'graph mut MergeBaseCommitGraph<'repo, 'cache>, + target_commit_id: gix::ObjectId, + upstream_tree_id: gix::ObjectId, upstream_commits: Vec, - /// A repository opened at the same path as `repo`, but with an in-memory ODB attached - /// to avoid writing intermediate objects. - inmemory_repo: git2::Repository, } -impl<'repo> IsCommitIntegrated<'repo> { - pub(crate) fn new(ctx: &'repo CommandContext, target: &Target) -> anyhow::Result { +impl<'repo, 'cache, 'graph> IsCommitIntegrated<'repo, 'cache, 'graph> { + pub(crate) fn new( + ctx: &'repo CommandContext, + target: &Target, + gix_repo: &'repo gix::Repository, + graph: &'graph mut MergeBaseCommitGraph<'repo, 'cache>, + ) -> anyhow::Result { let remote_branch = ctx .repository() .maybe_find_branch_by_refname(&target.branch.clone().into())? .ok_or(anyhow!("failed to get branch"))?; let remote_head = remote_branch.get().peel_to_commit()?; - let upstream_commits = + let mut upstream_commits = ctx.repository() .l(remote_head.id(), LogUntil::Commit(target.sha), false)?; - let inmemory_repo = ctx.repository().in_memory_repo()?; + upstream_commits.sort(); + let upstream_tree_id = ctx.repository().find_commit(remote_head.id())?.tree_id(); Ok(Self { - repo: ctx.repository(), - target_commit_id: target.sha, - remote_head_id: remote_head.id(), + gix_repo, + graph, + target_commit_id: git2_to_gix_object_id(target.sha), + upstream_tree_id: git2_to_gix_object_id(upstream_tree_id), upstream_commits, - inmemory_repo, }) } } -impl IsCommitIntegrated<'_> { - pub(crate) fn is_integrated(&self, commit: &git2::Commit) -> Result { - if self.target_commit_id == commit.id() { +impl IsCommitIntegrated<'_, '_, '_> { + pub(crate) fn is_integrated(&mut self, commit: &git2::Commit) -> Result { + if self.target_commit_id == git2_to_gix_object_id(commit.id()) { // could not be integrated if heads are the same. return Ok(false); } @@ -986,44 +1015,54 @@ impl IsCommitIntegrated<'_> { return Ok(false); } - if self.upstream_commits.contains(&commit.id()) { + if self.upstream_commits.binary_search(&commit.id()).is_ok() { return Ok(true); } - let merge_base_id = self.repo.merge_base(self.target_commit_id, commit.id())?; - if merge_base_id.eq(&commit.id()) { + let merge_base_id = self.gix_repo.merge_base_with_graph( + self.target_commit_id, + git2_to_gix_object_id(commit.id()), + self.graph, + )?; + if gix_to_git2_oid(merge_base_id).eq(&commit.id()) { // if merge branch is the same as branch head and there are upstream commits // then it's integrated return Ok(true); } - let merge_base = self.repo.find_commit(merge_base_id)?; - let merge_base_tree = merge_base.tree()?; - let upstream = self.repo.find_commit(self.remote_head_id)?; - let upstream_tree = upstream.tree()?; - - if merge_base_tree.id() == upstream_tree.id() { + let merge_base_tree_id = self.gix_repo.find_commit(merge_base_id)?.tree_id()?; + if merge_base_tree_id == self.upstream_tree_id { // if merge base is the same as upstream tree, then it's integrated return Ok(true); } // try to merge our tree into the upstream tree - let mut merge_index = self - .repo - .merge_trees(&merge_base_tree, &commit.tree()?, &upstream_tree, None) + let merge_options = self.gix_repo.tree_merge_options()?; + let mut merge_output = self + .gix_repo + .merge_trees( + merge_base_tree_id, + git2_to_gix_object_id(commit.tree_id()), + self.upstream_tree_id, + Default::default(), + merge_options, + ) .context("failed to merge trees")?; - if merge_index.has_conflicts() { + if merge_output + .has_unresolved_conflicts(gix::merge::tree::UnresolvedConflict::ConflictMarkers) + { return Ok(false); } - let merge_tree_oid = merge_index - .write_tree_to(&self.inmemory_repo) - .context("failed to write tree")?; + let merge_tree_id = merge_output + .tree + .write(|tree| self.gix_repo.write(tree)) + .map_err(|err| anyhow!("failed to write tree: {err}"))?; // if the merge_tree is the same as the new_target_tree and there are no files (uncommitted changes) // then the vbranch is fully merged - Ok(merge_tree_oid == upstream_tree.id()) + Ok(merge_tree_id == self.upstream_tree_id) } }