diff --git a/Cargo.lock b/Cargo.lock index 09dd3fa..77f50a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -101,7 +101,7 @@ dependencies = [ [[package]] name = "eh" -version = "0.1.4" +version = "0.1.5" dependencies = [ "clap", "dialoguer", @@ -116,7 +116,7 @@ dependencies = [ [[package]] name = "eh-log" -version = "0.1.4" +version = "0.1.5" dependencies = [ "yansi", ] @@ -421,7 +421,7 @@ checksum = "d7249219f66ced02969388cf2bb044a09756a083d0fab1e566056b04d9fbcaa5" [[package]] name = "xtask" -version = "0.1.4" +version = "0.1.5" dependencies = [ "clap", "clap_complete", diff --git a/Cargo.toml b/Cargo.toml index a0f139a..f627886 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,8 +9,8 @@ description = "Ergonomic Nix CLI helper" edition = "2024" license = "MPL-2.0" readme = true -rust-version = "1.90" -version = "0.1.4" +rust-version = "1.91.0" +version = "0.1.5" [workspace.dependencies] clap = { default-features = false, features = [ "std", "help", "derive" ], version = "4.5.56" } diff --git a/eh/src/build.rs b/eh/src/build.rs deleted file mode 100644 index 4ec4540..0000000 --- a/eh/src/build.rs +++ /dev/null @@ -1,18 +0,0 @@ -use crate::{ - error::Result, - util::{ - HashExtractor, - NixErrorClassifier, - NixFileFixer, - handle_nix_with_retry, - }, -}; - -pub fn handle_nix_build( - args: &[String], - hash_extractor: &dyn HashExtractor, - fixer: &dyn NixFileFixer, - classifier: &dyn NixErrorClassifier, -) -> Result { - handle_nix_with_retry("build", args, hash_extractor, fixer, classifier, false) -} diff --git a/eh/src/command.rs b/eh/src/commands/mod.rs similarity index 65% rename from eh/src/command.rs rename to eh/src/commands/mod.rs index a6ff260..6cc417d 100644 --- a/eh/src/command.rs +++ b/eh/src/commands/mod.rs @@ -6,15 +6,26 @@ use std::{ time::{Duration, Instant}, }; -use crate::error::{EhError, Result}; +use crate::{ + error::{EhError, Result}, + util::{ + HashExtractor, + NixErrorClassifier, + NixFileFixer, + handle_nix_with_retry, + }, +}; + +pub mod update; + +const DEFAULT_BUFFER_SIZE: usize = 4096; +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(300); -/// Trait for log interception and output handling. pub trait LogInterceptor: Send { fn on_stderr(&mut self, chunk: &[u8]); fn on_stdout(&mut self, chunk: &[u8]); } -/// Default log interceptor that just writes to stdio. pub struct StdIoInterceptor; impl LogInterceptor for StdIoInterceptor { @@ -26,19 +37,13 @@ impl LogInterceptor for StdIoInterceptor { } } -/// Default buffer size for reading command output -const DEFAULT_BUFFER_SIZE: usize = 4096; - -/// Default timeout for command execution -const DEFAULT_TIMEOUT: Duration = Duration::from_secs(300); // 5 minutes - +#[derive(Debug)] enum PipeEvent { Stdout(Vec), Stderr(Vec), Error(io::Error), } -/// Drain a pipe reader, sending chunks through the channel. fn read_pipe( mut reader: R, tx: mpsc::Sender, @@ -66,7 +71,6 @@ fn read_pipe( } } -/// Builder and executor for Nix commands. pub struct NixCommand { subcommand: String, args: Vec, @@ -126,8 +130,6 @@ impl NixCommand { self } - /// Build the underlying `std::process::Command` with all configured - /// arguments, environment variables, and flags. fn build_command(&self) -> Command { let mut cmd = Command::new("nix"); cmd.arg(&self.subcommand); @@ -147,10 +149,6 @@ impl NixCommand { cmd } - /// Run the command, streaming output to the provided interceptor. - /// - /// Stdout and stderr are read concurrently using background threads - /// so that neither pipe blocks the other. pub fn run_with_logs( &self, mut interceptor: I, @@ -212,7 +210,6 @@ impl NixCommand { return Err(EhError::Io(e)); }, Err(mpsc::RecvTimeoutError::Timeout) => {}, - // All senders dropped — both reader threads finished Err(mpsc::RecvTimeoutError::Disconnected) => break, } } @@ -224,7 +221,6 @@ impl NixCommand { Ok(status) } - /// Run the command and capture all output (with timeout). pub fn output(&self) -> Result { let mut cmd = self.build_command(); @@ -317,3 +313,156 @@ impl NixCommand { }) } } + +pub fn handle_nix_command( + command: &str, + args: &[String], + hash_extractor: &dyn HashExtractor, + fixer: &dyn NixFileFixer, + classifier: &dyn NixErrorClassifier, +) -> Result { + let intercept_env = matches!(command, "run" | "shell"); + handle_nix_with_retry( + command, + args, + hash_extractor, + fixer, + classifier, + intercept_env, + ) +} + +#[cfg(test)] +mod tests { + use std::io::{Cursor, Error}; + + use super::*; + + #[test] + fn test_read_pipe_stdout() { + let data = b"hello world"; + let cursor = Cursor::new(data); + let (tx, rx) = mpsc::channel(); + + let tx_clone = tx.clone(); + std::thread::spawn(move || { + read_pipe(cursor, tx_clone, false); + }); + + drop(tx); + + let events: Vec = rx.iter().take(10).collect(); + assert!(!events.is_empty()); + + let stdout_events: Vec<_> = events + .iter() + .filter(|e| matches!(e, PipeEvent::Stdout(_))) + .collect(); + assert!(!stdout_events.is_empty()); + + let combined: Vec = events + .iter() + .filter_map(|e| { + match e { + PipeEvent::Stdout(b) => Some(b.clone()), + _ => None, + } + }) + .flatten() + .collect(); + assert_eq!(combined, data); + } + + #[test] + fn test_read_pipe_stderr() { + let data = b"error output"; + let cursor = Cursor::new(data); + let (tx, rx) = mpsc::channel(); + + let tx_clone = tx.clone(); + std::thread::spawn(move || { + read_pipe(cursor, tx_clone, true); + }); + + drop(tx); + + let events: Vec = rx.iter().take(10).collect(); + + let stderr_events: Vec<_> = events + .iter() + .filter(|e| matches!(e, PipeEvent::Stderr(_))) + .collect(); + assert!(!stderr_events.is_empty()); + + let combined: Vec = events + .iter() + .filter_map(|e| { + match e { + PipeEvent::Stderr(b) => Some(b.clone()), + _ => None, + } + }) + .flatten() + .collect(); + assert_eq!(combined, data); + } + + #[test] + fn test_read_pipe_empty() { + let cursor = Cursor::new(b""); + let (tx, rx) = mpsc::channel(); + + let tx_clone = tx.clone(); + std::thread::spawn(move || { + read_pipe(cursor, tx_clone, false); + }); + + drop(tx); + + let events: Vec = rx.iter().take(10).collect(); + assert!(events.is_empty()); + } + + #[test] + fn test_read_pipe_error() { + struct ErrorReader; + impl Read for ErrorReader { + fn read(&mut self, _buf: &mut [u8]) -> std::io::Result { + Err(std::io::Error::other("test error")) + } + } + + let reader = ErrorReader; + let (tx, rx) = mpsc::channel(); + + let tx_clone = tx.clone(); + std::thread::spawn(move || { + read_pipe(reader, tx_clone, false); + }); + + drop(tx); + + let events: Vec = rx.iter().take(10).collect(); + + let error_events: Vec<_> = events + .iter() + .filter(|e| matches!(e, PipeEvent::Error(_))) + .collect(); + assert!(!error_events.is_empty()); + } + + #[test] + fn test_pipe_event_debug() { + let stdout_event = PipeEvent::Stdout(b"test".to_vec()); + let stderr_event = PipeEvent::Stderr(b"error".to_vec()); + let error_event = PipeEvent::Error(Error::other("test")); + + let debug_stdout = format!("{:?}", stdout_event); + let debug_stderr = format!("{:?}", stderr_event); + let debug_error = format!("{:?}", error_event); + + assert!(debug_stdout.contains("Stdout")); + assert!(debug_stderr.contains("Stderr")); + assert!(debug_error.contains("Error")); + } +} diff --git a/eh/src/update.rs b/eh/src/commands/update.rs similarity index 94% rename from eh/src/update.rs rename to eh/src/commands/update.rs index df4184e..4640b13 100644 --- a/eh/src/update.rs +++ b/eh/src/commands/update.rs @@ -1,14 +1,15 @@ use crate::{ - command::{NixCommand, StdIoInterceptor}, + commands::{NixCommand, StdIoInterceptor}, error::{EhError, Result}, }; /// Parse flake input names from `nix flake metadata --json` output. pub fn parse_flake_inputs(stdout: &str) -> Result> { - let value: serde_json::Value = - serde_json::from_str(stdout).map_err(|e| EhError::JsonParse { + let value: serde_json::Value = serde_json::from_str(stdout).map_err(|e| { + EhError::JsonParse { detail: e.to_string(), - })?; + } + })?; let inputs = value .get("locks") diff --git a/eh/src/error.rs b/eh/src/error.rs index 9ebc9b6..2846bb9 100644 --- a/eh/src/error.rs +++ b/eh/src/error.rs @@ -81,7 +81,7 @@ impl EhError { } #[must_use] - pub fn hint(&self) -> Option<&str> { + pub const fn hint(&self) -> Option<&str> { match self { Self::NixCommandFailed { .. } => { Some("run with --show-trace for more details") @@ -175,13 +175,7 @@ mod tests { 12 ); assert_eq!(EhError::ProcessExit { code: 42 }.exit_code(), 42); - assert_eq!( - EhError::JsonParse { - detail: "x".into(), - } - .exit_code(), - 13 - ); + assert_eq!(EhError::JsonParse { detail: "x".into() }.exit_code(), 13); assert_eq!(EhError::NoFlakeInputs.exit_code(), 14); assert_eq!(EhError::UpdateCancelled.exit_code(), 0); } @@ -249,13 +243,7 @@ mod tests { .hint() .is_some() ); - assert!( - EhError::JsonParse { - detail: "x".into(), - } - .hint() - .is_some() - ); + assert!(EhError::JsonParse { detail: "x".into() }.hint().is_some()); assert!(EhError::NoFlakeInputs.hint().is_some()); // Variants without hints assert!( diff --git a/eh/src/lib.rs b/eh/src/lib.rs index 23c5e78..f76e705 100644 --- a/eh/src/lib.rs +++ b/eh/src/lib.rs @@ -1,9 +1,5 @@ -pub mod build; -pub mod command; +pub mod commands; pub mod error; -pub mod run; -pub mod shell; -pub mod update; pub mod util; pub use clap::{CommandFactory, Parser, Subcommand}; diff --git a/eh/src/main.rs b/eh/src/main.rs index 8ce8b6b..890e474 100644 --- a/eh/src/main.rs +++ b/eh/src/main.rs @@ -4,12 +4,8 @@ use eh::{Cli, Command, CommandFactory, Parser}; use error::Result; use yansi::Paint; -mod build; -mod command; +mod commands; mod error; -mod run; -mod shell; -mod update; mod util; fn main() { @@ -66,7 +62,7 @@ fn dispatch_multicall( } if subcommand == "update" { - return Some(update::handle_update(&rest)); + return Some(commands::update::handle_update(&rest)); } let hash_extractor = util::RegexHashExtractor; @@ -74,12 +70,14 @@ fn dispatch_multicall( let classifier = util::DefaultNixErrorClassifier; Some(match subcommand { - "run" => run::handle_nix_run(&rest, &hash_extractor, &fixer, &classifier), - "shell" => { - shell::handle_nix_shell(&rest, &hash_extractor, &fixer, &classifier) - }, - "build" => { - build::handle_nix_build(&rest, &hash_extractor, &fixer, &classifier) + "run" | "shell" | "build" => { + commands::handle_nix_command( + subcommand, + &rest, + &hash_extractor, + &fixer, + &classifier, + ) }, // subcommand is assigned from the match on app_name above; // only "run"/"shell"/"build" are possible values. @@ -108,18 +106,36 @@ fn run_app() -> Result { match cli.command { Some(Command::Run { args }) => { - run::handle_nix_run(&args, &hash_extractor, &fixer, &classifier) + commands::handle_nix_command( + "run", + &args, + &hash_extractor, + &fixer, + &classifier, + ) }, Some(Command::Shell { args }) => { - shell::handle_nix_shell(&args, &hash_extractor, &fixer, &classifier) + commands::handle_nix_command( + "shell", + &args, + &hash_extractor, + &fixer, + &classifier, + ) }, Some(Command::Build { args }) => { - build::handle_nix_build(&args, &hash_extractor, &fixer, &classifier) + commands::handle_nix_command( + "build", + &args, + &hash_extractor, + &fixer, + &classifier, + ) }, - Some(Command::Update { args }) => update::handle_update(&args), + Some(Command::Update { args }) => commands::update::handle_update(&args), None => { Cli::command().print_help()?; diff --git a/eh/src/run.rs b/eh/src/run.rs deleted file mode 100644 index fff81a1..0000000 --- a/eh/src/run.rs +++ /dev/null @@ -1,18 +0,0 @@ -use crate::{ - error::Result, - util::{ - HashExtractor, - NixErrorClassifier, - NixFileFixer, - handle_nix_with_retry, - }, -}; - -pub fn handle_nix_run( - args: &[String], - hash_extractor: &dyn HashExtractor, - fixer: &dyn NixFileFixer, - classifier: &dyn NixErrorClassifier, -) -> Result { - handle_nix_with_retry("run", args, hash_extractor, fixer, classifier, true) -} diff --git a/eh/src/shell.rs b/eh/src/shell.rs deleted file mode 100644 index c0f4409..0000000 --- a/eh/src/shell.rs +++ /dev/null @@ -1,18 +0,0 @@ -use crate::{ - error::Result, - util::{ - HashExtractor, - NixErrorClassifier, - NixFileFixer, - handle_nix_with_retry, - }, -}; - -pub fn handle_nix_shell( - args: &[String], - hash_extractor: &dyn HashExtractor, - fixer: &dyn NixFileFixer, - classifier: &dyn NixErrorClassifier, -) -> Result { - handle_nix_with_retry("shell", args, hash_extractor, fixer, classifier, true) -} diff --git a/eh/src/util.rs b/eh/src/util.rs index 10227de..ee008b8 100644 --- a/eh/src/util.rs +++ b/eh/src/util.rs @@ -11,10 +11,13 @@ use walkdir::WalkDir; use yansi::Paint; use crate::{ - command::{NixCommand, StdIoInterceptor}, + commands::{NixCommand, StdIoInterceptor}, error::{EhError, Result}, }; +/// Maximum directory depth when searching for .nix files. +const MAX_DIR_DEPTH: usize = 3; + /// Compiled regex patterns for extracting the actual hash from nix stderr. static HASH_EXTRACT_PATTERNS: LazyLock<[Regex; 3]> = LazyLock::new(|| { [ @@ -39,11 +42,15 @@ static HASH_FIX_PATTERNS: LazyLock<[Regex; 3]> = LazyLock::new(|| { ] }); +/// Trait for extracting store paths and hashes from nix output. pub trait HashExtractor { + /// Extract the new store path/hash from nix output. fn extract_hash(&self, stderr: &str) -> Option; + /// Extract the old store path/hash from nix output (for hash updates). fn extract_old_hash(&self, stderr: &str) -> Option; } +/// Default implementation of [`HashExtractor`] using regex patterns. pub struct RegexHashExtractor; impl HashExtractor for RegexHashExtractor { @@ -66,13 +73,19 @@ impl HashExtractor for RegexHashExtractor { } } +/// Trait for fixing hash mismatches in nix files. pub trait NixFileFixer { + /// Attempt to fix hash in all nix files found in the current directory. + /// Returns `true` if at least one file was fixed. fn fix_hash_in_files( &self, old_hash: Option<&str>, new_hash: &str, ) -> Result; + /// Find all .nix files in the current directory (respects MAX_DIR_DEPTH). fn find_nix_files(&self) -> Result>; + /// Attempt to fix hash in a single file. + /// Returns `true` if the file was modified. fn fix_hash_in_file( &self, file_path: &Path, @@ -81,6 +94,7 @@ pub trait NixFileFixer { ) -> Result; } +/// Default implementation of [`NixFileFixer`] that walks the directory tree. pub struct DefaultNixFileFixer; impl NixFileFixer for DefaultNixFileFixer { @@ -112,7 +126,7 @@ impl NixFileFixer for DefaultNixFileFixer { }; let files: Vec = WalkDir::new(".") - .max_depth(3) + .max_depth(MAX_DIR_DEPTH) .into_iter() .filter_entry(|e| !should_skip(e)) .filter_map(std::result::Result::ok) @@ -144,7 +158,7 @@ impl NixFileFixer for DefaultNixFileFixer { let mut result_content = content; if let Some(old) = old_hash { - // Targeted replacement: only replace attributes whose value matches the + // 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 = [ @@ -171,7 +185,7 @@ impl NixFileFixer for DefaultNixFileFixer { } } } else { - // Fallback: replace all hash attributes (original behavior) + // Fallback: replace all hash attributes let replacements = [ format!(r#"hash = "{new_hash}""#), format!(r#"sha256 = "{new_hash}""#), @@ -208,23 +222,34 @@ impl NixFileFixer for DefaultNixFileFixer { } } +/// Trait for classifying nix errors and determining if a retry with modified +/// environment is appropriate. pub trait NixErrorClassifier { + /// Determine if the given stderr output should trigger a retry with modified + /// environment variables (e.g., NIXPKGS_ALLOW_UNFREE). fn should_retry(&self, stderr: &str) -> bool; } /// Classifies what retry action should be taken based on nix stderr output. #[derive(Debug, PartialEq, Eq)] pub enum RetryAction { + /// Package has an unfree license, retry with NIXPKGS_ALLOW_UNFREE=1 AllowUnfree, + /// Package is marked insecure, retry with + /// NIXPKGS_ALLOW_INSECURE_DERIVATIONS=1 AllowInsecure, + /// Package is marked broken, retry with NIXPKGS_ALLOW_BROKEN=1 AllowBroken, + /// No retry needed 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)> { + /// # Returns + /// + /// `(env_var, reason)` for this retry action, or `None` if no retry is + /// needed. + const fn env_override(&self) -> Option<(&str, &str)> { match self { Self::AllowUnfree => { Some(("NIXPKGS_ALLOW_UNFREE", "has an unfree license")) @@ -245,8 +270,7 @@ fn package_name(args: &[String]) -> &str { args .iter() .find(|a| !a.starts_with('-')) - .map(String::as_str) - .unwrap_or("") + .map_or("", String::as_str) } /// Print a retry message with consistent formatting. @@ -261,6 +285,7 @@ fn print_retry_msg(pkg: &str, reason: &str, env_var: &str) { } /// Classify stderr into a retry action. +#[must_use] pub fn classify_retry_action(stderr: &str) -> RetryAction { if stderr.contains("has an unfree license") && stderr.contains("refusing") { RetryAction::AllowUnfree @@ -284,22 +309,98 @@ fn is_hash_mismatch_error(stderr: &str) -> bool { || (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('-')); +/// Check if a package has an unfree, insecure, or broken attribute set. +/// Returns the appropriate `RetryAction` if any of these are true. Makes a +/// single nix eval call to minimize overhead. +fn check_package_flags(args: &[String]) -> Result { + // Default to "." if no argument provided (like `nix build` without args) + let eval_arg = args + .iter() + .find(|arg| !arg.starts_with('-')) + .cloned() + .unwrap_or_else(|| ".".to_string()); - let Some(eval_arg) = eval_arg else { - return Ok(RetryAction::None); // No expression to evaluate + let eval_expr = format!("nixpkgs#{eval_arg}.meta"); + let eval_cmd = NixCommand::new("eval") + .arg("--json") + .arg(&eval_expr) + .print_build_logs(false); + + let output = match eval_cmd.output() { + Ok(o) if o.status.success() => o, + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr); + if stderr.contains("does not provide attribute") { + return Ok(RetryAction::None); + } + log_warn!( + "failed to check package flags for '{}': {}", + eval_arg, + stderr.trim() + ); + return Ok(RetryAction::None); + }, + + Err(e) => { + log_warn!("failed to check package flags for '{}': {}", eval_arg, e); + return Ok(RetryAction::None); + }, }; + let meta: serde_json::Value = match serde_json::from_slice(&output.stdout) { + Ok(v) => v, + Err(e) => { + log_warn!("failed to parse package metadata for '{}': {}", eval_arg, e); + return Ok(RetryAction::None); + }, + }; + + let flags = [ + ("unfree", RetryAction::AllowUnfree), + ("insecure", RetryAction::AllowInsecure), + ("broken", RetryAction::AllowBroken), + ]; + + for (key, action) in flags { + if meta + .get(key) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false) + { + return Ok(action); + } + } + + Ok(RetryAction::None) +} + +/// Pre-evaluate expression to catch errors early. +/// +/// Returns a `RetryAction` if the package has retryable flags +/// (unfree/insecure/broken), allowing the caller to retry with the right +/// environment variables. +fn pre_evaluate(args: &[String]) -> Result { + // First, check package meta flags directly to avoid error message parsing + let action = check_package_flags(args)?; + if action != RetryAction::None { + return Ok(action); + } + + // Find flake references or expressions to evaluate + // Only take the first non-flag argument (the package/expression) + // Default to "." if no argument provided (like `nix build` without args) + let eval_arg = args + .iter() + .find(|arg| !arg.starts_with('-')) + .cloned() + .unwrap_or_else(|| { + log_warn!("no package specified, defaulting to '.' (current directory)"); + ".".to_string() + }); + + let eval_arg_ref = &eval_arg; let eval_cmd = NixCommand::new("eval") - .arg(eval_arg) + .arg(eval_arg_ref) .print_build_logs(false); let output = eval_cmd.output()?; @@ -311,12 +412,13 @@ fn pre_evaluate(args: &[String]) -> Result { let stderr = String::from_utf8_lossy(&output.stderr); // Classify whether this is a retryable error (unfree/insecure/broken) + // Fallback for errors that slip through (e.g., from dependencies) let action = classify_retry_action(&stderr); if action != RetryAction::None { return Ok(action); } - // Non-retryable eval failure — fail fast with a clear message + // Non-retryable eval failure, we should fail fast with a clear message // rather than running the full command and showing the same error again. let stderr_clean = stderr .trim()