nh checks v2

[NotAShelf]

The current nh environment checks are "sufficient", but they create weird conflicts based on the kind of Nix implementation, version or simply the configuration. I propose an alternative, non-monolithic approach to checking experimental features in check_nix_features where all features are checked upfront regardless of what command will actually be executed.

What I have in mind is new trait and structs to represent feature requirements. Something like:

use std::collections::HashSet;

pub trait FeatureRequirements {
    fn required_features(&self) -> Vec<&'static str>;
    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(());
        }
        
        let missing = util::get_missing_experimental_features(&required)?;
        if !missing.is_empty() {
            return Err(eyre::eyre!(
                "Missing required experimental features for this command: {}",
                missing.join(", ")
            ));
        }
        Ok(())
    }
}

#[derive(Debug)]
pub struct FlakeFeatures;

#[derive(Debug)] 
pub struct LegacyFeatures;

#[derive(Debug)]
pub struct ReplFeatures {
    pub is_flake: bool,
}

impl FeatureRequirements for FlakeFeatures {
    fn required_features(&self) -> Vec<&'static str> {
        vec!["nix-command", "flakes"]
    }
}

impl FeatureRequirements for LegacyFeatures {
    fn required_features(&self) -> Vec<&'static str> {
        vec!["nix-command"]
    }
}

impl FeatureRequirements for ReplFeatures {
    fn required_features(&self) -> Vec<&'static str> {
        let mut features = vec!["nix-command"];
        
        if self.is_flake {
            features.push("flakes");
            
            // Lix-specific repl-flake feature for older versions
            if let Ok(true) = util::is_lix() {
                if let Ok(version) = util::get_nix_version() {
                    if let Ok(current) = semver::Version::parse(&version) {
                        if let Ok(threshold) = semver::Version::parse("2.93.0") {
                            if current < threshold {
                                features.push("repl-flake");
                            }
                        }
                    }
                }
            }
        }
        
        features
    }
}

Then we would extend the command interfaces to include feature requirement information, likely as follows:

use crate::checks::FeatureRequirements;

impl NHCommand {
    pub fn get_feature_requirements(&self) -> Box<dyn FeatureRequirements> {
        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(checks::LegacyFeatures),
            Self::Clean(_) => Box::new(checks::LegacyFeatures),
            Self::Completions(_) => Box::new(checks::LegacyFeatures),
        }
    }

    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 {
                    std::env::set_var("NH_CURRENT_COMMAND", "os");
                }
                args.run()
            }
            // ...
        }
    }
}

impl OsArgs {
    fn get_feature_requirements(&self) -> Box<dyn FeatureRequirements> {
        match &self.subcommand {
            OsSubcommand::Repl(args) => {
                let is_flake = matches!(args.installable, Installable::Flake { .. }) ||
                              env::var("NH_OS_FLAKE").is_ok();
                Box::new(checks::ReplFeatures { is_flake })
            }
            _ => {
                // Check if using flakes based on installable or environment
                if self.uses_flakes() {
                    Box::new(checks::FlakeFeatures)
                } else {
                    Box::new(checks::LegacyFeatures)
                }
            }
        }
    }
    
    fn uses_flakes(&self) -> bool {
        // Check environment variables first
        if env::var("NH_OS_FLAKE").is_ok() {
            return true;
        }
        
        // Check installable type based on subcommand
        match &self.subcommand {
            OsSubcommand::Switch(args) | 
            OsSubcommand::Boot(args) |
            OsSubcommand::Test(args) |
            OsSubcommand::Build(args) => {
                matches!(args.common.installable, Installable::Flake { .. })
            }
            OsSubcommand::Repl(args) => {
                matches!(args.installable, Installable::Flake { .. })
            }
            _ => false
        }
    }
}

and lastly, the global one-off check would be removed. As such we wouldl only check features actually needed for the specific command being executed and take into account whether flakes are being used based on installable type and environment variables.

[NotAShelf]

Relevant #305

[viperML]

That snippet feels like too much code for the task

[NotAShelf]

It looks a bit too complex, yes, but I think it's the only future-proof way of dealing with weird Nix version/feature/vendor permutations. I'm going to submit a PR tomorrow, and we can decide if it's the way to go. Though, for reference, I am quite fond of how its structured rn.

I also wanted to keep things trait-based, and ended up removing some helpers like is_lix for an enum in its place. This implementation solves a lot of unnecessary version/feature checks, and is able to deal with the expanding number of Nix vendors that insist their way of doing things is the correct one.