Allow discovery of venv in VIRTUAL_ENV env variable #16853

MatthewMckee4 · 2025-03-19T17:30:35Z

Summary

Fixes astral-sh/ty#171

Allows the cli to find a virtual environment from the VIRTUAL_ENV environment variable if no --python is set

Test Plan

Unsure how to mock a venv to test that this works

github-actions · 2025-03-19T17:37:36Z

`mypy_primer` results

No ecosystem changes detected ✅

MichaReiser

Nice

MichaReiser · 2025-03-19T17:41:20Z

crates/red_knot_python_semantic/src/program.rs

+impl PythonPath {
+    pub fn find_virtual_env() -> Option<Self> {
+        let virtual_env = std::env::var("VIRTUAL_ENV").ok();
+        if let Some(virtual_env) = virtual_env {
+            tracing::debug!("Found virtual environment at {:?}", virtual_env);
+            return Some(Self::ActiveEnvironment(SystemPathBuf::from(virtual_env)));
+        }
+        None
+    }
+}


Using this approach for when VIRTUAL_ENV is set does make sense to me because we probably want Red Knot to fail if whatever VIRTUAL_ENV is pointing to doesn't turn out to be a virtual env.

I'm not sure if it's the right approach to e.g. support .venv because we then only want to use the folder if it actually is a virtual env.

I should probably look at this for parity with uv..

