From 5dc193c6fcda8df9d9d397f3ae20b8c16d291683 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 4 Jun 2025 22:28:51 +0300 Subject: [PATCH 01/11] util: add helper for determinate nix --- src/util.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/util.rs b/src/util.rs index fdf46881..16d1eb7f 100644 --- a/src/util.rs +++ b/src/util.rs @@ -72,11 +72,25 @@ pub fn is_lix() -> Result { let output = Command::new("nix") .arg("--version") .run_capture()? - .ok_or_else(|| eyre::eyre!("No output from command"))?; + .ok_or_else(|| eyre::eyre!("Failed to determine Nix variant"))?; Ok(output.to_lowercase().contains("lix")) } +/// Determines if the Nix binary is Determinate Nix +/// +/// # Returns +/// +/// * `Result` - True if the binary is Determinate Nix, false otherwise +pub fn is_determinate() -> Result { + let output = Command::new("nix") + .arg("--version") + .run_capture()? + .ok_or_else(|| eyre::eyre!("Failed to determine Nix variant"))?; + + Ok(output.to_lowercase().contains("determinate")) +} + /// Represents an object that may be a temporary path pub trait MaybeTempPath: std::fmt::Debug { fn get_path(&self) -> &Path; From 62a75ee5f5eff43da3c9bf03c6eed3cee7ff674f Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 4 Jun 2025 23:16:49 +0300 Subject: [PATCH 02/11] treewide: improve Nix version checks and feature requirements handling --- src/checks.rs | 259 ++++++++++++++++++++++++++++++++++++----------- src/interface.rs | 157 ++++++++++++++++++++++++++++ src/main.rs | 2 +- src/util.rs | 58 ++++++----- 4 files changed, 386 insertions(+), 90 deletions(-) diff --git a/src/checks.rs b/src/checks.rs index c82fef8c..638e0e8f 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -1,10 +1,10 @@ use std::{cmp::Ordering, env}; -use color_eyre::{Result, eyre}; +use color_eyre::Result; use semver::Version; -use tracing::warn; +use tracing::{debug, warn}; -use crate::util; +use crate::util::{self, NixVariant}; /// Verifies if the installed Nix version meets requirements /// @@ -17,10 +17,14 @@ pub fn check_nix_version() -> Result<()> { } let version = util::get_nix_version()?; - let is_lix_binary = util::is_lix()?; + let nix_variant = util::get_nix_variant()?; // XXX: Both Nix and Lix follow semantic versioning (semver). Update the // versions below once latest stable for either of those packages change. + // We *also* cannot (or rather, will not) make this check for non-nixpkgs + // Nix variants, since there is no good baseline for what to support + // without the understanding of stable/unstable branches. What do we check + // for, whether upstream made an announcement? No thanks. // TODO: Set up a CI to automatically update those in the future. const MIN_LIX_VERSION: &str = "2.91.1"; const MIN_NIX_VERSION: &str = "2.24.14"; @@ -37,10 +41,9 @@ pub fn check_nix_version() -> Result<()> { // to try and support too many versions. NixOS stable and unstable // will ALWAYS be supported, but outdated versions will not. If your // Nix fork uses a different versioning scheme, please open an issue. - let min_version = if is_lix_binary { - MIN_LIX_VERSION - } else { - MIN_NIX_VERSION + let min_version = match nix_variant { + util::NixVariant::Lix => MIN_LIX_VERSION, + _ => MIN_NIX_VERSION, }; let current = Version::parse(&version)?; @@ -48,7 +51,11 @@ pub fn check_nix_version() -> Result<()> { match current.cmp(&required) { Ordering::Less => { - let binary_name = if is_lix_binary { "Lix" } else { "Nix" }; + let binary_name = match nix_variant { + util::NixVariant::Lix => "Lix", + util::NixVariant::Determinate => "Determinate Nix", + util::NixVariant::Nix => "Nix", + }; warn!( "Warning: {} version {} is older than the recommended minimum version {}. You may encounter issues.", binary_name, version, min_version @@ -59,57 +66,6 @@ pub fn check_nix_version() -> Result<()> { } } -/// Verifies if the required experimental features are enabled -/// -/// # Returns -/// -/// * `Result<()>` - Ok if all required features are enabled, error otherwise -pub fn check_nix_features() -> Result<()> { - if env::var("NH_NO_CHECKS").is_ok() { - return Ok(()); - } - - let mut required_features = vec!["nix-command", "flakes"]; - - // Lix up until 2.93.0 uses repl-flake, which is removed in the latest version of Nix. - if util::is_lix()? { - let repl_flake_removed_in_lix_version = Version::parse("2.93.0")?; - let current_lix_version = Version::parse(&util::get_nix_version()?)?; - if current_lix_version < repl_flake_removed_in_lix_version { - required_features.push("repl-flake"); - } - } - - tracing::debug!("Required Nix features: {}", required_features.join(", ")); - - // Get currently enabled features - match util::get_nix_experimental_features() { - Ok(enabled_features) => { - let features_vec: Vec<_> = enabled_features.into_iter().collect(); - tracing::debug!("Enabled Nix features: {}", features_vec.join(", ")); - } - Err(e) => { - tracing::warn!("Failed to get enabled Nix features: {}", e); - } - } - - let missing_features = util::get_missing_experimental_features(&required_features)?; - - if !missing_features.is_empty() { - tracing::warn!( - "Missing required Nix features: {}", - missing_features.join(", ") - ); - return Err(eyre::eyre!( - "Missing required experimental features. Please enable: {}", - missing_features.join(", ") - )); - } - - tracing::debug!("All required Nix features are enabled"); - Ok(()) -} - /// Handles environment variable setup and returns if a warning should be shown /// /// # Returns @@ -144,6 +100,9 @@ pub fn setup_environment() -> Result { /// function. This will be executed in the main function, but can be executed /// before critical commands to double-check if necessary. /// +/// NOTE: Experimental feature checks are now done per-command to avoid +/// redundant error messages for features not needed by the specific command. +/// /// # Returns /// /// * `Result<()>` - Ok if all checks pass, error otherwise @@ -152,7 +111,185 @@ pub fn verify_nix_environment() -> Result<()> { return Ok(()); } + // Only check version globally. Features are checked per-command now. + // This function is kept as is for backwards compatibility. check_nix_version()?; - check_nix_features()?; Ok(()) } + +/// Trait for types that have feature requirements +pub trait FeatureRequirements { + /// Returns the list of required experimental features + fn required_features(&self) -> Vec<&'static str>; + + /// Checks if all required features are enabled + fn check_features(&self) -> Result<()> { + if env::var("NH_NO_CHECKS").is_ok() { + return Ok(()); + } + + let required = self.required_features(); + if required.is_empty() { + return Ok(()); + } + + debug!("Required Nix features: {}", required.join(", ")); + + let missing = util::get_missing_experimental_features(&required)?; + if !missing.is_empty() { + return Err(color_eyre::eyre::eyre!( + "Missing required experimental features for this command: {}", + missing.join(", ") + )); + } + + debug!("All required Nix features are enabled"); + Ok(()) + } +} + +/// Feature requirements for commands that use flakes +#[derive(Debug)] +pub struct FlakeFeatures; + +impl FeatureRequirements for FlakeFeatures { + fn required_features(&self) -> Vec<&'static str> { + let mut features = vec![]; + + // Determinate Nix doesn't require nix-command or flakes to be experimental + // as they simply decided to mark those as no-longer-experimental-lol. Remove + // redundant experimental features if the Nix variant is determinate. + if let Ok(variant) = util::get_nix_variant() { + if !matches!(variant, NixVariant::Determinate) { + features.push("nix-command"); + features.push("flakes"); + } + } + + features + } +} + +/// Feature requirements for legacy (non-flake) commands +/// XXX: There are actually no experimental feature requirements for legacy (nix2) CLI +/// but since move-fast-break-everything is a common mantra among Nix & Nix-adjecent +/// software, I've implemented this. Do not remove, this is simply for futureproofing. +#[derive(Debug)] +pub struct LegacyFeatures; + +impl FeatureRequirements for LegacyFeatures { + fn required_features(&self) -> Vec<&'static str> { + vec![] + } +} + +/// Feature requirements for OS repl commands +#[derive(Debug)] +pub struct OsReplFeatures { + pub is_flake: bool, +} + +impl FeatureRequirements for OsReplFeatures { + fn required_features(&self) -> Vec<&'static str> { + let mut features = vec![]; + + // For non-flake repls, no experimental features needed + if !self.is_flake { + return features; + } + + // For flake repls, check if we need experimental features + if let Ok(variant) = util::get_nix_variant() { + match variant { + NixVariant::Determinate => { + // Determinate Nix doesn't need experimental features + } + NixVariant::Lix => { + features.push("nix-command"); + features.push("flakes"); + + // Lix-specific repl-flake feature for older versions + if let Ok(version) = util::get_nix_version() { + if let Ok(current) = Version::parse(&version) { + if let Ok(threshold) = Version::parse("2.93.0") { + if current < threshold { + features.push("repl-flake"); + } + } + } + } + } + NixVariant::Nix => { + features.push("nix-command"); + features.push("flakes"); + } + } + } + + features + } +} + +/// Feature requirements for Home Manager repl commands +#[derive(Debug)] +pub struct HomeReplFeatures { + pub is_flake: bool, +} + +impl FeatureRequirements for HomeReplFeatures { + fn required_features(&self) -> Vec<&'static str> { + let mut features = vec![]; + + // For non-flake repls, no experimental features needed + if !self.is_flake { + return features; + } + + // For flake repls, only need nix-command and flakes + if let Ok(variant) = util::get_nix_variant() { + if !matches!(variant, NixVariant::Determinate) { + features.push("nix-command"); + features.push("flakes"); + } + } + + features + } +} + +/// Feature requirements for Darwin repl commands +#[derive(Debug)] +pub struct DarwinReplFeatures { + pub is_flake: bool, +} + +impl FeatureRequirements for DarwinReplFeatures { + fn required_features(&self) -> Vec<&'static str> { + let mut features = vec![]; + + // For non-flake repls, no experimental features needed + if !self.is_flake { + return features; + } + + // For flake repls, only need nix-command and flakes + if let Ok(variant) = util::get_nix_variant() { + if !matches!(variant, NixVariant::Determinate) { + features.push("nix-command"); + features.push("flakes"); + } + } + + features + } +} + +/// Feature requirements for commands that don't need experimental features +#[derive(Debug)] +pub struct NoFeatures; + +impl FeatureRequirements for NoFeatures { + fn required_features(&self) -> Vec<&'static str> { + vec![] + } +} diff --git a/src/interface.rs b/src/interface.rs index 3d93a3c2..074d9b65 100644 --- a/src/interface.rs +++ b/src/interface.rs @@ -1,3 +1,4 @@ +use std::env; use std::path::PathBuf; use anstyle::Style; @@ -5,6 +6,10 @@ use clap::ValueEnum; use clap::{Args, Parser, Subcommand, builder::Styles}; use crate::Result; +use crate::checks::{ + DarwinReplFeatures, FeatureRequirements, FlakeFeatures, HomeReplFeatures, LegacyFeatures, + NoFeatures, OsReplFeatures, +}; use crate::installable::Installable; const fn make_style() -> Styles { @@ -53,7 +58,22 @@ pub enum NHCommand { } impl NHCommand { + pub fn get_feature_requirements(&self) -> Box { + match self { + Self::Os(args) => args.get_feature_requirements(), + Self::Home(args) => args.get_feature_requirements(), + Self::Darwin(args) => args.get_feature_requirements(), + Self::Search(_) => Box::new(NoFeatures), + Self::Clean(_) => Box::new(NoFeatures), + Self::Completions(_) => Box::new(NoFeatures), + } + } + pub fn run(self) -> Result<()> { + // Check features specific to this command + let requirements = self.get_feature_requirements(); + requirements.check_features()?; + match self { Self::Os(args) => { unsafe { @@ -90,6 +110,35 @@ pub struct OsArgs { pub subcommand: OsSubcommand, } +impl OsArgs { + pub fn get_feature_requirements(&self) -> Box { + match &self.subcommand { + OsSubcommand::Repl(args) => { + let is_flake = args.uses_flakes(); + Box::new(OsReplFeatures { is_flake }) + } + OsSubcommand::Switch(args) + | OsSubcommand::Boot(args) + | OsSubcommand::Test(args) + | OsSubcommand::Build(args) => { + if args.uses_flakes() { + Box::new(FlakeFeatures) + } else { + Box::new(LegacyFeatures) + } + } + OsSubcommand::BuildVm(args) => { + if args.common.uses_flakes() { + Box::new(FlakeFeatures) + } else { + Box::new(LegacyFeatures) + } + } + OsSubcommand::Info(_) | OsSubcommand::Rollback(_) => Box::new(LegacyFeatures), + } + } +} + #[derive(Debug, Subcommand)] pub enum OsSubcommand { /// Build and activate the new configuration, and make it the boot default @@ -164,6 +213,18 @@ pub struct OsRebuildArgs { pub build_host: Option, } +impl OsRebuildArgs { + pub fn uses_flakes(&self) -> bool { + // Check environment variables first + if env::var("NH_OS_FLAKE").is_ok() { + return true; + } + + // Check installable type + matches!(self.common.installable, Installable::Flake { .. }) + } +} + #[derive(Debug, Args)] pub struct OsRollbackArgs { /// Only print actions, without performing them @@ -223,6 +284,18 @@ pub struct OsReplArgs { pub hostname: Option, } +impl OsReplArgs { + pub fn uses_flakes(&self) -> bool { + // Check environment variables first + if env::var("NH_OS_FLAKE").is_ok() { + return true; + } + + // Check installable type + matches!(self.installable, Installable::Flake { .. }) + } +} + #[derive(Debug, Args)] pub struct OsGenerationsArgs { /// Path to Nix' profiles directory @@ -329,6 +402,24 @@ pub struct HomeArgs { pub subcommand: HomeSubcommand, } +impl HomeArgs { + pub fn get_feature_requirements(&self) -> Box { + match &self.subcommand { + HomeSubcommand::Repl(args) => { + let is_flake = args.uses_flakes(); + Box::new(HomeReplFeatures { is_flake }) + } + HomeSubcommand::Switch(args) | HomeSubcommand::Build(args) => { + if args.uses_flakes() { + Box::new(FlakeFeatures) + } else { + Box::new(LegacyFeatures) + } + } + } + } +} + #[derive(Debug, Subcommand)] pub enum HomeSubcommand { /// Build and activate a home-manager configuration @@ -372,6 +463,18 @@ pub struct HomeRebuildArgs { pub backup_extension: Option, } +impl HomeRebuildArgs { + pub fn uses_flakes(&self) -> bool { + // Check environment variables first + if env::var("NH_HOME_FLAKE").is_ok() { + return true; + } + + // Check installable type + matches!(self.common.installable, Installable::Flake { .. }) + } +} + #[derive(Debug, Args)] pub struct HomeReplArgs { #[command(flatten)] @@ -388,6 +491,18 @@ pub struct HomeReplArgs { pub extra_args: Vec, } +impl HomeReplArgs { + pub fn uses_flakes(&self) -> bool { + // Check environment variables first + if env::var("NH_HOME_FLAKE").is_ok() { + return true; + } + + // Check installable type + matches!(self.installable, Installable::Flake { .. }) + } +} + #[derive(Debug, Parser)] /// Generate shell completion files into stdout pub struct CompletionArgs { @@ -404,6 +519,24 @@ pub struct DarwinArgs { pub subcommand: DarwinSubcommand, } +impl DarwinArgs { + pub fn get_feature_requirements(&self) -> Box { + match &self.subcommand { + DarwinSubcommand::Repl(args) => { + let is_flake = args.uses_flakes(); + Box::new(DarwinReplFeatures { is_flake }) + } + DarwinSubcommand::Switch(args) | DarwinSubcommand::Build(args) => { + if args.uses_flakes() { + Box::new(FlakeFeatures) + } else { + Box::new(LegacyFeatures) + } + } + } + } +} + #[derive(Debug, Subcommand)] pub enum DarwinSubcommand { /// Build and activate a nix-darwin configuration @@ -431,6 +564,18 @@ pub struct DarwinRebuildArgs { pub extra_args: Vec, } +impl DarwinRebuildArgs { + pub fn uses_flakes(&self) -> bool { + // Check environment variables first + if env::var("NH_DARWIN_FLAKE").is_ok() { + return true; + } + + // Check installable type + matches!(self.common.installable, Installable::Flake { .. }) + } +} + #[derive(Debug, Args)] pub struct DarwinReplArgs { #[command(flatten)] @@ -441,6 +586,18 @@ pub struct DarwinReplArgs { pub hostname: Option, } +impl DarwinReplArgs { + pub fn uses_flakes(&self) -> bool { + // Check environment variables first + if env::var("NH_DARWIN_FLAKE").is_ok() { + return true; + } + + // Check installable type + matches!(self.installable, Installable::Flake { .. }) + } +} + #[derive(Debug, Args)] pub struct UpdateArgs { #[arg(short = 'u', long = "update")] diff --git a/src/main.rs b/src/main.rs index 87f80f7b..ac0c1fad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -28,7 +28,7 @@ fn main() -> Result<()> { tracing::debug!("{args:#?}"); tracing::debug!(%NH_VERSION, ?NH_REV); - // Verify the Nix environment before running commands + // Check Nix version upfront checks::verify_nix_environment()?; // Once we assert required Nix features, validate NH environment checks diff --git a/src/util.rs b/src/util.rs index 16d1eb7f..939b9d0c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -4,10 +4,40 @@ use std::process::{Command as StdCommand, Stdio}; use std::str; use color_eyre::{Result, eyre}; +use once_cell::sync::OnceCell; use tempfile::TempDir; use crate::commands::Command; +#[derive(Debug, Clone, PartialEq)] +pub enum NixVariant { + Nix, + Lix, + Determinate, +} + +static NIX_VARIANT: OnceCell = OnceCell::new(); + +/// Get the Nix variant (cached) +pub fn get_nix_variant() -> Result<&'static NixVariant> { + NIX_VARIANT.get_or_try_init(|| { + let output = Command::new("nix") + .arg("--version") + .run_capture()? + .ok_or_else(|| eyre::eyre!("Failed to determine Nix variant"))?; + + let output_lower = output.to_lowercase(); + + if output_lower.contains("determinate") { + Ok(NixVariant::Determinate) + } else if output_lower.contains("lix") { + Ok(NixVariant::Lix) + } else { + Ok(NixVariant::Nix) + } + }) +} + /// Retrieves the installed Nix version as a string. /// /// This function executes the `nix --version` command, parses the output to @@ -63,34 +93,6 @@ pub fn ensure_ssh_key_login() -> Result<()> { Ok(()) } -/// Determines if the Nix binary is actually Lix -/// -/// # Returns -/// -/// * `Result` - True if the binary is Lix, false if it's standard Nix -pub fn is_lix() -> Result { - let output = Command::new("nix") - .arg("--version") - .run_capture()? - .ok_or_else(|| eyre::eyre!("Failed to determine Nix variant"))?; - - Ok(output.to_lowercase().contains("lix")) -} - -/// Determines if the Nix binary is Determinate Nix -/// -/// # Returns -/// -/// * `Result` - True if the binary is Determinate Nix, false otherwise -pub fn is_determinate() -> Result { - let output = Command::new("nix") - .arg("--version") - .run_capture()? - .ok_or_else(|| eyre::eyre!("Failed to determine Nix variant"))?; - - Ok(output.to_lowercase().contains("determinate")) -} - /// Represents an object that may be a temporary path pub trait MaybeTempPath: std::fmt::Debug { fn get_path(&self) -> &Path; From fdb6d2a2181ce7ae4170401bfc6d55186c6aebf1 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 5 Jun 2025 06:19:19 +0300 Subject: [PATCH 03/11] checks: add property tests --- Cargo.lock | 232 +++++++++++++++++++++++++++++++++++++- Cargo.toml | 21 +++- src/checks.rs | 304 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 547 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af04efdc..e526e1b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -118,6 +118,21 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "2.9.0" @@ -356,6 +371,21 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "futures" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65bc07b1a8bc7c85c5f2e110c476c7389b4554ba72af57d8445ea63a576b0876" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + [[package]] name = "futures-channel" version = "0.3.31" @@ -372,6 +402,17 @@ version = "0.3.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "05f29059c0c2090612e8d742178b0580d2dc940c837851ad723096f87af6663e" +[[package]] +name = "futures-executor" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e28d1d997f585e54aebc3f97d39e72338912123a67330d723fdbb564d646c9f" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + [[package]] name = "futures-io" version = "0.3.31" @@ -396,6 +437,7 @@ version = "0.3.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" dependencies = [ + "futures-channel", "futures-core", "futures-io", "futures-sink", @@ -780,6 +822,16 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23fb14cb19457329c82206317a5663005a4d404783dc74f4252769b0d5f42856" +[[package]] +name = "lock_api" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96936507f153605bddfcda068dd804796c84324ed2510809e5b2a624c81da765" +dependencies = [ + "autocfg", + "scopeguard", +] + [[package]] name = "log" version = "0.4.27" @@ -845,11 +897,13 @@ dependencies = [ "nix", "once_cell", "owo-colors", + "proptest", "regex", "reqwest", "semver", "serde", "serde_json", + "serial_test", "subprocess", "supports-hyperlinks", "system-configuration", @@ -920,6 +974,29 @@ version = "4.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1036865bb9422d3300cf723f657c2851d0e9ab12567854b1f4eba3d77decf564" +[[package]] +name = "parking_lot" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70d58bf43669b5795d1576d0641cfb6fbb2057bf629506267a92807158584a13" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc838d2a56b5b1a6c25f55575dfc605fabb63bb2365f6c2353ef9159aa69e4a5" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-targets 0.52.6", +] + [[package]] name = "percent-encoding" version = "2.3.1" @@ -956,6 +1033,32 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "proptest" +version = "1.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "14cae93065090804185d3b75f0bf93b8eeda30c7a9b4a33d3bdb3988d6229e50" +dependencies = [ + "bit-set", + "bit-vec", + "bitflags", + "lazy_static", + "num-traits", + "rand 0.8.5", + "rand_chacha 0.3.1", + "rand_xorshift", + "regex-syntax 0.8.5", + "rusty-fork", + "tempfile", + "unarray", +] + +[[package]] +name = "quick-error" +version = "1.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" + [[package]] name = "quinn" version = "0.11.7" @@ -984,7 +1087,7 @@ checksum = "bcbafbbdbb0f638fe3f35f3c56739f77a8a1d070cb25603226c83339b391472b" dependencies = [ "bytes", "getrandom 0.3.2", - "rand", + "rand 0.9.1", "ring", "rustc-hash", "rustls", @@ -1025,14 +1128,35 @@ version = "5.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74765f6d916ee2faa39bc8e68e4f3ed8949b48cccdac59983d287a7cb71ce9c5" +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha 0.3.1", + "rand_core 0.6.4", +] + [[package]] name = "rand" version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9fbfd9d094a40bf3ae768db9361049ace4c0e04a4fd6b359518bd7b73a73dd97" dependencies = [ - "rand_chacha", - "rand_core", + "rand_chacha 0.9.0", + "rand_core 0.9.3", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core 0.6.4", ] [[package]] @@ -1042,7 +1166,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb" dependencies = [ "ppv-lite86", - "rand_core", + "rand_core 0.9.3", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom 0.2.16", ] [[package]] @@ -1054,6 +1187,24 @@ dependencies = [ "getrandom 0.3.2", ] +[[package]] +name = "rand_xorshift" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d25bf25ec5ae4a3f1b92f929810509a2f53d7dca2f50b794ff57e3face536c8f" +dependencies = [ + "rand_core 0.6.4", +] + +[[package]] +name = "redox_syscall" +version = "0.5.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "928fca9cf2aa042393a8325b9ead81d2f0df4cb12e1e24cef072922ccd99c5af" +dependencies = [ + "bitflags", +] + [[package]] name = "regex" version = "1.11.1" @@ -1230,12 +1381,45 @@ version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eded382c5f5f786b989652c49544c4877d9f015cc22e145a5ea8ea66c2921cd2" +[[package]] +name = "rusty-fork" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb3dcc6e454c328bb824492db107ab7c0ae8fcffe4ad210136ef014458c1bc4f" +dependencies = [ + "fnv", + "quick-error", + "tempfile", + "wait-timeout", +] + [[package]] name = "ryu" version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" +[[package]] +name = "scc" +version = "2.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22b2d775fb28f245817589471dd49c5edf64237f4a19d10ce9a92ff4651a27f4" +dependencies = [ + "sdd", +] + +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + +[[package]] +name = "sdd" +version = "3.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "584e070911c7017da6cb2eb0788d09f43d789029b5877d3e5ecc8acf86ceee21" + [[package]] name = "semver" version = "1.0.26" @@ -1286,6 +1470,31 @@ dependencies = [ "serde", ] +[[package]] +name = "serial_test" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b258109f244e1d6891bf1053a55d63a5cd4f8f4c30cf9a1280989f80e7a1fa9" +dependencies = [ + "futures", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d69265a08751de7844521fd15003ae0a888e035773ba05695c5c759a6f89eef" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -1659,6 +1868,12 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "unarray" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94" + [[package]] name = "unicode-ident" version = "1.0.18" @@ -1727,6 +1942,15 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" +[[package]] +name = "wait-timeout" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ac3b126d3914f9849036f826e054cbabdc8519970b8998ddaf3b5bd3c65f11" +dependencies = [ + "libc", +] + [[package]] name = "want" version = "0.3.1" diff --git a/Cargo.toml b/Cargo.toml index a03482ce..05eb07bc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,15 +26,20 @@ dialoguer = { version = "0.11.0", default-features = false } elasticsearch-dsl = "0.4.19" hostname = "0.4" humantime = "2.1.0" -nix = { version = "0.30.1", default-features = false, features = ["fs", "user"] } +nix = { version = "0.30.1", default-features = false, features = [ + "fs", + "user", +] } once_cell = "1.18.0" owo-colors = "4.0.0" regex = "1.8.4" -reqwest = { version = "0.12.0", features = ["rustls-tls", "blocking", "json"], default-features = false } +reqwest = { version = "0.12.0", features = [ + "rustls-tls", + "blocking", + "json", +], default-features = false } semver = "1.0.22" -serde = { version = "1.0.166", features = [ - "derive", -] } +serde = { version = "1.0.166", features = ["derive"] } serde_json = "1.0.100" subprocess = "0.2" supports-hyperlinks = "3.0.0" @@ -46,9 +51,13 @@ tracing = "0.1.40" tracing-subscriber = { version = "0.3.18", features = [ "env-filter", "registry", - "std" + "std", ] } uzers = { version = "0.12.0", default-features = false } [target.'cfg(target_os="macos")'.dependencies] system-configuration = "0.6.1" + +[dev-dependencies] +proptest = "1.6.0" +serial_test = "3.2.0" diff --git a/src/checks.rs b/src/checks.rs index 638e0e8f..7ee82ae1 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -293,3 +293,307 @@ impl FeatureRequirements for NoFeatures { vec![] } } + +#[cfg(test)] +mod tests { + use std::env; + + use proptest::prelude::*; + use serial_test::serial; + + use super::*; + + // This helps set environment variables safely in tests + struct EnvGuard { + key: String, + original: Option, + } + + impl EnvGuard { + fn new(key: &str, value: &str) -> Self { + let original = env::var(key).ok(); + unsafe { + env::set_var(key, value); + } + EnvGuard { + key: key.to_string(), + original, + } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + unsafe { + match &self.original { + Some(val) => env::set_var(&self.key, val), + None => env::remove_var(&self.key), + } + } + } + } + + proptest! { + #[test] + fn test_flake_features_always_returns_consistent_results( + _dummy in 0..100u32 + ) { + let features = FlakeFeatures; + let result1 = features.required_features(); + let result2 = features.required_features(); + + // Property: Multiple calls should return identical results + prop_assert_eq!(result1.clone(), result2.clone()); + + // Property: Should only contain known experimental features + for feature in &result1 { + prop_assert!( + *feature == "nix-command" || + *feature == "flakes", + "Unknown feature: {}", feature + ); + } + + // Property: Results should be deterministic based on variant + // We can't control the actual variant in this test, but we can verify + // that the logic is consistent + if result1.is_empty() { + // If empty, variant should be Determinate (when available) + // This property holds when the system has Determinate Nix + } else { + // If not empty, should contain both nix-command and flakes + prop_assert!(result1.contains(&"nix-command")); + prop_assert!(result1.contains(&"flakes")); + prop_assert_eq!(result1.len(), 2); + } + } + + #[test] + fn test_legacy_features_always_empty( + _dummy in 0..100u32 + ) { + let features = LegacyFeatures; + let result = features.required_features(); + + // Property: Legacy features should always be empty + prop_assert!(result.is_empty()); + } + + #[test] + fn test_no_features_always_empty( + _dummy in 0..100u32 + ) { + let features = NoFeatures; + let result = features.required_features(); + + // Property: NoFeatures should always be empty + prop_assert!(result.is_empty()); + } + + #[test] + fn test_repl_features_consistency_with_flake_flag( + is_flake in any::() + ) { + // Test OS repl features + let os_features = OsReplFeatures { is_flake }; + let os_result = os_features.required_features(); + + // Test Home repl features + let home_features = HomeReplFeatures { is_flake }; + let home_result = home_features.required_features(); + + // Test Darwin repl features + let darwin_features = DarwinReplFeatures { is_flake }; + let darwin_result = darwin_features.required_features(); + + if !is_flake { + // Property: Non-flake repls should never require features + prop_assert!(os_result.is_empty()); + prop_assert!(home_result.is_empty()); + prop_assert!(darwin_result.is_empty()); + } else { + // Property: All flake repls should have consistent base features + // (when features are required, they should include nix-command and flakes) + for result in [&os_result, &home_result, &darwin_result] { + if !result.is_empty() { + prop_assert!(result.contains(&"nix-command")); + prop_assert!(result.contains(&"flakes")); + } + } + + // Property: Only OS repl may have additional features (repl-flake for older Lix) + // Home and Darwin should never have more than the base features + if !home_result.is_empty() { + prop_assert_eq!(home_result.len(), 2); + } + if !darwin_result.is_empty() { + prop_assert_eq!(darwin_result.len(), 2); + } + + // Property: OS repl may have 2 or 3 features (base + optional repl-flake) + if !os_result.is_empty() { + prop_assert!(os_result.len() >= 2 && os_result.len() <= 3); + if os_result.len() == 3 { + prop_assert!(os_result.contains(&"repl-flake")); + } + } + } + } + + #[test] + fn test_feature_requirements_trait_idempotency( + is_flake in any::() + ) { + let test_cases = vec![ + Box::new(FlakeFeatures) as Box, + Box::new(LegacyFeatures) as Box, + Box::new(OsReplFeatures { is_flake }) as Box, + Box::new(HomeReplFeatures { is_flake }) as Box, + Box::new(DarwinReplFeatures { is_flake }) as Box, + Box::new(NoFeatures) as Box, + ]; + + for feature_req in test_cases { + let result1 = feature_req.required_features(); + let result2 = feature_req.required_features(); + + // Property: Multiple calls should be idempotent + prop_assert_eq!(result1.clone(), result2.clone()); + + // Property: All features should be valid strings + for feature in &result1 { + prop_assert!(!feature.is_empty()); + prop_assert!(feature.chars().all(|c| c.is_ascii_alphanumeric() || c == '-')); + } + + // Property: No duplicate features + let mut sorted = result1.clone(); + sorted.sort(); + sorted.dedup(); + prop_assert_eq!(result1.len(), sorted.len()); + } + } + } + + // Regular unit tests for specific scenarios + #[test] + #[serial] + fn test_setup_environment_flake_to_nh_flake_migration() { + unsafe { + env::remove_var("FLAKE"); + env::remove_var("NH_FLAKE"); + env::remove_var("NH_OS_FLAKE"); + env::remove_var("NH_HOME_FLAKE"); + env::remove_var("NH_DARWIN_FLAKE"); + } + + let _guard = EnvGuard::new("FLAKE", "/test/flake"); + + let result = setup_environment().expect("setup_environment should succeed"); + + assert!(result, "Should warn when migrating FLAKE to NH_FLAKE"); + assert_eq!(env::var("NH_FLAKE").unwrap(), "/test/flake"); + } + + #[test] + #[serial] + fn test_setup_environment_no_migration_when_nh_flake_exists() { + unsafe { + env::remove_var("FLAKE"); + env::remove_var("NH_FLAKE"); + env::remove_var("NH_OS_FLAKE"); + env::remove_var("NH_HOME_FLAKE"); + env::remove_var("NH_DARWIN_FLAKE"); + } + + let _guard1 = EnvGuard::new("FLAKE", "/test/flake"); + let _guard2 = EnvGuard::new("NH_FLAKE", "/existing/flake"); + + let result = setup_environment().expect("setup_environment should succeed"); + + assert!(!result, "Should not warn when NH_FLAKE already exists"); + assert_eq!(env::var("NH_FLAKE").unwrap(), "/existing/flake"); + } + + #[test] + #[serial] + fn test_setup_environment_no_migration_when_specific_flake_vars_exist() { + unsafe { + env::remove_var("FLAKE"); + env::remove_var("NH_FLAKE"); + env::remove_var("NH_OS_FLAKE"); + env::remove_var("NH_HOME_FLAKE"); + env::remove_var("NH_DARWIN_FLAKE"); + } + + let _guard1 = EnvGuard::new("FLAKE", "/test/flake"); + let _guard2 = EnvGuard::new("NH_OS_FLAKE", "/os/flake"); + + let result = setup_environment().expect("setup_environment should succeed"); + + assert!(!result, "Should not warn when specific flake vars exist"); + assert_eq!(env::var("NH_FLAKE").unwrap(), "/test/flake"); + } + + #[test] + #[serial] + fn test_check_features_bypassed_with_nh_no_checks() { + let _guard = EnvGuard::new("NH_NO_CHECKS", "1"); + + let features = FlakeFeatures; + let result = features.check_features(); + + assert!( + result.is_ok(), + "check_features should succeed when NH_NO_CHECKS is set" + ); + } + + #[test] + #[serial] + fn test_verify_nix_environment_bypassed_with_nh_no_checks() { + let _guard = EnvGuard::new("NH_NO_CHECKS", "1"); + + let result = verify_nix_environment(); + + assert!( + result.is_ok(), + "verify_nix_environment should succeed when NH_NO_CHECKS is set" + ); + } + + #[test] + #[serial] + fn test_check_nix_version_bypassed_with_nh_no_checks() { + let _guard = EnvGuard::new("NH_NO_CHECKS", "1"); + + let result = check_nix_version(); + + assert!( + result.is_ok(), + "check_nix_version should succeed when NH_NO_CHECKS is set" + ); + } + + proptest! { + #[test] + #[serial] + fn test_env_guard_cleanup_property( + key in "[A-Z_]{1,20}", + value in "[a-zA-Z0-9/._-]{1,50}" + ) { + let original = env::var(&key).ok(); + + { + let _guard = EnvGuard::new(&key, &value); + prop_assert_eq!(env::var(&key).unwrap(), value); + } + + // Property: Environment should be restored after guard is dropped + match original { + Some(orig_val) => prop_assert_eq!(env::var(&key).unwrap(), orig_val), + None => prop_assert!(env::var(&key).is_err()), + } + } + } +} From 2f19cd976dec6a055950cee871dde74a18067e4b Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 5 Jun 2025 06:29:56 +0300 Subject: [PATCH 04/11] util: improve Nix variant detection with graceful handling for missing output --- src/util.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/util.rs b/src/util.rs index 939b9d0c..73c93f10 100644 --- a/src/util.rs +++ b/src/util.rs @@ -21,13 +21,19 @@ static NIX_VARIANT: OnceCell = OnceCell::new(); /// Get the Nix variant (cached) pub fn get_nix_variant() -> Result<&'static NixVariant> { NIX_VARIANT.get_or_try_init(|| { - let output = Command::new("nix") - .arg("--version") - .run_capture()? - .ok_or_else(|| eyre::eyre!("Failed to determine Nix variant"))?; + let output = Command::new("nix").arg("--version").run_capture()?; - let output_lower = output.to_lowercase(); + // XXX: If running with dry=true or Nix is not installed, output might be None + // The latter is less likely to occur, but we still want graceful handling. + let output_str = match output { + Some(output) => output, + None => return Ok(NixVariant::Nix), // default to standard Nix variant + }; + + let output_lower = output_str.to_lowercase(); + // FIXME: This fails to account for Nix variants we don't check for and + // assumes the environment is mainstream Nix. if output_lower.contains("determinate") { Ok(NixVariant::Determinate) } else if output_lower.contains("lix") { From 0232ac0b8345d2d0b18a9af9b6e97246b1aa0845 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 5 Jun 2025 08:09:00 +0300 Subject: [PATCH 05/11] checks: normalize version strings or bail gracefully --- src/checks.rs | 104 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 2 deletions(-) diff --git a/src/checks.rs b/src/checks.rs index 7ee82ae1..cefa4093 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -6,6 +6,43 @@ use tracing::{debug, warn}; use crate::util::{self, NixVariant}; +/// Normalizes a version string to be compatible with semver parsing. +/// +/// This function handles Nix's complex version formats by extracting just the +/// semantic version part. Examples of supported formats: +/// - "2.25.0-pre" -> "2.25.0" +/// - "2.24.14-1" -> "2.24.14" +/// - "2.30pre20250521_76a4d4c2" -> "2.30.0" +/// - "2.91.1" -> "2.91.1" +/// +/// # Arguments +/// +/// * `version` - The raw version string to normalize +/// +/// # Returns +/// +/// * `String` - The normalized version string suitable for semver parsing +fn normalize_version_string(version: &str) -> String { + // First, try to extract a version pattern like X.Y or X.Y.Z from the beginning + if let Some(captures) = regex::Regex::new(r"^(\d+)\.(\d+)(?:\.(\d+))?") + .unwrap() + .captures(version) + { + let major = captures.get(1).unwrap().as_str(); + let minor = captures.get(2).unwrap().as_str(); + let patch = captures.get(3).map(|m| m.as_str()).unwrap_or("0"); + + format!("{}.{}.{}", major, minor, patch) + } else { + // Fallback: split on common separators and take the first part + version + .split(&['-', '+', 'p', '_'][..]) + .next() + .unwrap_or(version) + .to_string() + } +} + /// Verifies if the installed Nix version meets requirements /// /// # Returns @@ -46,7 +83,20 @@ pub fn check_nix_version() -> Result<()> { _ => MIN_NIX_VERSION, }; - let current = Version::parse(&version)?; + // Normalize the version string to handle pre-release versions and distro suffixes + let normalized_version = normalize_version_string(&version); + + let current = match Version::parse(&normalized_version) { + Ok(ver) => ver, + Err(e) => { + warn!( + "Failed to parse Nix version '{}' (normalized: '{}'): {}. Skipping version check.", + version, normalized_version, e + ); + return Ok(()); + } + }; + let required = Version::parse(min_version)?; match current.cmp(&required) { @@ -210,7 +260,8 @@ impl FeatureRequirements for OsReplFeatures { // Lix-specific repl-flake feature for older versions if let Ok(version) = util::get_nix_version() { - if let Ok(current) = Version::parse(&version) { + let normalized_version = normalize_version_string(&version); + if let Ok(current) = Version::parse(&normalized_version) { if let Ok(threshold) = Version::parse("2.93.0") { if current < threshold { features.push("repl-flake"); @@ -334,6 +385,35 @@ mod tests { } proptest! { + #[test] + fn test_normalize_version_string_handles_various_formats( + major in 1u32..10, + minor in 0u32..99, + patch in 0u32..99 + ) { + // Test basic semver format + let basic = format!("{}.{}.{}", major, minor, patch); + prop_assert_eq!(normalize_version_string(&basic), basic.clone()); + + // Test with pre-release suffix + let pre_release = format!("{}.{}.{}-pre", major, minor, patch); + prop_assert_eq!(normalize_version_string(&pre_release), basic.clone()); + + // Test with distro suffix + let distro = format!("{}.{}.{}-1", major, minor, patch); + prop_assert_eq!(normalize_version_string(&distro), basic.clone()); + + // Test Nix-style version without patch (should add .0) + let no_patch = format!("{}.{}", major, minor); + let expected_no_patch = format!("{}.{}.0", major, minor); + prop_assert_eq!(normalize_version_string(&no_patch), expected_no_patch); + + // Test complex Nix format like "2.30pre20250521_76a4d4c2" + let complex = format!("{}.{}pre20250521_76a4d4c2", major, minor); + let expected_complex = format!("{}.{}.0", major, minor); + prop_assert_eq!(normalize_version_string(&complex), expected_complex); + } + #[test] fn test_flake_features_always_returns_consistent_results( _dummy in 0..100u32 @@ -476,6 +556,26 @@ mod tests { } // Regular unit tests for specific scenarios + #[test] + fn test_normalize_version_string_with_real_nix_versions() { + // Test the exact format you mentioned + assert_eq!( + normalize_version_string("2.30pre20250521_76a4d4c2"), + "2.30.0" + ); + + // Test other real Nix version formats + assert_eq!(normalize_version_string("2.25.0-pre"), "2.25.0"); + assert_eq!(normalize_version_string("2.24.14-1"), "2.24.14"); + assert_eq!(normalize_version_string("2.91.1"), "2.91.1"); + assert_eq!(normalize_version_string("2.18"), "2.18.0"); + + // Test edge cases + assert_eq!(normalize_version_string("3.0dev"), "3.0.0"); + assert_eq!(normalize_version_string("2.22rc1"), "2.22.0"); + assert_eq!(normalize_version_string("2.19_git_abc123"), "2.19.0"); + } + #[test] #[serial] fn test_setup_environment_flake_to_nh_flake_migration() { From 791d80067616c95387cc72b97da53a556327b393 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 5 Jun 2025 08:38:19 +0300 Subject: [PATCH 06/11] docs: update changelog --- CHANGELOG.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72da8dd7..c8ab07cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,20 @@ - + # NH Changelog +## Unreleased + +### Changed + +- Nh checks are now more robust in the sense that unnecessary features will not + be required when the underlying command does not depend on them. + +### Fixed + +- Nh will now correctly detect non-semver version strings, such as `x.ygit`. + Instead of failing the check, we now try to normalize the string and simply + skip the check with a warning. + ## 4.1.1 ### Changed From c1fee5aa7b05ef22a3cb2f0243e8993a600534a2 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 5 Jun 2025 12:23:45 +0300 Subject: [PATCH 07/11] checks: compile version reegex once to avoid per-call overhead --- src/checks.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/checks.rs b/src/checks.rs index cefa4093..454cde34 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -1,11 +1,16 @@ use std::{cmp::Ordering, env}; use color_eyre::Result; +use once_cell::sync::Lazy; +use regex::Regex; use semver::Version; use tracing::{debug, warn}; use crate::util::{self, NixVariant}; +// Static regex compiled once for version string normalization +static VERSION_REGEX: Lazy = Lazy::new(|| Regex::new(r"^(\d+)\.(\d+)(?:\.(\d+))?").unwrap()); + /// Normalizes a version string to be compatible with semver parsing. /// /// This function handles Nix's complex version formats by extracting just the @@ -24,10 +29,7 @@ use crate::util::{self, NixVariant}; /// * `String` - The normalized version string suitable for semver parsing fn normalize_version_string(version: &str) -> String { // First, try to extract a version pattern like X.Y or X.Y.Z from the beginning - if let Some(captures) = regex::Regex::new(r"^(\d+)\.(\d+)(?:\.(\d+))?") - .unwrap() - .captures(version) - { + if let Some(captures) = VERSION_REGEX.captures(version) { let major = captures.get(1).unwrap().as_str(); let minor = captures.get(2).unwrap().as_str(); let patch = captures.get(3).map(|m| m.as_str()).unwrap_or("0"); From 7e863e42e6b9c119d4bb5ec316482b98c34f97cc Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Fri, 6 Jun 2025 14:34:19 +0300 Subject: [PATCH 08/11] checks: improve flake detection; check that environment variables are not empty --- src/interface.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/interface.rs b/src/interface.rs index 074d9b65..8bab097c 100644 --- a/src/interface.rs +++ b/src/interface.rs @@ -216,7 +216,7 @@ pub struct OsRebuildArgs { impl OsRebuildArgs { pub fn uses_flakes(&self) -> bool { // Check environment variables first - if env::var("NH_OS_FLAKE").is_ok() { + if env::var("NH_OS_FLAKE").is_ok_and(|v| !v.is_empty()) { return true; } @@ -466,7 +466,7 @@ pub struct HomeRebuildArgs { impl HomeRebuildArgs { pub fn uses_flakes(&self) -> bool { // Check environment variables first - if env::var("NH_HOME_FLAKE").is_ok() { + if env::var("NH_HOME_FLAKE").is_ok_and(|v| !v.is_empty()) { return true; } @@ -494,7 +494,7 @@ pub struct HomeReplArgs { impl HomeReplArgs { pub fn uses_flakes(&self) -> bool { // Check environment variables first - if env::var("NH_HOME_FLAKE").is_ok() { + if env::var("NH_HOME_FLAKE").is_ok_and(|v| !v.is_empty()) { return true; } @@ -567,7 +567,7 @@ pub struct DarwinRebuildArgs { impl DarwinRebuildArgs { pub fn uses_flakes(&self) -> bool { // Check environment variables first - if env::var("NH_DARWIN_FLAKE").is_ok() { + if env::var("NH_DARWIN_FLAKE").is_ok_and(|v| !v.is_empty()) { return true; } @@ -589,7 +589,7 @@ pub struct DarwinReplArgs { impl DarwinReplArgs { pub fn uses_flakes(&self) -> bool { // Check environment variables first - if env::var("NH_DARWIN_FLAKE").is_ok() { + if env::var("NH_DARWIN_FLAKE").is_ok_and(|v| !v.is_empty()) { return true; } From 47151f84ae466f7b0cb61dc4e3ac057cc39d39a0 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Fri, 6 Jun 2025 14:45:23 +0300 Subject: [PATCH 09/11] checks: handle missing components in version normalization --- src/checks.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/checks.rs b/src/checks.rs index 454cde34..0f9a7e23 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -37,11 +37,18 @@ fn normalize_version_string(version: &str) -> String { format!("{}.{}.{}", major, minor, patch) } else { // Fallback: split on common separators and take the first part - version + let base_version = version .split(&['-', '+', 'p', '_'][..]) .next() - .unwrap_or(version) - .to_string() + .unwrap_or(version); + + // Version should have all three components (major.minor.patch) + let parts: Vec<&str> = base_version.split('.').collect(); + match parts.len() { + 1 => format!("{}.0.0", parts[0]), // "1" -> "1.0.0" + 2 => format!("{}.{}.0", parts[0], parts[1]), // "1.2" -> "1.2.0" + _ => base_version.to_string(), // "1.2.3" or more parts, use as-is + } } } @@ -576,6 +583,16 @@ mod tests { assert_eq!(normalize_version_string("3.0dev"), "3.0.0"); assert_eq!(normalize_version_string("2.22rc1"), "2.22.0"); assert_eq!(normalize_version_string("2.19_git_abc123"), "2.19.0"); + + // Test fallback cases where patch component is missing + assert_eq!(normalize_version_string("1.2-beta"), "1.2.0"); + assert_eq!(normalize_version_string("3.4+build.1"), "3.4.0"); + assert_eq!(normalize_version_string("5.6_alpha"), "5.6.0"); + + // Test fallback cases where both minor and patch are missing + assert_eq!(normalize_version_string("2-rc1"), "2.0.0"); + assert_eq!(normalize_version_string("4+build"), "4.0.0"); + assert_eq!(normalize_version_string("7_dev"), "7.0.0"); } #[test] From 32a2c267336ae8fe91ee128cb27fc6b1e5d15560 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Fri, 6 Jun 2025 16:45:28 +0300 Subject: [PATCH 10/11] chore: ignore cargo-mutants output --- .gitignore | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 0b8bffb0..d698b78f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,12 @@ +# Rust +target + +# Nix .direnv result* -target + +# From cargo-mutants +mutants* + +# Misc report* From 2faa956dc31264a73491b49b371949b07236be2c Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Fri, 6 Jun 2025 19:05:40 +0300 Subject: [PATCH 11/11] treewide: get rid of once_cell stdlib has a clean alternative since Rust 1.70 --- Cargo.lock | 1 - Cargo.toml | 1 - src/checks.rs | 5 +++-- src/util.rs | 24 +++++++++++++++--------- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e526e1b2..10b6e780 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -895,7 +895,6 @@ dependencies = [ "hostname", "humantime", "nix", - "once_cell", "owo-colors", "proptest", "regex", diff --git a/Cargo.toml b/Cargo.toml index 05eb07bc..e9515f2b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,6 @@ nix = { version = "0.30.1", default-features = false, features = [ "fs", "user", ] } -once_cell = "1.18.0" owo-colors = "4.0.0" regex = "1.8.4" reqwest = { version = "0.12.0", features = [ diff --git a/src/checks.rs b/src/checks.rs index 0f9a7e23..c9ce7112 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -1,7 +1,7 @@ +use std::sync::LazyLock; use std::{cmp::Ordering, env}; use color_eyre::Result; -use once_cell::sync::Lazy; use regex::Regex; use semver::Version; use tracing::{debug, warn}; @@ -9,7 +9,8 @@ use tracing::{debug, warn}; use crate::util::{self, NixVariant}; // Static regex compiled once for version string normalization -static VERSION_REGEX: Lazy = Lazy::new(|| Regex::new(r"^(\d+)\.(\d+)(?:\.(\d+))?").unwrap()); +static VERSION_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r"^(\d+)\.(\d+)(?:\.(\d+))?").unwrap()); /// Normalizes a version string to be compatible with semver parsing. /// diff --git a/src/util.rs b/src/util.rs index 73c93f10..d31abf18 100644 --- a/src/util.rs +++ b/src/util.rs @@ -2,9 +2,9 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::process::{Command as StdCommand, Stdio}; use std::str; +use std::sync::OnceLock; use color_eyre::{Result, eyre}; -use once_cell::sync::OnceCell; use tempfile::TempDir; use crate::commands::Command; @@ -16,18 +16,22 @@ pub enum NixVariant { Determinate, } -static NIX_VARIANT: OnceCell = OnceCell::new(); +static NIX_VARIANT: OnceLock = OnceLock::new(); /// Get the Nix variant (cached) pub fn get_nix_variant() -> Result<&'static NixVariant> { - NIX_VARIANT.get_or_try_init(|| { - let output = Command::new("nix").arg("--version").run_capture()?; + NIX_VARIANT.get_or_init(|| { + let output = Command::new("nix") + .arg("--version") + .run_capture() + .ok() + .flatten(); // XXX: If running with dry=true or Nix is not installed, output might be None // The latter is less likely to occur, but we still want graceful handling. let output_str = match output { Some(output) => output, - None => return Ok(NixVariant::Nix), // default to standard Nix variant + None => return NixVariant::Nix, // default to standard Nix variant }; let output_lower = output_str.to_lowercase(); @@ -35,13 +39,15 @@ pub fn get_nix_variant() -> Result<&'static NixVariant> { // FIXME: This fails to account for Nix variants we don't check for and // assumes the environment is mainstream Nix. if output_lower.contains("determinate") { - Ok(NixVariant::Determinate) + NixVariant::Determinate } else if output_lower.contains("lix") { - Ok(NixVariant::Lix) + NixVariant::Lix } else { - Ok(NixVariant::Nix) + NixVariant::Nix } - }) + }); + + Ok(NIX_VARIANT.get().unwrap()) } /// Retrieves the installed Nix version as a string.