Use PythonEnvironment API in uv venv
#4029
Conversation
8ece47b to
4a42f7f
Compare
crates/uv/src/commands/venv.rs
Outdated
| .into_interpreter(); | ||
| // Locate the Python interpreter to use in the environment | ||
| let interpreter = | ||
| PythonEnvironment::find(python_request, SystemPython::Required, preview, cache) |
There was a problem hiding this comment.
Does uv venv from within a venv still work?
There was a problem hiding this comment.
I'll verify this — probably not. I think the find_default_interpreter branch is actually important here.
Thanks for flagging.
There was a problem hiding this comment.
I wonder if we can add test coverage for that case too...
There was a problem hiding this comment.
So what I have here is actually equivalent to the current code. I think uv venv from an active virtual environment doesn't read the interpreter from the virtual environment; I tried to write a test case and it's not working on main. Should it? Maybe this regressed in #3266 and we missed it because we didn't have test coverage?
There was a problem hiding this comment.
Adding test coverage in #4045
There was a problem hiding this comment.
Discussed in Discord, the behavior that Charlie mentioned was changed in #3266 since we skip interpreters in the PATH that are not virtual environments.
There is no behavior change in this pull request and I think the existing behavior is okay for now since we haven't gotten any reports about the change that was released in 0.2.0.
4a42f7f to
38051f5
Compare
…nvironment (#4073) Closes #4072 This was an accidental change in #4029 in which I had updated the pull request to support virtual environments as requested in review and forgot to revert it. Separately, we shouldn't fail if `VIRTUAL_ENV` points to an empty directory and `SystemPython::Allowed` is used, will address that separately.
There's no reason to be reaching into the lower-level
find_interpretermanually here.