refactor: git: use thiserror and cleanup errors

crates/git/Cargo.toml

@@ -41,6 +41,7 @@ secrecy = { workspace = true }
 # error handling
 derive_more = { workspace = true }
 derive-with = { workspace = true }
+thiserror = { workspace = true }
 
 # # file watcher
 # inotify = { workspace = true }

crates/git/src/branch.rs

@@ -4,21 +4,26 @@ use git_next_config as config;
 
 pub type Result<T> = core::result::Result<T, Error>;
 
-#[derive(Debug, derive_more::Display, derive_more::From)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
-    Network(kxio::network::NetworkError),
+    #[error("network: {0}")]
+    Network(#[from] kxio::network::NetworkError),
 
-    Fetch(git::fetch::Error),
+    #[error("fetch: {0}")]
+    Fetch(#[from] git::fetch::Error),
 
-    Push(git::push::Error),
+    #[error("push: {0}")]
+    Push(#[from] git::push::Error),
 
+    #[error("lock")]
     Lock,
 
-    Generic(String),
+    #[error("gix iter: {0}")]
-    I1(gix::reference::iter::Error),
+    GixIter(#[from] gix::reference::iter::Error),
-    I2(gix::reference::iter::init::Error),
+
+    #[error("gix iter init: {0}")]
+    GixIterInit(#[from] gix::reference::iter::init::Error),
 }
-impl std::error::Error for Error {}
 
 pub fn reset(
     repository: &git::OpenRepository,

crates/git/src/commit.rs

@@ -39,14 +39,21 @@ pub struct Histories {
 }
 
 pub mod log {
-    use derive_more::{Display, From};
 
     pub type Result<T> = core::result::Result<T, Error>;
 
-    #[derive(Debug, Display, From)]
+    #[derive(Debug, thiserror::Error)]
     pub enum Error {
+        #[error("gix: {0}")]
         Gix(String),
+
+        #[error("lock")]
         Lock,
     }
-    impl std::error::Error for Error {}
+
+    impl From<String> for Error {
+        fn from(e: String) -> Self {
+            Self::Gix(e)
+        }
+    }
 }

crates/git/src/fetch.rs

@@ -1,20 +1,20 @@
-#[derive(Debug, derive_more::Display)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
+    #[error("unable to open repo: {0}")]
     UnableToOpenRepo(String),
+
+    #[error("no remote found")]
     NoFetchRemoteFound,
+
+    #[error("remote connect: {0}")]
     Connect(String),
-    Fetch(String),
+
+    #[error("prepare: {0}")]
+    Prepare(String),
+
+    #[error("receive: {0}")]
+    Receive(String),
+
+    #[error("lock")]
     Lock,
 }
-impl std::error::Error for Error {}
-
-mod gix_error {
-    #![cfg(not(tarpaulin_include))] // third-party library errors
-    use super::Error;
-
-    impl From<gix::remote::connect::Error> for Error {
-        fn from(gix_error: gix::remote::connect::Error) -> Self {
-            Self::Connect(gix_error.to_string())
-        }
-    }
-}

crates/git/src/file.rs

@@ -1,54 +1,57 @@
 //
 pub type Result<T> = core::result::Result<T, Error>;
 
-#[derive(Debug, derive_more::Display)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
+    #[error("lock")]
     Lock,
 
-    #[display("File not found: {}", 0)]
+    #[error("File not found: {}", 0)]
     NotFound(String),
 
-    #[display("Unable to parse file contents")]
+    #[error("Unable to parse file contents")]
     ParseContent,
 
-    #[display("Unable to decode from base64")]
+    #[error("Unable to decode from base64")]
     DecodeFromBase64,
 
-    #[display("Unable to decode from UTF-8")]
+    #[error("Unable to decode from UTF-8")]
     DecodeFromUtf8,
 
-    #[display("Unknown file encoding: {}", 0)]
+    #[error("Unknown file encoding: {}", 0)]
     UnknownEncoding(String),
 
-    #[display("Not a file: {}", 0)]
+    #[error("Not a file: {}", 0)]
     NotFile(String),
 
-    #[display("Unknown error (status: {})", 0)]
+    #[error("Unknown error (status: {})", 0)]
     Unknown(String),
 
-    CommitLog(crate::commit::log::Error),
+    #[error("commit log: {0}")]
+    CommitLog(#[from] crate::commit::log::Error),
 
+    #[error("commit not found")]
     CommitNotFound,
 
+    #[error("no tree in commit")]
     NoTreeInCommit(String),
 
+    #[error("no .git-next.toml file found in repo")]
     NoGitNextToml,
 
+    #[error("find reference: {0}")]
     FindReference(String),
 
+    #[error("find object: {0}")]
     FindObject(String),
 
+    #[error("Non-UTF-8 in blob: {0}")]
     NonUtf8Blob(String),
 
+    #[error("try id")]
     TryId,
 }
-impl std::error::Error for Error {}
 
-impl From<crate::commit::log::Error> for Error {
-    fn from(value: crate::commit::log::Error) -> Self {
-        Self::CommitLog(value)
-    }
-}
 impl From<gix::reference::find::existing::Error> for Error {
     fn from(value: gix::reference::find::existing::Error) -> Self {
         Self::FindReference(value.to_string())

crates/git/src/forge/webhook.rs

@@ -1,35 +1,29 @@
-use derive_more::Display;
+//
-
 pub type Result<T> = core::result::Result<T, Error>;
 
-#[derive(Debug, Display)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
-    #[display("network: {}", 0)]
+    #[error("network")]
-    Network(kxio::network::NetworkError),
+    Network(#[from] kxio::network::NetworkError),
 
+    #[error("failed to register: {0}")]
     FailedToRegister(String),
 
+    #[error("network response was empty")]
     NetworkResponseEmpty,
 
+    #[error("repo config not loaded")]
     NoRepoConfig,
 
+    #[error("failed to notify self of loaded config")]
     FailedToNotifySelf(String),
 
-    Serde(serde_json::error::Error),
+    #[error("(de)serialisation")]
+    Serde(#[from] serde_json::error::Error),
 
+    #[error("unknown branch: {0}")]
     UnknownBranch(String),
+
+    #[error("failed to list: {0}")]
     FailedToList(String),
 }
-impl std::error::Error for Error {}
-
-impl From<kxio::network::NetworkError> for Error {
-    fn from(value: kxio::network::NetworkError) -> Self {
-        Self::Network(value)
-    }
-}
-
-impl From<serde_json::error::Error> for Error {
-    fn from(value: serde_json::error::Error) -> Self {
-        Self::Serde(value)
-    }
-}

crates/git/src/push.rs

@@ -14,12 +14,16 @@ impl std::fmt::Display for Force {
     }
 }
 
-#[derive(Debug, derive_more::From, derive_more::Display)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
-    Open(Box<gix::open::Error>),
+    #[error("gix open: {0}")]
-    Push,
+    Open(#[from] Box<gix::open::Error>),
+
+    #[error("io: {0}")]
+    Push(#[from] std::io::Error),
+
+    #[error("lock")]
     Lock,
 }
-impl std::error::Error for Error {}
 
 pub type Result<T> = core::result::Result<T, Error>;

crates/git/src/repository/mod.rs

@@ -81,19 +81,35 @@ impl From<Direction> for gix::remote::Direction {
 
 pub type Result<T> = core::result::Result<T, Error>;
 
-#[derive(Debug, derive_more::Display)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
+    #[error("invalid git dir: {0}")]
     InvalidGitDir(git_next_config::GitDir),
+
+    #[error("io: {0}")]
     Io(std::io::Error),
+
+    #[error("git exec wait: {0}")]
     Wait(std::io::Error),
+
+    #[error("git exec spawn: {0}")]
     Spawn(std::io::Error),
+
+    #[error("validation: {0}")]
     Validation(String),
+
+    #[error("git clone: {0}")]
     Clone(String),
+
+    #[error("open: {0}")]
     Open(String),
+
+    #[error("git fetch: {0}")]
     Fetch(String),
+
+    #[error("mock lock")]
     MockLock,
 }
-impl std::error::Error for Error {}
 
 mod gix_errors {
     #![cfg(not(tarpaulin_include))] // third-party library errors

crates/git/src/repository/open/oreal.rs

@@ -56,17 +56,13 @@ impl super::OpenRepositoryLike for RealOpenRepository {
             #[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes
             return Err(git::fetch::Error::NoFetchRemoteFound);
         };
-        let prepared_fetch = remote
+        remote
-            .connect(gix::remote::Direction::Fetch)?
+            .connect(gix::remote::Direction::Fetch)
-            .prepare_fetch(gix::progress::Discard, Default::default());
+            .map_err(|gix| git::fetch::Error::Connect(gix.to_string()))?
-        match prepared_fetch {
+            .prepare_fetch(gix::progress::Discard, Default::default())
-            Ok(fetch) => {
+            .map_err(|gix| git::fetch::Error::Prepare(gix.to_string()))?
-                fetch
             .receive(gix::progress::Discard, &Default::default())
-                    .map_err(|e| git::fetch::Error::Fetch(e.to_string()))?;
+            .map_err(|gix| git::fetch::Error::Receive(gix.to_string()))?;
-            }
-            Err(e) => Err(git::fetch::Error::Fetch(e.to_string()))?,
-        }
         info!("Fetch okay");
         Ok(())
     }
@@ -101,32 +97,18 @@ impl super::OpenRepositoryLike for RealOpenRepository {
             .map_err(|_| git::push::Error::Lock)
             .map(|r| r.git_dir().to_path_buf())?;
         let ctx = gix::diff::command::Context {
-            git_dir: Some(git_dir.clone()),
+            git_dir: Some(git_dir),
             ..Default::default()
         };
-        match gix::command::prepare(command.expose_secret())
+        gix::command::prepare(command.expose_secret())
             .with_context(ctx)
             .with_shell_allow_argument_splitting()
             .stdout(std::process::Stdio::null())
             .stderr(std::process::Stdio::null())
-            .spawn()
+            .spawn()?
-        {
+            .wait()?;
-            Ok(mut child) => match child.wait() {
-                Ok(_) => {
-                    info!("Push okay");
         Ok(())
     }
-                Err(err) => {
-                    warn!(?err, ?git_dir, "Failed (wait)");
-                    Err(git::push::Error::Push)
-                }
-            },
-            Err(err) => {
-                warn!(?err, ?git_dir, "Failed (spawn)");
-                Err(git::push::Error::Push)
-            }
-        }
-    }
 
     fn commit_log(
         &self,

crates/git/src/validation/positions.rs

@@ -48,9 +48,10 @@ pub fn validate_positions(
             repo_config.branches().main(),
             repo_config.branches().main(),
         );
-        return Err(git::validation::positions::Error::DevBranchNotBasedOn(
+        return Err(git::validation::positions::Error::DevBranchNotBasedOn {
-            repo_config.branches().main(),
+            dev: repo_config.branches().dev(),
-        ));
+            other: repo_config.branches().main(),
+        });
     }
     // verify that next is an ancestor of dev, and force update to it main if it isn't
     let Some(next) = commit_histories.next.first().cloned() else {
@@ -78,7 +79,7 @@ pub fn validate_positions(
                 commit: next,
             });
         }
-        return Err(git::validation::positions::Error::BranchReset(
+        return Err(git::validation::positions::Error::NextBranchResetRequired(
             repo_config.branches().next(),
         ));
     }
@@ -108,7 +109,7 @@ pub fn validate_positions(
                 commit: next,
             });
         }
-        return Err(git::validation::positions::Error::BranchReset(
+        return Err(git::validation::positions::Error::NextBranchResetRequired(
             repo_config.branches().next(),
         ));
     }
@@ -131,9 +132,10 @@ pub fn validate_positions(
             repo_config.branches().dev(),
             repo_config.branches().next()
         );
-        return Err(git::validation::positions::Error::DevBranchNotBasedOn(
+        return Err(git::validation::positions::Error::DevBranchNotBasedOn {
-            repo_config.branches().next(),
+            dev: repo_config.branches().dev(),
-        )); // dev is not based on next
+            other: repo_config.branches().next(),
+        }); // dev is not based on next
     }
     let Some(dev) = commit_histories.dev.first().cloned() else {
         warn!(
@@ -169,28 +171,29 @@ fn get_commit_histories(
     let histories = git::commit::Histories { main, next, dev };
     Ok(histories)
 }
-#[derive(Debug, derive_more::Display)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
-    Fetch(git::fetch::Error),
+    #[error("fetch: {0}")]
+    Fetch(#[from] git::fetch::Error),
 
-    CommitLog(git::commit::log::Error),
+    #[error("commit log: {0}")]
+    CommitLog(#[from] git::commit::log::Error),
 
-    #[display("Failed to Reset Branch {branch} to {commit}")]
+    #[error("failed to reset branch '{branch}' to {commit}")]
     FailedToResetBranch {
         branch: config::BranchName,
         commit: git::Commit,
     },
 
-    BranchReset(config::BranchName),
+    #[error("next branch '{0}' needs to be reset")]
+    NextBranchResetRequired(config::BranchName),
 
+    #[error("branch '{0}' has no commits")]
     BranchHasNoCommits(config::BranchName),
 
-    DevBranchNotBasedOn(config::BranchName),
+    #[error("dev branch '{dev}' not based on branch '{other}' ")]
-}
+    DevBranchNotBasedOn {
-impl std::error::Error for Error {}
+        dev: config::BranchName,
-
+        other: config::BranchName,
-impl From<git::fetch::Error> for Error {
+    },
-    fn from(value: git::fetch::Error) -> Self {
-        Self::Fetch(value)
-    }
 }

crates/git/src/validation/repo.rs

@@ -32,23 +32,35 @@ pub fn validate_repo(
 }
 
 type Result<T> = core::result::Result<T, Error>;
-#[derive(Debug, derive_more::Display)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
+    #[error("no default push remote")]
     NoDefaultPushRemote,
+
+    #[error("no default fetch remote")]
     NoDefaultFetchRemote,
+
+    #[error("no url for default push remote")]
     NoUrlForDefaultPushRemote,
+
+    #[error("no hostname for default push remote")]
     NoHostnameForDefaultPushRemote,
+
+    #[error("unable to open repo: {0}")]
     UnableToOpenRepo(String),
-    Io(std::io::Error),
+
-    #[display("MismatchDefaultPushRemote(found: {found}, expected: {expected})")]
+    #[error("io")]
+    Io(#[from] std::io::Error),
+
+    #[error("MismatchDefaultPushRemote(found: {found}, expected: {expected})")]
     MismatchDefaultPushRemote {
         found: git::GitRemote,
         expected: git::GitRemote,
     },
-    #[display("MismatchDefaultFetchRemote(found: {found}, expected: {expected})")]
+
+    #[error("MismatchDefaultFetchRemote(found: {found}, expected: {expected})")]
     MismatchDefaultFetchRemote {
         found: git::GitRemote,
         expected: git::GitRemote,
     },
 }
-impl std::error::Error for Error {}