diff --git a/crates/gitbutler-core/src/project_repository/repository.rs b/crates/gitbutler-core/src/project_repository/repository.rs index 9ef417963..60ace9689 100644 --- a/crates/gitbutler-core/src/project_repository/repository.rs +++ b/crates/gitbutler-core/src/project_repository/repository.rs @@ -26,12 +26,12 @@ pub struct Repository { impl Repository { pub fn open(project: &projects::Project) -> Result { - let repo = git::Repository::open(&project.path).or_else(|err| match err { - git::Error::NotFound(_) => Err(anyhow::Error::from(err)).context(format!( + let repo = git::Repository::open(&project.path).map_err(|err| match err { + git::Error::NotFound(_) => anyhow::Error::from(err).context(format!( "repository not found at \"{}\"", project.path.display() )), - other => Err(other.into()), + other => other.into(), })?; // XXX(qix-): This is a temporary measure to disable GC on the project repository. diff --git a/crates/gitbutler-core/src/projects/controller.rs b/crates/gitbutler-core/src/projects/controller.rs index c7a080350..c83f60013 100644 --- a/crates/gitbutler-core/src/projects/controller.rs +++ b/crates/gitbutler-core/src/projects/controller.rs @@ -158,7 +158,8 @@ impl Controller { } pub fn get(&self, id: ProjectId) -> Result { - let project = self.projects_storage.get(id)?; + #[cfg_attr(not(windows), allow(unused_mut))] + let mut project = self.projects_storage.get(id)?; if !project.gb_dir().exists() { if let Err(error) = std::fs::create_dir_all(project.gb_dir()) { tracing::error!(project_id = %project.id, ?error, "failed to create \"{}\" on project get", project.gb_dir().display()); diff --git a/crates/gitbutler-core/src/virtual_branches/controller.rs b/crates/gitbutler-core/src/virtual_branches/controller.rs index e6ecb0063..467058c78 100644 --- a/crates/gitbutler-core/src/virtual_branches/controller.rs +++ b/crates/gitbutler-core/src/virtual_branches/controller.rs @@ -521,7 +521,7 @@ impl ControllerInner { self.with_verify_branch(project_id, |project_repository, user| { super::create_virtual_branch_from_branch(project_repository, branch, user) - .map_err(Error::from_err) + .map_err(Into::into) }) } @@ -539,7 +539,7 @@ impl ControllerInner { let project = self.projects.get(project_id)?; let project_repository = project_repository::Repository::open(&project)?; super::list_remote_commit_files(&project_repository.git_repository, commit_oid) - .map_err(Error::from_err) + .map_err(Into::into) } pub fn set_base_branch( @@ -714,7 +714,7 @@ impl ControllerInner { to_commit_oid, ownership, ) - .map_err(Error::from_err) + .map_err(Into::into) }) } @@ -729,8 +729,7 @@ impl ControllerInner { self.with_verify_branch(project_id, |project_repository, _| { let snapshot_tree = project_repository.project().prepare_snapshot(); let result: Result<(), Error> = - super::undo_commit(project_repository, branch_id, commit_oid) - .map_err(Error::from_err); + super::undo_commit(project_repository, branch_id, commit_oid).map_err(Into::into); let _ = snapshot_tree.and_then(|snapshot_tree| { project_repository.project().snapshot_commit_undo( snapshot_tree, @@ -756,7 +755,7 @@ impl ControllerInner { .project() .create_snapshot(SnapshotDetails::new(OperationKind::InsertBlankCommit)); super::insert_blank_commit(project_repository, branch_id, commit_oid, user, offset) - .map_err(Error::from_err) + .map_err(Into::into) }) } @@ -774,7 +773,7 @@ impl ControllerInner { .project() .create_snapshot(SnapshotDetails::new(OperationKind::ReorderCommit)); super::reorder_commit(project_repository, branch_id, commit_oid, offset) - .map_err(Error::from_err) + .map_err(Into::into) }) } @@ -791,7 +790,7 @@ impl ControllerInner { .project() .create_snapshot(SnapshotDetails::new(OperationKind::UndoCommit)); super::reset_branch(project_repository, branch_id, target_commit_oid) - .map_err(Error::from_err) + .map_err(Into::into) }) } @@ -848,7 +847,7 @@ impl ControllerInner { let _ = project_repository .project() .create_snapshot(SnapshotDetails::new(OperationKind::CherryPick)); - super::cherry_pick(project_repository, branch_id, commit_oid).map_err(Error::from_err) + super::cherry_pick(project_repository, branch_id, commit_oid).map_err(Into::into) }) } @@ -883,7 +882,7 @@ impl ControllerInner { let _ = project_repository .project() .create_snapshot(SnapshotDetails::new(OperationKind::SquashCommit)); - super::squash(project_repository, branch_id, commit_oid).map_err(Error::from_err) + super::squash(project_repository, branch_id, commit_oid).map_err(Into::into) }) } @@ -900,7 +899,7 @@ impl ControllerInner { .project() .create_snapshot(SnapshotDetails::new(OperationKind::UpdateCommitMessage)); super::update_commit_message(project_repository, branch_id, commit_oid, message) - .map_err(Error::from_err) + .map_err(Into::into) }) } @@ -994,7 +993,7 @@ impl ControllerInner { let project = self.projects.get(project_id)?; let project_repository = project_repository::Repository::open(&project)?; let user = self.users.get_user()?; - super::integration::verify_branch(&project_repository).map_err(Error::from_err)?; + super::integration::verify_branch(&project_repository)?; action(&project_repository, user.as_ref()) } @@ -1008,7 +1007,7 @@ impl ControllerInner { let project = self.projects.get(project_id)?; let project_repository = project_repository::Repository::open(&project)?; let user = self.users.get_user()?; - super::integration::verify_branch(&project_repository).map_err(Error::from_err)?; + super::integration::verify_branch(&project_repository)?; Ok(tokio::task::spawn_blocking(move || { action(&project_repository, user.as_ref()) })) diff --git a/crates/gitbutler-core/src/virtual_branches/errors.rs b/crates/gitbutler-core/src/virtual_branches/errors.rs index 6c5d53df6..51d1a7352 100644 --- a/crates/gitbutler-core/src/virtual_branches/errors.rs +++ b/crates/gitbutler-core/src/virtual_branches/errors.rs @@ -1,58 +1,28 @@ -use super::{branch::BranchOwnershipClaims, BranchId, GITBUTLER_INTEGRATION_REFERENCE}; +use super::BranchId; use crate::error::{AnyhowContextExt, Context, ErrorWithContext}; -use crate::{ - git, - project_repository::{self, RemoteError}, - projects::ProjectId, -}; +use crate::{git, project_repository::RemoteError, projects::ProjectId}; -// Generic error enum for use in the virtual branches module. -#[derive(Debug, thiserror::Error)] -pub enum VirtualBranchError { - #[error("target ownership not found")] - TargetOwnerhshipNotFound(BranchOwnershipClaims), - #[error("git object {0} not found")] - GitObjectNotFound(git::Oid), - #[error("commit failed")] - CommitFailed, - #[error("rebase failed")] - RebaseFailed, - #[error("force push not allowed")] - ForcePushNotAllowed(ForcePushNotAllowed), - #[error("Branch has no commits - there is nothing to amend to")] - BranchHasNoCommits, - #[error(transparent)] - Other(#[from] anyhow::Error), +/// A way to mark errors using `[anyhow::Context::context]` for later retrieval. +/// +/// Note that the display implementation is visible to users in logs, so it's a bit 'special' +/// to signify its marker status. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum Marker { + /// Invalid state was detected, making the repository invalid for operation. + VerificationFailure, } -#[derive(Debug, thiserror::Error)] -pub enum VerifyError { - #[error("project in detached head state. Please checkout {} to continue", GITBUTLER_INTEGRATION_REFERENCE.branch())] - DetachedHead, - #[error("project is on {0}. Please checkout {} to continue", GITBUTLER_INTEGRATION_REFERENCE.branch())] - InvalidHead(String), - #[error("Repo HEAD is unavailable")] - HeadNotFound, - #[error("GibButler's integration commit not found on head.")] - NoIntegrationCommit, - #[error(transparent)] - GitError(#[from] git::Error), - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -#[derive(Debug, thiserror::Error)] -pub enum ResetBranchError { - #[error("commit {0} not in the branch")] - CommitNotFoundInBranch(git::Oid), - #[error(transparent)] - Other(#[from] anyhow::Error), - #[error(transparent)] - Git(#[from] git::Error), +impl std::fmt::Display for Marker { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Marker::VerificationFailure => f.write_str(""), + } + } } #[derive(Debug, thiserror::Error)] pub enum ApplyBranchError { + // TODO(ST): use local Marker to detect this case, apply the same to ProjectConflict #[error("branch {0} is in a conflicting state")] BranchConflicts(BranchId), #[error(transparent)] @@ -61,38 +31,10 @@ pub enum ApplyBranchError { Other(#[from] anyhow::Error), } -#[derive(Debug, thiserror::Error)] -pub enum ListVirtualBranchesError { - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for ListVirtualBranchesError { - fn context(&self) -> Option { - match self { - ListVirtualBranchesError::Other(error) => error.custom_context_or_root_cause().into(), - } - } -} - -#[derive(Debug, thiserror::Error)] -pub enum CreateVirtualBranchError { - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for CreateVirtualBranchError { - fn context(&self) -> Option { - match self { - CreateVirtualBranchError::Other(error) => error.custom_context_or_root_cause().into(), - } - } -} - #[derive(Debug, thiserror::Error)] pub enum PushError { #[error(transparent)] - Remote(#[from] project_repository::RemoteError), + Remote(#[from] RemoteError), #[error(transparent)] Other(#[from] anyhow::Error), } @@ -106,47 +48,11 @@ impl ErrorWithContext for PushError { } } -#[derive(Debug, thiserror::Error)] -pub enum IsVirtualBranchMergeable { - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for IsVirtualBranchMergeable { - fn context(&self) -> Option { - match self { - IsVirtualBranchMergeable::Other(error) => error.custom_context_or_root_cause().into(), - } - } -} - #[derive(Debug)] pub struct ForcePushNotAllowed { pub project_id: ProjectId, } -#[derive(Debug, thiserror::Error)] -pub enum CherryPickError { - #[error("commit {0} not found ")] - CommitNotFound(git::Oid), - #[error("can not cherry pick not applied branch")] - NotApplied, - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -#[derive(Debug, thiserror::Error)] -pub enum SquashError { - #[error("force push not allowed")] - ForcePushNotAllowed(ForcePushNotAllowed), - #[error("commit {0} not in the branch")] - CommitNotFound(git::Oid), - #[error("can not squash root commit")] - CantSquashRootCommit, - #[error(transparent)] - Other(#[from] anyhow::Error), -} - #[derive(Debug, thiserror::Error)] pub enum FetchFromTargetError { #[error("failed to fetch")] @@ -163,63 +69,3 @@ impl ErrorWithContext for FetchFromTargetError { } } } - -#[derive(Debug, thiserror::Error)] -pub enum UpdateCommitMessageError { - #[error("force push not allowed")] - ForcePushNotAllowed(ForcePushNotAllowed), - #[error("Commit message can not be empty")] - EmptyMessage, - #[error("commit {0} not in the branch")] - CommitNotFound(git::Oid), - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -#[derive(Debug, thiserror::Error)] -pub enum CreateVirtualBranchFromBranchError { - #[error("failed to apply")] - ApplyBranch(ApplyBranchError), - #[error("can not create a branch from default target")] - CantMakeBranchFromDefaultTarget, - #[error("branch {0} not found")] - BranchNotFound(git::Refname), - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -#[derive(Debug, thiserror::Error)] -pub enum ListRemoteCommitFilesError { - #[error("commit {0} not found")] - CommitNotFound(git::Oid), - #[error("failed to find commit")] - Other(#[from] anyhow::Error), -} - -#[derive(Debug, thiserror::Error)] -pub enum ListRemoteBranchesError { - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for ListRemoteBranchesError { - fn context(&self) -> Option { - match self { - ListRemoteBranchesError::Other(error) => error.custom_context_or_root_cause().into(), - } - } -} - -#[derive(Debug, thiserror::Error)] -pub enum GetRemoteBranchDataError { - #[error(transparent)] - Other(#[from] anyhow::Error), -} - -impl ErrorWithContext for GetRemoteBranchDataError { - fn context(&self) -> Option { - match self { - GetRemoteBranchDataError::Other(error) => error.custom_context_or_root_cause().into(), - } - } -} diff --git a/crates/gitbutler-core/src/virtual_branches/files.rs b/crates/gitbutler-core/src/virtual_branches/files.rs index 4149aa190..c89128d9a 100644 --- a/crates/gitbutler-core/src/virtual_branches/files.rs +++ b/crates/gitbutler-core/src/virtual_branches/files.rs @@ -1,9 +1,8 @@ use std::path; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use serde::Serialize; -use super::errors; use crate::git::{self, diff}; #[derive(Debug, PartialEq, Clone, Serialize)] @@ -16,15 +15,12 @@ pub struct RemoteBranchFile { pub fn list_remote_commit_files( repository: &git::Repository, - commit_oid: git::Oid, -) -> Result, errors::ListRemoteCommitFilesError> { - let commit = match repository.find_commit(commit_oid) { - Ok(commit) => Ok(commit), - Err(git::Error::NotFound(_)) => Err(errors::ListRemoteCommitFilesError::CommitNotFound( - commit_oid, - )), - Err(error) => Err(errors::ListRemoteCommitFilesError::Other(error.into())), - }?; + commit_id: git::Oid, +) -> Result> { + let commit = repository.find_commit(commit_id).map_err(|err| match err { + git::Error::NotFound(_) => anyhow!("commit {commit_id} not found"), + err => err.into(), + })?; // If it's a merge commit, we list nothing. In the future we could to a fork exec of `git diff-tree --cc` if commit.parent_count() != 1 { diff --git a/crates/gitbutler-core/src/virtual_branches/integration.rs b/crates/gitbutler-core/src/virtual_branches/integration.rs index c306b6a6b..d4492a0b1 100644 --- a/crates/gitbutler-core/src/virtual_branches/integration.rs +++ b/crates/gitbutler-core/src/virtual_branches/integration.rs @@ -1,10 +1,11 @@ use std::{path::PathBuf, vec}; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use bstr::ByteSlice; use lazy_static::lazy_static; -use super::{errors::VerifyError, VirtualBranchesHandle}; +use super::VirtualBranchesHandle; +use crate::virtual_branches::errors::Marker; use crate::{ git::{self, CommitExt}, project_repository::{self, conflicts, LogUntil}, @@ -291,142 +292,143 @@ pub fn update_gitbutler_integration( Ok(final_commit.into()) } -pub fn verify_branch( - project_repository: &project_repository::Repository, -) -> Result<(), VerifyError> { - verify_current_branch_name(project_repository)?; - verify_head_is_set(project_repository)?; - verify_head_is_clean(project_repository)?; +pub fn verify_branch(project_repository: &project_repository::Repository) -> Result<()> { + project_repository + .verify_current_branch_name() + .and_then(|me| me.verify_head_is_set()) + .and_then(|me| me.verify_head_is_clean()) + .context(Marker::VerificationFailure)?; Ok(()) } -fn verify_head_is_clean( - project_repository: &project_repository::Repository, -) -> Result<(), VerifyError> { - let head_commit = project_repository - .git_repository - .head() - .context("failed to get head")? - .peel_to_commit() - .context("failed to peel to commit")?; - - let vb_handle = VirtualBranchesHandle::new(project_repository.project().gb_dir()); - let default_target = vb_handle - .get_default_target() - .context("failed to get default target")?; - - let mut extra_commits = project_repository - .log( - head_commit.id().into(), - LogUntil::Commit(default_target.sha), - ) - .context("failed to get log")?; - - let integration_commit = extra_commits.pop(); - - if integration_commit.is_none() { - // no integration commit found - return Err(VerifyError::NoIntegrationCommit); +impl project_repository::Repository { + fn verify_head_is_set(&self) -> Result<&Self> { + match self.get_head().context("failed to get head")?.name() { + Some(refname) if refname.to_string() == GITBUTLER_INTEGRATION_REFERENCE.to_string() => { + Ok(self) + } + Some(head_name) => Err(invalid_head_err(&head_name.to_string())), + None => Err(anyhow!( + "project in detached head state. Please checkout {} to continue", + GITBUTLER_INTEGRATION_REFERENCE.branch() + )), + } } - if extra_commits.is_empty() { - // no extra commits found, so we're good - return Ok(()); + // Returns an error if repo head is not pointing to the integration branch. + fn verify_current_branch_name(&self) -> Result<&Self> { + match self.get_head()?.name() { + Some(head) => { + let head_name = head.to_string(); + if head_name != GITBUTLER_INTEGRATION_REFERENCE.to_string() { + return Err(invalid_head_err(&head_name)); + } + Ok(self) + } + None => Err(anyhow!("Repo HEAD is unavailable")), + } } - project_repository - .git_repository - .reset( - integration_commit.as_ref().unwrap(), - git2::ResetType::Soft, - None, - ) - .context("failed to reset to integration commit")?; - - let mut new_branch = super::create_virtual_branch( - project_repository, - &BranchCreateRequest { - name: extra_commits - .last() - .map(|commit| commit.message_bstr().to_string()), - ..Default::default() - }, - ) - .context("failed to create virtual branch")?; - - // rebasing the extra commits onto the new branch - let vb_state = project_repository.project().virtual_branches(); - extra_commits.reverse(); - let mut head = new_branch.head; - for commit in extra_commits { - let new_branch_head = project_repository + fn verify_head_is_clean(&self) -> Result<&Self> { + let head_commit = self .git_repository - .find_commit(head) - .context("failed to find new branch head")?; + .head() + .context("failed to get head")? + .peel_to_commit() + .context("failed to peel to commit")?; - let rebased_commit_oid = project_repository - .git_repository - .commit( - None, - &commit.author(), - &commit.committer(), - &commit.message_bstr().to_str_lossy(), - &commit.tree().unwrap(), - &[&new_branch_head], + let vb_handle = VirtualBranchesHandle::new(self.project().gb_dir()); + let default_target = vb_handle + .get_default_target() + .context("failed to get default target")?; + + let mut extra_commits = self + .log( + head_commit.id().into(), + LogUntil::Commit(default_target.sha), + ) + .context("failed to get log")?; + + let integration_commit = extra_commits.pop(); + + if integration_commit.is_none() { + // no integration commit found + bail!("gibButler's integration commit not found on head"); + } + + if extra_commits.is_empty() { + // no extra commits found, so we're good + return Ok(self); + } + + self.git_repository + .reset( + integration_commit.as_ref().unwrap(), + git2::ResetType::Soft, None, ) - .context(format!( - "failed to rebase commit {} onto new branch", - commit.id() - ))?; + .context("failed to reset to integration commit")?; - let rebased_commit = project_repository - .git_repository - .find_commit(rebased_commit_oid) - .context(format!( - "failed to find rebased commit {}", - rebased_commit_oid - ))?; + let mut new_branch = super::create_virtual_branch( + self, + &BranchCreateRequest { + name: extra_commits + .last() + .map(|commit| commit.message_bstr().to_string()), + ..Default::default() + }, + ) + .context("failed to create virtual branch")?; - new_branch.head = rebased_commit.id().into(); - new_branch.tree = rebased_commit.tree_id().into(); - vb_state - .set_branch(new_branch.clone()) - .context("failed to write branch")?; + // rebasing the extra commits onto the new branch + let vb_state = self.project().virtual_branches(); + extra_commits.reverse(); + let mut head = new_branch.head; + for commit in extra_commits { + let new_branch_head = self + .git_repository + .find_commit(head) + .context("failed to find new branch head")?; - head = rebased_commit.id().into(); - } - Ok(()) -} + let rebased_commit_oid = self + .git_repository + .commit( + None, + &commit.author(), + &commit.committer(), + &commit.message_bstr().to_str_lossy(), + &commit.tree().unwrap(), + &[&new_branch_head], + None, + ) + .context(format!( + "failed to rebase commit {} onto new branch", + commit.id() + ))?; -fn verify_head_is_set( - project_repository: &project_repository::Repository, -) -> Result<(), VerifyError> { - match project_repository - .get_head() - .context("failed to get head") - .map_err(VerifyError::Other)? - .name() - { - Some(refname) if refname.to_string() == GITBUTLER_INTEGRATION_REFERENCE.to_string() => { - Ok(()) + let rebased_commit = self + .git_repository + .find_commit(rebased_commit_oid) + .context(format!( + "failed to find rebased commit {}", + rebased_commit_oid + ))?; + + new_branch.head = rebased_commit.id().into(); + new_branch.tree = rebased_commit.tree_id().into(); + vb_state + .set_branch(new_branch.clone()) + .context("failed to write branch")?; + + head = rebased_commit.id().into(); } - None => Err(VerifyError::DetachedHead), - Some(head_name) => Err(VerifyError::InvalidHead(head_name.to_string())), + Ok(self) } } -// Returns an error if repo head is not pointing to the integration branch. -pub fn verify_current_branch_name( - project_repository: &project_repository::Repository, -) -> Result { - match project_repository.get_head()?.name() { - Some(head) => { - if head.to_string() != GITBUTLER_INTEGRATION_REFERENCE.to_string() { - return Err(VerifyError::InvalidHead(head.to_string())); - } - Ok(true) - } - None => Err(VerifyError::HeadNotFound), - } +fn invalid_head_err(head_name: &str) -> anyhow::Error { + anyhow!( + "project is on {head_name}. Please checkout {} to continue", + GITBUTLER_INTEGRATION_REFERENCE.branch() + ) } diff --git a/crates/gitbutler-core/src/virtual_branches/remote.rs b/crates/gitbutler-core/src/virtual_branches/remote.rs index a8655c333..5838f3b8e 100644 --- a/crates/gitbutler-core/src/virtual_branches/remote.rs +++ b/crates/gitbutler-core/src/virtual_branches/remote.rs @@ -4,7 +4,7 @@ use anyhow::{Context, Result}; use bstr::BString; use serde::Serialize; -use super::{errors, target, Author, VirtualBranchesHandle}; +use super::{target, Author, VirtualBranchesHandle}; use crate::{ git::{self, CommitExt}, project_repository::{self, LogUntil}, @@ -55,7 +55,7 @@ pub struct RemoteCommit { // a list of all the normal (non-gitbutler) git branches. pub fn list_remote_branches( project_repository: &project_repository::Repository, -) -> Result, errors::ListRemoteBranchesError> { +) -> Result> { let default_target = default_target(&project_repository.project().gb_dir())?; let mut remote_branches = vec![]; @@ -85,7 +85,7 @@ pub fn list_remote_branches( pub fn get_branch_data( project_repository: &project_repository::Repository, refname: &git::Refname, -) -> Result { +) -> Result { let default_target = default_target(&project_repository.project().gb_dir())?; let branch = project_repository @@ -97,9 +97,7 @@ pub fn get_branch_data( .context("failed to get branch data")?; branch_data - .ok_or_else(|| { - errors::GetRemoteBranchDataError::Other(anyhow::anyhow!("no data found for branch")) - }) + .context("no data found for branch") .map(|branch_data| RemoteBranchData { sha: branch_data.sha, name: branch_data.name, diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index e252255ed..cb657eed6 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -719,7 +719,7 @@ fn find_base_tree<'a>( pub fn list_virtual_branches( project_repository: &project_repository::Repository, -) -> Result<(Vec, Vec), errors::ListVirtualBranchesError> { +) -> Result<(Vec, Vec)> { let mut branches: Vec = Vec::new(); let vb_state = project_repository.project().virtual_branches(); @@ -993,7 +993,7 @@ fn commit_to_vbranch_commit( pub fn create_virtual_branch( project_repository: &project_repository::Repository, create: &BranchCreateRequest, -) -> Result { +) -> Result { let vb_state = project_repository.project().virtual_branches(); let default_target = vb_state.get_default_target()?; @@ -1943,33 +1943,31 @@ fn virtual_hunks_into_virtual_files( pub fn reset_branch( project_repository: &project_repository::Repository, branch_id: BranchId, - target_commit_oid: git::Oid, -) -> Result<(), errors::ResetBranchError> { + target_commit_id: git::Oid, +) -> Result<()> { let vb_state = project_repository.project().virtual_branches(); let default_target = vb_state.get_default_target()?; let mut branch = vb_state.get_branch(branch_id)?; - if branch.head == target_commit_oid { + if branch.head == target_commit_id { // nothing to do return Ok(()); } - if default_target.sha != target_commit_oid + if default_target.sha != target_commit_id && !project_repository .l(branch.head, LogUntil::Commit(default_target.sha))? - .contains(&target_commit_oid) + .contains(&target_commit_id) { - return Err(errors::ResetBranchError::CommitNotFoundInBranch( - target_commit_oid, - )); + bail!("commit {target_commit_id} not in the branch"); } // Compute the old workspace before resetting, so we can figure out // what hunks were released by this reset, and assign them to this branch. let old_head = get_workspace_head(&vb_state, project_repository)?; - branch.head = target_commit_oid; + branch.head = target_commit_id; vb_state .set_branch(branch.clone()) .context("failed to write branch")?; @@ -2495,7 +2493,7 @@ pub fn is_remote_branch_mergeable( git::Error::NotFound(_) => { anyhow!("Remote branch {} not found", branch_name.clone()) } - err => anyhow::Error::from(err), + err => err.into(), })?; let branch_oid = branch.target().context("detatched head")?; let branch_commit = project_repository @@ -2524,7 +2522,7 @@ pub fn is_remote_branch_mergeable( pub fn is_virtual_branch_mergeable( project_repository: &project_repository::Repository, branch_id: BranchId, -) -> Result { +) -> Result { let vb_state = project_repository.project().virtual_branches(); let branch = vb_state.get_branch(branch_id)?; if branch.applied { @@ -2585,19 +2583,19 @@ pub fn is_virtual_branch_mergeable( pub fn move_commit_file( project_repository: &project_repository::Repository, branch_id: BranchId, - from_commit_oid: git::Oid, - to_commit_oid: git::Oid, + from_commit_id: git::Oid, + to_commit_id: git::Oid, target_ownership: &BranchOwnershipClaims, -) -> Result { +) -> Result { let vb_state = project_repository.project().virtual_branches(); let Some(mut target_branch) = vb_state.try_branch(branch_id)? else { - return Ok(to_commit_oid); // this is wrong + return Ok(to_commit_id); // this is wrong }; let default_target = vb_state.get_default_target()?; - let mut to_amend_oid = to_commit_oid; + let mut to_amend_oid = to_commit_id; let mut amend_commit = project_repository .git_repository .find_commit(to_amend_oid) @@ -2645,13 +2643,11 @@ pub fn move_commit_file( // if we're not moving anything, return an error if diffs_to_amend.is_empty() { - return Err(errors::VirtualBranchError::TargetOwnerhshipNotFound( - target_ownership.clone(), - )); + bail!("target ownership not found"); } // is from_commit_oid in upstream_commits? - if !upstream_commits.contains(&from_commit_oid) { + if !upstream_commits.contains(&from_commit_id) { // this means that the "from" commit is _below_ the "to" commit in the history // which makes things a little more complicated because in this case we need to // remove the changes from the lower "from" commit, rebase everything, then add the changes @@ -2660,7 +2656,7 @@ pub fn move_commit_file( // first, let's get the from commit data and it's parent data let from_commit = project_repository .git_repository - .find_commit(from_commit_oid) + .find_commit(from_commit_id) .context("failed to find commit")?; let from_tree = from_commit.tree().context("failed to find tree")?; let from_parent = from_commit.parent(0).context("failed to find parent")?; @@ -2707,11 +2703,11 @@ pub fn move_commit_file( let repo = &project_repository.git_repository; // write our new tree and commit for the new "from" commit without the moved changes - let new_from_tree_oid = + let new_from_tree_id = write_tree_onto_commit(project_repository, from_parent.id().into(), &diffs_to_keep)?; let new_from_tree = &repo - .find_tree(new_from_tree_oid) - .map_err(|_error| errors::VirtualBranchError::GitObjectNotFound(new_from_tree_oid))?; + .find_tree(new_from_tree_id) + .with_context(|| "tree {new_from_tree_oid} not found")?; let change_id = from_commit.change_id(); let new_from_commit_oid = repo .commit( @@ -2723,19 +2719,18 @@ pub fn move_commit_file( &[&from_parent], change_id.as_deref(), ) - .map_err(|_error| errors::VirtualBranchError::CommitFailed)?; + .context("commit failed")?; // rebase everything above the new "from" commit that has the moved changes removed let new_head = match cherry_rebase( project_repository, new_from_commit_oid, - from_commit_oid, + from_commit_id, target_branch.head, ) { Ok(Some(new_head)) => new_head, - _ => { - return Err(errors::VirtualBranchError::RebaseFailed); - } + Ok(None) => bail!("no rebase was performed"), + Err(err) => return Err(err).context("rebase failed"), }; // ok, now we need to identify which the new "to" commit is in the rebased history @@ -2755,14 +2750,12 @@ pub fn move_commit_file( let to_commit_offset = old_upstream_commit_oids .iter() .position(|c| *c == to_amend_oid) - .ok_or(errors::VirtualBranchError::Other(anyhow!( - "failed to find commit in old commits" - )))?; + .context("failed to find commit in old commits")?; // find the new "to" commit in our new rebased upstream commits - to_amend_oid = *new_upstream_commit_oids.get(to_commit_offset).ok_or( - errors::VirtualBranchError::Other(anyhow!("failed to find commit in new commits")), - )?; + to_amend_oid = *new_upstream_commit_oids + .get(to_commit_offset) + .context("failed to find commit in new commits")?; // reset the "to" commit variable for writing the changes back to amend_commit = project_repository @@ -2830,7 +2823,7 @@ pub fn move_commit_file( super::integration::update_gitbutler_integration(&vb_state, project_repository)?; Ok(commit_oid) } else { - Err(errors::VirtualBranchError::RebaseFailed) + Err(anyhow!("rebase failed")) } } @@ -2994,7 +2987,7 @@ pub fn reorder_commit( branch_id: BranchId, commit_oid: git::Oid, offset: i32, -) -> Result<(), errors::VirtualBranchError> { +) -> Result<()> { let vb_state = project_repository.project().virtual_branches(); let default_target = vb_state.get_default_target()?; @@ -3025,20 +3018,16 @@ pub fn reorder_commit( ids_to_rebase.push(commit_oid); ids_to_rebase.push(last_oid); - match cherry_rebase_group(project_repository, parent_oid.into(), &mut ids_to_rebase) { - Ok(new_head) => { - branch.head = new_head; - vb_state - .set_branch(branch.clone()) - .context("failed to write branch")?; + let new_head = + cherry_rebase_group(project_repository, parent_oid.into(), &mut ids_to_rebase) + .context("rebase failed")?; + branch.head = new_head; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; - super::integration::update_gitbutler_integration(&vb_state, project_repository) - .context("failed to update gitbutler integration")?; - } - _ => { - return Err(errors::VirtualBranchError::RebaseFailed); - } - } + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; } else { // move commit down if default_target.sha == parent_oid.into() { @@ -3057,20 +3046,17 @@ pub fn reorder_commit( ids_to_rebase.push(parent_oid.into()); ids_to_rebase.push(commit_oid); - match cherry_rebase_group(project_repository, target_oid.into(), &mut ids_to_rebase) { - Ok(new_head) => { - branch.head = new_head; - vb_state - .set_branch(branch.clone()) - .context("failed to write branch")?; + let new_head = + cherry_rebase_group(project_repository, target_oid.into(), &mut ids_to_rebase) + .context("rebase failed")?; - super::integration::update_gitbutler_integration(&vb_state, project_repository) - .context("failed to update gitbutler integration")?; - } - _ => { - return Err(errors::VirtualBranchError::RebaseFailed); - } - } + branch.head = new_head; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; + + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; } Ok(()) @@ -3085,7 +3071,7 @@ pub fn insert_blank_commit( commit_oid: git::Oid, user: Option<&users::User>, offset: i32, -) -> Result<(), errors::VirtualBranchError> { +) -> Result<()> { let vb_state = project_repository.project().virtual_branches(); let mut branch = vb_state.get_branch(branch_id)?; @@ -3127,8 +3113,9 @@ pub fn insert_blank_commit( super::integration::update_gitbutler_integration(&vb_state, project_repository) .context("failed to update gitbutler integration")?; } - _ => { - return Err(errors::VirtualBranchError::RebaseFailed); + Ok(None) => bail!("no rebase happened"), + Err(err) => { + return Err(err).context("rebase failed"); } } } @@ -3142,7 +3129,7 @@ pub fn undo_commit( project_repository: &project_repository::Repository, branch_id: BranchId, commit_oid: git::Oid, -) -> Result<(), errors::VirtualBranchError> { +) -> Result<()> { let vb_state = project_repository.project().virtual_branches(); let mut branch = vb_state.get_branch(branch_id)?; @@ -3169,8 +3156,9 @@ pub fn undo_commit( Ok(Some(new_head)) => { new_commit_oid = new_head.into(); } - _ => { - return Err(errors::VirtualBranchError::RebaseFailed); + Ok(None) => bail!("no rebase happened"), + Err(err) => { + return Err(err).context("rebase failed"); } } } @@ -3196,7 +3184,7 @@ pub fn cherry_rebase( target_commit_oid: git::Oid, start_commit_oid: git::Oid, end_commit_oid: git::Oid, -) -> Result, anyhow::Error> { +) -> Result> { // get a list of the commits to rebase let mut ids_to_rebase = project_repository.l( end_commit_oid, @@ -3221,7 +3209,7 @@ fn cherry_rebase_group( project_repository: &project_repository::Repository, target_commit_oid: git::Oid, ids_to_rebase: &mut [git::Oid], -) -> Result { +) -> Result { ids_to_rebase.reverse(); // now, rebase unchanged commits onto the new commit let commits_to_rebase = ids_to_rebase @@ -3287,8 +3275,8 @@ fn cherry_rebase_group( pub fn cherry_pick( project_repository: &project_repository::Repository, branch_id: BranchId, - target_commit_oid: git::Oid, -) -> Result, errors::CherryPickError> { + target_commit_id: git::Oid, +) -> Result> { project_repository.assure_unconflicted()?; let vb_state = project_repository.project().virtual_branches(); @@ -3299,15 +3287,15 @@ pub fn cherry_pick( if !branch.applied { // todo? - return Err(errors::CherryPickError::NotApplied); + bail!("can not cherry pick a branch that is not applied") } let target_commit = project_repository .git_repository - .find_commit(target_commit_oid) + .find_commit(target_commit_id) .map_err(|error| match error { - git::Error::NotFound(_) => errors::CherryPickError::CommitNotFound(target_commit_oid), - error => errors::CherryPickError::Other(error.into()), + git::Error::NotFound(_) => anyhow!("commit {target_commit_id} not found "), + err => err.into(), })?; let branch_head_commit = project_repository @@ -3325,8 +3313,7 @@ pub fn cherry_pick( .filter(|b| b.applied) .collect::>(); - let integration_commit_id = - super::integration::get_workspace_head(&vb_state, project_repository)?; + let integration_commit_id = get_workspace_head(&vb_state, project_repository)?; let (applied_statuses, _) = get_applied_status( project_repository, @@ -3462,12 +3449,12 @@ pub fn cherry_pick( Ok(commit_oid) } -/// squashes a commit from a virtual branch into it's parent. +/// squashes a commit from a virtual branch into its parent. pub fn squash( project_repository: &project_repository::Repository, branch_id: BranchId, - commit_oid: git::Oid, -) -> Result<(), errors::SquashError> { + commit_id: git::Oid, +) -> Result<()> { project_repository.assure_resolved()?; let vb_state = project_repository.project().virtual_branches(); @@ -3478,13 +3465,13 @@ pub fn squash( project_repository::LogUntil::Commit(default_target.sha), )?; - if !branch_commit_oids.contains(&commit_oid) { - return Err(errors::SquashError::CommitNotFound(commit_oid)); + if !branch_commit_oids.contains(&commit_id) { + bail!("commit {commit_id} not in the branch") } let commit_to_squash = project_repository .git_repository - .find_commit(commit_oid) + .find_commit(commit_id) .context("failed to find commit")?; let parent_commit = commit_to_squash @@ -3505,15 +3492,11 @@ pub fn squash( && !project_repository.project().ok_with_force_push { // squashing into a pushed commit will cause a force push that is not allowed - return Err(errors::SquashError::ForcePushNotAllowed( - errors::ForcePushNotAllowed { - project_id: project_repository.project().id, - }, - )); + bail!("force push not allowed"); } if !branch_commit_oids.contains(&parent_commit.id().into()) { - return Err(errors::SquashError::CantSquashRootCommit); + bail!("can not squash root commit"); } // create a commit that: @@ -3544,11 +3527,11 @@ pub fn squash( let ids_to_rebase = { let ids = branch_commit_oids - .split(|oid| oid.eq(&commit_oid)) + .split(|oid| oid.eq(&commit_id)) .collect::>(); ids.first().copied() } - .ok_or(errors::SquashError::CommitNotFound(commit_oid))?; + .with_context(|| format!("commit {commit_id} not in the branch"))?; let mut ids_to_rebase = ids_to_rebase.to_vec(); match cherry_rebase_group(project_repository, new_commit_oid, &mut ids_to_rebase) { @@ -3563,7 +3546,7 @@ pub fn squash( .context("failed to update gitbutler integration")?; Ok(()) } - _ => Err(errors::SquashError::Other(anyhow!("rebase error"))), + Err(err) => Err(err.context("rebase error").context(Code::Unknown)), } } @@ -3571,11 +3554,11 @@ pub fn squash( pub fn update_commit_message( project_repository: &project_repository::Repository, branch_id: BranchId, - commit_oid: git::Oid, + commit_id: git::Oid, message: &str, -) -> Result<(), errors::UpdateCommitMessageError> { +) -> Result<()> { if message.is_empty() { - return Err(errors::UpdateCommitMessageError::EmptyMessage); + bail!("commit message can not be empty"); } project_repository.assure_unconflicted()?; @@ -3588,8 +3571,8 @@ pub fn update_commit_message( project_repository::LogUntil::Commit(default_target.sha), )?; - if !branch_commit_oids.contains(&commit_oid) { - return Err(errors::UpdateCommitMessageError::CommitNotFound(commit_oid)); + if !branch_commit_oids.contains(&commit_id) { + bail!("commit {commit_id} not in the branch"); } let pushed_commit_oids = branch.upstream_head.map_or_else( @@ -3602,19 +3585,14 @@ pub fn update_commit_message( }, )?; - if pushed_commit_oids.contains(&commit_oid) && !project_repository.project().ok_with_force_push - { + if pushed_commit_oids.contains(&commit_id) && !project_repository.project().ok_with_force_push { // updating the message of a pushed commit will cause a force push that is not allowed - return Err(errors::UpdateCommitMessageError::ForcePushNotAllowed( - errors::ForcePushNotAllowed { - project_id: project_repository.project().id, - }, - )); + bail!("force push not allowed"); } let target_commit = project_repository .git_repository - .find_commit(commit_oid) + .find_commit(commit_id) .context("failed to find commit")?; let parents: Vec<_> = target_commit.parents().collect(); @@ -3636,29 +3614,24 @@ pub fn update_commit_message( let ids_to_rebase = { let ids = branch_commit_oids - .split(|oid| oid.eq(&commit_oid)) + .split(|oid| oid.eq(&commit_id)) .collect::>(); ids.first().copied() } - .ok_or(errors::UpdateCommitMessageError::CommitNotFound(commit_oid))?; + .with_context(|| format!("commit {commit_id} not in the branch"))?; let mut ids_to_rebase = ids_to_rebase.to_vec(); - match cherry_rebase_group(project_repository, new_commit_oid, &mut ids_to_rebase) { - Ok(new_head_id) => { - // save new branch head - branch.head = new_head_id; - vb_state - .set_branch(branch.clone()) - .context("failed to write branch")?; + let new_head_id = cherry_rebase_group(project_repository, new_commit_oid, &mut ids_to_rebase) + .map_err(|err| err.context("rebase error"))?; + // save new branch head + branch.head = new_head_id; + vb_state + .set_branch(branch.clone()) + .context("failed to write branch")?; - super::integration::update_gitbutler_integration(&vb_state, project_repository) - .context("failed to update gitbutler integration")?; - Ok(()) - } - _ => Err(errors::UpdateCommitMessageError::Other(anyhow!( - "rebase error" - ))), - } + super::integration::update_gitbutler_integration(&vb_state, project_repository) + .context("failed to update gitbutler integration")?; + Ok(()) } /// moves commit from the branch it's in to the top of the target branch @@ -3809,12 +3782,16 @@ pub fn create_virtual_branch_from_branch( project_repository: &project_repository::Repository, upstream: &git::Refname, user: Option<&users::User>, -) -> Result { - if !matches!(upstream, git::Refname::Local(_) | git::Refname::Remote(_)) { - return Err(errors::CreateVirtualBranchFromBranchError::BranchNotFound( - upstream.clone(), - )); - } +) -> Result { + // only set upstream if it's not the default target + let upstream_branch = match upstream { + git::Refname::Other(_) | git::Refname::Virtual(_) => { + // we only support local or remote branches + bail!("branch {upstream} must be a local or remote branch"); + } + git::Refname::Remote(remote) => Some(remote.clone()), + git::Refname::Local(local) => local.remote().cloned(), + }; let branch_name = upstream .branch() @@ -3830,23 +3807,16 @@ pub fn create_virtual_branch_from_branch( let default_target = vb_state.get_default_target()?; if let git::Refname::Remote(remote_upstream) = upstream { - if default_target.branch.eq(remote_upstream) { - return Err( - errors::CreateVirtualBranchFromBranchError::CantMakeBranchFromDefaultTarget, - ); + if default_target.branch == *remote_upstream { + bail!("cannot create a branch from default target") } } let repo = &project_repository.git_repository; - let head_reference = match repo.find_reference(upstream) { - Ok(head) => Ok(head), - Err(git::Error::NotFound(_)) => Err( - errors::CreateVirtualBranchFromBranchError::BranchNotFound(upstream.clone()), - ), - Err(error) => Err(errors::CreateVirtualBranchFromBranchError::Other( - error.into(), - )), - }?; + let head_reference = repo.find_reference(upstream).map_err(|err| match err { + git::Error::NotFound(_) => anyhow!("branch {upstream} was not found"), + err => err.into(), + })?; let head_commit = head_reference .peel_to_commit() .context("failed to peel to commit")?; @@ -3867,30 +3837,10 @@ pub fn create_virtual_branch_from_branch( let now = crate::time::now_ms(); - // only set upstream if it's not the default target - let upstream_branch = match upstream { - git::Refname::Other(_) | git::Refname::Virtual(_) => { - // we only support local or remote branches - return Err(errors::CreateVirtualBranchFromBranchError::BranchNotFound( - upstream.clone(), - )); - } - git::Refname::Remote(remote) => Some(remote.clone()), - git::Refname::Local(local) => local.remote().cloned(), - }; - // add file ownership based off the diff - let target_commit = repo - .find_commit(default_target.sha) - .map_err(|error| errors::CreateVirtualBranchFromBranchError::Other(error.into()))?; - let merge_base_oid = repo - .merge_base(target_commit.id().into(), head_commit.id().into()) - .map_err(|error| errors::CreateVirtualBranchFromBranchError::Other(error.into()))?; - let merge_base_tree = repo - .find_commit(merge_base_oid) - .map_err(|error| errors::CreateVirtualBranchFromBranchError::Other(error.into()))? - .tree() - .map_err(|error| errors::CreateVirtualBranchFromBranchError::Other(error.into()))?; + let target_commit = repo.find_commit(default_target.sha)?; + let merge_base_oid = repo.merge_base(target_commit.id().into(), head_commit.id().into())?; + let merge_base_tree = repo.find_commit(merge_base_oid)?.tree()?; // do a diff between the head of this branch and the target base let diff = diff::trees( @@ -3946,9 +3896,7 @@ pub fn create_virtual_branch_from_branch( // if branch conflicts with the workspace, it's ok. keep it unapplied Ok(branch.id) } - Err(error) => Err(errors::CreateVirtualBranchFromBranchError::ApplyBranch( - error, - )), + Err(err) => Err(err).context("failed to apply"), } } diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/cherry_pick.rs b/crates/gitbutler-core/tests/suite/virtual_branches/cherry_pick.rs index 30d72220f..24769965e 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/cherry_pick.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/cherry_pick.rs @@ -212,14 +212,14 @@ mod cleanly { .await .unwrap(); - assert!(matches!( + assert_eq!( controller .cherry_pick(*project_id, branch_id, commit_three_oid) .await .unwrap_err() - .downcast_ref(), - Some(errors::CherryPickError::NotApplied) - )); + .to_string(), + "can not cherry pick a branch that is not applied" + ); } } @@ -374,13 +374,13 @@ mod with_conflicts { .await .unwrap(); - assert!(matches!( + assert_eq!( controller .cherry_pick(*project_id, branch_id, commit_oid) .await .unwrap_err() - .downcast_ref(), - Some(errors::CherryPickError::NotApplied) - )); + .to_string(), + "can not cherry pick a branch that is not applied" + ); } } diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/create_virtual_branch_from_branch.rs b/crates/gitbutler-core/tests/suite/virtual_branches/create_virtual_branch_from_branch.rs index 1c17e7c87..1051c43d3 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/create_virtual_branch_from_branch.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/create_virtual_branch_from_branch.rs @@ -293,7 +293,7 @@ async fn from_default_target() { // branch should be created unapplied, because of the conflict - assert!(matches!( + assert_eq!( controller .create_virtual_branch_from_branch( *project_id, @@ -301,9 +301,9 @@ async fn from_default_target() { ) .await .unwrap_err() - .downcast_ref(), - Some(errors::CreateVirtualBranchFromBranchError::CantMakeBranchFromDefaultTarget) - )); + .to_string(), + "cannot create a branch from default target" + ); } #[tokio::test] @@ -321,7 +321,7 @@ async fn from_non_existent_branch() { // branch should be created unapplied, because of the conflict - assert!(matches!( + assert_eq!( controller .create_virtual_branch_from_branch( *project_id, @@ -329,11 +329,9 @@ async fn from_non_existent_branch() { ) .await .unwrap_err() - .downcast_ref(), - Some(errors::CreateVirtualBranchFromBranchError::BranchNotFound( - _ - )) - )); + .to_string(), + "branch refs/remotes/origin/branch was not found" + ); } #[tokio::test] diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/reset_virtual_branch.rs b/crates/gitbutler-core/tests/suite/virtual_branches/reset_virtual_branch.rs index d2ecf276c..8625c9afb 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/reset_virtual_branch.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/reset_virtual_branch.rs @@ -1,6 +1,6 @@ use std::fs; -use gitbutler_core::virtual_branches::{branch, errors::ResetBranchError}; +use gitbutler_core::virtual_branches::branch; use crate::suite::virtual_branches::Test; @@ -252,7 +252,7 @@ async fn to_non_existing() { oid }; - assert!(matches!( + assert_eq!( controller .reset_virtual_branch( *project_id, @@ -261,7 +261,7 @@ async fn to_non_existing() { ) .await .unwrap_err() - .downcast_ref(), - Some(ResetBranchError::CommitNotFoundInBranch(_)) - )); + .to_string(), + "commit fe14df8c66b73c6276f7bb26102ad91da680afcb not in the branch" + ); } diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/squash.rs b/crates/gitbutler-core/tests/suite/virtual_branches/squash.rs index 7f331da5e..b9590c184 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/squash.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/squash.rs @@ -310,18 +310,18 @@ async fn forcepush_forbidden() { .unwrap() }; - assert!(matches!( + assert_eq!( controller .squash(*project_id, branch_id, commit_two_oid) .await .unwrap_err() - .downcast_ref(), - Some(errors::SquashError::ForcePushNotAllowed(_)) - )); + .to_string(), + "force push not allowed" + ); } #[tokio::test] -async fn root() { +async fn root_forbidden() { let Test { repository, project_id, @@ -347,12 +347,12 @@ async fn root() { .unwrap() }; - assert!(matches!( + assert_eq!( controller .squash(*project_id, branch_id, commit_one_oid) .await .unwrap_err() - .downcast_ref(), - Some(errors::SquashError::CantSquashRootCommit) - )); + .to_string(), + "can not squash root commit" + ); } diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs b/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs index b833ca48f..55af136ef 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs @@ -260,14 +260,14 @@ async fn forcepush_forbidden() { .await .unwrap(); - assert!(matches!( + assert_eq!( controller .update_commit_message(*project_id, branch_id, commit_one_oid, "commit one updated",) .await .unwrap_err() - .downcast_ref(), - Some(errors::UpdateCommitMessageError::ForcePushNotAllowed(_)) - )); + .to_string(), + "force push not allowed" + ); } #[tokio::test] @@ -365,12 +365,12 @@ async fn empty() { .unwrap() }; - assert!(matches!( + assert_eq!( controller .update_commit_message(*project_id, branch_id, commit_one_oid, "",) .await .unwrap_err() - .downcast_ref(), - Some(errors::UpdateCommitMessageError::EmptyMessage) - )); + .to_string(), + "commit message can not be empty" + ); } diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/verify_branch.rs b/crates/gitbutler-core/tests/suite/virtual_branches/verify_branch.rs index 4f2151438..1c29f70b9 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/verify_branch.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/verify_branch.rs @@ -1,5 +1,3 @@ -use gitbutler_core::virtual_branches::errors::VerifyError; - use super::*; // Ensures that `verify_branch` returns an error when not on the integration branch. @@ -16,10 +14,9 @@ async fn should_fail_on_incorrect_branch() { repository.checkout(&branch_name); let result = controller.list_virtual_branches(*project_id).await; - let error = result.err(); - assert!(&error.is_some()); - - let error = error.unwrap(); - let error = error.downcast_ref::(); - assert!(matches!(error, Some(VerifyError::InvalidHead(_)))); + let err = result.unwrap_err(); + assert_eq!( + format!("{err:#}"), + ": project is on refs/heads/somebranch. Please checkout gitbutler/integration to continue" + ); } diff --git a/crates/gitbutler-core/tests/virtual_branches/mod.rs b/crates/gitbutler-core/tests/virtual_branches/mod.rs index f5416a6a5..992c4b54d 100644 --- a/crates/gitbutler-core/tests/virtual_branches/mod.rs +++ b/crates/gitbutler-core/tests/virtual_branches/mod.rs @@ -2092,8 +2092,8 @@ fn verify_branch_not_integration() -> Result<()> { let verify_result = verify_branch(project_repository); assert!(verify_result.is_err()); assert_eq!( - verify_result.unwrap_err().to_string(), - "project is on refs/heads/master. Please checkout gitbutler/integration to continue" + format!("{:#}", verify_result.unwrap_err()), + ": project is on refs/heads/master. Please checkout gitbutler/integration to continue" ); Ok(()) diff --git a/crates/gitbutler-tauri/src/error.rs b/crates/gitbutler-tauri/src/error.rs index fbc9e5607..68d10e56c 100644 --- a/crates/gitbutler-tauri/src/error.rs +++ b/crates/gitbutler-tauri/src/error.rs @@ -94,6 +94,11 @@ mod frontend { #[test] fn no_context_or_code() { let err = anyhow!("err msg"); + assert_eq!( + format!("{:#}", err), + "err msg", + "just one error on display here" + ); assert_eq!( json(err), "{\"code\":\"errors.unknown\",\"message\":\"Something went wrong\"}", @@ -104,6 +109,11 @@ mod frontend { #[test] fn find_code() { let err = anyhow!("err msg").context(Code::Validation); + assert_eq!( + format!("{:#}", err), + "errors.validation: err msg", + "note how the context becomes an error, in front of the original one" + ); assert_eq!( json(err), "{\"code\":\"errors.validation\",\"message\":\"err msg\"}", @@ -111,9 +121,29 @@ mod frontend { ); } + #[test] + fn find_code_after_cause() { + let original_err = std::io::Error::new(std::io::ErrorKind::Other, "actual cause"); + let err = anyhow::Error::from(original_err) + .context("err msg") + .context(Code::Validation); + + assert_eq!( + format!("{:#}", err), + "errors.validation: err msg: actual cause", + "an even longer chain, with the cause as root as one might expect" + ); + assert_eq!( + json(err), + "{\"code\":\"errors.validation\",\"message\":\"err msg\"}", + "in order to attach a custom message to an original cause, our messaging (and Code) is the tail" + ); + } + #[test] fn find_context() { let err = anyhow!("err msg").context(Context::new_static(Code::Validation, "ctx msg")); + assert_eq!(format!("{:#}", err), "ctx msg: err msg"); assert_eq!( json(err), "{\"code\":\"errors.validation\",\"message\":\"ctx msg\"}", @@ -124,6 +154,11 @@ mod frontend { #[test] fn find_context_without_message() { let err = anyhow!("err msg").context(Context::from(Code::Validation)); + assert_eq!( + format!("{:#}", err), + "Something went wrong: err msg", + "on display, `Context` does just insert a generic message" + ); assert_eq!( json(err), "{\"code\":\"errors.validation\",\"message\":\"err msg\"}", @@ -136,6 +171,11 @@ mod frontend { let err = anyhow!("bottom msg") .context("top msg") .context(Code::Validation); + assert_eq!( + format!("{:#}", err), + "errors.validation: top msg: bottom msg", + "now it's clear why bottom is bottom" + ); assert_eq!( json(err), "{\"code\":\"errors.validation\",\"message\":\"top msg\"}", @@ -149,6 +189,11 @@ mod frontend { .context(Code::ProjectGitAuth) .context("top msg") .context(Code::Validation); + assert_eq!( + format!("{:#}", err), + "errors.validation: top msg: errors.projects.git.auth: bottom msg", + "each code is treated like its own error in the chain" + ); assert_eq!( json(err), "{\"code\":\"errors.validation\",\"message\":\"top msg\"}", diff --git a/crates/gitbutler-watcher/src/handler.rs b/crates/gitbutler-watcher/src/handler.rs index 3b84377b1..82711718a 100644 --- a/crates/gitbutler-watcher/src/handler.rs +++ b/crates/gitbutler-watcher/src/handler.rs @@ -99,7 +99,14 @@ impl Handler { }, }) } - Err(err) if err.is::() => Ok(()), + Err(err) + if matches!( + err.downcast_ref::(), + Some(virtual_branches::errors::Marker::VerificationFailure) + ) => + { + Ok(()) + } Err(err) => Err(err.context("failed to list virtual branches").into()), } }