Skip to content

Commit eaef718

Browse files
authored
Enhance Python environment handling with error reporting (#325)
Fixes #321 Improve handling of Python environments by adding error reporting for broken environments. Include tests to ensure functionality.
1 parent 1498e00 commit eaef718

File tree

7 files changed

+425
-2
lines changed

7 files changed

+425
-2
lines changed

.vscode/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
"git.branchProtectionPrompt": "alwaysCommitToNewBranch",
1212
"git.branchRandomName.enable": true,
1313
"chat.tools.terminal.autoApprove": {
14-
"cargo test": true
14+
"cargo test": true,
15+
"cargo fmt": true
1516
}
1617
}

crates/pet-core/src/python_environment.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ pub struct PythonEnvironment {
7070
// Some of the known symlinks for the environment.
7171
// E.g. in the case of Homebrew there are a number of symlinks that are created.
7272
pub symlinks: Option<Vec<PathBuf>>,
73+
/// An error message if the environment is known to be in a bad state.
74+
/// For example, when the Python executable is a broken symlink.
75+
/// If None, no known issues have been detected (but this doesn't guarantee
76+
/// the environment is fully functional - we don't spawn Python to verify).
77+
pub error: Option<String>,
7378
}
7479

7580
impl Ord for PythonEnvironment {
@@ -176,6 +181,9 @@ impl std::fmt::Display for PythonEnvironment {
176181
}
177182
}
178183
}
184+
if let Some(error) = &self.error {
185+
writeln!(f, " Error : {error}").unwrap_or_default();
186+
}
179187
Ok(())
180188
}
181189
}
@@ -194,6 +202,7 @@ pub struct PythonEnvironmentBuilder {
194202
project: Option<PathBuf>,
195203
arch: Option<Architecture>,
196204
symlinks: Option<Vec<PathBuf>>,
205+
error: Option<String>,
197206
}
198207

199208
impl PythonEnvironmentBuilder {
@@ -209,6 +218,7 @@ impl PythonEnvironmentBuilder {
209218
project: None,
210219
arch: None,
211220
symlinks: None,
221+
error: None,
212222
}
213223
}
214224
pub fn from_environment(env: PythonEnvironment) -> Self {
@@ -223,6 +233,7 @@ impl PythonEnvironmentBuilder {
223233
project: env.project,
224234
arch: env.arch,
225235
symlinks: env.symlinks,
236+
error: env.error,
226237
}
227238
}
228239

@@ -285,6 +296,11 @@ impl PythonEnvironmentBuilder {
285296
self
286297
}
287298

299+
pub fn error(mut self, error: Option<String>) -> Self {
300+
self.error = error;
301+
self
302+
}
303+
288304
fn update_symlinks_and_exe(&mut self, symlinks: Option<Vec<PathBuf>>) {
289305
let mut all = self.symlinks.clone().unwrap_or_default();
290306
if let Some(ref exe) = self.executable {
@@ -340,6 +356,7 @@ impl PythonEnvironmentBuilder {
340356
project: self.project,
341357
arch: self.arch,
342358
symlinks,
359+
error: self.error,
343360
}
344361
}
345362
}

