From 53a30c8b95a277c0c01b1e809b25bf10d84c19fc Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Mon, 6 Jan 2025 14:01:40 -0500 Subject: [PATCH 1/2] health: Handle non-symlink dotfiles Previously these are silently ignored. But we they show up in "Unmanaged". Also, panic on dotfile errors instead of silently ignoring them. --- crates/omnix-health/src/check/shell.rs | 41 ++++++++++++++++++++------ 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/crates/omnix-health/src/check/shell.rs b/crates/omnix-health/src/check/shell.rs index ef74dbc8..631267be 100644 --- a/crates/omnix-health/src/check/shell.rs +++ b/crates/omnix-health/src/check/shell.rs @@ -45,24 +45,32 @@ impl Checkable for ShellCheck { let mut managed: HashMap = HashMap::new(); let mut unmanaged: Vec = Vec::new(); for path in &shell.get_dotfiles() { - match std::fs::read_link(path) { - Ok(target) => { + match resolve_symlink(path) { + Ok(Some(target)) => { if super::direnv::is_path_in_nix_store(&target) { managed.insert(path.clone(), target); } else { + // A symlink to a non-nix path unmanaged.push(path.clone()); - }; + } + } + Ok(None) => { + // Not a symlink; cannot be managed by nix + unmanaged.push(path.clone()); } Err(err) => { - tracing::warn!("Dotfile {:?} symlink error: {:?}; ignoring.", path, err); + panic!("Dotfile {:?} symlink error: {:?}.", path, err); } } } let title = "Shell dotfiles".to_string(); let info = format!( - "Shell={:?}; Managed: {:?}; Unmanaged: {:?}", - shell, managed, unmanaged + "Shell={:?}; HOME={:?}; Managed: {:?}; Unmanaged: {:?}", + shell, + get_home_dir(), + managed, + unmanaged ); let result = if !managed.is_empty() { CheckResult::Green @@ -127,12 +135,27 @@ impl Shell { /// Get the currently existing dotfiles under $HOME fn get_dotfiles(&self) -> Vec { - let home_dir = - PathBuf::from(std::env::var("HOME").expect("Environment variable `HOME` not set")); self.dotfile_names() .iter() - .map(|dotfile| home_dir.join(dotfile)) + .map(|dotfile| get_home_dir().join(dotfile)) .filter(|path| path.exists()) .collect() } } + +fn get_home_dir() -> PathBuf { + PathBuf::from(std::env::var("HOME").expect("Environment variable `HOME` not set")) +} + +/// Resolve symlink to its target path. +/// +/// Returns `None` if the path is not a symlink. This function can thus be used to both resolve and check if a path is a symlink. +fn resolve_symlink(path: &PathBuf) -> std::io::Result> { + let meta = std::fs::symlink_metadata(path)?; + if meta.file_type().is_symlink() { + let target = std::fs::read_link(path)?; + Ok(Some(target)) + } else { + Ok(None) + } +} From c1f03dd41aadde6d1f509f264f4c0680cbc7d536 Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Mon, 6 Jan 2025 15:15:17 -0500 Subject: [PATCH 2/2] refactor: Rewrite the shell detection code --- crates/omnix-health/src/check/shell.rs | 149 ++++++++++++++----------- 1 file changed, 83 insertions(+), 66 deletions(-) diff --git a/crates/omnix-health/src/check/shell.rs b/crates/omnix-health/src/check/shell.rs index 631267be..90a44af5 100644 --- a/crates/omnix-health/src/check/shell.rs +++ b/crates/omnix-health/src/check/shell.rs @@ -1,5 +1,8 @@ use serde::{Deserialize, Serialize}; -use std::{collections::HashMap, path::PathBuf}; +use std::{ + collections::HashMap, + path::{Path, PathBuf}, +}; use crate::traits::{Check, CheckResult, Checkable}; @@ -28,55 +31,40 @@ impl Checkable for ShellCheck { if !self.enable { return vec![]; } - let shell = match Shell::current_shell() { - Some(shell) => shell, - None => { - let msg = "Unsupported shell. Please file an issue at "; + let user_shell_env = match CurrentUserShellEnv::new() { + Ok(shell) => shell, + Err(err) => { + tracing::error!("Skipping shell dotfile check! {:?}", err); if self.required { - panic!("{}", msg); + panic!("Unable to determine user's shell environment (see above)"); } else { - tracing::warn!("Skipping shell dotfile check! {}", msg); + tracing::warn!("Skipping shell dotfile check! (see above)"); return vec![]; } } }; - // Iterate over each dotfile and check if it is managed by nix - let mut managed: HashMap = HashMap::new(); + // Iterate over each dotfile and check if it is managed by Nix + let mut managed: HashMap<&'static str, PathBuf> = HashMap::new(); let mut unmanaged: Vec = Vec::new(); - for path in &shell.get_dotfiles() { - match resolve_symlink(path) { - Ok(Some(target)) => { - if super::direnv::is_path_in_nix_store(&target) { - managed.insert(path.clone(), target); - } else { - // A symlink to a non-nix path - unmanaged.push(path.clone()); - } - } - Ok(None) => { - // Not a symlink; cannot be managed by nix - unmanaged.push(path.clone()); - } - Err(err) => { - panic!("Dotfile {:?} symlink error: {:?}.", path, err); - } + for (name, path) in user_shell_env.dotfiles { + if super::direnv::is_path_in_nix_store(&path) { + managed.insert(name, path.clone()); + } else { + unmanaged.push(path.clone()); } } let title = "Shell dotfiles".to_string(); let info = format!( "Shell={:?}; HOME={:?}; Managed: {:?}; Unmanaged: {:?}", - shell, - get_home_dir(), - managed, - unmanaged + user_shell_env.shell, user_shell_env.home, managed, unmanaged ); let result = if !managed.is_empty() { CheckResult::Green } else { CheckResult::Red { - msg: format!("Default Shell: {:?} is not managed by Nix", shell), + msg: format!("Default Shell: {:?} is not managed by Nix", user_shell_env.shell), suggestion: "You can use `home-manager` to manage shell configuration. See ".to_string(), } }; @@ -91,37 +79,73 @@ impl Checkable for ShellCheck { } } +/// The shell environment of the current user +struct CurrentUserShellEnv { + /// The user's home directory + home: PathBuf, + /// Current shell + shell: Shell, + /// *Absolute* paths to the dotfiles + dotfiles: HashMap<&'static str, PathBuf>, +} + +impl CurrentUserShellEnv { + /// Get the current user's shell environment + fn new() -> Result { + let home = PathBuf::from(std::env::var("HOME")?); + let shell = Shell::current_shell()?; + let dotfiles = shell.get_dotfiles(&home)?; + let v = CurrentUserShellEnv { + home, + shell, + dotfiles, + }; + Ok(v) + } +} + +#[derive(thiserror::Error, Debug)] +enum ShellError { + #[error("IO error: {0}")] + Io(#[from] std::io::Error), + + #[error("Environment variable error: {0}")] + Var(#[from] std::env::VarError), + + #[error("Bad $SHELL value")] + BadShellPath, + + #[error("Unsupported shell. Please file an issue at ")] + UnsupportedShell, +} + /// An Unix shell #[derive(Debug, Serialize, Deserialize, Clone, Hash, Eq, PartialEq)] #[serde(rename_all = "lowercase")] -pub enum Shell { +enum Shell { Zsh, Bash, } impl Shell { /// Returns the user's current [Shell] - fn current_shell() -> Option { - let shell_path = - PathBuf::from(std::env::var("SHELL").expect("Environment variable `SHELL` not set")); + fn current_shell() -> Result { + let shell_path = PathBuf::from(std::env::var("SHELL")?); Self::from_path(shell_path) } /// Lookup [Shell] from the given executable path /// For example if path is `/bin/zsh`, it would return `Zsh` - fn from_path(exe_path: PathBuf) -> Option { + fn from_path(exe_path: PathBuf) -> Result { let shell_name = exe_path .file_name() - .expect("Path does not have a file name component") + .ok_or(ShellError::BadShellPath)? .to_string_lossy(); match shell_name.as_ref() { - "zsh" => Some(Shell::Zsh), - "bash" => Some(Shell::Bash), - _ => { - tracing::warn!("Unrecognized shell: {:?}. Please file an issue at ", exe_path); - None - } + "zsh" => Ok(Shell::Zsh), + "bash" => Ok(Shell::Bash), + _ => Err(ShellError::UnsupportedShell), } } @@ -134,28 +158,21 @@ impl Shell { } /// Get the currently existing dotfiles under $HOME - fn get_dotfiles(&self) -> Vec { - self.dotfile_names() - .iter() - .map(|dotfile| get_home_dir().join(dotfile)) - .filter(|path| path.exists()) - .collect() - } -} - -fn get_home_dir() -> PathBuf { - PathBuf::from(std::env::var("HOME").expect("Environment variable `HOME` not set")) -} - -/// Resolve symlink to its target path. -/// -/// Returns `None` if the path is not a symlink. This function can thus be used to both resolve and check if a path is a symlink. -fn resolve_symlink(path: &PathBuf) -> std::io::Result> { - let meta = std::fs::symlink_metadata(path)?; - if meta.file_type().is_symlink() { - let target = std::fs::read_link(path)?; - Ok(Some(target)) - } else { - Ok(None) + /// + /// Returned paths will be absolute (i.e., symlinks are resolved). + fn get_dotfiles(&self, home_dir: &Path) -> std::io::Result> { + let mut paths = HashMap::new(); + for dotfile in self.dotfile_names() { + match std::fs::canonicalize(home_dir.join(dotfile)) { + Ok(path) => { + paths.insert(dotfile, path); + } + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + // If file not found, skip + } + Err(err) => return Err(err), + } + } + Ok(paths) } }