Skip to content

Commit e600bba

Browse files
committed
fix: improve symlink directory detection in environment finder
1 parent 248575c commit e600bba

File tree

3 files changed

+157
-3
lines changed

3 files changed

+157
-3
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/pet/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ lazy_static = "1.4.0"
5050

5151
[dev-dependencies]
5252
regex = "1.10.4"
53+
tempfile = "3.10"
5354

5455
[features]
5556
ci = []

crates/pet/src/find.rs

Lines changed: 155 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ pub fn find_and_report_envs(
162162
possible_environments.append(
163163
&mut reader
164164
.filter_map(Result::ok)
165-
.filter(|d| d.file_type().is_ok_and(|f| f.is_dir()))
165+
// Use path().is_dir() instead of file_type().is_dir() to follow symlinks
166+
// See: https://github.com/microsoft/python-environment-tools/issues/196
167+
.filter(|d| d.path().is_dir())
166168
.map(|p| p.path())
167169
.collect(),
168170
);
@@ -285,7 +287,8 @@ pub fn find_python_environments_in_workspace_folder_recursive(
285287
if let Ok(reader) = fs::read_dir(workspace_folder.join(".pixi").join("envs")) {
286288
reader
287289
.filter_map(Result::ok)
288-
.filter(|d| d.file_type().is_ok_and(|f| f.is_dir()))
290+
// Use path().is_dir() instead of file_type().is_dir() to follow symlinks
291+
.filter(|d| d.path().is_dir())
289292
.map(|p| p.path())
290293
.for_each(|p| paths_to_search_first.push(p));
291294
}
@@ -310,7 +313,8 @@ pub fn find_python_environments_in_workspace_folder_recursive(
310313
if let Ok(reader) = fs::read_dir(workspace_folder) {
311314
for folder in reader
312315
.filter_map(Result::ok)
313-
.filter(|d| d.file_type().is_ok_and(|f| f.is_dir()))
316+
// Use path().is_dir() instead of file_type().is_dir() to follow symlinks
317+
.filter(|d| d.path().is_dir())
314318
.map(|p| p.path())
315319
.filter(|p| {
316320
// If this directory is a sub directory or is in the environment_directories, then do not search in this directory.
@@ -431,3 +435,151 @@ pub fn identify_python_executables_using_locators(
431435
}
432436
}
433437
}
438+
439+
#[cfg(test)]
440+
mod tests {
441+
use std::fs;
442+
use tempfile::TempDir;
443+
444+
/// Test that `path().is_dir()` properly follows symlinks to directories.
445+
/// This is the fix for https://github.com/microsoft/python-environment-tools/issues/196
446+
///
447+
/// The issue was that `DirEntry::file_type().is_dir()` returns false for symlinks
448+
/// to directories on Unix, causing symlinked virtual environments to be missed.
449+
#[test]
450+
#[cfg(unix)]
451+
fn test_symlinked_directory_is_detected() {
452+
use std::os::unix::fs::symlink;
453+
454+
// Create temporary directories
455+
let tmp = TempDir::new().expect("Failed to create temp dir");
456+
let target_dir = tmp.path().join("actual_venv");
457+
let container_dir = tmp.path().join("envs");
458+
let symlink_dir = container_dir.join("linked_venv");
459+
460+
// Create the target directory (simulating a venv)
461+
fs::create_dir_all(&target_dir).expect("Failed to create target dir");
462+
fs::create_dir_all(&container_dir).expect("Failed to create container dir");
463+
464+
// Create a symlink from envs/linked_venv -> actual_venv
465+
symlink(&target_dir, &symlink_dir).expect("Failed to create symlink");
466+
467+
// Verify the symlink was created
468+
assert!(symlink_dir.exists(), "Symlink should exist");
469+
470+
// Test that path().is_dir() follows the symlink
471+
let entries: Vec<_> = fs::read_dir(&container_dir)
472+
.expect("Failed to read dir")
473+
.filter_map(Result::ok)
474+
.collect();
475+
476+
assert_eq!(entries.len(), 1, "Should have one entry");
477+
478+
let entry = &entries[0];
479+
480+
// This is the OLD behavior that caused the bug:
481+
// file_type().is_dir() does NOT follow symlinks
482+
let file_type_is_dir = entry.file_type().is_ok_and(|ft| ft.is_dir());
483+
assert!(
484+
!file_type_is_dir,
485+
"file_type().is_dir() should return false for symlinks (this is the bug)"
486+
);
487+
488+
// This is the NEW behavior that fixes the bug:
489+
// path().is_dir() DOES follow symlinks
490+
let path_is_dir = entry.path().is_dir();
491+
assert!(
492+
path_is_dir,
493+
"path().is_dir() should return true for symlinks to directories"
494+
);
495+
}
496+
497+
/// Test that regular directories still work with the fix
498+
#[test]
499+
fn test_regular_directory_is_detected() {
500+
let tmp = TempDir::new().expect("Failed to create temp dir");
501+
let container_dir = tmp.path().join("envs");
502+
let sub_dir = container_dir.join("my_venv");
503+
504+
fs::create_dir_all(&sub_dir).expect("Failed to create dirs");
505+
506+
let entries: Vec<_> = fs::read_dir(&container_dir)
507+
.expect("Failed to read dir")
508+
.filter_map(Result::ok)
509+
.filter(|d| d.path().is_dir())
510+
.collect();
511+
512+
assert_eq!(entries.len(), 1, "Should detect the regular directory");
513+
assert!(
514+
entries[0].path().ends_with("my_venv"),
515+
"Should be the my_venv directory"
516+
);
517+
}
518+
519+
/// Test that files are not incorrectly detected as directories
520+
#[test]
521+
fn test_file_is_not_detected_as_directory() {
522+
let tmp = TempDir::new().expect("Failed to create temp dir");
523+
let container_dir = tmp.path().join("envs");
524+
let file_path = container_dir.join("some_file.txt");
525+
526+
fs::create_dir_all(&container_dir).expect("Failed to create dirs");
527+
fs::write(&file_path, "test content").expect("Failed to write file");
528+
529+
let dirs: Vec<_> = fs::read_dir(&container_dir)
530+
.expect("Failed to read dir")
531+
.filter_map(Result::ok)
532+
.filter(|d| d.path().is_dir())
533+
.collect();
534+
535+
assert!(dirs.is_empty(), "Should not detect files as directories");
536+
}
537+
538+
/// Test symlinked directory scenario matching the original issue:
539+
/// User has ~/envs with symlinks to venvs in other locations
540+
#[test]
541+
#[cfg(unix)]
542+
fn test_symlinked_venv_in_envs_directory() {
543+
use std::os::unix::fs::symlink;
544+
545+
let tmp = TempDir::new().expect("Failed to create temp dir");
546+
547+
// Simulate user's actual venv location
548+
let project_dir = tmp.path().join("projects").join("myproject");
549+
let actual_venv = project_dir.join(".venv");
550+
551+
// Simulate ~/envs directory with symlink
552+
let envs_dir = tmp.path().join("envs");
553+
let symlinked_venv = envs_dir.join("myproject_venv");
554+
555+
// Create the actual venv structure
556+
fs::create_dir_all(actual_venv.join("bin")).expect("Failed to create venv");
557+
fs::write(actual_venv.join("bin").join("python"), "").expect("Failed to create python");
558+
fs::write(actual_venv.join("pyvenv.cfg"), "home = /usr/bin")
559+
.expect("Failed to create pyvenv.cfg");
560+
561+
// Create envs directory with symlink
562+
fs::create_dir_all(&envs_dir).expect("Failed to create envs dir");
563+
symlink(&actual_venv, &symlinked_venv).expect("Failed to create symlink");
564+
565+
// The fix ensures this symlinked directory is discovered
566+
let discovered: Vec<_> = fs::read_dir(&envs_dir)
567+
.expect("Failed to read envs dir")
568+
.filter_map(Result::ok)
569+
.filter(|d| d.path().is_dir()) // The fix: using path().is_dir()
570+
.map(|d| d.path())
571+
.collect();
572+
573+
assert_eq!(discovered.len(), 1, "Should discover the symlinked venv");
574+
assert_eq!(
575+
discovered[0], symlinked_venv,
576+
"Should be the symlinked venv path"
577+
);
578+
579+
// Verify it's actually a venv by checking for pyvenv.cfg
580+
assert!(
581+
discovered[0].join("pyvenv.cfg").exists(),
582+
"Symlink should point to a valid venv"
583+
);
584+
}
585+
}

0 commit comments

Comments
 (0)