Skip to content

Commit 7020c0d

Browse files
edvilmeCopilot
andcommitted
Address PR feedback: fix symlink handling, empty .venv guard, doc accuracy
- Use fs::metadata instead of symlink_metadata to follow symlinks, so .venv symlinks to directories are correctly treated as directories - Add empty-string guard after trimming .venv file content to prevent whitespace-only files from resolving to the project directory - Fix doc comment to accurately describe that relative paths are not canonicalized - Add tests for whitespace-only .venv file and symlink-to-directory cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 427439c commit 7020c0d

1 file changed

Lines changed: 43 additions & 2 deletions

File tree

crates/pet-fs/src/path.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,18 +343,23 @@ fn get_user_home() -> Option<PathBuf> {
343343
/// 1. If `<dir>/.venv` is a directory, return that path.
344344
/// 2. If `<dir>/.venv` is a file, read its contents, trim whitespaces, and resolve the path:
345345
/// - If the path is absolute, return it.
346-
/// - If the path is relative, resolve it against `<dir>` and return the absolute path.
346+
/// - If the path is relative, resolve it against `<dir>` using `dir.join(path)`.
347+
/// The returned path is not further canonicalized, so it may remain relative if `<dir>`
348+
/// is relative and may preserve `.` or `..` components.
347349
/// - If the resolved path does not exist or is not a directory, return `None`.
348350
/// 3. If `<dir>/.venv` does not exist, return `None`.
349351
///
350352
/// See: <https://www.python.org/dev/peps/pep-0832/#specification>
351353
pub fn resolve_dot_venv(dir: &Path) -> Option<PathBuf> {
352354
let dot_venv = dir.join(".venv");
353-
let meta = std::fs::symlink_metadata(&dot_venv).ok()?;
355+
let meta = std::fs::metadata(&dot_venv).ok()?;
354356
if meta.is_dir() {
355357
Some(dot_venv)
356358
} else if meta.is_file() {
357359
let content = std::fs::read_to_string(&dot_venv).ok()?.trim().to_string();
360+
if content.is_empty() {
361+
return None;
362+
}
358363
let path = PathBuf::from(content);
359364
let resolved_path = if path.is_absolute() {
360365
path
@@ -842,6 +847,42 @@ mod tests {
842847
let _ = std::fs::remove_dir_all(&test_dir);
843848
}
844849

850+
#[test]
851+
fn test_resolve_dot_venv_file_whitespace_only_returns_none() {
852+
// A whitespace-only .venv file should not resolve to the project directory
853+
let temp_dir = std::env::temp_dir();
854+
let test_dir = temp_dir.join("pet_test_dot_venv_whitespace_only");
855+
856+
let _ = std::fs::remove_dir_all(&test_dir);
857+
std::fs::create_dir_all(&test_dir).expect("Failed to create test dir");
858+
859+
let dot_venv_file = test_dir.join(".venv");
860+
std::fs::write(&dot_venv_file, " \n\t ").expect("Failed to write .venv file");
861+
862+
assert_eq!(resolve_dot_venv(&test_dir), None);
863+
864+
let _ = std::fs::remove_dir_all(&test_dir);
865+
}
866+
867+
#[cfg(unix)]
868+
#[test]
869+
fn test_resolve_dot_venv_symlink_to_directory() {
870+
// A .venv symlink to a directory should be treated like a directory
871+
let temp_dir = std::env::temp_dir();
872+
let test_dir = temp_dir.join("pet_test_dot_venv_symlink_dir");
873+
let target_venv = test_dir.join("actual_venv");
874+
let dot_venv = test_dir.join(".venv");
875+
876+
let _ = std::fs::remove_dir_all(&test_dir);
877+
std::fs::create_dir_all(&target_venv).expect("Failed to create target venv dir");
878+
std::os::unix::fs::symlink(&target_venv, &dot_venv)
879+
.expect("Failed to create .venv symlink");
880+
881+
assert_eq!(resolve_dot_venv(&test_dir), Some(dot_venv.clone()));
882+
883+
let _ = std::fs::remove_dir_all(&test_dir);
884+
}
885+
845886
#[test]
846887
fn test_resolve_dot_venv_file_points_to_file_not_dir() {
847888
// If .venv file points to a path that exists but is a file (not a dir), return None

0 commit comments

Comments
 (0)