restore detection of project conflicts

Previously that variant was removed even though we need it now to
easily detect this case.

crates/gitbutler-core/src/error.rs

@@ -120,7 +120,7 @@
 //! here. It is made to only automatically convert from types that have context information.
 //! Those who have not will need to be converted by hand using [`Error::from_err()`].
 use std::borrow::Cow;
-use std::fmt::{self, Debug, Display};
+use std::fmt::{Debug, Display, Formatter};
 
 /// A unique code that consumers of the API may rely on to identify errors.
 ///
@@ -138,7 +138,9 @@ pub enum Code {
     Unknown,
     Validation,
     ProjectGitAuth,
-    // TODO(ST): try to remove this and replace it with downcasting or thiserror pattern matching
+    /// An indicator for a conflict in the project which is used for flow-control.
+    ///
+    /// See usages for details on what these conflicts can be.
     ProjectConflict,
 }
 
@@ -281,12 +283,6 @@ impl From<Error> for anyhow::Error {
     }
 }
 
-impl Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        Display::fmt(&self.0, f)
-    }
-}
-
 impl<E> From<E> for Error
 where
     E: ErrorWithContext + Send + Sync + 'static,
@@ -296,6 +292,12 @@ where
     }
 }
 
+impl Display for Error {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        Display::fmt(&self.0, f)
+    }
+}
+
 impl Error {
     /// A manual, none-overlapping implementation of `From` (or else there are conflicts).
     pub fn from_err(err: impl std::error::Error + Send + Sync + 'static) -> Self {

crates/gitbutler-core/src/virtual_branches/base.rs

@@ -1,6 +1,6 @@
 use std::{path::Path, time};
 
-use anyhow::{Context, Result};
+use anyhow::{anyhow, bail, Context, Result};
 use git2::Index;
 use serde::Serialize;
 
@@ -11,6 +11,7 @@ use super::{
     },
     target, BranchId, RemoteCommit, VirtualBranchHunk, VirtualBranchesHandle,
 };
+use crate::error::Code;
 use crate::{
     git::{self, diff},
     project_repository::{self, LogUntil},
@@ -46,7 +47,7 @@ pub fn get_base_branch_data(
 fn go_back_to_integration(
     project_repository: &project_repository::Repository,
     default_target: &target::Target,
-) -> Result<super::BaseBranch, errors::SetBaseBranchError> {
+) -> Result<BaseBranch> {
     let statuses = project_repository
         .git_repository
         .statuses(Some(
@@ -56,7 +57,7 @@ fn go_back_to_integration(
         ))
         .context("failed to get status")?;
     if !statuses.is_empty() {
-        return Err(errors::SetBaseBranchError::DirtyWorkingDirectory);
+        return Err(anyhow!("current HEAD is dirty")).context(Code::ProjectConflict);
     }
 
     let vb_state = project_repository.project().virtual_branches();
@@ -117,7 +118,7 @@ fn go_back_to_integration(
 pub fn set_base_branch(
     project_repository: &project_repository::Repository,
     target_branch_ref: &git::RemoteRefname,
-) -> Result<super::BaseBranch, errors::SetBaseBranchError> {
+) -> Result<BaseBranch> {
     let repo = &project_repository.git_repository;
 
     // if target exists, and it is the same as the requested branch, we should go back
@@ -129,12 +130,10 @@ pub fn set_base_branch(
 
     // lookup a branch by name
     let target_branch = match repo.find_branch(&target_branch_ref.clone().into()) {
-        Ok(branch) => Ok(branch),
-        Err(git::Error::NotFound(_)) => Err(errors::SetBaseBranchError::BranchNotFound(
-            target_branch_ref.clone(),
-        )),
-        Err(error) => Err(errors::SetBaseBranchError::Other(error.into())),
-    }?;
+        Ok(branch) => branch,
+        Err(git::Error::NotFound(_)) => bail!("remote branch '{}' not found", target_branch_ref),
+        Err(err) => return Err(err.into()),
+    };
 
     let remote = repo
         .find_remote(target_branch_ref.remote())
@@ -276,24 +275,21 @@ pub fn set_base_branch(
 pub fn set_target_push_remote(
     project_repository: &project_repository::Repository,
     push_remote_name: &str,
-) -> Result<(), errors::SetBaseBranchError> {
-    let repo = &project_repository.git_repository;
-
-    let remote = repo
+) -> Result<()> {
+    let remote = project_repository
+        .git_repository
         .find_remote(push_remote_name)
         .context(format!("failed to find remote {}", push_remote_name))?;
 
     // if target exists, and it is the same as the requested branch, we should go back
     let mut target = default_target(&project_repository.project().gb_dir())?;
-
-    target.push_remote_name = Some(
-        remote
-            .name()
-            .context("failed to get remote name")?
-            .to_string(),
-    );
+    target.push_remote_name = remote
+        .name()
+        .context("failed to get remote name")?
+        .to_string()
+        .into();
     let vb_state = project_repository.project().virtual_branches();
-    vb_state.set_default_target(target.clone())?;
+    vb_state.set_default_target(target)?;
 
     Ok(())
 }

crates/gitbutler-core/src/virtual_branches/controller.rs

@@ -137,7 +137,7 @@ impl Controller {
         &self,
         project_id: ProjectId,
         target_branch: &git::RemoteRefname,
-    ) -> Result<super::BaseBranch, Error> {
+    ) -> Result<BaseBranch, Error> {
         self.inner(project_id)
             .await
             .set_base_branch(project_id, target_branch)
@@ -546,13 +546,13 @@ impl ControllerInner {
         &self,
         project_id: ProjectId,
         target_branch: &git::RemoteRefname,
-    ) -> Result<super::BaseBranch, Error> {
+    ) -> Result<BaseBranch, Error> {
         let project = self.projects.get(project_id)?;
         let project_repository = project_repository::Repository::open(&project)?;
         let _ = project_repository
             .project()
             .create_snapshot(SnapshotDetails::new(OperationKind::SetBaseBranch));
-        super::set_base_branch(&project_repository, target_branch).map_err(Error::from_err)
+        Ok(super::set_base_branch(&project_repository, target_branch)?)
     }
 
     pub fn set_target_push_remote(
@@ -562,7 +562,10 @@ impl ControllerInner {
     ) -> Result<(), Error> {
         let project = self.projects.get(project_id)?;
         let project_repository = project_repository::Repository::open(&project)?;
-        super::set_target_push_remote(&project_repository, push_remote).map_err(Error::from_err)
+        Ok(super::set_target_push_remote(
+            &project_repository,
+            push_remote,
+        )?)
     }
 
     pub async fn integrate_upstream_commits(

crates/gitbutler-core/src/virtual_branches/errors.rs

@@ -306,16 +306,6 @@ pub enum UpdateCommitMessageError {
     Other(#[from] anyhow::Error),
 }
 
-#[derive(Debug, thiserror::Error)]
-pub enum SetBaseBranchError {
-    #[error("Current HEAD is dirty.")]
-    DirtyWorkingDirectory,
-    #[error("remote branch '{0}' not found")]
-    BranchNotFound(git::RemoteRefname),
-    #[error(transparent)]
-    Other(#[from] anyhow::Error),
-}
-
 #[derive(Debug, thiserror::Error)]
 pub enum UpdateBaseBranchError {
     #[error("project is in conflicting state")]
@@ -379,6 +369,7 @@ impl ProjectConflict {
             "project {} is in a conflicted state",
             self.project_id
         ))
+        .with_code(Code::ProjectConflict)
     }
 }
 
@@ -389,10 +380,11 @@ pub struct DefaultTargetNotSet {
 
 impl DefaultTargetNotSet {
     fn to_context(&self) -> error::Context {
-        error::Context::new(format!(
+        Context::new(format!(
             "project {} does not have a default target set",
             self.project_id
         ))
+        .with_code(Code::ProjectConflict)
     }
 }
 

crates/gitbutler-core/src/virtual_branches/virtual.rs

@@ -1233,7 +1233,7 @@ pub fn integrate_upstream_commits(
     if has_rebased_commits && !can_use_force {
         let message = "Aborted because force push is disallowed and commits have been rebased.";
         return Err(anyhow!("Cannot merge rebased commits without force push")
-            .context(error::Context::new(message)));
+            .context(error::Context::new(message).with_code(Code::ProjectConflict)));
     }
 
     let integration_result = match can_use_force {
@@ -1243,7 +1243,7 @@ pub fn integrate_upstream_commits(
                 let message =
                     "Aborted because force push is disallowed and commits have been rebased.";
                 return Err(anyhow!("Cannot merge rebased commits without force push")
-                    .context(error::Context::new(message)));
+                    .context(error::Context::new(message).with_code(Code::ProjectConflict)));
             }
             integrate_with_merge(
                 project_repository,
@@ -1255,14 +1255,11 @@ pub fn integrate_upstream_commits(
         }
     };
 
-    // TODO: Use thiserror for the two integrate_with functions instead of this?
-    if let Err(err) = &integration_result {
-        if err
-            .custom_context()
+    if integration_result.as_ref().err().map_or(false, |err| {
+        err.custom_context()
             .is_some_and(|c| c.code == Code::ProjectConflict)
-        {
-            return Ok(());
-        };
+    }) {
+        return Ok(());
     };
 
     let new_head = integration_result?;

crates/gitbutler-core/tests/suite/virtual_branches/set_base_branch.rs

@@ -25,7 +25,7 @@ mod error {
             ..
         } = &Test::default();
 
-        assert!(matches!(
+        assert_eq!(
             controller
                 .set_base_branch(
                     *project_id,
@@ -33,13 +33,14 @@ mod error {
                 )
                 .await
                 .unwrap_err()
-                .downcast_ref(),
-            Some(errors::SetBaseBranchError::BranchNotFound(_))
-        ));
+                .to_string(),
+            "remote branch 'refs/remotes/origin/missing' not found"
+        );
     }
 }
 
 mod go_back_to_integration {
+    use gitbutler_core::error::Code;
     use pretty_assertions::assert_eq;
 
     use super::*;
@@ -123,7 +124,7 @@ mod go_back_to_integration {
                 .await
                 .unwrap_err()
                 .downcast_ref(),
-            Some(errors::SetBaseBranchError::DirtyWorkingDirectory)
+            Some(Code::ProjectConflict)
         ));
     }
 
@@ -159,7 +160,7 @@ mod go_back_to_integration {
                 .await
                 .unwrap_err()
                 .downcast_ref(),
-            Some(errors::SetBaseBranchError::DirtyWorkingDirectory)
+            Some(Code::ProjectConflict)
         ));
     }