crates/pet-pyenv/tests/pyenv_test.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ fn find_pyenv_envs() {
221221
home.to_str().unwrap(),
222222
".pyenv/versions/3.9.9/bin/python",
223223
])]),
224+
error: None,
224225
};
225226
let expected_virtual_env = PythonEnvironment {
226227
display_name: None,
@@ -242,6 +243,7 @@ fn find_pyenv_envs() {
242243
home.to_str().unwrap(),
243244
".pyenv/versions/my-virtual-env/bin/python",
244245
])]),
246+
error: None,
245247
};
246248
let expected_3_12_1 = PythonEnvironment {
247249
display_name: None,
@@ -263,6 +265,7 @@ fn find_pyenv_envs() {
263265
home.to_str().unwrap(),
264266
".pyenv/versions/3.12.1/bin/python",
265267
])]),
268+
error: None,
266269
};
267270
let expected_3_13_dev = PythonEnvironment {
268271
display_name: None,
@@ -284,6 +287,7 @@ fn find_pyenv_envs() {
284287
home.to_str().unwrap(),
285288
".pyenv/versions/3.13-dev/bin/python",
286289
])]),
290+
error: None,
287291
};
288292
let expected_3_12_1a3 = PythonEnvironment {
289293
display_name: None,
@@ -305,6 +309,7 @@ fn find_pyenv_envs() {
305309
home.to_str().unwrap(),
306310
".pyenv/versions/3.12.1a3/bin/python",
307311
])]),
312+
error: None,
308313
};
309314
let expected_no_gil = PythonEnvironment {
310315
display_name: None,
@@ -326,6 +331,7 @@ fn find_pyenv_envs() {
326331
home.to_str().unwrap(),
327332
".pyenv/versions/nogil-3.9.10-1/bin/python",
328333
])]),
334+
error: None,
329335
};
330336
let expected_pypy = PythonEnvironment {
331337
display_name: None,
@@ -347,6 +353,7 @@ fn find_pyenv_envs() {
347353
home.to_str().unwrap(),
348354
".pyenv/versions/pypy3.9-7.3.15/bin/python",
349355
])]),
356+
error: None,
350357
};
351358

