From d86d81f3c1408f6400904a6d9e57b2da5420178c Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sat, 2 Aug 2025 20:24:29 +0300 Subject: [PATCH] fix stdout handling for nr; better catching logic Signed-off-by: NotAShelf Change-Id: I6a6a69643bbfdb289145019aa47012caa3bf3c04 --- eh/Cargo.toml | 4 ++ eh/src/command.rs | 2 + eh/src/lib.rs | 34 +++++++++ eh/src/main.rs | 29 +------- eh/src/run.rs | 2 +- eh/src/util.rs | 102 +++++++++++++++++++------- eh/tests/basic.rs | 178 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 296 insertions(+), 55 deletions(-) create mode 100644 eh/src/lib.rs create mode 100644 eh/tests/basic.rs diff --git a/eh/Cargo.toml b/eh/Cargo.toml index db2736d..8da1e92 100644 --- a/eh/Cargo.toml +++ b/eh/Cargo.toml @@ -6,6 +6,10 @@ edition.workspace = true authors.workspace = true rust-version.workspace = true +[lib] +name = "eh" +crate-type = ["lib"] + [dependencies] clap.workspace = true regex.workspace = true diff --git a/eh/src/command.rs b/eh/src/command.rs index f94623e..75e6970 100644 --- a/eh/src/command.rs +++ b/eh/src/command.rs @@ -98,6 +98,7 @@ impl NixCommand { if self.interactive { cmd.stdout(Stdio::inherit()); cmd.stderr(Stdio::inherit()); + cmd.stdin(Stdio::inherit()); return cmd.status(); } @@ -167,6 +168,7 @@ impl NixCommand { if self.interactive { cmd.stdout(Stdio::inherit()); cmd.stderr(Stdio::inherit()); + cmd.stdin(Stdio::inherit()); } else { cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); diff --git a/eh/src/lib.rs b/eh/src/lib.rs new file mode 100644 index 0000000..df062fa --- /dev/null +++ b/eh/src/lib.rs @@ -0,0 +1,34 @@ +pub mod build; +pub mod command; +pub mod run; +pub mod shell; +pub mod util; + +pub use clap::{CommandFactory, Parser, Subcommand}; + +#[derive(Parser)] +#[command(name = "eh")] +#[command(about = "Ergonomic Nix helper", long_about = None)] +pub struct Cli { + #[command(subcommand)] + pub command: Option, +} + +#[derive(Subcommand)] +pub enum Command { + /// Run a Nix derivation + Run { + #[arg(trailing_var_arg = true)] + args: Vec, + }, + /// Enter a Nix shell + Shell { + #[arg(trailing_var_arg = true)] + args: Vec, + }, + /// Build a Nix derivation + Build { + #[arg(trailing_var_arg = true)] + args: Vec, + }, +} diff --git a/eh/src/main.rs b/eh/src/main.rs index c7aad53..4aab85f 100644 --- a/eh/src/main.rs +++ b/eh/src/main.rs @@ -1,4 +1,4 @@ -use clap::{CommandFactory, Parser, Subcommand}; +use eh::{Cli, Command, CommandFactory, Parser}; use std::env; use std::path::Path; @@ -8,33 +8,6 @@ mod run; mod shell; mod util; -#[derive(Parser)] -#[command(name = "eh")] -#[command(about = "Ergonomic Nix helper", long_about = None)] -struct Cli { - #[command(subcommand)] - command: Option, -} - -#[derive(Subcommand)] -enum Command { - /// Run a Nix derivation - Run { - #[arg(trailing_var_arg = true)] - args: Vec, - }, - /// Enter a Nix shell - Shell { - #[arg(trailing_var_arg = true)] - args: Vec, - }, - /// Build a Nix derivation - Build { - #[arg(trailing_var_arg = true)] - args: Vec, - }, -} - fn main() { let format = tracing_subscriber::fmt::format() .with_level(true) // don't include levels in formatted output diff --git a/eh/src/run.rs b/eh/src/run.rs index dc1b1e7..0fa322c 100644 --- a/eh/src/run.rs +++ b/eh/src/run.rs @@ -6,5 +6,5 @@ pub fn handle_nix_run( fixer: &dyn NixFileFixer, classifier: &dyn NixErrorClassifier, ) { - handle_nix_with_retry("run", args, hash_extractor, fixer, classifier, false); + handle_nix_with_retry("run", args, hash_extractor, fixer, classifier, true); } diff --git a/eh/src/util.rs b/eh/src/util.rs index fc1f9ca..46dbb38 100644 --- a/eh/src/util.rs +++ b/eh/src/util.rs @@ -15,9 +15,9 @@ pub struct RegexHashExtractor; impl HashExtractor for RegexHashExtractor { fn extract_hash(&self, stderr: &str) -> Option { let patterns = [ - r"got:\s+([a-zA-Z0-9+/=]+)", - r"actual:\s+([a-zA-Z0-9+/=]+)", - r"have:\s+([a-zA-Z0-9+/=]+)", + 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) { @@ -110,6 +110,43 @@ pub trait NixErrorClassifier { fn should_retry(&self, stderr: &str) -> bool; } +/// Pre-evaluate expression to catch errors early +fn pre_evaluate(_subcommand: &str, args: &[String]) -> bool { + // 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 true; // No expression to evaluate + }; + + let eval_cmd = NixCommand::new("eval").arg(eval_arg).arg("--raw"); + + let output = match eval_cmd.output() { + Ok(output) => output, + Err(_) => return false, + }; + + if output.status.success() { + return true; + } + + 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 true; + } + + // For other eval failures, fail early + false +} + /// Shared retry logic for nix commands (build/run/shell). pub fn handle_nix_with_retry( subcommand: &str, @@ -119,29 +156,36 @@ pub fn handle_nix_with_retry( classifier: &dyn NixErrorClassifier, interactive: bool, ) -> ! { - let mut cmd = NixCommand::new(subcommand).print_build_logs(true); - if interactive { - cmd = cmd.interactive(true); - } - for arg in args { - cmd = cmd.arg(arg); - } - let status = cmd - .run_with_logs(StdIoInterceptor) - .expect("failed to run nix command"); - if status.success() { - std::process::exit(0); + // Pre-evaluate for build commands to catch errors early + if !pre_evaluate(subcommand, args) { + eprintln!("Error: Expression evaluation failed"); + std::process::exit(1); } - let mut output_cmd = NixCommand::new(subcommand) + // For run commands, try interactive first to avoid breaking terminal + if subcommand == "run" && interactive { + let mut cmd = NixCommand::new(subcommand) + .print_build_logs(true) + .interactive(true); + for arg in args { + cmd = cmd.arg(arg); + } + let status = cmd + .run_with_logs(StdIoInterceptor) + .expect("failed to run nix command"); + if status.success() { + std::process::exit(0); + } + } + + // First, always capture output to check for errors that need retry + let output_cmd = NixCommand::new(subcommand) .print_build_logs(true) .args(args.iter().cloned()); - if interactive { - output_cmd = output_cmd.interactive(true); - } let output = output_cmd.output().expect("failed to capture output"); let stderr = String::from_utf8_lossy(&output.stderr); + // Check if we need to retry with special flags if let Some(new_hash) = hash_extractor.extract_hash(&stderr) { if fixer.fix_hash_in_files(&new_hash) { info!("{}", Paint::green("✔ Fixed hash mismatch, retrying...")); @@ -157,7 +201,7 @@ pub fn handle_nix_with_retry( } if classifier.should_retry(&stderr) { - if stderr.contains("unfree") { + if stderr.contains("has an unfree license") && stderr.contains("refusing") { warn!( "{}", Paint::yellow("⚠ Unfree package detected, retrying with NIXPKGS_ALLOW_UNFREE=1...") @@ -173,7 +217,7 @@ pub fn handle_nix_with_retry( let retry_status = retry_cmd.run_with_logs(StdIoInterceptor).unwrap(); std::process::exit(retry_status.code().unwrap_or(1)); } - if stderr.contains("insecure") { + if stderr.contains("has been marked as insecure") && stderr.contains("refusing") { warn!( "{}", Paint::yellow( @@ -191,7 +235,7 @@ pub fn handle_nix_with_retry( let retry_status = retry_cmd.run_with_logs(StdIoInterceptor).unwrap(); std::process::exit(retry_status.code().unwrap_or(1)); } - if stderr.contains("broken") { + if stderr.contains("has been marked as broken") && stderr.contains("refusing") { warn!( "{}", Paint::yellow("⚠ Broken package detected, retrying with NIXPKGS_ALLOW_BROKEN=1...") @@ -209,16 +253,22 @@ pub fn handle_nix_with_retry( } } + // If the first attempt succeeded, we're done + if output.status.success() { + std::process::exit(0); + } + + // Otherwise, show the error and exit std::io::stderr().write_all(output.stderr.as_ref()).unwrap(); - std::process::exit(status.code().unwrap_or(1)); + std::process::exit(output.status.code().unwrap_or(1)); } pub struct DefaultNixErrorClassifier; impl NixErrorClassifier for DefaultNixErrorClassifier { fn should_retry(&self, stderr: &str) -> bool { RegexHashExtractor.extract_hash(stderr).is_some() - || (stderr.contains("unfree") && stderr.contains("refusing")) - || (stderr.contains("insecure") && stderr.contains("refusing")) - || (stderr.contains("broken") && stderr.contains("refusing")) + || (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")) } } diff --git a/eh/tests/basic.rs b/eh/tests/basic.rs new file mode 100644 index 0000000..4b69805 --- /dev/null +++ b/eh/tests/basic.rs @@ -0,0 +1,178 @@ +//! I hate writing tests, and I hate writing integration tests. This is the best +//! that you are getting, deal with it. +use std::process::{Command, Stdio}; + +#[test] +fn nix_eval_validation() { + // Test that invalid expressions are caught early for all commands + let commands = ["build", "run", "shell"]; + + for cmd in &commands { + let output = Command::new("timeout") + .args([ + "10", + "cargo", + "run", + "--bin", + "eh", + "--", + cmd, + "invalid-flake-ref", + ]) + .output() + .expect("Failed to execute command"); + + // Should fail fast with eval error + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("Error: Expression evaluation failed") || !output.status.success()); + } +} + +#[test] +fn unfree_package_handling() { + // Test that unfree packages are detected and handled correctly + let output = Command::new("timeout") + .args([ + "30", + "cargo", + "run", + "--bin", + "eh", + "--", + "build", + "nixpkgs#discord", + ]) + .output() + .expect("Failed to execute command"); + + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let combined = format!("{}{}", stdout, stderr); + + // Should detect unfree package and show appropriate message + assert!( + combined.contains("has an unfree license") + || combined.contains("NIXPKGS_ALLOW_UNFREE") + || combined.contains("⚠ Unfree package detected") + ); +} + +#[test] +fn insecure_package_handling() { + // Test that error classification works for insecure packages + use eh::util::{DefaultNixErrorClassifier, NixErrorClassifier}; + + let classifier = DefaultNixErrorClassifier; + let stderr_insecure = + "Package 'example-1.0' has been marked as insecure, refusing to evaluate."; + + assert!(classifier.should_retry(stderr_insecure)); +} + +#[test] +fn broken_package_handling() { + // Test that error classification works for broken packages + use eh::util::{DefaultNixErrorClassifier, NixErrorClassifier}; + + let classifier = DefaultNixErrorClassifier; + let stderr_broken = "Package 'example-1.0' has been marked as broken, refusing to evaluate."; + + assert!(classifier.should_retry(stderr_broken)); +} + +#[test] +fn multicall_binary_dispatch() { + // Test that nb/nr/ns dispatch correctly based on binary name + let commands = [("nb", "build"), ("nr", "run"), ("ns", "shell")]; + + for (binary_name, _expected_cmd) in &commands { + let output = Command::new("timeout") + .args(["10", "cargo", "run", "--bin", "eh"]) + .env("CARGO_BIN_NAME", binary_name) + .arg("nixpkgs#hello") + .arg("--help") // Use help to avoid actually building + .output() + .expect("Failed to execute command"); + + // Should execute without panicking (status code may vary) + assert!(output.status.code().is_some()); + } +} + +#[test] +fn interactive_mode_inheritance() { + // Test that run commands inherit stdio properly + let mut child = Command::new("timeout") + .args([ + "10", + "cargo", + "run", + "--bin", + "eh", + "--", + "run", + "nixpkgs#echo", + "test", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("Failed to spawn command"); + + let status = child.wait().expect("Failed to wait for child"); + + // Should complete without hanging + assert!(status.code().is_some()); +} + +#[test] +fn hash_extraction() { + use eh::util::{HashExtractor, RegexHashExtractor}; + + let extractor = RegexHashExtractor; + let stderr = "error: hash mismatch in fixed-output derivation '/nix/store/...': + specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= + got: sha256-BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB="; + + let hash = extractor.extract_hash(stderr); + assert!(hash.is_some()); + assert_eq!( + hash.unwrap(), + "sha256-BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB=" + ); +} + +#[test] +fn error_classification() { + use eh::util::{DefaultNixErrorClassifier, NixErrorClassifier}; + + let classifier = DefaultNixErrorClassifier; + + assert!(classifier.should_retry("has an unfree license ('unfree'), refusing to evaluate")); + assert!(classifier.should_retry("has been marked as insecure, refusing to evaluate")); + assert!(classifier.should_retry("has been marked as broken, refusing to evaluate")); + assert!(!classifier.should_retry("random build error")); +} + +#[test] +fn hash_mismatch_auto_fix() { + // Test that hash mismatches are automatically detected and fixed + // This is harder to test without creating actual files, so we test the regex + // for the time being. Alternatively I could do this inside a temporary directory + // but cba for now. + use eh::util::{HashExtractor, RegexHashExtractor}; + + let extractor = RegexHashExtractor; + let stderr_with_mismatch = r#" +error: hash mismatch in fixed-output derivation + specified: sha256-oldhashaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa= + got: sha256-newhashbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb= +"#; + + let extracted = extractor.extract_hash(stderr_with_mismatch); + assert_eq!( + extracted, + Some("sha256-newhashbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb=".to_string()) + ); +}