FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Discover and resolve .venv files (PEP 832) by edvilme · Pull Request #433 · microsoft/python-environment-tools · GitHub

/ python-environment-tools Public

Discover and resolve .venv files (PEP 832) - #433

Draft
Eduardo Villalpando Mello (edvilme) wants to merge 7 commits into
mainfrom
pep832
ConversationCommits (7)ChecksFiles changed
Draft

Discover and resolve .venv files (PEP 832)#433
Eduardo Villalpando Mello (edvilme) wants to merge 7 commits into
mainfrom
pep832

Conversation

Eduardo Villalpando Mello (edvilme) commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

This pull request introduces robust support for resolving .venv entries in project directories, following both the traditional convention (where .venv is a directory) and PEP 832 (where .venv is a file pointing to the virtual environment path). The new utility function resolve_dot_venv is added in pet-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:

  • Added the resolve_dot_venv function to pet-fs, which resolves .venv as either a directory or a file (per PEP 832), handling absolute and relative paths and verifying the existence of the resolved directory.
  • Added extensive tests for 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:

  • Updated pet-poetry and pet-uv to use resolve_dot_venv for detecting project-local virtual environments, replacing previous direct .venv directory checks. This ensures compatibility with both directory and file-based .venv setups. [1] [2] [3] [4] [5] [6]
  • Updated the workspace folder search logic in pet to prioritize .venv as resolved by resolve_dot_venv, improving accuracy and supporting the new convention. [1] [2]

Add resolve_venv utility supporting .venv file (PEP 832)
cb6bfea
Replace .env search with resolve_dot_env
96c0cff
Fix missing imports
7a9e574
Fix issues
9d47e7f
fix: handle Option<PathBuf> from resolve_dot_venv correctly
5e66a8c 
- 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>
fix: add blank doc line before See link to fix clippy doc_lazy_contin…
427439c 
…uation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot commented Apr 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Performance Report (Linux) ✅

Metric PR (P50) PR (P95) Baseline (P50) Delta Change
Server Startup 1ms 1ms 1ms 0ms 0%
Full Refresh 45ms 268ms 54ms -9ms -10.0%

Results based on 10 iterations. P50 = median, P95 = 95th percentile.

Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

github-actions Bot commented Apr 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Performance Report (macOS)

Metric PR (P50) PR (P95) Baseline (P50) Delta
Server Startup 58ms 508ms 73ms -15ms
Full Refresh 117ms 30159ms 139ms -22ms

Results based on 10 iterations. P50 = median, P95 = 95th percentile.

Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

github-actions Bot commented Apr 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Coverage Report (Linux)

Metric Value
Current Coverage 76.3%
Base Branch Coverage 76.1%
Delta .2% ✅

Coverage increased! Great work!

github-actions Bot commented Apr 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Coverage Report (Windows)

Metric Value
Current Coverage 73.21%
Base Branch Coverage 73.02%
Delta 0.19% ✅

Coverage increased! Great work!

github-actions Bot commented Apr 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Performance Report (Windows) ✅

Metric PR (P50) PR (P95) Baseline (P50) Delta Change
Server Startup 9ms 12ms 10ms -1ms -10%
Full Refresh 147ms 572ms 174ms -27ms -15.5%

Results based on 10 iterations. P50 = median, P95 = 95th percentile.

Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

Closed
Copilot AI reviewed Apr 17, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 .venv as either a directory or a PEP 832 path file.
  • Update workspace-folder search ordering in pet to prioritize resolved .venv paths.
  • Update pet-uv and pet-poetry to use the shared .venv resolution logic instead of direct .venv directory 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.

Comment thread crates/pet-fs/src/path.rs
Comment thread crates/pet-fs/src/path.rs Outdated
Comment thread crates/pet-fs/src/path.rs
Comment thread crates/pet-fs/src/path.rs Outdated

Karthik Nadig (karthiknadig) commented Apr 22, 2026

Copy link
Copy Markdown
Member

Eduardo Villalpando Mello (@edvilme) Are you still working on this?

Address PR feedback: fix symlink handling, empty .venv guard, doc acc…
7020c0d 
…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>

Eduardo Villalpando Mello (edvilme) commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator Author

Eduardo Villalpando Mello (@edvilme) Are you still working on this?

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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL