From d86d81f3c1408f6400904a6d9e57b2da5420178c Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sat, 2 Aug 2025 20:24:29 +0300 Subject: [PATCH 1/2] 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()) + ); +} From 0174e390f321403b1d7e1cb66b1b9e28eaf4b330 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sat, 2 Aug 2025 22:46:24 +0300 Subject: [PATCH 2/2] try to get completions going Signed-off-by: NotAShelf Change-Id: I6a6a6964dcd3beb752ae2d3a90ab6e545815c692 --- Cargo.lock | 11 +++++++++ Cargo.toml | 1 + xtask/Cargo.toml | 2 ++ xtask/src/main.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 6e76d62..562e139 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -43,6 +43,15 @@ dependencies = [ "clap_lex", ] +[[package]] +name = "clap_complete" +version = "4.5.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5abde44486daf70c5be8b8f8f1b66c49f86236edf6fa2abadb4d961c4c6229a" +dependencies = [ + "clap", +] + [[package]] name = "clap_derive" version = "4.5.41" @@ -302,6 +311,8 @@ name = "xtask" version = "0.1.1" dependencies = [ "clap", + "clap_complete", + "eh", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e16edae..38fd81f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ version = "0.1.1" [workspace.dependencies] clap = { default-features = false, features = [ "std", "help", "derive" ], version = "4.5" } +clap_complete = "4.5" regex = "1.11" tracing = "0.1" tracing-subscriber = "0.3" diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index a8d8f20..54657fb 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -10,3 +10,5 @@ publish = false [dependencies] clap.workspace = true +clap_complete.workspace = true +eh = { path = "../eh" } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 6a2d7b7..4d92b6a 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -4,7 +4,8 @@ use std::{ process, }; -use clap::Parser; +use clap::{CommandFactory, Parser}; +use clap_complete::{Shell, generate}; #[derive(clap::Parser)] struct Cli { @@ -24,6 +25,15 @@ enum Command { #[arg(long, default_value = "target/release/eh")] main_binary: PathBuf, }, + /// Generate shell completion scripts + Completions { + /// Shell to generate completions for + #[arg(value_enum)] + shell: Shell, + /// Directory to output completion files + #[arg(long, default_value = "completions")] + output_dir: PathBuf, + }, } #[derive(Debug, Clone, Copy)] @@ -56,6 +66,12 @@ fn main() { process::exit(1); } } + Command::Completions { shell, output_dir } => { + if let Err(error) = generate_completions(shell, &output_dir) { + eprintln!("error generating completions: {error}"); + process::exit(1); + } + } } } @@ -117,3 +133,43 @@ fn create_multicall_binaries( Ok(()) } + +fn generate_completions(shell: Shell, output_dir: &Path) -> Result<(), Box> { + println!("generating {} completions...", shell); + + fs::create_dir_all(output_dir)?; + + let mut cmd = eh::Cli::command(); + let bin_name = "eh"; + + let completion_file = output_dir.join(format!("{}.{}", bin_name, shell)); + let mut file = fs::File::create(&completion_file)?; + + generate(shell, &mut cmd, bin_name, &mut file); + + println!("completion file generated: {}", completion_file.display()); + + // Create symlinks for multicall binaries + let multicall_names = ["nb", "nr", "ns"]; + for name in &multicall_names { + let symlink_path = output_dir.join(format!("{}.{}", name, shell)); + if symlink_path.exists() { + fs::remove_file(&symlink_path)?; + } + + #[cfg(unix)] + { + std::os::unix::fs::symlink(&completion_file, &symlink_path)?; + println!("completion symlink created: {}", symlink_path.display()); + } + + #[cfg(not(unix))] + { + fs::copy(&completion_file, &symlink_path)?; + println!("completion copy created: {}", symlink_path.display()); + } + } + + println!("completions generated successfully!"); + Ok(()) +}