Improve git error handling.

Don't panic on git error, but show message.
nabijaczleweli · Oct 1, 2024 · b753953 · b753953
1 parent 2d682fb
commit b753953
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 14 deletions.
diff --git a/src/main.rs b/src/main.rs
@@ -3,11 +3,11 @@ extern crate tabwriter;
 extern crate git2;
 
 use std::io::{ErrorKind as IoErrorKind, Write, stdout, sink};
+use std::fmt::{self, Formatter, Display};
 use std::process::{Command, exit};
 use std::collections::BTreeMap;
 use std::iter::FromIterator;
 use tabwriter::TabWriter;
-use std::fmt::Display;
 use std::ffi::OsStr;
 #[cfg(target_os="windows")]
 use std::fs::File;
@@ -368,11 +368,20 @@ fn actual_main() -> Result<(), i32> {
             writeln!(out, "Package\tInstalled\tLatest\tNeeds update").unwrap();
             packages.sort_by(|lhs, rhs| (!lhs.needs_update(), &lhs.name).cmp(&(!rhs.needs_update(), &rhs.name)));
             for package in &packages {
+                struct OidOrError<'a, Oid: Display, GitError: Display>(&'a Result<Oid, GitError>);
+                impl<Oid: Display, GitError: Display> Display for OidOrError<'_, Oid, GitError> {
+                    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), fmt::Error> {
+                        match self.0 {
+                            Ok(oid) => write!(f, "{}", oid),
+                            Err(err) => write!(f, "git error: {}", err),
+                        }
+                    }
+                }
                 writeln!(out,
                          "{}\t{}\t{}\t{}",
                          package.name,
                          package.id,
-                         package.newest_id.as_ref().unwrap(),
+                         OidOrError(&package.newest_id),
                          if package.needs_update() { "Yes" } else { "No" })
                     .unwrap();
             }

diff --git a/src/ops/mod.rs b/src/ops/mod.rs
@@ -142,18 +142,18 @@ pub struct RegistryPackage {
 ///                url: "https://github.com/jwilm/alacritty".to_string(),
 ///                branch: None,
 ///                id: git2::Oid::from_str("eb231b3e70b87875df4bdd1974d5e94704024d70").unwrap(),
-///                newest_id: None,
+///                newest_id: Err(git2::Error::from_str("")),
 ///                executables: vec!["alacritty".to_string()],
 ///            });
 ///
 /// # /*
 /// package.pull_version(&registry_tree, &registry);
 /// # */
-/// # package.newest_id = Some(git2::Oid::from_str("5f7885749c4d7e48869b1fc0be4d430601cdbbfa").unwrap());
-/// assert!(package.newest_id.is_some());
+/// # package.newest_id = git2::Oid::from_str("5f7885749c4d7e48869b1fc0be4d430601cdbbfa");
+/// assert!(package.newest_id.is_ok());
 /// # }
 /// ```
-#[derive(Debug, Clone, Hash, PartialEq, Eq)]
+#[derive(Debug, PartialEq)]
 pub struct GitRepoPackage {
     /// The package's name.
     pub name: String,
@@ -166,10 +166,27 @@ pub struct GitRepoPackage {
     /// The latest version of the package available at the main [`crates.io`](https://crates.io) repository.
     ///
     /// `None` by default, acquire via `GitRepoPackage::pull_version()`.
-    pub newest_id: Option<Oid>,
+    pub newest_id: Result<Oid, GitError>,
     /// Executables currently installed for this package.
     pub executables: Vec<String>,
 }
+impl Hash for GitRepoPackage {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.name.hash(state);
+        self.url.hash(state);
+        self.branch.hash(state);
+        self.id.hash(state);
+        match &self.newest_id {
+            Ok(nid) => nid.hash(state),
+            Err(err) => {
+                err.raw_code().hash(state);
+                err.raw_class().hash(state);
+                err.message().hash(state);
+            }
+        }
+        self.executables.hash(state);
+    }
+}
 
 
 impl RegistryPackage {
@@ -476,7 +493,7 @@ impl GitRepoPackage {
     ///                url: "https://github.com/jwilm/alacritty".to_string(),
     ///                branch: None,
     ///                id: git2::Oid::from_str("eb231b3e70b87875df4bdd1974d5e94704024d70").unwrap(),
-    ///                newest_id: None,
+    ///                newest_id: Err(git2::Error::from_str("")),
     ///                executables: vec!["alacritty".to_string()],
     ///            });
     ///
@@ -489,7 +506,7 @@ impl GitRepoPackage {
     ///                url: "https://github.com/nabijaczleweli/chattium-oxide-client".to_string(),
     ///                branch: Some("master".to_string()),
     ///                id: git2::Oid::from_str("108a7b94f0e0dcb2a875f70fc0459d5a682df14c").unwrap(),
-    ///                newest_id: None,
+    ///                newest_id: Err(git2::Error::from_str("")),
     ///                executables: vec!["chattium-oxide-client.exe".to_string()],
     ///            });
     /// # }
@@ -512,7 +529,7 @@ impl GitRepoPackage {
                 url: url.into(),
                 branch: branch,
                 id: Oid::from_str(sha).unwrap(),
-                newest_id: None,
+                newest_id: Err(GitError::from_str("")),
                 executables: executables,
             }
         })
@@ -531,7 +548,7 @@ impl GitRepoPackage {
 
         let repo = self.pull_version_repo(&clone_dir, http_proxy, fork_git);
 
-        self.newest_id = Some(repo.and_then(|r| r.head().and_then(|h| h.target().ok_or_else(|| GitError::from_str("HEAD not a direct reference")))).unwrap());
+        self.newest_id = repo.and_then(|r| r.head().and_then(|h| h.target().ok_or_else(|| GitError::from_str("HEAD not a direct reference"))));
     }
 
     fn pull_version_fresh_clone(&self, clone_dir: &Path, http_proxy: Option<&str>, fork_git: bool) -> Result<Repository, GitError> {
@@ -667,21 +684,21 @@ impl GitRepoPackage {
     ///             url: "https://github.com/jwilm/alacritty".to_string(),
     ///             branch: None,
     ///             id: git2::Oid::from_str("eb231b3e70b87875df4bdd1974d5e94704024d70").unwrap(),
-    ///             newest_id: Some(git2::Oid::from_str("5f7885749c4d7e48869b1fc0be4d430601cdbbfa").unwrap()),
+    ///             newest_id: git2::Oid::from_str("5f7885749c4d7e48869b1fc0be4d430601cdbbfa"),
     ///             executables: vec!["alacritty".to_string()],
     ///         }.needs_update());
     /// assert!(!GitRepoPackage {
     ///             name: "alacritty".to_string(),
     ///             url: "https://github.com/jwilm/alacritty".to_string(),
     ///             branch: None,
     ///             id: git2::Oid::from_str("5f7885749c4d7e48869b1fc0be4d430601cdbbfa").unwrap(),
-    ///             newest_id: Some(git2::Oid::from_str("5f7885749c4d7e48869b1fc0be4d430601cdbbfa").unwrap()),
+    ///             newest_id: git2::Oid::from_str("5f7885749c4d7e48869b1fc0be4d430601cdbbfa"),
     ///             executables: vec!["alacritty".to_string()],
     ///         }.needs_update());
     /// # }
     /// ```
     pub fn needs_update(&self) -> bool {
-        self.newest_id.is_some() && self.id != *self.newest_id.as_ref().unwrap()
+        self.newest_id.is_ok() && self.id != *self.newest_id.as_ref().unwrap()
     }
 }