(there's actually not much to review here — uv's discovery is far more complicated and doesn't really seem relevant for this particular change)

crates/red_knot_python_semantic/src/program.rs

github-actions · 2025-03-19T18:56:57Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

AlexWaygood

this is looking great!!

crates/red_knot_project/src/metadata/options.rs

crates/red_knot_python_semantic/src/module_resolver/resolver.rs

crates/red_knot_python_semantic/src/program.rs

crates/red_knot_python_semantic/src/site_packages.rs

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

crates/red_knot_python_semantic/src/site_packages.rs

MichaReiser

Nice!

I think we can use SysPrefixPath in more places to avoid having to reduce the number of arguments that need to be routed through the different levels.

crates/red_knot_python_semantic/src/site_packages.rs

crates/red_knot_python_semantic/src/program.rs

crates/red_knot_python_semantic/src/site_packages.rs

MatthewMckee4 · 2025-03-20T13:39:53Z

Could anyone help with testing, I'm unsure about how to go about testing this

AlexWaygood · 2025-03-20T13:42:34Z

Could anyone help with testing, I'm unsure about how to go about testing this

I'm curious if @MichaReiser has any ideas but my best idea here is lots of manual testing:

If we manually create an environment and activate it, can red-knot automatically resolve imports that refer to third-party packages in the venv's site_packages directory?
If the environment is implicitly activated via uv run, can can red-knot automatically resolve imports that refer to third-party packages in the venv's site_packages directory?
If we deliberately "break" a venv in one of the ways that we try to detect using the site_packages::SitePackagesDiscoveryError enum and then activate the venv and run red-knot, what do the error messages from red-knot look like?

MichaReiser · 2025-03-20T13:46:05Z

An automated CLI test would be great, but it's probably somewhat painful to set up because SysPrefixPath does a lot of validation, and the behavior is even platform-dependent. I'm fine skipping an automated test here because it doesn't do much more than --python <path>

crates/red_knot_python_semantic/src/site_packages.rs

AlexWaygood

I manually tested with both "good" and "broken" virtual environments, and it all looks great. Thank you!

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

* main: (26 commits) Use the common `OperatorPrecedence` for the parser (#16747) [red-knot] Check subtype relation between callable types (#16804) [red-knot] Check whether two callable types are equivalent (#16698) [red-knot] Ban most `Type::Instance` types in type expressions (#16872) Special-case value-expression inference of special form subscriptions (#16877) [syntax-errors] Fix star annotation before Python 3.11 (#16878) Recognize `SyntaxError:` as an error code for ecosystem checks (#16879) [red-knot] add test cases result in false positive errors (#16856) Bump 0.11.1 (#16871) Allow discovery of venv in VIRTUAL_ENV env variable (#16853) Split git pathspecs in change determination onto separate lines (#16869) Use the correct base commit for change determination (#16857) Separate `BitXorOr` into `BitXor` and `BitOr` precedence (#16844) Server: Allow `FixAll` action in presence of version-specific syntax errors (#16848) [`refurb`] Fix starred expressions fix (`FURB161`) (#16550) [`flake8-executable`] Add pytest and uv run to help message for `shebang-missing-python` (`EXE003`) (#16855) Show more precise messages in invalid type expressions (#16850) [`flake8-executables`] Allow `uv run` in shebang line for `shebang-missing-python` (`EXE003`) (#16849) Add `--exit-non-zero-on-format` (#16009) [red-knot] Ban list literals in most contexts in type expressions (#16847) ...

Allow discovery of venv in VIRTUAL_ENV env variable

b2526e2

AlexWaygood added the ty Multi-file analysis & type inference label Mar 19, 2025

MichaReiser reviewed Mar 19, 2025

View reviewed changes

Refactor PythonPath and add SysPrefixPath

f9536b0

Update SitePackagesDiscoveryError

ed94272

MatthewMckee4 marked this pull request as ready for review March 20, 2025 11:23

MatthewMckee4 requested review from carljm, AlexWaygood, sharkdp and dcreager as code owners March 20, 2025 11:23

AlexWaygood reviewed Mar 20, 2025

View reviewed changes

MatthewMckee4 and others added 4 commits March 20, 2025 11:53

Update crates/red_knot_python_semantic/src/program.rs

dfeb7cb

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Update crates/red_knot_python_semantic/src/module_resolver/resolver.rs

e0e477d

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Update crates/red_knot_project/src/metadata/options.rs

dbc3644

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Make SysPrefixPath fields private and clean up PythonPath methods

4072462

MatthewMckee4 requested a review from AlexWaygood March 20, 2025 12:10

AlexWaygood reviewed Mar 20, 2025

View reviewed changes

crates/red_knot_python_semantic/src/site_packages.rs Show resolved Hide resolved

Restructure structs

f423376

MichaReiser approved these changes Mar 20, 2025

View reviewed changes

crates/red_knot_python_semantic/src/site_packages.rs Show resolved Hide resolved

crates/red_knot_python_semantic/src/site_packages.rs Show resolved Hide resolved

crates/red_knot_python_semantic/src/program.rs Show resolved Hide resolved

AlexWaygood reviewed Mar 20, 2025

View reviewed changes

crates/red_knot_python_semantic/src/site_packages.rs Outdated Show resolved Hide resolved

crates/red_knot_python_semantic/src/site_packages.rs Outdated Show resolved Hide resolved

AlexWaygood reviewed Mar 20, 2025

View reviewed changes

crates/red_knot_python_semantic/src/site_packages.rs Outdated Show resolved Hide resolved

AlexWaygood approved these changes Mar 20, 2025

View reviewed changes

MatthewMckee4 and others added 2 commits March 20, 2025 13:49

Update site_packages.rs

9ba3272

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Apply suggestions from code review

1555afe

AlexWaygood enabled auto-merge (squash) March 20, 2025 13:52

AlexWaygood merged commit cdafd8e into astral-sh:main Mar 20, 2025
22 checks passed

MatthewMckee4 deleted the discovery-activated-venv branch March 20, 2025 14:00

Allow discovery of venv in VIRTUAL_ENV env variable #16853

Allow discovery of venv in VIRTUAL_ENV env variable #16853

Uh oh!

Conversation

MatthewMckee4 commented Mar 19, 2025

Summary

Test Plan

Uh oh!

github-actions bot commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

MichaReiser Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

zanieb Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

zanieb Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MatthewMckee4 commented Mar 20, 2025

Uh oh!

AlexWaygood commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaReiser commented Mar 20, 2025

Uh oh!

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 19, 2025 •

edited

Loading

`mypy_primer` results

github-actions bot commented Mar 19, 2025 •

edited

Loading

`ruff-ecosystem` results

AlexWaygood commented Mar 20, 2025 •

edited

Loading