352359
let expected_conda_root = PythonEnvironment {
@@ -360,6 +367,7 @@ fn find_pyenv_envs() {
360367
manager: Some(expected_conda_manager.clone()),
361368
arch: Some(Architecture::X64),
362369
symlinks: Some(vec![conda_dir.join("bin").join("python")]),
370+
error: None,
363371
};
364372
let expected_conda_one = PythonEnvironment {
365373
display_name: None,
@@ -372,6 +380,7 @@ fn find_pyenv_envs() {
372380
manager: Some(expected_conda_manager.clone()),
373381
arch: None,
374382
symlinks: Some(vec![conda_dir.join("envs").join("one").join("python")]),
383+
error: None,
375384
};
376385
let expected_conda_two = PythonEnvironment {
377386
display_name: None,
@@ -384,6 +393,7 @@ fn find_pyenv_envs() {
384393
manager: Some(expected_conda_manager.clone()),
385394
symlinks: Some(vec![conda_dir.join("envs").join("two").join("python")]),
386395
arch: None,
396+
error: None,
387397
};
388398

389399
let mut expected_envs = vec![
@@ -453,6 +463,7 @@ fn resolve_pyenv_environment() {
453463
manager: Some(expected_manager.clone()),
454464
arch: None,
455465
symlinks: Some(vec![executable]),
466+
error: None,
456467
};
457468
let expected_virtual_env = PythonEnvironment {
458469
display_name: None,
@@ -474,6 +485,7 @@ fn resolve_pyenv_environment() {
474485
home.to_str().unwrap(),
475486
".pyenv/versions/my-virtual-env/bin/python",
476487
])]),
488+
error: None,
477489
};
478490

479491
// Resolve regular Python installs in Pyenv

crates/pet-python-utils/src/executable.rs

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,31 @@ lazy_static! {
1717
Regex::new(r"python(\d+\.?)*$").expect("error parsing Unix executable regex");
1818
}
1919

20+
/// Checks if a path is a broken symlink (symlink that points to a non-existent target).
21+
/// Returns true if the path is a symlink and its target does not exist.
22+
pub fn is_broken_symlink(path: &Path) -> bool {
23+
// First check if it's a symlink using symlink_metadata (doesn't follow symlinks)
24+
if let Ok(metadata) = fs::symlink_metadata(path) {
25+
if metadata.file_type().is_symlink() {
26+
// Now check if the target exists using regular metadata (follows symlinks)
27+
// If this fails or returns false for exists(), then it's broken
28+
return !path.exists();
29+
}
30+
}
31+
false
32+
}
33+
34+
/// Result of looking for an executable in an environment path.
35+
#[derive(Debug, Clone)]
36+
pub enum ExecutableResult {
37+
/// A valid executable was found
38+
Found(PathBuf),
39+
/// An executable path exists but is broken (e.g., broken symlink)
40+
Broken(PathBuf),
41+
/// No executable was found
42+
NotFound,
43+
}
44+
2045
#[cfg(windows)]
2146
pub fn find_executable(env_path: &Path) -> Option<PathBuf> {
2247
[
@@ -43,6 +68,56 @@ pub fn find_executable(env_path: &Path) -> Option<PathBuf> {
4368
.find(|path| path.is_file())
4469
}
4570

71+
/// Finds an executable in the environment path, including broken symlinks.
72+
/// This is useful for detecting virtual environments that have broken Python executables.
73+
#[cfg(windows)]
74+
pub fn find_executable_or_broken(env_path: &Path) -> ExecutableResult {
75+
let candidates = [
76+
env_path.join("Scripts").join("python.exe"),
77+
env_path.join("Scripts").join("python3.exe"),
78+
env_path.join("bin").join("python.exe"),
79+
env_path.join("bin").join("python3.exe"),
80+
env_path.join("python.exe"),
81+
env_path.join("python3.exe"),
82+
];
83+
84+
// First try to find a valid executable
85+
if let Some(path) = candidates.iter().find(|path| path.is_file()) {
86+
return ExecutableResult::Found(path.clone());
87+
}
88+
89+
// Then check for broken symlinks
90+
if let Some(path) = candidates.iter().find(|path| is_broken_symlink(path)) {
91+
return ExecutableResult::Broken(path.clone());
92+
}
93+
94+
ExecutableResult::NotFound
95+
}
96+
97+
/// Finds an executable in the environment path, including broken symlinks.
98+
/// This is useful for detecting virtual environments that have broken Python executables.
99+
#[cfg(unix)]
100+
pub fn find_executable_or_broken(env_path: &Path) -> ExecutableResult {
101+
let candidates = [
102+
env_path.join("bin").join("python"),
103+
env_path.join("bin").join("python3"),
104+
env_path.join("python"),
105+
env_path.join("python3"),
106+
];
107+
108+
// First try to find a valid executable
109+
if let Some(path) = candidates.iter().find(|path| path.is_file()) {
110+
return ExecutableResult::Found(path.clone());
111+
}
112+
113+
// Then check for broken symlinks
114+
if let Some(path) = candidates.iter().find(|path| is_broken_symlink(path)) {
115+
return ExecutableResult::Broken(path.clone());
116+
}
117+
118+
ExecutableResult::NotFound
119+
}
120+
46121
pub fn find_executables<T: AsRef<Path>>(env_path: T) -> Vec<PathBuf> {
47122
let mut env_path = env_path.as_ref().to_path_buf();
48123
// Never find exes in pyenv shims folder, they are not valid exes.
@@ -306,4 +381,137 @@ mod tests {
306381
PathBuf::from("/home/user/project/shims").as_path()
307382
));
308383
}
384+
385+
#[test]
386+
fn test_is_broken_symlink_regular_file() {
387+
// A regular file should not be detected as a broken symlink
388+
let temp_dir = std::env::temp_dir();
389+
let test_file = temp_dir.join("pet_test_regular_file.txt");
390+
fs::write(&test_file, "test").unwrap();
391+
392+
assert!(!is_broken_symlink(&test_file));
393+
394+
let _ = fs::remove_file(&test_file);
395+
}
396+
397+
#[test]
398+
fn test_is_broken_symlink_nonexistent() {
399+
// A non-existent path should not be detected as a broken symlink
400+
let nonexistent = PathBuf::from("/this/path/does/not/exist/python");
401+
assert!(!is_broken_symlink(&nonexistent));
402+
}
403+
404+
#[test]
405+
#[cfg(unix)]
406+
fn test_is_broken_symlink_unix() {
407+
use std::os::unix::fs::symlink;
408+
409+
let temp_dir = std::env::temp_dir();
410+
let target = temp_dir.join("pet_test_symlink_target_nonexistent");
411+
let link = temp_dir.join("pet_test_broken_symlink");
412+
413+
// Clean up any previous test artifacts
414+
let _ = fs::remove_file(&link);
415+
let _ = fs::remove_file(&target);
416+
417+
// Create a symlink to a non-existent target
418+
symlink(&target, &link).unwrap();
419+
420+
// The symlink should be detected as broken
421+
assert!(is_broken_symlink(&link));
422+
423+
// Clean up
424+
let _ = fs::remove_file(&link);
425+
}
426+
427+
#[test]
428+
#[cfg(unix)]
429+
fn test_is_broken_symlink_valid_symlink() {
430+
use std::os::unix::fs::symlink;
431+
432+
let temp_dir = std::env::temp_dir();
433+
let target = temp_dir.join("pet_test_symlink_target_exists");
434+
let link = temp_dir.join("pet_test_valid_symlink");
435+
436+
// Clean up any previous test artifacts
437+
let _ = fs::remove_file(&link);
438+
let _ = fs::remove_file(&target);
439+
440+
// Create the target file
441+
fs::write(&target, "test").unwrap();
442+
443+
// Create a symlink to the existing target
444+
symlink(&target, &link).unwrap();
445+
446+
// The symlink should NOT be detected as broken
447+
assert!(!is_broken_symlink(&link));
448+
449+
// Clean up
450+
let _ = fs::remove_file(&link);
451+
let _ = fs::remove_file(&target);
452+
}
453+
454+
#[test]
455+
fn test_find_executable_or_broken_not_found() {
456+
let temp_dir = std::env::temp_dir().join("pet_test_empty_env");
457+
let _ = fs::create_dir_all(&temp_dir);
458+
459+
match find_executable_or_broken(&temp_dir) {
460+
ExecutableResult::NotFound => (),
461+
other => panic!("Expected NotFound, got {:?}", other),
462+
}
463+
464+
let _ = fs::remove_dir_all(&temp_dir);
465+
}
466+
467+
#[test]
468+
fn test_find_executable_or_broken_found() {
469+
let temp_dir = std::env::temp_dir().join("pet_test_valid_env");
470+
#[cfg(windows)]
471+
let bin_dir = temp_dir.join("Scripts");
472+
#[cfg(unix)]
473+
let bin_dir = temp_dir.join("bin");
474+
475+
let _ = fs::remove_dir_all(&temp_dir);
476+
fs::create_dir_all(&bin_dir).unwrap();
477+
478+
#[cfg(windows)]
479+
let python_exe = bin_dir.join("python.exe");
480+
#[cfg(unix)]
481+
let python_exe = bin_dir.join("python");
482+
483+
fs::write(&python_exe, "fake python").unwrap();
484+
485+
match find_executable_or_broken(&temp_dir) {
486+
ExecutableResult::Found(path) => assert_eq!(path, python_exe),
487+
other => panic!("Expected Found, got {:?}", other),
488+
}
489+
490+
let _ = fs::remove_dir_all(&temp_dir);
491+
}
492+
493+
#[test]
494+
#[cfg(unix)]
495+
fn test_find_executable_or_broken_broken_symlink() {
496+
use std::os::unix::fs::symlink;
497+
498+
let temp_dir = std::env::temp_dir().join("pet_test_broken_env");
499+
let bin_dir = temp_dir.join("bin");
500+
501+
let _ = fs::remove_dir_all(&temp_dir);
502+
fs::create_dir_all(&bin_dir).unwrap();
503+
504+
let python_exe = bin_dir.join("python");
505+
let nonexistent_target = PathBuf::from("/nonexistent/python3.10");
506+
507+
// Create a broken symlink
508+
symlink(&nonexistent_target, &python_exe).unwrap();
509+
510+
match find_executable_or_broken(&temp_dir) {
511+
ExecutableResult::Broken(path) => assert_eq!(path, python_exe),
512+
other => panic!("Expected Broken, got {:?}", other),
513+
}
514+
515+
let _ = fs::remove_dir_all(&temp_dir);
516+
}
309517
}

0 commit comments

Comments
 (0)