From 08f234d85a22ddd74ef286e40667f3801282fd7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christina=20S=C3=B8rensen?= Date: Mon, 16 Oct 2023 19:40:53 +0200 Subject: [PATCH 1/6] refactor(link): add LinkError and use for handle_file_exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christina Sørensen --- src/git.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/src/git.rs b/src/git.rs index bce9dbb..8f232fb 100644 --- a/src/git.rs +++ b/src/git.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use std::fs::canonicalize; use std::os::unix::fs::symlink; use std::path::Path; -use std::{fs, process::Command}; +use std::{fmt, fs, process::Command}; use crate::settings; use crate::utils::strings::{failure_str, success_str}; @@ -123,7 +123,40 @@ pub struct SeriesItem<'series> { pub closure: Box (bool)>, } -fn handle_file_exists(selff: &Link, tx_path: &Path, rx_path: &Path) -> bool { +#[derive(Debug, Clone)] +pub enum LinkError { + AlreadyLinked(String, String), + DifferentLink(String, String), + FileExists(String, String), +} + +impl std::fmt::Display for LinkError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + LinkError::AlreadyLinked(tx, rx) => { + write!(f, "Linking {tx} -> {rx} failed: file already linked") + } + LinkError::DifferentLink(tx, rx) => write!( + f, + "Linking {tx} -> {rx} failed: link to different file exists" + ), + LinkError::FileExists(tx, rx) => write!(f, "Linking {tx} -> {rx} failed: file exists"), + } + } +} + +impl std::error::Error for LinkError { + // TODO: I'm too tired to implement soucce... yawn, eepy + // fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + // match self { + // LinkError::AlreadyLinked(tx, rx) => Some(rx), + // LinkError::DifferentLink(tx, rx) => Some(rx), + // LinkError::FileExists(tx, rx) => Some(rx), + // } + // } +} + +fn handle_file_exists(selff: &Link, tx_path: &Path, rx_path: &Path) -> Result { match rx_path.read_link() { Ok(file) if file.canonicalize().expect("failed to canonicalize file") @@ -133,18 +166,27 @@ fn handle_file_exists(selff: &Link, tx_path: &Path, rx_path: &Path) -> bool { "Linking {} -> {} failed: file already linked", &selff.tx, &selff.rx ); - false + Err(LinkError::AlreadyLinked( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )) } Ok(file) => { error!( "Linking {} -> {} failed: link to different file exists", &selff.tx, &selff.rx ); - false + Err(LinkError::DifferentLink( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )) } Err(error) => { error!("Linking {} -> {} failed: file exists", &selff.tx, &selff.rx); - false + Err(LinkError::FileExists( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )) } } } @@ -155,7 +197,8 @@ impl Link { let tx_path: &Path = std::path::Path::new(&self.tx); let rx_path: &Path = std::path::Path::new(&self.rx); match rx_path.try_exists() { - Ok(true) => handle_file_exists(self, tx_path, rx_path), + // TODO: unwrap defeats the purpose here. + Ok(true) => handle_file_exists(self, tx_path, rx_path).unwrap(), Ok(false) if rx_path.is_symlink() => { error!( "Linking {} -> {} failed: broken symlink", -- 2.46.0 From 7a333b53bd3fc8e441ae2a97d80cf10a92963544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christina=20S=C3=B8rensen?= Date: Tue, 17 Oct 2023 06:47:18 +0200 Subject: [PATCH 2/6] refactor(link): add error handling for linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christina Sørensen --- src/git.rs | 54 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/src/git.rs b/src/git.rs index 8f232fb..13e672c 100644 --- a/src/git.rs +++ b/src/git.rs @@ -128,6 +128,8 @@ pub enum LinkError { AlreadyLinked(String, String), DifferentLink(String, String), FileExists(String, String), + BrokenSymlinkExists(String, String), + FailedCreatingLink(String, String), } impl std::fmt::Display for LinkError { @@ -141,6 +143,10 @@ impl std::fmt::Display for LinkError { "Linking {tx} -> {rx} failed: link to different file exists" ), LinkError::FileExists(tx, rx) => write!(f, "Linking {tx} -> {rx} failed: file exists"), + LinkError::BrokenSymlinkExists(tx, rx) => { + write!(f, "Linking {tx} -> {rx} failed: broken symlink") + } + LinkError::FailedCreatingLink(tx, rx) => write!(f, "Linking {tx} -> {rx} failed"), } } } @@ -193,26 +199,35 @@ fn handle_file_exists(selff: &Link, tx_path: &Path, rx_path: &Path) -> Result bool { + pub fn link(&self) -> Result { let tx_path: &Path = std::path::Path::new(&self.tx); let rx_path: &Path = std::path::Path::new(&self.rx); match rx_path.try_exists() { // TODO: unwrap defeats the purpose here. - Ok(true) => handle_file_exists(self, tx_path, rx_path).unwrap(), + Ok(true) => handle_file_exists(self, tx_path, rx_path), Ok(false) if rx_path.is_symlink() => { error!( "Linking {} -> {} failed: broken symlink", &self.tx, &self.rx ); - false + Err(LinkError::FileExists( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )) } Ok(false) => { symlink(&self.tx, &self.rx).expect("failed to create link"); - true + Err(LinkError::FailedCreatingLink( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )) } Err(error) => { error!("Linking {} -> {} failed: {}", &self.tx, &self.rx, error); - false + Err(LinkError::FailedCreatingLink( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )) } } } @@ -594,7 +609,7 @@ impl Config { /// Runs associated function on all links in config fn on_all_links_spinner(&self, op: &str, f: F) where - F: Fn(&Link) -> bool, + F: Fn(&Link) -> Result, { for category in self.categories.values() { match category.links.as_ref() { @@ -603,16 +618,27 @@ impl Config { if !settings::QUIET.load(std::sync::atomic::Ordering::Relaxed) { let mut sp = Spinner::new(Spinners::Dots10, format!("{}: {}", link.name, op)); - if f(link) { - sp.stop_and_persist( - success_str(), - format!("{}: {}", link.name, op), - ); - } else { - sp.stop_and_persist( + match f(link) { + Err(e @ LinkError::AlreadyLinked(_, _)) => { + sp.stop_and_persist(success_str(), format!("{e}")) + } + Err(e @ LinkError::DifferentLink(_, _)) => { + sp.stop_and_persist(failure_str(), format!("{e}")) + } + Err(e @ LinkError::FileExists(_, _)) => { + sp.stop_and_persist(failure_str(), format!("{e}")) + } + Err(e @ LinkError::BrokenSymlinkExists(_, _)) => { + sp.stop_and_persist(failure_str(), format!("{e}")) + } + Err(e @ LinkError::FailedCreatingLink(_, _)) => { + sp.stop_and_persist(failure_str(), format!("{e}")) + } + Err(e) => sp.stop_and_persist(failure_str(), format!("{e}")), + _ => sp.stop_and_persist( failure_str(), format!("{}: {}", link.name, op), - ); + ), } } else { f(link); -- 2.46.0 From 9c474d665e724bf603a4e80c4ad54dd699b7e47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christina=20S=C3=B8rensen?= Date: Tue, 17 Oct 2023 06:59:06 +0200 Subject: [PATCH 3/6] refactor(link): add LinkError::IoError wrapping, fix happy path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christina Sørensen --- src/git.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/git.rs b/src/git.rs index 13e672c..55ed35b 100644 --- a/src/git.rs +++ b/src/git.rs @@ -123,13 +123,14 @@ pub struct SeriesItem<'series> { pub closure: Box (bool)>, } -#[derive(Debug, Clone)] +#[derive(Debug)] pub enum LinkError { AlreadyLinked(String, String), DifferentLink(String, String), FileExists(String, String), BrokenSymlinkExists(String, String), FailedCreatingLink(String, String), + IoError(std::io::Error), } impl std::fmt::Display for LinkError { @@ -147,6 +148,7 @@ impl std::fmt::Display for LinkError { write!(f, "Linking {tx} -> {rx} failed: broken symlink") } LinkError::FailedCreatingLink(tx, rx) => write!(f, "Linking {tx} -> {rx} failed"), + LinkError::IoError(err) => write!(f, "IO Error: {err}"), } } } @@ -162,6 +164,12 @@ impl std::error::Error for LinkError { // } } +impl From for LinkError { + fn from(err: std::io::Error) -> LinkError { + LinkError::IoError(err) + } +} + fn handle_file_exists(selff: &Link, tx_path: &Path, rx_path: &Path) -> Result { match rx_path.read_link() { Ok(file) @@ -216,11 +224,8 @@ impl Link { )) } Ok(false) => { - symlink(&self.tx, &self.rx).expect("failed to create link"); - Err(LinkError::FailedCreatingLink( - tx_path.to_string_lossy().to_string(), - rx_path.to_string_lossy().to_string(), - )) + symlink(&self.tx, &self.rx)?; + Ok(true) } Err(error) => { error!("Linking {} -> {} failed: {}", &self.tx, &self.rx, error); @@ -634,6 +639,9 @@ impl Config { Err(e @ LinkError::FailedCreatingLink(_, _)) => { sp.stop_and_persist(failure_str(), format!("{e}")) } + Err(e @ LinkError::IoError(_)) => { + sp.stop_and_persist(failure_str(), format!("{e}")) + } Err(e) => sp.stop_and_persist(failure_str(), format!("{e}")), _ => sp.stop_and_persist( failure_str(), -- 2.46.0 From 7b88057b3f382315c637a28558be9b4bf609f053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christina=20S=C3=B8rensen?= Date: Tue, 17 Oct 2023 07:02:22 +0200 Subject: [PATCH 4/6] refactor(link): remove debug error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christina Sørensen --- src/git.rs | 54 ++++++++++++++++-------------------------------------- 1 file changed, 16 insertions(+), 38 deletions(-) diff --git a/src/git.rs b/src/git.rs index 55ed35b..ff87b7a 100644 --- a/src/git.rs +++ b/src/git.rs @@ -176,32 +176,19 @@ fn handle_file_exists(selff: &Link, tx_path: &Path, rx_path: &Path) -> Result { - debug!( - "Linking {} -> {} failed: file already linked", - &selff.tx, &selff.rx - ); Err(LinkError::AlreadyLinked( tx_path.to_string_lossy().to_string(), rx_path.to_string_lossy().to_string(), )) } - Ok(file) => { - error!( - "Linking {} -> {} failed: link to different file exists", - &selff.tx, &selff.rx - ); - Err(LinkError::DifferentLink( - tx_path.to_string_lossy().to_string(), - rx_path.to_string_lossy().to_string(), - )) - } - Err(error) => { - error!("Linking {} -> {} failed: file exists", &selff.tx, &selff.rx); - Err(LinkError::FileExists( - tx_path.to_string_lossy().to_string(), - rx_path.to_string_lossy().to_string(), - )) - } + Ok(file) => Err(LinkError::DifferentLink( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )), + Err(error) => Err(LinkError::FileExists( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )), } } @@ -213,27 +200,18 @@ impl Link { match rx_path.try_exists() { // TODO: unwrap defeats the purpose here. Ok(true) => handle_file_exists(self, tx_path, rx_path), - Ok(false) if rx_path.is_symlink() => { - error!( - "Linking {} -> {} failed: broken symlink", - &self.tx, &self.rx - ); - Err(LinkError::FileExists( - tx_path.to_string_lossy().to_string(), - rx_path.to_string_lossy().to_string(), - )) - } + Ok(false) if rx_path.is_symlink() => Err(LinkError::FileExists( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )), Ok(false) => { symlink(&self.tx, &self.rx)?; Ok(true) } - Err(error) => { - error!("Linking {} -> {} failed: {}", &self.tx, &self.rx, error); - Err(LinkError::FailedCreatingLink( - tx_path.to_string_lossy().to_string(), - rx_path.to_string_lossy().to_string(), - )) - } + Err(error) => Err(LinkError::FailedCreatingLink( + tx_path.to_string_lossy().to_string(), + rx_path.to_string_lossy().to_string(), + )), } } } -- 2.46.0 From 41334cdaf00b37ebe7cbc2bfd18d0a8b349bb140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christina=20S=C3=B8rensen?= Date: Tue, 17 Oct 2023 07:02:45 +0200 Subject: [PATCH 5/6] refactor(link): remove TODO comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christina Sørensen --- src/git.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/git.rs b/src/git.rs index ff87b7a..e7ec2d5 100644 --- a/src/git.rs +++ b/src/git.rs @@ -198,7 +198,6 @@ impl Link { let tx_path: &Path = std::path::Path::new(&self.tx); let rx_path: &Path = std::path::Path::new(&self.rx); match rx_path.try_exists() { - // TODO: unwrap defeats the purpose here. Ok(true) => handle_file_exists(self, tx_path, rx_path), Ok(false) if rx_path.is_symlink() => Err(LinkError::FileExists( tx_path.to_string_lossy().to_string(), -- 2.46.0 From a1a52b10f3d2571f59092d579b29932974322d70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christina=20S=C3=B8rensen?= Date: Tue, 17 Oct 2023 07:04:27 +0200 Subject: [PATCH 6/6] refactor(link): propagate canonicalization errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christina Sørensen --- src/git.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/git.rs b/src/git.rs index e7ec2d5..02175b7 100644 --- a/src/git.rs +++ b/src/git.rs @@ -172,10 +172,7 @@ impl From for LinkError { fn handle_file_exists(selff: &Link, tx_path: &Path, rx_path: &Path) -> Result { match rx_path.read_link() { - Ok(file) - if file.canonicalize().expect("failed to canonicalize file") - == tx_path.canonicalize().expect("failed to canonicalize path") => - { + Ok(file) if file.canonicalize()? == tx_path.canonicalize()? => { Err(LinkError::AlreadyLinked( tx_path.to_string_lossy().to_string(), rx_path.to_string_lossy().to_string(), -- 2.46.0