From dde5723cee321f20e61fecaf05b8f38ac2342662 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 5 May 2025 02:11:18 +0300 Subject: [PATCH] refactor: implement thiserror for better error handling --- Cargo.lock | 21 ++++ Cargo.toml | 1 + src/main.rs | 319 ++++++++++++++++++++++++++++++---------------------- 3 files changed, 209 insertions(+), 132 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b0cd346..526dade 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -131,6 +131,7 @@ version = "0.1.0" dependencies = [ "clap", "regex", + "thiserror", "yansi", ] @@ -204,6 +205,26 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "thiserror" +version = "2.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "567b8a2dae586314f7be2a752ec7474332959c6460e02bde30d702a66d488708" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "unicode-ident" version = "1.0.18" diff --git a/Cargo.toml b/Cargo.toml index 71a793f..b24c26a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,4 +6,5 @@ edition = "2024" [dependencies] clap = { version = "4.5.37", features = ["derive"] } regex = "1.11.1" +thiserror = "2.0.12" yansi = "1.0.1" diff --git a/src/main.rs b/src/main.rs index 6a17233..d235e2c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,12 +4,35 @@ use regex::Regex; use std::{ collections::{HashMap, HashSet}, process::Command, - string::{String, ToString}, + string::String, sync::OnceLock, thread, }; +use thiserror::Error; use yansi::Paint; +/// Application errors with thiserror +#[derive(Debug, Error)] +enum AppError { + #[error("Command failed: {0}")] + CommandFailed(String), + + #[error("Failed to decode command output: {0}")] + CommandOutputError(#[from] std::str::Utf8Error), + + #[error("Failed to parse data: {0}")] + ParseError(String), + + #[error("Regex error: {0}")] + RegexError(#[from] regex::Error), + + #[error("IO error: {0}")] + IoError(#[from] std::io::Error), +} + +// Use type alias for Result with our custom error type +type Result = std::result::Result; + #[derive(Parser, Debug)] #[command(name = "Nix not Python diff tool")] #[command(version = "1.0")] @@ -28,6 +51,7 @@ struct Args { closure_size: bool, } +#[derive(Debug, Clone)] struct Package<'a> { name: &'a str, versions: HashSet<&'a str>, @@ -35,6 +59,22 @@ struct Package<'a> { is_dep: bool, } +impl<'a> Package<'a> { + fn new(name: &'a str, version: &'a str, is_dep: bool) -> Self { + let mut versions = HashSet::new(); + versions.insert(version); + Self { + name, + versions, + is_dep, + } + } + + fn add_version(&mut self, version: &'a str) { + self.versions.insert(version); + } +} + fn main() { let args = Args::parse(); @@ -43,7 +83,7 @@ fn main() { println!("<<< {}", args.path.to_string_lossy()); println!(">>> {}", args.path2.to_string_lossy()); - // handles to the threads collecting closure size information + // Handles to the threads collecting closure size information // We do this as early as possible because nix is slow. let closure_size_handles = if args.closure_size { let path = args.path.clone(); @@ -56,30 +96,61 @@ fn main() { None }; - let package_list_pre = get_packages(&args.path); - let package_list_post = get_packages(&args.path2); + // Get package lists and handle potential errors + let package_list_pre = match get_packages(&args.path) { + Ok(packages) => packages, + Err(e) => { + eprintln!("Error getting packages from path {}: {}", args.path.display(), e); + Vec::new() + } + }; + + let package_list_post = match get_packages(&args.path2) { + Ok(packages) => packages, + Err(e) => { + eprintln!("Error getting packages from path {}: {}", args.path2.display(), e); + Vec::new() + } + }; // Map from packages of the first closure to their version let mut pre = HashMap::<&str, HashSet<&str>>::new(); let mut post = HashMap::<&str, HashSet<&str>>::new(); for p in &package_list_pre { - let (name, version) = get_version(&**p); - pre.entry(name).or_default().insert(version); + match get_version(&**p) { + Ok((name, version)) => { + pre.entry(name).or_default().insert(version); + }, + Err(e) => { + if cfg!(debug_assertions) { + eprintln!("Error parsing package version: {e}"); + } + } + } } + for p in &package_list_post { - let (name, version) = get_version(&**p); - post.entry(name).or_default().insert(version); + match get_version(&**p) { + Ok((name, version)) => { + post.entry(name).or_default().insert(version); + }, + Err(e) => { + if cfg!(debug_assertions) { + eprintln!("Error parsing package version: {e}"); + } + } + } } // Compare the package names of both versions let pre_keys: HashSet<&str> = pre.keys().copied().collect(); let post_keys: HashSet<&str> = post.keys().copied().collect(); - // difference gives us added and removed packages + // Difference gives us added and removed packages let added: HashSet<&str> = &post_keys - &pre_keys; let removed: HashSet<&str> = &pre_keys - &post_keys; - // get the intersection of the package names for version changes + // Get the intersection of the package names for version changes let changed: HashSet<&str> = &pre_keys & &post_keys; println!("Difference between the two generations:"); @@ -93,12 +164,15 @@ fn main() { if let Some((pre_handle, post_handle)) = closure_size_handles { match (pre_handle.join(), post_handle.join()) { - (Ok(pre_size), Ok(post_size)) => { + (Ok(Ok(pre_size)), Ok(Ok(post_size))) => { println!("{}", "Closure Size:".underline().bold()); println!("Before: {pre_size} MiB"); println!("After: {post_size} MiB"); println!("Difference: {} MiB", post_size - pre_size); } + (Ok(Err(e)), _) | (_, Ok(Err(e))) => { + eprintln!("Error getting closure size: {e}"); + } _ => { eprintln!("Error: Failed to get closure size information due to a thread error"); } @@ -106,56 +180,44 @@ fn main() { } } -// gets the packages in a closure -fn get_packages(path: &std::path::Path) -> Vec { - // get the nix store paths using nix-store --query --references +/// Gets the packages in a closure +fn get_packages(path: &std::path::Path) -> Result> { + // Get the nix store paths using `nix-store --query --references `` let output = Command::new("nix-store") .arg("--query") .arg("--references") .arg(path.join("sw")) - .output(); + .output()?; - match output { - Ok(query) => { - match str::from_utf8(&query.stdout) { - Ok(list) => list.lines().map(ToString::to_string).collect(), - Err(e) => { - eprintln!("Error decoding command output: {}", e); - Vec::new() - } - } - } - Err(e) => { - eprintln!("Error executing nix-store command: {}", e); - Vec::new() - } + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(AppError::CommandFailed(format!( + "nix-store command failed: {stderr}" + ))); } + + let list = str::from_utf8(&output.stdout)?; + Ok(list.lines().map(str::to_owned).collect()) } -// gets the dependencies of the packages in a closure -fn get_dependencies(path: &std::path::Path) -> Vec { - // get the nix store paths using nix-store --query --references +/// Gets the dependencies of the packages in a closure +fn get_dependencies(path: &std::path::Path) -> Result> { + // Get the nix store paths using `nix-store --query --requisites `` let output = Command::new("nix-store") .arg("--query") .arg("--requisites") .arg(path.join("sw")) - .output(); + .output()?; - match output { - Ok(query) => { - match str::from_utf8(&query.stdout) { - Ok(list) => list.lines().map(ToString::to_string).collect(), - Err(e) => { - eprintln!("Error decoding command output: {}", e); - Vec::new() - } - } - } - Err(e) => { - eprintln!("Error executing nix-store command: {}", e); - Vec::new() - } + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(AppError::CommandFailed(format!( + "nix-store command failed: {stderr}" + ))); } + + let list = str::from_utf8(&output.stdout)?; + Ok(list.lines().map(str::to_owned).collect()) } // Returns a reference to the compiled regex pattern. @@ -168,19 +230,28 @@ fn store_path_regex() -> &'static Regex { }) } -fn get_version<'a>(pack: impl Into<&'a str>) -> (&'a str, &'a str) { - // funny regex to split a nix store path into its name and its version. +// Parse the nix store path to extract package name and version +fn get_version<'a>(pack: impl Into<&'a str>) -> Result<(&'a str, &'a str)> { let path = pack.into(); - + // Match the regex against the input if let Some(cap) = store_path_regex().captures(path) { // Handle potential missing captures safely let name = cap.get(1).map_or("", |m| m.as_str()); let version = cap.get(2).map_or("", |m| m.as_str()); - return (name, version); + + if name.is_empty() || version.is_empty() { + return Err(AppError::ParseError(format!( + "Failed to extract name or version from path: {path}" + ))); + } + + return Ok((name, version)); } - ("", "") + Err(AppError::ParseError(format!( + "Path does not match expected nix store format: {path}" + ))) } fn check_nix_available() -> bool { @@ -191,78 +262,61 @@ fn check_nix_available() -> bool { nix_available.is_some() && nix_query_available.is_some() } -fn get_closure_size(path: &std::path::Path) -> i64 { +fn get_closure_size(path: &std::path::Path) -> Result { // Run nix path-info command to get closure size - match Command::new("nix") + let output = Command::new("nix") .arg("path-info") .arg("--closure-size") .arg(path.join("sw")) - .output() - { - Ok(output) if output.status.success() => { - // Parse command output to extract the size - match str::from_utf8(&output.stdout) { - Ok(stdout) => { - // Parse the last word in the output as an integer - stdout - .split_whitespace() - .last() - .and_then(|s| s.parse::().ok()) - .map_or_else( - || { - eprintln!("Failed to parse closure size from output: {}", stdout); - 0 - }, - |size| size / 1024 / 1024, // Convert to MiB - ) - } - Err(e) => { - eprintln!("Error decoding command output: {}", e); - 0 - } - } - } - Ok(output) => { - // Command ran but returned an error - match str::from_utf8(&output.stderr) { - Ok(stderr) if !stderr.is_empty() => { - eprintln!("nix path-info command failed: {}", stderr); - } - _ => { - eprintln!("nix path-info command failed with status: {}", output.status); - } - } - 0 - } - Err(e) => { - // Command failed to run - eprintln!("Failed to execute nix path-info command: {}", e); - 0 - } + .output()?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(AppError::CommandFailed(format!( + "nix path-info command failed: {stderr}" + ))); } + + let stdout = str::from_utf8(&output.stdout)?; + + // Parse the last word in the output as an integer (in bytes) + stdout + .split_whitespace() + .last() + .ok_or_else(|| AppError::ParseError("Unexpected output format from nix path-info".into()))? + .parse::() + .map_err(|e| AppError::ParseError(format!("Failed to parse size value: {e}"))) + .map(|size| size / 1024 / 1024) // Convert to MiB } fn print_added(set: HashSet<&str>, post: &HashMap<&str, HashSet<&str>>) { println!("{}", "Packages added:".underline().bold()); - for p in set { - let posts = post.get(p); - if let Some(ver) = posts { - let version_str = ver.iter().copied().collect::>().join(" "); - println!( - "{} {} {} {}", - "[A:]".green().bold(), - p, - "@".yellow().bold(), - version_str.blue() - ); - } + + // Use sorted output + let mut sorted: Vec<_> = set.iter() + .filter_map(|p| post.get(p).map(|ver| (*p, ver))) + .collect(); + + // Sort by package name for consistent output + sorted.sort_by(|(a, _), (b, _)| a.cmp(b)); + + for (p, ver) in sorted { + let version_str = ver.iter().copied().collect::>().join(" "); + println!( + "{} {} {} {}", + "[A:]".green().bold(), + p, + "@".yellow().bold(), + version_str.blue() + ); } } + fn print_removed(set: HashSet<&str>, pre: &HashMap<&str, HashSet<&str>>) { println!("{}", "Packages removed:".underline().bold()); - for p in set { - let pre = pre.get(p); - if let Some(ver) = pre { + set.iter() + .filter_map(|p| pre.get(p).map(|ver| (*p, ver))) + .for_each(|(p, ver)| { let version_str = ver.iter().copied().collect::>().join(" "); println!( "{} {} {} {}", @@ -271,35 +325,36 @@ fn print_removed(set: HashSet<&str>, pre: &HashMap<&str, HashSet<&str>>) { "@".yellow(), version_str.blue() ); - } - } + }); } + fn print_changes( set: HashSet<&str>, pre: &HashMap<&str, HashSet<&str>>, post: &HashMap<&str, HashSet<&str>>, ) { println!("{}", "Version changes:".underline().bold()); - for p in set { - if p.is_empty() { - continue; - } - - // We should handle the case where the package might not exist in one of the maps - if let (Some(ver_pre), Some(ver_post)) = (pre.get(p), post.get(p)) { + set.iter() + .filter(|p| !p.is_empty()) + .filter_map(|p| { + match (pre.get(p), post.get(p)) { + (Some(ver_pre), Some(ver_post)) if ver_pre != ver_post => { + Some((p, ver_pre, ver_post)) + } + _ => None, + } + }) + .for_each(|(p, ver_pre, ver_post)| { let version_str_pre = ver_pre.iter().copied().collect::>().join(" "); let version_str_post = ver_post.iter().copied().collect::>().join(", "); - if ver_pre != ver_post { - println!( - "{} {} {} {} ~> {}", - "[C:]".bold().bright_yellow(), - p, - "@".yellow(), - version_str_pre.magenta(), - version_str_post.blue() - ); - } - } - } + println!( + "{} {} {} {} ~> {}", + "[C:]".bold().bright_yellow(), + p, + "@".yellow(), + version_str_pre.magenta(), + version_str_post.blue() + ); + }); }