Skip to content

Commit

Permalink
Merge pull request #4793 from Byron/git2-to-gix
Browse files Browse the repository at this point in the history
performance improvements and bug fixes
  • Loading branch information
krlvi authored Aug 30, 2024
2 parents 717078d + 4d495cb commit 2c7773a
Show file tree
Hide file tree
Showing 26 changed files with 268 additions and 148 deletions.
2 changes: 1 addition & 1 deletion apps/desktop/src/lib/stores/remoteBranches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class RemoteBranchService {
try {
const remoteBranches = plainToInstance(
Branch,
await invoke<any[]>('list_remote_branches', { projectId: this.projectId })
await invoke<any[]>('list_local_branches', { projectId: this.projectId })
);
this.projectMetrics?.setMetric('normal_branch_count', remoteBranches.length);
this.branches.set(remoteBranches);
Expand Down
11 changes: 9 additions & 2 deletions apps/desktop/src/lib/vbranches/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,19 @@ export class DetailedCommit {
conflicted!: boolean;
// Set if a GitButler branch reference pointing to this commit exists. In the format of "refs/remotes/origin/my-branch"
remoteRef?: string | undefined;
// Identifies the remote commit id from which this local commit was copied. The backend figured this out by comparing
// author, commit and message.
copiedFromRemoteId?: string;

prev?: DetailedCommit;
next?: DetailedCommit;

get status(): CommitStatus {
if (this.isIntegrated) return 'integrated';
if (this.isRemote && (!this.relatedTo || this.id === this.relatedTo.id))
if (
(this.isRemote && (!this.relatedTo || this.id === this.relatedTo.id)) ||
(this.copiedFromRemoteId && this.relatedTo && this.copiedFromRemoteId === this.relatedTo.id)
)
return 'localAndRemote';
return 'local';
}
Expand Down Expand Up @@ -240,9 +246,10 @@ export class Commit {

export type AnyCommit = DetailedCommit | Commit;

export function commitCompare(left: AnyCommit, right: AnyCommit): boolean {
export function commitCompare(left: AnyCommit, right: DetailedCommit): boolean {
if (left.id === right.id) return true;
if (left.changeId && right.changeId && left.changeId === right.changeId) return true;
if (right.copiedFromRemoteId && right.copiedFromRemoteId === left.id) return true;
return false;
}

Expand Down
7 changes: 4 additions & 3 deletions crates/gitbutler-branch-actions/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
branch::get_uncommited_files,
branch_manager::BranchManagerExt,
file::RemoteBranchFile,
remote::{get_branch_data, list_remote_branches, RemoteBranch, RemoteBranchData},
remote::{get_branch_data, list_local_branches, RemoteBranch, RemoteBranchData},
VirtualBranchesExt,
};
use anyhow::{Context, Result};
Expand Down Expand Up @@ -485,9 +485,9 @@ impl VirtualBranchActions {
branch::push(&ctx, branch_id, with_force, &helper, askpass)
}

pub fn list_remote_branches(project: Project) -> Result<Vec<RemoteBranch>> {
pub fn list_local_branches(project: Project) -> Result<Vec<RemoteBranch>> {
let ctx = CommandContext::open(&project)?;
list_remote_branches(&ctx)
list_local_branches(&ctx)
}

pub fn get_remote_branch_data(
Expand Down Expand Up @@ -581,6 +581,7 @@ impl VirtualBranchActions {
branch::move_commit(&ctx, target_branch_id, commit_oid).map_err(Into::into)
}

#[instrument(level = tracing::Level::DEBUG, skip(self, project), err(Debug))]
pub fn create_virtual_branch_from_branch(
&self,
project: &Project,
Expand Down
2 changes: 1 addition & 1 deletion crates/gitbutler-branch-actions/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ pub(crate) fn set_base_branch(
// if there are any commits on the head branch or uncommitted changes in the working directory, we need to
// put them into a virtual branch

let wd_diff = gitbutler_diff::workdir(repo, &current_head_commit.id())?;
let wd_diff = gitbutler_diff::workdir(repo, current_head_commit.id())?;
if !wd_diff.is_empty() || current_head_commit.id() != target.sha {
// assign ownership to the branch
let ownership = wd_diff.iter().fold(
Expand Down
11 changes: 6 additions & 5 deletions crates/gitbutler-branch-actions/src/branch.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::integration::get_workspace_head;
use crate::{RemoteBranchFile, VirtualBranchesExt};
use anyhow::{bail, Context, Result};
use bstr::{BStr, ByteSlice};
Expand All @@ -9,7 +10,7 @@ use gitbutler_command_context::CommandContext;
use gitbutler_diff::DiffByPathMap;
use gitbutler_project::access::WorktreeReadPermission;
use gitbutler_reference::normalize_branch_name;
use gitbutler_repo::{GixRepositoryExt, RepositoryExt};
use gitbutler_repo::GixRepositoryExt;
use gitbutler_serde::BStringForFrontend;
use gix::object::tree::diff::Action;
use gix::prelude::ObjectIdExt;
Expand All @@ -23,14 +24,14 @@ use std::{
fmt::Debug,
vec,
};
use tracing::instrument;

#[instrument(level = tracing::Level::DEBUG, skip(ctx, _permission))]
pub(crate) fn get_uncommited_files_raw(
context: &CommandContext,
ctx: &CommandContext,
_permission: &WorktreeReadPermission,
) -> Result<DiffByPathMap> {
let repository = context.repository();
let head_commit = repository.head_commit()?;
gitbutler_diff::workdir(repository, &head_commit.id())
gitbutler_diff::workdir(ctx.repository(), get_workspace_head(ctx)?)
.context("Failed to list uncommited files")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_reference::{Refname, RemoteRefname};
use gitbutler_repo::{rebase::cherry_rebase, RepoActionsExt, RepositoryExt};
use gitbutler_time::time::now_since_unix_epoch_ms;
use tracing::instrument;

use super::BranchManager;
use crate::{
Expand All @@ -20,6 +21,7 @@ use crate::{
};

impl BranchManager<'_> {
#[instrument(level = tracing::Level::DEBUG, skip(self, perm), err(Debug))]
pub fn create_virtual_branch(
&self,
create: &BranchCreateRequest,
Expand Down Expand Up @@ -281,6 +283,7 @@ impl BranchManager<'_> {

/// Holding private methods associated to branch creation
impl BranchManager<'_> {
#[instrument(level = tracing::Level::DEBUG, skip(self, perm), err(Debug))]
fn apply_branch(
&self,
branch_id: BranchId,
Expand Down Expand Up @@ -310,6 +313,7 @@ impl BranchManager<'_> {
default_target.sha, branch.head
))?;
if merge_base != default_target.sha {
let _span = tracing::debug_span!("merge-base isn't default-target").entered();
// Branch is out of date, merge or rebase it
let merge_base_tree = repo
.find_commit(merge_base)
Expand Down Expand Up @@ -454,6 +458,8 @@ impl BranchManager<'_> {
vb_state.set_branch(branch.clone())?;
}

let _span = tracing::debug_span!("finalize").entered();

let wd_tree = self.ctx.repository().create_wd_tree()?;

let branch_tree = repo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use gitbutler_oplog::SnapshotExt;
use gitbutler_project::access::WorktreeWritePermission;
use gitbutler_reference::{normalize_branch_name, ReferenceName, Refname};
use gitbutler_repo::{RepoActionsExt, RepositoryExt};
use tracing::instrument;

use super::BranchManager;
use crate::{
Expand All @@ -19,6 +20,7 @@ use crate::{

impl BranchManager<'_> {
// to unapply a branch, we need to write the current tree out, then remove those file changes from the wd
#[instrument(level = tracing::Level::DEBUG, skip(self, perm), err(Debug))]
pub fn convert_to_real_branch(
&self,
branch_id: BranchId,
Expand Down Expand Up @@ -52,6 +54,7 @@ impl BranchManager<'_> {
real_branch.reference_name()
}

#[instrument(level = tracing::Level::DEBUG, skip(self, perm), err(Debug))]
pub(crate) fn delete_branch(
&self,
branch_id: BranchId,
Expand Down Expand Up @@ -88,30 +91,38 @@ impl BranchManager<'_> {

// go through the other applied branches and merge them into the final tree
// then check that out into the working directory
let final_tree = 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?;
let branch = status.0;
let files = status
.1
.into_iter()
.map(|file| (file.path, file.hunks))
.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 final_tree = {
let _span = tracing::debug_span!(
"new tree without deleted branch",
num_branches = applied_statuses.len() - 1
)
.entered();
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?;
let branch = status.0;
let files = status
.1
.into_iter()
.map(|file| (file.path, file.hunks))
.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 _span = tracing::debug_span!("checkout final tree").entered();
// checkout final_tree into the working directory
repo.checkout_tree_builder(&final_tree)
.force()
Expand All @@ -128,6 +139,7 @@ impl BranchManager<'_> {
}

impl BranchManager<'_> {
#[instrument(level = tracing::Level::DEBUG, skip(self, vbranch), err(Debug))]
fn build_real_branch(&self, vbranch: &mut Branch) -> Result<git2::Branch<'_>> {
let repo = self.ctx.repository();
let target_commit = repo.find_commit(vbranch.head)?;
Expand Down
7 changes: 7 additions & 0 deletions crates/gitbutler-branch-actions/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ pub struct VirtualBranchCommit {
pub change_id: Option<String>,
pub is_signed: bool,
pub conflicted: bool,
/// The id of the remote commit from which this one was copied, as identified by
/// having equal author, committer, and commit message.
/// This is used by the frontend similar to the `change_id` to group matching commits.
#[serde(with = "gitbutler_serde::oid_opt")]
pub copied_from_remote_id: Option<git2::Oid>,
pub remote_ref: Option<ReferenceName>,
}

Expand All @@ -45,6 +50,7 @@ pub(crate) fn commit_to_vbranch_commit(
commit: &git2::Commit,
is_integrated: bool,
is_remote: bool,
copied_from_remote_id: Option<git2::Oid>,
) -> Result<VirtualBranchCommit> {
let timestamp = u128::try_from(commit.time().seconds())?;
let message = commit.message_bstr().to_owned();
Expand Down Expand Up @@ -81,6 +87,7 @@ pub(crate) fn commit_to_vbranch_commit(
change_id: commit.change_id(),
is_signed: commit.is_signed(),
conflicted: commit.is_conflicted(),
copied_from_remote_id,
remote_ref,
};

Expand Down
1 change: 1 addition & 0 deletions crates/gitbutler-branch-actions/src/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ fn write_integration_file(head: &git2::Reference, path: PathBuf) -> Result<()> {
std::fs::write(path, format!(":{}", sha))?;
Ok(())
}
#[instrument(level = tracing::Level::DEBUG, skip(vb_state, ctx), err(Debug))]
pub fn update_gitbutler_integration(
vb_state: &VirtualBranchesHandle,
ctx: &CommandContext,
Expand Down
2 changes: 1 addition & 1 deletion crates/gitbutler-branch-actions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ mod file;
pub use file::{Get, RemoteBranchFile};

mod remote;
pub use remote::{list_remote_branches, RemoteBranch, RemoteBranchData, RemoteCommit};
pub use remote::{list_local_branches, RemoteBranch, RemoteBranchData, RemoteCommit};

pub mod conflicts;

Expand Down
Loading

0 comments on commit 2c7773a

Please sign in to comment.