Conversation
- find.rs: resolve_dot_venv returns Option<PathBuf>, use if-let to conditionally insert into search paths vec - environment_locations.rs: fix bug where ? operator on None would short-circuit the function, losing already-collected environments - path.rs: remove trailing whitespace in doc comments (fmt check) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Performance Report (Linux) ✅
Legend
|
Performance Report (macOS)
Legend
|
Test Coverage Report (Linux)
Coverage increased! Great work! |
Test Coverage Report (Windows)
Coverage increased! Great work! |
Performance Report (Windows) ✅
Legend
|
There was a problem hiding this comment.
Pull request overview
Adds first-class support for project-local .venv discovery across PET, including the PEP 832 convention where .venv is a file pointing to a virtual environment directory, and integrates this resolution into workspace scanning and relevant locators.
Changes:
- Introduce
pet_fs::path::resolve_dot_venv()to resolve.venvas either a directory or a PEP 832 path file. - Update workspace-folder search ordering in
petto prioritize resolved.venvpaths. - Update
pet-uvandpet-poetryto use the shared.venvresolution logic instead of direct.venvdirectory checks, and add tests for the resolver.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/pet-fs/src/path.rs | Adds resolve_dot_venv() plus unit tests for directory- and file-based .venv resolution. |
| crates/pet/src/find.rs | Uses resolve_dot_venv() to prioritize .venv during workspace recursive discovery. |
| crates/pet-uv/src/lib.rs | Uses resolve_dot_venv() when building uv workspace/project environments from .venv. |
| crates/pet-poetry/src/environment_locations.rs | Uses resolve_dot_venv() when considering project-local Poetry environments. |
Member
|
@edvilme Are you still working on this? |
…uracy - 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>
Collaborator
Author
Hello. Yes, I will be updating it as the discussions on the PEP evolve. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces robust support for resolving
.venventries in project directories, following both the traditional convention (where.venvis a directory) and PEP 832 (where.venvis a file pointing to the virtual environment path). The new utility functionresolve_dot_venvis added inpet-fs, and is integrated throughout the codebase to ensure consistent and correct detection of virtual environments. Comprehensive tests are also included to validate the new logic.Core functionality:
resolve_dot_venvfunction topet-fs, which resolves.venvas either a directory or a file (per PEP 832), handling absolute and relative paths and verifying the existence of the resolved directory.resolve_dot_venv, covering all major cases: missing.venv, directory, file with absolute/relative path, whitespace trimming, non-existent targets, and file targets.Integration into environment discovery:
pet-poetryandpet-uvto useresolve_dot_venvfor detecting project-local virtual environments, replacing previous direct.venvdirectory checks. This ensures compatibility with both directory and file-based.venvsetups. [1] [2] [3] [4] [5] [6]petto prioritize.venvas resolved byresolve_dot_venv, improving accuracy and supporting the new convention. [1] [2]