fix stdout handling for nr; better catching logic

Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: I6a6a69643bbfdb289145019aa47012caa3bf3c04

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

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());

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<Command>,
+}
+
+#[derive(Subcommand)]
+pub enum Command {
+    /// Run a Nix derivation
+    Run {
+        #[arg(trailing_var_arg = true)]
+        args: Vec<String>,
+    },
+    /// Enter a Nix shell
+    Shell {
+        #[arg(trailing_var_arg = true)]
+        args: Vec<String>,
+    },
+    /// Build a Nix derivation
+    Build {
+        #[arg(trailing_var_arg = true)]
+        args: Vec<String>,
+    },
+}

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<Command>,
-}
-
-#[derive(Subcommand)]
-enum Command {
-    /// Run a Nix derivation
-    Run {
-        #[arg(trailing_var_arg = true)]
-        args: Vec<String>,
-    },
-    /// Enter a Nix shell
-    Shell {
-        #[arg(trailing_var_arg = true)]
-        args: Vec<String>,
-    },
-    /// Build a Nix derivation
-    Build {
-        #[arg(trailing_var_arg = true)]
-        args: Vec<String>,
-    },
-}
-
 fn main() {
     let format = tracing_subscriber::fmt::format()
         .with_level(true) // don't include levels in formatted output

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);
 }

eh/src/util.rs

@@ -15,9 +15,9 @@ pub struct RegexHashExtractor;
 impl HashExtractor for RegexHashExtractor {
     fn extract_hash(&self, stderr: &str) -> Option<String> {
         let patterns = [
-            r"got:\s+([a-zA-Z0-9+/=]+)",
+            r"got:\s+(sha256-[a-zA-Z0-9+/=]+)",
-            r"actual:\s+([a-zA-Z0-9+/=]+)",
+            r"actual:\s+(sha256-[a-zA-Z0-9+/=]+)",
-            r"have:\s+([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);
+    // Pre-evaluate for build commands to catch errors early
-    if interactive {
+    if !pre_evaluate(subcommand, args) {
-        cmd = cmd.interactive(true);
+        eprintln!("Error: Expression evaluation failed");
-    }
+        std::process::exit(1);
-    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);
     }
 
-    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("has an unfree license") && stderr.contains("refusing"))
-            || (stderr.contains("insecure") && stderr.contains("refusing"))
+            || (stderr.contains("has been marked as insecure") && stderr.contains("refusing"))
-            || (stderr.contains("broken") && stderr.contains("refusing"))
+            || (stderr.contains("has been marked as broken") && stderr.contains("refusing"))
     }
 }

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())
+    );
+}