From f4ec31f81b785efca71a06b79de2b897586db31f Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Wed, 5 Jun 2024 14:31:26 +0200 Subject: [PATCH] fix a bug in handling the case of remote refname being none --- .../gitbutler-core/src/git/repository_ext.rs | 14 ++++-- .../src/virtual_branches/base.rs | 15 +++--- .../src/virtual_branches/virtual.rs | 23 ++++----- .../gitbutler-testsupport/src/test_project.rs | 48 ++++++++++++------- 4 files changed, 57 insertions(+), 43 deletions(-) diff --git a/crates/gitbutler-core/src/git/repository_ext.rs b/crates/gitbutler-core/src/git/repository_ext.rs index 274ff0f29..95785b10d 100644 --- a/crates/gitbutler-core/src/git/repository_ext.rs +++ b/crates/gitbutler-core/src/git/repository_ext.rs @@ -19,7 +19,7 @@ pub trait RepositoryExt { fn checkout_index_builder<'a>(&'a self, index: &'a mut git2::Index) -> CheckoutIndexBuilder; fn checkout_index_path_builder>(&self, path: P) -> Result<()>; fn checkout_tree_builder<'a>(&'a self, tree: &'a git2::Tree<'a>) -> CheckoutTreeBuidler; - fn find_branch_by_refname(&self, name: &Refname) -> Result; + fn find_branch_by_refname(&self, name: &Refname) -> Result>; /// Based on the index, add all data similar to `git add .` and create a tree from it, which is returned. fn get_wd_tree(&self) -> Result; @@ -79,8 +79,8 @@ impl RepositoryExt for Repository { } } - fn find_branch_by_refname(&self, name: &Refname) -> Result { - self.find_branch( + fn find_branch_by_refname(&self, name: &Refname) -> Result> { + let branch = self.find_branch( &name.simple_name(), match name { Refname::Virtual(_) | Refname::Local(_) | Refname::Other(_) => { @@ -88,8 +88,12 @@ impl RepositoryExt for Repository { } Refname::Remote(_) => git2::BranchType::Remote, }, - ) - // .map_err(Into::into) + ); + match branch { + Ok(branch) => Ok(Some(branch)), + Err(e) if e.code() == git2::ErrorCode::NotFound => Ok(None), + Err(e) => Err(e.into()), + } } #[instrument(level = tracing::Level::DEBUG, skip(self), err(Debug))] diff --git a/crates/gitbutler-core/src/virtual_branches/base.rs b/crates/gitbutler-core/src/virtual_branches/base.rs index c14e88eff..e450f8531 100644 --- a/crates/gitbutler-core/src/virtual_branches/base.rs +++ b/crates/gitbutler-core/src/virtual_branches/base.rs @@ -1,6 +1,6 @@ use std::{path::Path, time}; -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::{anyhow, Context, Result}; use git2::Index; use serde::Serialize; @@ -131,11 +131,9 @@ pub fn set_base_branch( // lookup a branch by name let target_branch = match repo.find_branch_by_refname(&target_branch_ref.clone().into()) { Ok(branch) => branch, - Err(err) if err.code() == git2::ErrorCode::NotFound => { - bail!("remote branch '{}' not found", target_branch_ref) - } - Err(err) => return Err(err.into()), - }; + Err(err) => return Err(err), + } + .ok_or(anyhow!("remote branch '{}' not found", target_branch_ref))?; let remote = repo .find_remote(target_branch_ref.remote()) @@ -340,6 +338,7 @@ pub fn update_base_branch( .context(format!("failed to find branch {}", target.branch))?; let new_target_commit = target_branch + .ok_or(anyhow!("failed to get branch"))? .get() .peel_to_commit() .context(format!("failed to peel branch {} to commit", target.branch))?; @@ -577,7 +576,9 @@ pub fn target_to_base_branch( target: &target::Target, ) -> Result { let repo = project_repository.repo(); - let branch = repo.find_branch_by_refname(&target.branch.clone().into())?; + let branch = repo + .find_branch_by_refname(&target.branch.clone().into())? + .ok_or(anyhow!("failed to get branch"))?; let commit = branch.get().peel_to_commit()?; let oid = commit.id(); diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index 432686eb8..288bf2303 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -732,19 +732,10 @@ pub fn list_virtual_branches( let repo = project_repository.repo(); update_conflict_markers(project_repository, &files)?; - let upstream_branch = match branch - .upstream - .as_ref() - .map(|name| repo.find_branch_by_refname(&git::Refname::from(name))) - .transpose() - { - Ok(branch) => Ok(branch), - Err(error) => Err(error), - } - .context(format!( - "failed to find upstream branch for {}", - branch.name - ))?; + let upstream_branch = match branch.clone().upstream { + Some(upstream) => repo.find_branch_by_refname(&git::Refname::from(upstream))?, + None => None, + }; let upstram_branch_commit = upstream_branch .as_ref() @@ -2398,7 +2389,8 @@ fn is_commit_integrated( ) -> Result { let remote_branch = project_repository .repo() - .find_branch_by_refname(&target.branch.clone().into())?; + .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 = project_repository.l( remote_head.id().into(), @@ -2471,7 +2463,8 @@ pub fn is_remote_branch_mergeable( let branch = project_repository .repo() - .find_branch_by_refname(&branch_name.into())?; + .find_branch_by_refname(&branch_name.into())? + .ok_or(anyhow!("branch not found"))?; let branch_oid = branch.get().target().context("detatched head")?; let branch_commit = project_repository .repo() diff --git a/crates/gitbutler-testsupport/src/test_project.rs b/crates/gitbutler-testsupport/src/test_project.rs index 2c841ca1e..5f3a9d712 100644 --- a/crates/gitbutler-testsupport/src/test_project.rs +++ b/crates/gitbutler-testsupport/src/test_project.rs @@ -149,7 +149,7 @@ impl TestProject { .remote_repository .find_branch_by_refname(&branch_name) .unwrap(); - let branch_commit = branch.get().peel_to_commit().unwrap(); + let branch_commit = branch.unwrap().get().peel_to_commit().unwrap(); let master_branch = { let name: git::Refname = "refs/heads/master".parse().unwrap(); @@ -157,7 +157,7 @@ impl TestProject { .find_branch_by_refname(&name) .unwrap() }; - let master_branch_commit = master_branch.get().peel_to_commit().unwrap(); + let master_branch_commit = master_branch.unwrap().get().peel_to_commit().unwrap(); let mut rebase_options = git2::RebaseOptions::new(); rebase_options.quiet(true); @@ -228,7 +228,7 @@ impl TestProject { .remote_repository .find_branch_by_refname(&branch_name) .unwrap(); - let branch_commit = branch.get().peel_to_commit().unwrap(); + let branch_commit = branch.as_ref().unwrap().get().peel_to_commit().unwrap(); let master_branch = { let name: git::Refname = "refs/heads/master".parse().unwrap(); @@ -236,7 +236,12 @@ impl TestProject { .find_branch_by_refname(&name) .unwrap() }; - let master_branch_commit = master_branch.get().peel_to_commit().unwrap(); + let master_branch_commit = master_branch + .as_ref() + .unwrap() + .get() + .peel_to_commit() + .unwrap(); let merge_base = { let oid = self @@ -250,8 +255,8 @@ impl TestProject { .remote_repository .merge_trees( &merge_base.tree().unwrap(), - &master_branch.get().peel_to_tree().unwrap(), - &branch.get().peel_to_tree().unwrap(), + &master_branch.unwrap().get().peel_to_tree().unwrap(), + &branch.unwrap().get().peel_to_tree().unwrap(), None, ) .unwrap(); @@ -295,24 +300,35 @@ impl TestProject { } pub fn checkout(&self, branch: &git::LocalRefname) { - let branch: git::Refname = branch.into(); + let refname: git::Refname = branch.into(); let head_commit = self .local_repository .head() .unwrap() .peel_to_commit() .unwrap(); - let tree = match self.local_repository.find_branch_by_refname(&branch) { - Ok(branch) => branch.get().peel_to_tree().unwrap(), - Err(err) if err.code() == git2::ErrorCode::NotFound => { - self.local_repository - .reference(&branch.to_string(), head_commit.id(), false, "new branch") - .unwrap(); - head_commit.tree().unwrap() - } + let tree = match self.local_repository.find_branch_by_refname(&refname) { + Ok(branch) => match branch { + Some(branch) => branch.get().peel_to_tree().unwrap(), + None => { + self.local_repository + .reference(&refname.to_string(), head_commit.id(), false, "new branch") + .unwrap(); + head_commit.tree().unwrap() + } + }, + // Ok(branch) => branch.get().peel_to_tree().unwrap(), + // Err(err) if err.code() == git2::ErrorCode::NotFound => { + // self.local_repository + // .reference(&branch.to_string(), head_commit.id(), false, "new branch") + // .unwrap(); + // head_commit.tree().unwrap() + // } Err(error) => panic!("{:?}", error), }; - self.local_repository.set_head(&branch.to_string()).unwrap(); + self.local_repository + .set_head(&refname.to_string()) + .unwrap(); self.local_repository .checkout_tree_builder(&tree) .force()