diff --git a/.vscode/settings.json b/.vscode/settings.json index 9262f343..5ca3cc78 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -11,6 +11,7 @@ "git.branchProtectionPrompt": "alwaysCommitToNewBranch", "git.branchRandomName.enable": true, "chat.tools.terminal.autoApprove": { - "cargo test": true + "cargo test": true, + "cargo fmt": true } } diff --git a/crates/pet-core/src/python_environment.rs b/crates/pet-core/src/python_environment.rs index 361ec66b..50b99dcf 100644 --- a/crates/pet-core/src/python_environment.rs +++ b/crates/pet-core/src/python_environment.rs @@ -70,6 +70,11 @@ pub struct PythonEnvironment { // Some of the known symlinks for the environment. // E.g. in the case of Homebrew there are a number of symlinks that are created. pub symlinks: Option>, + /// An error message if the environment is known to be in a bad state. + /// For example, when the Python executable is a broken symlink. + /// If None, no known issues have been detected (but this doesn't guarantee + /// the environment is fully functional - we don't spawn Python to verify). + pub error: Option, } impl Ord for PythonEnvironment { @@ -176,6 +181,9 @@ impl std::fmt::Display for PythonEnvironment { } } } + if let Some(error) = &self.error { + writeln!(f, " Error : {error}").unwrap_or_default(); + } Ok(()) } } @@ -194,6 +202,7 @@ pub struct PythonEnvironmentBuilder { project: Option, arch: Option, symlinks: Option>, + error: Option, } impl PythonEnvironmentBuilder { @@ -209,6 +218,7 @@ impl PythonEnvironmentBuilder { project: None, arch: None, symlinks: None, + error: None, } } pub fn from_environment(env: PythonEnvironment) -> Self { @@ -223,6 +233,7 @@ impl PythonEnvironmentBuilder { project: env.project, arch: env.arch, symlinks: env.symlinks, + error: env.error, } } @@ -285,6 +296,11 @@ impl PythonEnvironmentBuilder { self } + pub fn error(mut self, error: Option) -> Self { + self.error = error; + self + } + fn update_symlinks_and_exe(&mut self, symlinks: Option>) { let mut all = self.symlinks.clone().unwrap_or_default(); if let Some(ref exe) = self.executable { @@ -340,6 +356,7 @@ impl PythonEnvironmentBuilder { project: self.project, arch: self.arch, symlinks, + error: self.error, } } } diff --git a/crates/pet-pyenv/tests/pyenv_test.rs b/crates/pet-pyenv/tests/pyenv_test.rs index 0a79ccfe..69706f31 100644 --- a/crates/pet-pyenv/tests/pyenv_test.rs +++ b/crates/pet-pyenv/tests/pyenv_test.rs @@ -221,6 +221,7 @@ fn find_pyenv_envs() { home.to_str().unwrap(), ".pyenv/versions/3.9.9/bin/python", ])]), + error: None, }; let expected_virtual_env = PythonEnvironment { display_name: None, @@ -242,6 +243,7 @@ fn find_pyenv_envs() { home.to_str().unwrap(), ".pyenv/versions/my-virtual-env/bin/python", ])]), + error: None, }; let expected_3_12_1 = PythonEnvironment { display_name: None, @@ -263,6 +265,7 @@ fn find_pyenv_envs() { home.to_str().unwrap(), ".pyenv/versions/3.12.1/bin/python", ])]), + error: None, }; let expected_3_13_dev = PythonEnvironment { display_name: None, @@ -284,6 +287,7 @@ fn find_pyenv_envs() { home.to_str().unwrap(), ".pyenv/versions/3.13-dev/bin/python", ])]), + error: None, }; let expected_3_12_1a3 = PythonEnvironment { display_name: None, @@ -305,6 +309,7 @@ fn find_pyenv_envs() { home.to_str().unwrap(), ".pyenv/versions/3.12.1a3/bin/python", ])]), + error: None, }; let expected_no_gil = PythonEnvironment { display_name: None, @@ -326,6 +331,7 @@ fn find_pyenv_envs() { home.to_str().unwrap(), ".pyenv/versions/nogil-3.9.10-1/bin/python", ])]), + error: None, }; let expected_pypy = PythonEnvironment { display_name: None, @@ -347,6 +353,7 @@ fn find_pyenv_envs() { home.to_str().unwrap(), ".pyenv/versions/pypy3.9-7.3.15/bin/python", ])]), + error: None, }; let expected_conda_root = PythonEnvironment { @@ -360,6 +367,7 @@ fn find_pyenv_envs() { manager: Some(expected_conda_manager.clone()), arch: Some(Architecture::X64), symlinks: Some(vec![conda_dir.join("bin").join("python")]), + error: None, }; let expected_conda_one = PythonEnvironment { display_name: None, @@ -372,6 +380,7 @@ fn find_pyenv_envs() { manager: Some(expected_conda_manager.clone()), arch: None, symlinks: Some(vec![conda_dir.join("envs").join("one").join("python")]), + error: None, }; let expected_conda_two = PythonEnvironment { display_name: None, @@ -384,6 +393,7 @@ fn find_pyenv_envs() { manager: Some(expected_conda_manager.clone()), symlinks: Some(vec![conda_dir.join("envs").join("two").join("python")]), arch: None, + error: None, }; let mut expected_envs = vec![ @@ -453,6 +463,7 @@ fn resolve_pyenv_environment() { manager: Some(expected_manager.clone()), arch: None, symlinks: Some(vec![executable]), + error: None, }; let expected_virtual_env = PythonEnvironment { display_name: None, @@ -474,6 +485,7 @@ fn resolve_pyenv_environment() { home.to_str().unwrap(), ".pyenv/versions/my-virtual-env/bin/python", ])]), + error: None, }; // Resolve regular Python installs in Pyenv diff --git a/crates/pet-python-utils/src/executable.rs b/crates/pet-python-utils/src/executable.rs index 32ad6780..cea3337a 100644 --- a/crates/pet-python-utils/src/executable.rs +++ b/crates/pet-python-utils/src/executable.rs @@ -17,6 +17,31 @@ lazy_static! { Regex::new(r"python(\d+\.?)*$").expect("error parsing Unix executable regex"); } +/// Checks if a path is a broken symlink (symlink that points to a non-existent target). +/// Returns true if the path is a symlink and its target does not exist. +pub fn is_broken_symlink(path: &Path) -> bool { + // First check if it's a symlink using symlink_metadata (doesn't follow symlinks) + if let Ok(metadata) = fs::symlink_metadata(path) { + if metadata.file_type().is_symlink() { + // Now check if the target exists using regular metadata (follows symlinks) + // If this fails or returns false for exists(), then it's broken + return !path.exists(); + } + } + false +} + +/// Result of looking for an executable in an environment path. +#[derive(Debug, Clone)] +pub enum ExecutableResult { + /// A valid executable was found + Found(PathBuf), + /// An executable path exists but is broken (e.g., broken symlink) + Broken(PathBuf), + /// No executable was found + NotFound, +} + #[cfg(windows)] pub fn find_executable(env_path: &Path) -> Option { [ @@ -43,6 +68,56 @@ pub fn find_executable(env_path: &Path) -> Option { .find(|path| path.is_file()) } +/// Finds an executable in the environment path, including broken symlinks. +/// This is useful for detecting virtual environments that have broken Python executables. +#[cfg(windows)] +pub fn find_executable_or_broken(env_path: &Path) -> ExecutableResult { + let candidates = [ + env_path.join("Scripts").join("python.exe"), + env_path.join("Scripts").join("python3.exe"), + env_path.join("bin").join("python.exe"), + env_path.join("bin").join("python3.exe"), + env_path.join("python.exe"), + env_path.join("python3.exe"), + ]; + + // First try to find a valid executable + if let Some(path) = candidates.iter().find(|path| path.is_file()) { + return ExecutableResult::Found(path.clone()); + } + + // Then check for broken symlinks + if let Some(path) = candidates.iter().find(|path| is_broken_symlink(path)) { + return ExecutableResult::Broken(path.clone()); + } + + ExecutableResult::NotFound +} + +/// Finds an executable in the environment path, including broken symlinks. +/// This is useful for detecting virtual environments that have broken Python executables. +#[cfg(unix)] +pub fn find_executable_or_broken(env_path: &Path) -> ExecutableResult { + let candidates = [ + env_path.join("bin").join("python"), + env_path.join("bin").join("python3"), + env_path.join("python"), + env_path.join("python3"), + ]; + + // First try to find a valid executable + if let Some(path) = candidates.iter().find(|path| path.is_file()) { + return ExecutableResult::Found(path.clone()); + } + + // Then check for broken symlinks + if let Some(path) = candidates.iter().find(|path| is_broken_symlink(path)) { + return ExecutableResult::Broken(path.clone()); + } + + ExecutableResult::NotFound +} + pub fn find_executables>(env_path: T) -> Vec { let mut env_path = env_path.as_ref().to_path_buf(); // Never find exes in pyenv shims folder, they are not valid exes. @@ -306,4 +381,137 @@ mod tests { PathBuf::from("/home/user/project/shims").as_path() )); } + + #[test] + fn test_is_broken_symlink_regular_file() { + // A regular file should not be detected as a broken symlink + let temp_dir = std::env::temp_dir(); + let test_file = temp_dir.join("pet_test_regular_file.txt"); + fs::write(&test_file, "test").unwrap(); + + assert!(!is_broken_symlink(&test_file)); + + let _ = fs::remove_file(&test_file); + } + + #[test] + fn test_is_broken_symlink_nonexistent() { + // A non-existent path should not be detected as a broken symlink + let nonexistent = PathBuf::from("/this/path/does/not/exist/python"); + assert!(!is_broken_symlink(&nonexistent)); + } + + #[test] + #[cfg(unix)] + fn test_is_broken_symlink_unix() { + use std::os::unix::fs::symlink; + + let temp_dir = std::env::temp_dir(); + let target = temp_dir.join("pet_test_symlink_target_nonexistent"); + let link = temp_dir.join("pet_test_broken_symlink"); + + // Clean up any previous test artifacts + let _ = fs::remove_file(&link); + let _ = fs::remove_file(&target); + + // Create a symlink to a non-existent target + symlink(&target, &link).unwrap(); + + // The symlink should be detected as broken + assert!(is_broken_symlink(&link)); + + // Clean up + let _ = fs::remove_file(&link); + } + + #[test] + #[cfg(unix)] + fn test_is_broken_symlink_valid_symlink() { + use std::os::unix::fs::symlink; + + let temp_dir = std::env::temp_dir(); + let target = temp_dir.join("pet_test_symlink_target_exists"); + let link = temp_dir.join("pet_test_valid_symlink"); + + // Clean up any previous test artifacts + let _ = fs::remove_file(&link); + let _ = fs::remove_file(&target); + + // Create the target file + fs::write(&target, "test").unwrap(); + + // Create a symlink to the existing target + symlink(&target, &link).unwrap(); + + // The symlink should NOT be detected as broken + assert!(!is_broken_symlink(&link)); + + // Clean up + let _ = fs::remove_file(&link); + let _ = fs::remove_file(&target); + } + + #[test] + fn test_find_executable_or_broken_not_found() { + let temp_dir = std::env::temp_dir().join("pet_test_empty_env"); + let _ = fs::create_dir_all(&temp_dir); + + match find_executable_or_broken(&temp_dir) { + ExecutableResult::NotFound => (), + other => panic!("Expected NotFound, got {:?}", other), + } + + let _ = fs::remove_dir_all(&temp_dir); + } + + #[test] + fn test_find_executable_or_broken_found() { + let temp_dir = std::env::temp_dir().join("pet_test_valid_env"); + #[cfg(windows)] + let bin_dir = temp_dir.join("Scripts"); + #[cfg(unix)] + let bin_dir = temp_dir.join("bin"); + + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&bin_dir).unwrap(); + + #[cfg(windows)] + let python_exe = bin_dir.join("python.exe"); + #[cfg(unix)] + let python_exe = bin_dir.join("python"); + + fs::write(&python_exe, "fake python").unwrap(); + + match find_executable_or_broken(&temp_dir) { + ExecutableResult::Found(path) => assert_eq!(path, python_exe), + other => panic!("Expected Found, got {:?}", other), + } + + let _ = fs::remove_dir_all(&temp_dir); + } + + #[test] + #[cfg(unix)] + fn test_find_executable_or_broken_broken_symlink() { + use std::os::unix::fs::symlink; + + let temp_dir = std::env::temp_dir().join("pet_test_broken_env"); + let bin_dir = temp_dir.join("bin"); + + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&bin_dir).unwrap(); + + let python_exe = bin_dir.join("python"); + let nonexistent_target = PathBuf::from("/nonexistent/python3.10"); + + // Create a broken symlink + symlink(&nonexistent_target, &python_exe).unwrap(); + + match find_executable_or_broken(&temp_dir) { + ExecutableResult::Broken(path) => assert_eq!(path, python_exe), + other => panic!("Expected Broken, got {:?}", other), + } + + let _ = fs::remove_dir_all(&temp_dir); + } } diff --git a/crates/pet-venv/src/lib.rs b/crates/pet-venv/src/lib.rs index 5ca5575c..852faad2 100644 --- a/crates/pet-venv/src/lib.rs +++ b/crates/pet-venv/src/lib.rs @@ -10,7 +10,7 @@ use pet_core::{ reporter::Reporter, Locator, LocatorKind, }; -use pet_python_utils::executable::find_executables; +use pet_python_utils::executable::{find_executable_or_broken, find_executables, ExecutableResult}; use pet_python_utils::version; fn is_venv_internal(env: &PythonEnv) -> Option { @@ -26,6 +26,54 @@ pub fn is_venv(env: &PythonEnv) -> bool { pub fn is_venv_dir(path: &Path) -> bool { PyVenvCfg::find(path).is_some() } + +/// Tries to create a PythonEnvironment from a directory that might be a venv. +/// This function can detect broken environments (e.g., with broken symlinks) +/// and will return them with an error field set. +pub fn try_environment_from_venv_dir(path: &Path) -> Option { + // Check if this is a venv directory + let cfg = PyVenvCfg::find(path)?; + + let prefix = path.to_path_buf(); + let version = version::from_creator_for_virtual_env(&prefix).or(Some(cfg.version.clone())); + let name = cfg.prompt; + + match find_executable_or_broken(path) { + ExecutableResult::Found(executable) => { + let symlinks = find_executables(&prefix); + Some( + PythonEnvironmentBuilder::new(Some(PythonEnvironmentKind::Venv)) + .name(name) + .executable(Some(executable)) + .version(version) + .prefix(Some(prefix)) + .symlinks(Some(symlinks)) + .build(), + ) + } + ExecutableResult::Broken(executable) => Some( + PythonEnvironmentBuilder::new(Some(PythonEnvironmentKind::Venv)) + .name(name) + .executable(Some(executable)) + .version(version) + .prefix(Some(prefix)) + .error(Some("Python executable is a broken symlink".to_string())) + .build(), + ), + ExecutableResult::NotFound => { + // pyvenv.cfg exists but no Python executable found at all + Some( + PythonEnvironmentBuilder::new(Some(PythonEnvironmentKind::Venv)) + .name(name) + .version(version) + .prefix(Some(prefix)) + .error(Some("Python executable not found".to_string())) + .build(), + ) + } + } +} + pub struct Venv {} impl Venv { @@ -88,3 +136,125 @@ impl Locator for Venv { // We expect the user of this class to call `is_compatible` } } + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + #[test] + fn test_try_environment_from_venv_dir_not_a_venv() { + // A directory without pyvenv.cfg should return None + let temp_dir = std::env::temp_dir().join("pet_test_not_a_venv"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).unwrap(); + + let result = try_environment_from_venv_dir(&temp_dir); + assert!(result.is_none()); + + let _ = fs::remove_dir_all(&temp_dir); + } + + #[test] + fn test_try_environment_from_venv_dir_missing_executable() { + // A venv with pyvenv.cfg but no Python executable + let temp_dir = std::env::temp_dir().join("pet_test_venv_no_exe"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).unwrap(); + + // Create pyvenv.cfg + fs::write( + temp_dir.join("pyvenv.cfg"), + "version = 3.10.0\nprompt = test-env\n", + ) + .unwrap(); + + let result = try_environment_from_venv_dir(&temp_dir); + assert!(result.is_some()); + + let env = result.unwrap(); + assert_eq!(env.kind, Some(PythonEnvironmentKind::Venv)); + assert!(env.error.is_some()); + assert!(env.error.unwrap().contains("not found")); + assert_eq!(env.name, Some("test-env".to_string())); + + let _ = fs::remove_dir_all(&temp_dir); + } + + #[test] + fn test_try_environment_from_venv_dir_valid() { + // A valid venv with pyvenv.cfg and Python executable + let temp_dir = std::env::temp_dir().join("pet_test_venv_valid"); + let _ = fs::remove_dir_all(&temp_dir); + + #[cfg(windows)] + let bin_dir = temp_dir.join("Scripts"); + #[cfg(unix)] + let bin_dir = temp_dir.join("bin"); + + fs::create_dir_all(&bin_dir).unwrap(); + + // Create pyvenv.cfg + fs::write( + temp_dir.join("pyvenv.cfg"), + "version = 3.11.0\nprompt = my-project\n", + ) + .unwrap(); + + // Create python executable + #[cfg(windows)] + let python_exe = bin_dir.join("python.exe"); + #[cfg(unix)] + let python_exe = bin_dir.join("python"); + + fs::write(&python_exe, "fake python").unwrap(); + + let result = try_environment_from_venv_dir(&temp_dir); + assert!(result.is_some()); + + let env = result.unwrap(); + assert_eq!(env.kind, Some(PythonEnvironmentKind::Venv)); + assert!(env.error.is_none()); + assert!(env.executable.is_some()); + assert_eq!(env.name, Some("my-project".to_string())); + + let _ = fs::remove_dir_all(&temp_dir); + } + + #[test] + #[cfg(unix)] + fn test_try_environment_from_venv_dir_broken_symlink() { + use std::os::unix::fs::symlink; + + // A venv with pyvenv.cfg but a broken symlink for Python + let temp_dir = std::env::temp_dir().join("pet_test_venv_broken_symlink"); + let _ = fs::remove_dir_all(&temp_dir); + + let bin_dir = temp_dir.join("bin"); + fs::create_dir_all(&bin_dir).unwrap(); + + // Create pyvenv.cfg + fs::write( + temp_dir.join("pyvenv.cfg"), + "version = 3.9.0\nprompt = broken-env\n", + ) + .unwrap(); + + // Create a broken symlink + let python_exe = bin_dir.join("python"); + let nonexistent_target = std::path::PathBuf::from("/nonexistent/python3.9"); + symlink(&nonexistent_target, &python_exe).unwrap(); + + let result = try_environment_from_venv_dir(&temp_dir); + assert!(result.is_some()); + + let env = result.unwrap(); + assert_eq!(env.kind, Some(PythonEnvironmentKind::Venv)); + assert!(env.error.is_some()); + assert!(env.error.as_ref().unwrap().contains("broken symlink")); + assert_eq!(env.name, Some("broken-env".to_string())); + assert!(env.executable.is_some()); + + let _ = fs::remove_dir_all(&temp_dir); + } +} diff --git a/crates/pet/src/find.rs b/crates/pet/src/find.rs index 9361ff0e..129aac4e 100644 --- a/crates/pet/src/find.rs +++ b/crates/pet/src/find.rs @@ -14,6 +14,7 @@ use pet_pixi::is_pixi_env; use pet_python_utils::executable::{ find_executable, find_executables, should_search_for_environments_in_path, }; +use pet_venv::try_environment_from_venv_dir; use pet_virtualenv::is_virtualenv_dir; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; @@ -374,6 +375,13 @@ fn find_python_environments_in_paths_with_locators( if let Some(executable) = find_executable(path) { vec![executable] } else { + // No valid executable found. Check if this is a broken venv. + // If so, report it with an error instead of silently skipping. + if let Some(broken_env) = try_environment_from_venv_dir(path) { + if broken_env.error.is_some() { + reporter.report_environment(&broken_env); + } + } vec![] } } else { diff --git a/docs/JSONRPC.md b/docs/JSONRPC.md index dec64c64..0e4a3cff 100644 --- a/docs/JSONRPC.md +++ b/docs/JSONRPC.md @@ -242,6 +242,13 @@ interface Environment { * Thats because there could be multiple conda installations on the system, hence we try not to make any assumptions. */ manager?: Manager; + /** + * An error message if the environment is known to be in a bad state. + * For example: "Python executable is a broken symlink" + * If undefined, no known issues have been detected (but this doesn't guarantee + * the environment is fully functional - we don't spawn Python to verify). + */ + error?: string; } interface Manager {