From 304a7e1a1adfa4909b5acc6e26bf49d55b8c16d9 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Fri, 30 Jan 2026 18:02:09 +0300 Subject: [PATCH] eh: remove unused tracing dep Signed-off-by: NotAShelf Change-Id: Idd2818fa3d590b192c1bdecefb25da066a6a6964 --- Cargo.lock | 116 --------- Cargo.toml | 7 +- eh/Cargo.toml | 2 - eh/src/main.rs | 69 +++--- eh/src/util.rs | 634 ++++++++++++++++++++++++++++++++++++++----------- 5 files changed, 535 insertions(+), 293 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd32f1d..d68884d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -84,8 +84,6 @@ dependencies = [ "regex", "tempfile", "thiserror", - "tracing", - "tracing-subscriber", "walkdir", "yansi", ] @@ -124,12 +122,6 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" -[[package]] -name = "lazy_static" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" - [[package]] name = "libc" version = "0.2.177" @@ -142,39 +134,18 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df1d3c3b53da64cf5760482273a98e575c651a67eec7f77df96b5b642de8f039" -[[package]] -name = "log" -version = "0.4.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13dc2df351e3202783a1fe0d44375f7295ffb4049267b0f3018346dc122a1d94" - [[package]] name = "memchr" version = "2.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a282da65faaf38286cf3be983213fcf1d2e2a58700e808f83f4ea9a4804bc0" -[[package]] -name = "nu-ansi-term" -version = "0.50.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" -dependencies = [ - "windows-sys", -] - [[package]] name = "once_cell" version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" -[[package]] -name = "pin-project-lite" -version = "0.2.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" - [[package]] name = "proc-macro2" version = "1.0.95" @@ -250,21 +221,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "sharded-slab" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" -dependencies = [ - "lazy_static", -] - -[[package]] -name = "smallvec" -version = "1.15.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" - [[package]] name = "syn" version = "2.0.104" @@ -309,84 +265,12 @@ dependencies = [ "syn", ] -[[package]] -name = "thread_local" -version = "1.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f60246a4944f24f6e018aa17cdeffb7818b76356965d03b07d6a9886e8962185" -dependencies = [ - "cfg-if", -] - -[[package]] -name = "tracing" -version = "0.1.41" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "784e0ac535deb450455cbfa28a6f0df145ea1bb7ae51b821cf5e7927fdcfbdd0" -dependencies = [ - "pin-project-lite", - "tracing-attributes", - "tracing-core", -] - -[[package]] -name = "tracing-attributes" -version = "0.1.30" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81383ab64e72a7a8b8e13130c49e3dab29def6d0c7d76a03087b3cf71c5c6903" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - -[[package]] -name = "tracing-core" -version = "0.1.34" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9d12581f227e93f094d3af2ae690a574abb8a2b9b7a96e7cfe9647b2b617678" -dependencies = [ - "once_cell", - "valuable", -] - -[[package]] -name = "tracing-log" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee855f1f400bd0e5c02d150ae5de3840039a3f54b025156404e34c23c03f47c3" -dependencies = [ - "log", - "once_cell", - "tracing-core", -] - -[[package]] -name = "tracing-subscriber" -version = "0.3.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2054a14f5307d601f88daf0553e1cbf472acc4f2c51afab632431cdcd72124d5" -dependencies = [ - "nu-ansi-term", - "sharded-slab", - "smallvec", - "thread_local", - "tracing-core", - "tracing-log", -] - [[package]] name = "unicode-ident" version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5a5f39404a5da50712a4c1eecf25e90dd62b613502b7e925fd4e4d19b5c96512" -[[package]] -name = "valuable" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" - [[package]] name = "walkdir" version = "2.5.0" diff --git a/Cargo.toml b/Cargo.toml index 9cc10d5..59825fb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,6 @@ [workspace] -members = [ "eh", "xtask" ] +members = [ "eh", "crates/*" ] +default-members = ["eh"] resolver = "3" [workspace.package] @@ -17,11 +18,11 @@ clap_complete = "4.5.60" regex = "1.12.2" tempfile = "3.23.0" thiserror = "2.0.17" -tracing = "0.1.41" -tracing-subscriber = "0.3.20" walkdir = "2.5.0" yansi = "1.0.1" +eh = { path = "./eh" } + [profile.release] codegen-units = 1 lto = true diff --git a/eh/Cargo.toml b/eh/Cargo.toml index 1e62b9b..f194a62 100644 --- a/eh/Cargo.toml +++ b/eh/Cargo.toml @@ -15,7 +15,5 @@ clap.workspace = true regex.workspace = true tempfile.workspace = true thiserror.workspace = true -tracing.workspace = true -tracing-subscriber.workspace = true walkdir.workspace = true yansi.workspace = true diff --git a/eh/src/main.rs b/eh/src/main.rs index 370049f..e090375 100644 --- a/eh/src/main.rs +++ b/eh/src/main.rs @@ -11,20 +11,15 @@ mod shell; mod util; fn main() { - let format = tracing_subscriber::fmt::format() - .with_level(true) // don't include levels in formatted output - .with_target(true) // don't include targets - .with_thread_ids(false) // include the thread ID of the current thread - .with_thread_names(false) // include the name of the current thread - .compact(); // use the `Compact` formatting style. - tracing_subscriber::fmt().event_format(format).init(); - let result = run_app(); match result { Ok(code) => std::process::exit(code), Err(e) => { eprintln!("Error: {e}"); + if let Some(hint) = e.hint() { + eprintln!("Hint: {hint}"); + } std::process::exit(e.exit_code()); }, } @@ -37,42 +32,42 @@ fn dispatch_multicall( ) -> Option> { let rest: Vec = args.collect(); - // Validate arguments before processing - if let Err(e) = util::validate_nix_args(&rest) { - return Some(Err(e)); + let subcommand = match app_name { + "nr" => "run", + "ns" => "shell", + "nb" => "build", + _ => return None, + }; + + // Handle --help/-h/--version before forwarding to nix + if rest.iter().any(|a| a == "--help" || a == "-h") { + eprintln!("{app_name}: shorthand for 'eh {subcommand}'"); + eprintln!("Usage: {app_name} [args...]"); + eprintln!("All arguments are forwarded to 'nix {subcommand}'."); + return Some(Ok(0)); + } + + if rest.iter().any(|a| a == "--version") { + eprintln!("{app_name} (eh {})", env!("CARGO_PKG_VERSION")); + return Some(Ok(0)); } let hash_extractor = util::RegexHashExtractor; let fixer = util::DefaultNixFileFixer; let classifier = util::DefaultNixErrorClassifier; - match app_name { - "nr" => { - Some(run::handle_nix_run( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) + Some(match subcommand { + "run" => run::handle_nix_run(&rest, &hash_extractor, &fixer, &classifier), + "shell" => { + shell::handle_nix_shell(&rest, &hash_extractor, &fixer, &classifier) }, - "ns" => { - Some(shell::handle_nix_shell( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) + "build" => { + build::handle_nix_build(&rest, &hash_extractor, &fixer, &classifier) }, - "nb" => { - Some(build::handle_nix_build( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - _ => None, - } + // subcommand is assigned from the match on app_name above; + // only "run"/"shell"/"build" are possible values. + _ => unreachable!(), + }) } fn run_app() -> Result { @@ -107,7 +102,7 @@ fn run_app() -> Result { build::handle_nix_build(&args, &hash_extractor, &fixer, &classifier) }, - _ => { + None => { Cli::command().print_help()?; println!(); Ok(0) diff --git a/eh/src/util.rs b/eh/src/util.rs index f452296..78671a9 100644 --- a/eh/src/util.rs +++ b/eh/src/util.rs @@ -1,11 +1,11 @@ use std::{ io::{BufWriter, Write}, path::{Path, PathBuf}, + sync::LazyLock, }; use regex::Regex; use tempfile::NamedTempFile; -use tracing::{info, warn}; use walkdir::WalkDir; use yansi::Paint; @@ -14,22 +14,41 @@ use crate::{ error::{EhError, Result}, }; +/// Compiled regex patterns for extracting the actual hash from nix stderr. +static HASH_EXTRACT_PATTERNS: LazyLock<[Regex; 3]> = LazyLock::new(|| { + [ + Regex::new(r"got:\s+(sha256-[a-zA-Z0-9+/=]+)").unwrap(), + Regex::new(r"actual:\s+(sha256-[a-zA-Z0-9+/=]+)").unwrap(), + Regex::new(r"have:\s+(sha256-[a-zA-Z0-9+/=]+)").unwrap(), + ] +}); + +/// Compiled regex pattern for extracting the old (specified) hash from nix +/// stderr. +static HASH_OLD_EXTRACT_PATTERN: LazyLock = LazyLock::new(|| { + Regex::new(r"specified:\s+(sha256-[a-zA-Z0-9+/=]+)").unwrap() +}); + +/// Compiled regex patterns for matching hash attributes in .nix files. +static HASH_FIX_PATTERNS: LazyLock<[Regex; 3]> = LazyLock::new(|| { + [ + Regex::new(r#"hash\s*=\s*"[^"]*""#).unwrap(), + Regex::new(r#"sha256\s*=\s*"[^"]*""#).unwrap(), + Regex::new(r#"outputHash\s*=\s*"[^"]*""#).unwrap(), + ] +}); + pub trait HashExtractor { fn extract_hash(&self, stderr: &str) -> Option; + fn extract_old_hash(&self, stderr: &str) -> Option; } pub struct RegexHashExtractor; impl HashExtractor for RegexHashExtractor { fn extract_hash(&self, stderr: &str) -> Option { - let patterns = [ - r"got:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"actual:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"have:\s+(sha256-[a-zA-Z0-9+/=]+)", - ]; - for pattern in &patterns { - if let Ok(re) = Regex::new(pattern) - && let Some(captures) = re.captures(stderr) + for re in HASH_EXTRACT_PATTERNS.iter() { + if let Some(captures) = re.captures(stderr) && let Some(hash) = captures.get(1) { return Some(hash.as_str().to_string()); @@ -37,22 +56,42 @@ impl HashExtractor for RegexHashExtractor { } None } + + fn extract_old_hash(&self, stderr: &str) -> Option { + HASH_OLD_EXTRACT_PATTERN + .captures(stderr) + .and_then(|c| c.get(1)) + .map(|m| m.as_str().to_string()) + } } pub trait NixFileFixer { - fn fix_hash_in_files(&self, new_hash: &str) -> Result; + fn fix_hash_in_files( + &self, + old_hash: Option<&str>, + new_hash: &str, + ) -> Result; fn find_nix_files(&self) -> Result>; - fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result; + fn fix_hash_in_file( + &self, + file_path: &Path, + old_hash: Option<&str>, + new_hash: &str, + ) -> Result; } pub struct DefaultNixFileFixer; impl NixFileFixer for DefaultNixFileFixer { - fn fix_hash_in_files(&self, new_hash: &str) -> Result { + fn fix_hash_in_files( + &self, + old_hash: Option<&str>, + new_hash: &str, + ) -> Result { let nix_files = self.find_nix_files()?; let mut fixed = false; for file_path in nix_files { - if self.fix_hash_in_file(&file_path, new_hash)? { + if self.fix_hash_in_file(&file_path, old_hash, new_hash)? { println!("Updated hash in {}", file_path.display()); fixed = true; } @@ -61,8 +100,20 @@ impl NixFileFixer for DefaultNixFileFixer { } fn find_nix_files(&self) -> Result> { + let should_skip = |entry: &walkdir::DirEntry| -> bool { + // Never skip the root entry, otherwise the entire walk is empty + if entry.depth() == 0 || !entry.file_type().is_dir() { + return false; + } + let name = entry.file_name().to_string_lossy(); + name.starts_with('.') + || matches!(name.as_ref(), "node_modules" | "target" | "result") + }; + let files: Vec = WalkDir::new(".") + .max_depth(3) .into_iter() + .filter_entry(|e| !should_skip(e)) .filter_map(std::result::Result::ok) .filter(|entry| { entry.file_type().is_file() @@ -80,39 +131,59 @@ impl NixFileFixer for DefaultNixFileFixer { Ok(files) } - fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result { - // Pre-compile regex patterns once to avoid repeated compilation - let patterns: Vec<(Regex, String)> = [ - (r#"hash\s*=\s*"[^"]*""#, format!(r#"hash = "{new_hash}""#)), - ( - r#"sha256\s*=\s*"[^"]*""#, - format!(r#"sha256 = "{new_hash}""#), - ), - ( - r#"outputHash\s*=\s*"[^"]*""#, - format!(r#"outputHash = "{new_hash}""#), - ), - ] - .into_iter() - .map(|(pattern, replacement)| { - Regex::new(pattern) - .map(|re| (re, replacement)) - .map_err(EhError::Regex) - }) - .collect::>>()?; - + fn fix_hash_in_file( + &self, + file_path: &Path, + old_hash: Option<&str>, + new_hash: &str, + ) -> Result { // Read the entire file content let content = std::fs::read_to_string(file_path)?; let mut replaced = false; let mut result_content = content; - // Apply replacements - for (re, replacement) in &patterns { - if re.is_match(&result_content) { - result_content = re - .replace_all(&result_content, replacement.as_str()) - .into_owned(); - replaced = true; + if let Some(old) = old_hash { + // Targeted replacement: only replace attributes whose value matches the + // old hash. Uses regexes to handle variable whitespace around `=`. + let old_escaped = regex::escape(old); + let targeted_patterns = [ + ( + Regex::new(&format!(r#"hash\s*=\s*"{old_escaped}""#)).unwrap(), + format!(r#"hash = "{new_hash}""#), + ), + ( + Regex::new(&format!(r#"sha256\s*=\s*"{old_escaped}""#)).unwrap(), + format!(r#"sha256 = "{new_hash}""#), + ), + ( + Regex::new(&format!(r#"outputHash\s*=\s*"{old_escaped}""#)).unwrap(), + format!(r#"outputHash = "{new_hash}""#), + ), + ]; + + for (re, replacement) in &targeted_patterns { + if re.is_match(&result_content) { + result_content = re + .replace_all(&result_content, replacement.as_str()) + .into_owned(); + replaced = true; + } + } + } else { + // Fallback: replace all hash attributes (original behavior) + let replacements = [ + format!(r#"hash = "{new_hash}""#), + format!(r#"sha256 = "{new_hash}""#), + format!(r#"outputHash = "{new_hash}""#), + ]; + + for (re, replacement) in HASH_FIX_PATTERNS.iter().zip(&replacements) { + if re.is_match(&result_content) { + result_content = re + .replace_all(&result_content, replacement.as_str()) + .into_owned(); + replaced = true; + } } } @@ -140,38 +211,119 @@ pub trait NixErrorClassifier { fn should_retry(&self, stderr: &str) -> bool; } -/// Pre-evaluate expression to catch errors early -fn pre_evaluate(_subcommand: &str, args: &[String]) -> Result { +/// Classifies what retry action should be taken based on nix stderr output. +#[derive(Debug, PartialEq, Eq)] +pub enum RetryAction { + AllowUnfree, + AllowInsecure, + AllowBroken, + None, +} + +impl RetryAction { + /// Returns `(env_var, reason)` for this retry action, + /// or `None` if no retry is needed. + fn env_override(&self) -> Option<(&str, &str)> { + match self { + Self::AllowUnfree => { + Some(("NIXPKGS_ALLOW_UNFREE", "has an unfree license")) + }, + Self::AllowInsecure => { + Some(("NIXPKGS_ALLOW_INSECURE", "has been marked as insecure")) + }, + Self::AllowBroken => { + Some(("NIXPKGS_ALLOW_BROKEN", "has been marked as broken")) + }, + Self::None => None, + } + } +} + +/// Extract the package/expression name from args (first non-flag argument). +fn package_name(args: &[String]) -> &str { + args + .iter() + .find(|a| !a.starts_with('-')) + .map(String::as_str) + .unwrap_or("") +} + +/// Print a retry message with consistent formatting. +/// Format: ` -> : , setting =1` +fn print_retry_msg(pkg: &str, reason: &str, env_var: &str) { + eprintln!( + " {} {}: {}, setting {}", + "->".yellow().bold(), + pkg.bold(), + reason, + format!("{env_var}=1").bold(), + ); +} + +/// Classify stderr into a retry action. +pub fn classify_retry_action(stderr: &str) -> RetryAction { + if stderr.contains("has an unfree license") && stderr.contains("refusing") { + RetryAction::AllowUnfree + } else if stderr.contains("has been marked as insecure") + && stderr.contains("refusing") + { + RetryAction::AllowInsecure + } else if stderr.contains("has been marked as broken") + && stderr.contains("refusing") + { + RetryAction::AllowBroken + } else { + RetryAction::None + } +} + +/// Returns true if stderr looks like a genuine hash mismatch error +/// (not just any mention of "hash" or "sha256"). +fn is_hash_mismatch_error(stderr: &str) -> bool { + stderr.contains("hash mismatch") + || (stderr.contains("specified:") && stderr.contains("got:")) +} + +/// Pre-evaluate expression to catch errors early. +/// +/// Returns a `RetryAction` if the evaluation fails with a retryable error +/// (unfree/insecure/broken), allowing the caller to retry with the right +/// environment variables without ever streaming the verbose nix error output. +fn pre_evaluate(args: &[String]) -> Result { // Find flake references or expressions to evaluate // Only take the first non-flag argument (the package/expression) let eval_arg = args.iter().find(|arg| !arg.starts_with('-')); let Some(eval_arg) = eval_arg else { - return Ok(true); // No expression to evaluate + return Ok(RetryAction::None); // No expression to evaluate }; - let eval_cmd = NixCommand::new("eval").arg(eval_arg).arg("--raw"); + let eval_cmd = NixCommand::new("eval") + .arg(eval_arg) + .print_build_logs(false); let output = eval_cmd.output()?; if output.status.success() { - return Ok(true); + return Ok(RetryAction::None); } let stderr = String::from_utf8_lossy(&output.stderr); - // If eval fails due to unfree/insecure/broken, don't fail pre-evaluation - // Let the main command handle it with retry logic - if stderr.contains("has an unfree license") - || stderr.contains("refusing to evaluate") - || stderr.contains("has been marked as insecure") - || stderr.contains("has been marked as broken") - { - return Ok(true); + // Classify whether this is a retryable error (unfree/insecure/broken) + let action = classify_retry_action(&stderr); + if action != RetryAction::None { + return Ok(action); } - // For other eval failures, fail early - Ok(false) + // For other eval failures, warn but let the actual command handle the + // error with full streaming output rather than halting here. + let err = EhError::PreEvalFailed { + expression: eval_arg.clone(), + stderr: stderr.trim().to_string(), + }; + eprintln!(" {} {}", "->".yellow().bold(), err,); + Ok(RetryAction::None) } pub fn validate_nix_args(args: &[String]) -> Result<()> { @@ -202,15 +354,28 @@ pub fn handle_nix_with_retry( interactive: bool, ) -> Result { validate_nix_args(args)?; - // Pre-evaluate for build commands to catch errors early - if !pre_evaluate(subcommand, args)? { - return Err(EhError::NixCommandFailed( - "Expression evaluation failed".to_string(), - )); + + // Pre-evaluate to detect retryable errors (unfree/insecure/broken) before + // running the actual command. This avoids streaming verbose nix error output + // only to retry immediately after. + let pkg = package_name(args); + let pre_eval_action = pre_evaluate(args)?; + if let Some((env_var, reason)) = pre_eval_action.env_override() { + print_retry_msg(pkg, reason, env_var); + let mut retry_cmd = NixCommand::new(subcommand) + .print_build_logs(true) + .args_ref(args) + .env(env_var, "1") + .impure(true); + if interactive { + retry_cmd = retry_cmd.interactive(true); + } + let retry_status = retry_cmd.run_with_logs(StdIoInterceptor)?; + return Ok(retry_status.code().unwrap_or(1)); } - // For run commands, try interactive first to avoid breaking terminal - if subcommand == "run" && interactive { + // For run/shell commands, try interactive mode now that pre-eval passed + if interactive { let status = NixCommand::new(subcommand) .print_build_logs(true) .interactive(true) @@ -221,18 +386,23 @@ pub fn handle_nix_with_retry( } } - // First, always capture output to check for errors that need retry + // Capture output to check for errors that need retry (hash mismatches etc.) let output_cmd = NixCommand::new(subcommand) .print_build_logs(true) .args_ref(args); let output = output_cmd.output()?; let stderr = String::from_utf8_lossy(&output.stderr); - // Check if we need to retry with special flags + // Check for hash mismatch errors if let Some(new_hash) = hash_extractor.extract_hash(&stderr) { - match fixer.fix_hash_in_files(&new_hash) { + let old_hash = hash_extractor.extract_old_hash(&stderr); + match fixer.fix_hash_in_files(old_hash.as_deref(), &new_hash) { Ok(true) => { - info!("{}", Paint::green("✔ Fixed hash mismatch, retrying...")); + eprintln!( + " {} {}: hash mismatch corrected in local files, rebuilding", + "->".green().bold(), + pkg.bold(), + ); let mut retry_cmd = NixCommand::new(subcommand) .print_build_logs(true) .args_ref(args); @@ -246,72 +416,34 @@ pub fn handle_nix_with_retry( // No files were fixed, continue with normal error handling }, Err(EhError::NoNixFilesFound) => { - warn!("No .nix files found to fix hash in"); + eprintln!( + " {} {}: hash mismatch detected but no .nix files found to update", + "->".yellow().bold(), + pkg.bold(), + ); // Continue with normal error handling }, Err(e) => { return Err(e); }, } - } else if stderr.contains("hash") || stderr.contains("sha256") { - // If there's a hash-related error but we couldn't extract it, that's a - // failure - return Err(EhError::HashExtractionFailed); + } else if is_hash_mismatch_error(&stderr) { + // There's a genuine hash mismatch but we couldn't extract the new hash + return Err(EhError::HashExtractionFailed { + stderr: stderr.to_string(), + }); } + // Fallback: check for unfree/insecure/broken in captured output + // (in case pre_evaluate didn't catch it, e.g. from a dependency) if classifier.should_retry(&stderr) { - if stderr.contains("has an unfree license") && stderr.contains("refusing") { - warn!( - "{}", - Paint::yellow( - "⚠ Unfree package detected, retrying with NIXPKGS_ALLOW_UNFREE=1..." - ) - ); + let action = classify_retry_action(&stderr); + if let Some((env_var, reason)) = action.env_override() { + print_retry_msg(pkg, reason, env_var); let mut retry_cmd = NixCommand::new(subcommand) .print_build_logs(true) .args_ref(args) - .env("NIXPKGS_ALLOW_UNFREE", "1") - .impure(true); - if interactive { - retry_cmd = retry_cmd.interactive(true); - } - let retry_status = retry_cmd.run_with_logs(StdIoInterceptor)?; - return Ok(retry_status.code().unwrap_or(1)); - } - if stderr.contains("has been marked as insecure") - && stderr.contains("refusing") - { - warn!( - "{}", - Paint::yellow( - "⚠ Insecure package detected, retrying with \ - NIXPKGS_ALLOW_INSECURE=1..." - ) - ); - let mut retry_cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args_ref(args) - .env("NIXPKGS_ALLOW_INSECURE", "1") - .impure(true); - if interactive { - retry_cmd = retry_cmd.interactive(true); - } - let retry_status = retry_cmd.run_with_logs(StdIoInterceptor)?; - return Ok(retry_status.code().unwrap_or(1)); - } - if stderr.contains("has been marked as broken") - && stderr.contains("refusing") - { - warn!( - "{}", - Paint::yellow( - "⚠ Broken package detected, retrying with NIXPKGS_ALLOW_BROKEN=1..." - ) - ); - let mut retry_cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args_ref(args) - .env("NIXPKGS_ALLOW_BROKEN", "1") + .env(env_var, "1") .impure(true); if interactive { retry_cmd = retry_cmd.interactive(true); @@ -330,22 +462,23 @@ pub fn handle_nix_with_retry( std::io::stderr() .write_all(&output.stderr) .map_err(EhError::Io)?; - Err(EhError::ProcessExit { - code: output.status.code().unwrap_or(1), - }) + + match output.status.code() { + Some(code) => Err(EhError::ProcessExit { code }), + // No exit code means the process was killed by a signal + None => { + Err(EhError::NixCommandFailed { + command: subcommand.to_string(), + }) + }, + } } pub struct DefaultNixErrorClassifier; impl NixErrorClassifier for DefaultNixErrorClassifier { fn should_retry(&self, stderr: &str) -> bool { - RegexHashExtractor.extract_hash(stderr).is_some() - || (stderr.contains("has an unfree license") - && stderr.contains("refusing")) - || (stderr.contains("has been marked as insecure") - && stderr.contains("refusing")) - || (stderr.contains("has been marked as broken") - && stderr.contains("refusing")) + classify_retry_action(stderr) != RetryAction::None } } @@ -379,7 +512,7 @@ mod tests { let fixer = DefaultNixFileFixer; let result = fixer - .fix_hash_in_file(file_path, "sha256-newhash999") + .fix_hash_in_file(file_path, None, "sha256-newhash999") .unwrap(); assert!(result, "Hash replacement should return true"); @@ -413,7 +546,7 @@ mod tests { // Test hash replacement let fixer = DefaultNixFileFixer; let result = fixer - .fix_hash_in_file(&file_path, "sha256-newhash999") + .fix_hash_in_file(&file_path, None, "sha256-newhash999") .unwrap(); assert!( @@ -448,7 +581,7 @@ mod tests { // Test that streaming can handle large files without memory issues let fixer = DefaultNixFileFixer; let result = fixer - .fix_hash_in_file(file_path, "sha256-newhash999") + .fix_hash_in_file(file_path, None, "sha256-newhash999") .unwrap(); assert!(result, "Hash replacement should work for large files"); @@ -483,7 +616,9 @@ mod tests { // Test hash replacement let fixer = DefaultNixFileFixer; - let result = fixer.fix_hash_in_file(file_path, "sha256-newhash").unwrap(); + let result = fixer + .fix_hash_in_file(file_path, None, "sha256-newhash") + .unwrap(); assert!(result, "Hash replacement should succeed"); @@ -538,4 +673,233 @@ mod tests { safe_args ); } + + #[test] + fn test_input_validation_empty_args() { + let result = validate_nix_args(&[]); + assert!(result.is_ok(), "Empty args should be accepted"); + } + + #[test] + fn test_hash_extraction_got_pattern() { + let stderr = "hash mismatch in fixed-output derivation\n specified: \ + sha256-AAAA\n got: \ + sha256-BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB="; + let extractor = RegexHashExtractor; + let hash = extractor.extract_hash(stderr); + assert!(hash.is_some()); + assert!(hash.unwrap().starts_with("sha256-")); + } + + #[test] + fn test_hash_extraction_actual_pattern() { + let stderr = "hash mismatch\n actual: \ + sha256-CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC="; + let extractor = RegexHashExtractor; + let hash = extractor.extract_hash(stderr); + assert!(hash.is_some()); + assert!(hash.unwrap().starts_with("sha256-")); + } + + #[test] + fn test_hash_extraction_have_pattern() { + let stderr = "hash mismatch\n have: \ + sha256-DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD="; + let extractor = RegexHashExtractor; + let hash = extractor.extract_hash(stderr); + assert!(hash.is_some()); + assert!(hash.unwrap().starts_with("sha256-")); + } + + #[test] + fn test_hash_extraction_no_match() { + let stderr = "error: some other nix error without hashes"; + let extractor = RegexHashExtractor; + assert!(extractor.extract_hash(stderr).is_none()); + } + + #[test] + fn test_hash_extraction_partial_match() { + // Contains "got:" but no sha256 hash + let stderr = "got: some-other-value"; + let extractor = RegexHashExtractor; + assert!(extractor.extract_hash(stderr).is_none()); + } + + #[test] + fn test_false_positive_hash_detection() { + // Normal nix output mentioning "hash" or "sha256" without being a mismatch + let cases = [ + "evaluating attribute 'sha256' of derivation 'hello'", + "building '/nix/store/hash-something.drv'", + "copying path '/nix/store/sha256-abcdef-hello'", + "this derivation has a hash attribute set", + ]; + for stderr in &cases { + assert!( + !is_hash_mismatch_error(stderr), + "Should not detect hash mismatch in: {stderr}" + ); + } + } + + #[test] + fn test_genuine_hash_mismatch_detection() { + assert!(is_hash_mismatch_error( + "hash mismatch in fixed-output derivation" + )); + assert!(is_hash_mismatch_error( + "specified: sha256-AAAA\n got: sha256-BBBB" + )); + } + + #[test] + fn test_classify_retry_action_unfree() { + let stderr = + "error: Package 'foo' has an unfree license, refusing to evaluate."; + assert_eq!(classify_retry_action(stderr), RetryAction::AllowUnfree); + } + + #[test] + fn test_classify_retry_action_insecure() { + let stderr = + "error: Package 'bar' has been marked as insecure, refusing to evaluate."; + assert_eq!(classify_retry_action(stderr), RetryAction::AllowInsecure); + } + + #[test] + fn test_classify_retry_action_broken() { + let stderr = + "error: Package 'baz' has been marked as broken, refusing to evaluate."; + assert_eq!(classify_retry_action(stderr), RetryAction::AllowBroken); + } + + #[test] + fn test_classify_retry_action_none() { + let stderr = "error: attribute 'nonexistent' not found"; + assert_eq!(classify_retry_action(stderr), RetryAction::None); + } + + #[test] + fn test_retry_action_env_overrides() { + let (var, reason) = RetryAction::AllowUnfree.env_override().unwrap(); + assert_eq!(var, "NIXPKGS_ALLOW_UNFREE"); + assert!(reason.contains("unfree")); + + let (var, reason) = RetryAction::AllowInsecure.env_override().unwrap(); + assert_eq!(var, "NIXPKGS_ALLOW_INSECURE"); + assert!(reason.contains("insecure")); + + let (var, reason) = RetryAction::AllowBroken.env_override().unwrap(); + assert_eq!(var, "NIXPKGS_ALLOW_BROKEN"); + assert!(reason.contains("broken")); + + assert_eq!(RetryAction::None.env_override(), None); + } + + #[test] + fn test_classifier_should_retry() { + let classifier = DefaultNixErrorClassifier; + assert!( + classifier.should_retry( + "Package 'x' has an unfree license, refusing to evaluate" + ) + ); + assert!(classifier.should_retry( + "Package 'x' has been marked as insecure, refusing to evaluate" + )); + assert!(classifier.should_retry( + "Package 'x' has been marked as broken, refusing to evaluate" + )); + assert!(!classifier.should_retry("error: attribute not found")); + } + + #[test] + fn test_old_hash_extraction() { + let stderr = + "hash mismatch in fixed-output derivation\n specified: \ + sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\n got: \ + sha256-BBBB="; + let extractor = RegexHashExtractor; + let old = extractor.extract_old_hash(stderr); + assert!(old.is_some()); + assert_eq!( + old.unwrap(), + "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" + ); + } + + #[test] + fn test_old_hash_extraction_missing() { + let stderr = "hash mismatch\n got: sha256-BBBB="; + let extractor = RegexHashExtractor; + assert!(extractor.extract_old_hash(stderr).is_none()); + } + + #[test] + fn test_targeted_hash_replacement_only_matching() { + let temp_file = NamedTempFile::new().unwrap(); + let file_path = temp_file.path(); + + // File with two derivations, each with a different hash + let test_content = r#"{ pkgs }: +{ + a = pkgs.fetchurl { + url = "https://example.com/a.tar.gz"; + hash = "sha256-AAAA"; + }; + b = pkgs.fetchurl { + url = "https://example.com/b.tar.gz"; + hash = "sha256-BBBB"; + }; +}"#; + + let mut file = std::fs::File::create(file_path).unwrap(); + file.write_all(test_content.as_bytes()).unwrap(); + file.flush().unwrap(); + + let fixer = DefaultNixFileFixer; + // Only replace the hash matching "sha256-AAAA" + let result = fixer + .fix_hash_in_file(file_path, Some("sha256-AAAA"), "sha256-NEWW") + .unwrap(); + + assert!(result, "Targeted replacement should return true"); + + let updated = std::fs::read_to_string(file_path).unwrap(); + assert!( + updated.contains(r#"hash = "sha256-NEWW""#), + "Matching hash should be replaced" + ); + assert!( + updated.contains(r#"hash = "sha256-BBBB""#), + "Non-matching hash should be untouched" + ); + } + + #[test] + fn test_targeted_hash_replacement_no_match() { + let temp_file = NamedTempFile::new().unwrap(); + let file_path = temp_file.path(); + + let test_content = r#"{ hash = "sha256-XXXX"; }"#; + + let mut file = std::fs::File::create(file_path).unwrap(); + file.write_all(test_content.as_bytes()).unwrap(); + file.flush().unwrap(); + + let fixer = DefaultNixFileFixer; + // old_hash doesn't match anything in the file + let result = fixer + .fix_hash_in_file(file_path, Some("sha256-NOMATCH"), "sha256-NEWW") + .unwrap(); + + assert!(!result, "Should return false when old hash doesn't match"); + + let updated = std::fs::read_to_string(file_path).unwrap(); + assert!( + updated.contains("sha256-XXXX"), + "Original hash should be untouched" + ); + } }