Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite Python interpreter discovery #3266

Merged
merged 1 commit into from
May 21, 2024
Merged

Rewrite Python interpreter discovery #3266

merged 1 commit into from
May 21, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Apr 25, 2024

Updates our Python interpreter discovery to conform to the rules described in #2386, please see that issue for a full description of the behavior. Briefly, we now will search for interpreters that satisfy a requested version without stopping at the first Python executable. Additionally, if retrieving information about an interpreter fails we will continue to search for a working interpreter. We also add the plumbing necessary to request Python implementations other than CPython, though we do not add support for other implementations at this time.

A major internal goal of this work is to prepare for user-facing managed toolchains i.e. fetching a requested version during uv run. These APIs are not introduced, but there is some managed toolchain handling as required for our test suite.

Some noteworthy implementation changes:

  • The uv_interpreter::find_python module has been removed in favor of a uv_interpreter::discovery module.
  • There are new types to help structure interpreter requests and track sources
  • Executable discovery is implemented as a big lazy iterator and is a central authority for source precedence
  • uv_interpreter::Error variants were split into scoped types in each module
  • There's much more unit test coverage, but not for Windows yet

Remaining work:

  • Write new test cases
  • Determine correct behavior around executables in the current directory
  • Future: Combine PythonVersion and VersionRequest
  • Future: Consider splitting ManagedToolchain into local and remote variants
  • Future: Add Windows unit test coverage
  • Future: Explore behavior around implementation precedence (i.e. CPython over PyPy)

Refactors split into:

Closes #2386

@zanieb zanieb force-pushed the zb/interp-request-ii branch 5 times, most recently from 0972b45 to ea73ecf Compare April 30, 2024 16:28
@zanieb zanieb force-pushed the zb/interp-request branch 2 times, most recently from eb65979 to 3d0f4a6 Compare April 30, 2024 17:15
@zanieb zanieb force-pushed the zb/interp-request-ii branch 2 times, most recently from f233559 to 8b7967b Compare April 30, 2024 17:40
Copy link

codspeed-hq bot commented Apr 30, 2024

CodSpeed Performance Report

Merging #3266 will not alter performance

Comparing zb/interp-request-ii (1b212ab) with main (dfd6ccf)

Summary

✅ 12 untouched benchmarks

zanieb added a commit that referenced this pull request May 2, 2024
Split out of #3266 

Mostly an organizational change, with some error handling
simplification.
zanieb added a commit that referenced this pull request May 2, 2024
Split out of #3266

If `UV_BOOTSTRAP_DIR` and `CARGO_MANIFEST_DIR` are both unset, we
currently panic. This isn't good once we start to use managed toolchains
in production. We'll need to change this more later once the toolchain
directory is more user-facing.
zanieb added a commit that referenced this pull request May 2, 2024
…odules (#3332)

Split out of #3266

The "selector" concept doesn't seem well enough defined as-is. For
example, `PythonVersion` belongs there but isn't present. Going for
smaller modules instead.
zanieb added a commit that referenced this pull request May 2, 2024
@zanieb
Copy link
Member Author

zanieb commented May 20, 2024

Example test setup

gh pr checkout 3266
cargo build
alias uv=$(pwd)/target/debug/uv
uv version

I'll do some local QA and share helpful commands here

@charliermarsh
Copy link
Member

charliermarsh commented May 20, 2024

Not sure if intentional, but I have pypy (which is PyPy at Python 2.7) and pypy39 (which is PyPy at Python 3.9) in path, and cargo run venv --python pypy fails because it tries to use the one called pypy:

puffin on  zb/interp-request-ii [$] is 📦 v0.1.44 via 🐍 v3.11.8 via 🦀 v1.78.0 took 2s
❯ cargo run venv --python pypy
   Compiling uv v0.1.44 (/Users/crmarsh/workspace/puffin/crates/uv)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.75s
     Running `target/debug/uv venv --python pypy`
  × Can't use Python at `/opt/homebrew/bin/pypy`
  ╰─▶ Python executable does not support `-I` flag. Please use Python 3.8 or newer.

puffin on  zb/interp-request-ii [$] is 📦 v0.1.44 via 🐍 v3.11.8 via 🦀 v1.78.0 took 2s
❯ cargo run venv --python pypy3.9
   Compiling uv v0.1.44 (/Users/crmarsh/workspace/puffin/crates/uv)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.02s
     Running `target/debug/uv venv --python pypy3.9`
Using Python 3.9.18 interpreter at: /opt/homebrew/bin/pypy3.9
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate

Should it resolve to the pypy3.9?

cargo run venv --python pypy3.9 does work as expected.

@charliermarsh
Copy link
Member

Otherwise, my own smoke-testing works as expected.

@charliermarsh
Copy link
Member

I would personally expect this to work -- I copied a Python to foo locally, and --python ./foo discovers it but --python foo does not:

puffin on  zb/interp-request-ii [$?]
❯ cargo run venv --python foo
   Compiling uv v0.1.44 (/Users/crmarsh/workspace/puffin/crates/uv)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.87s
     Running `target/debug/uv venv --python foo`
  × Requested Python executable `foo` not found in PATH

puffin on  zb/interp-request-ii [$?]
❯ cargo run venv --python ./foo
   Compiling uv v0.1.44 (/Users/crmarsh/workspace/puffin/crates/uv)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.72s
     Running `target/debug/uv venv --python ./foo`
Using Python 3.9.18 interpreter at: foo
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate

I think we fixed this in the prior implementation and that it was considered a bug.

@zanieb
Copy link
Member Author

zanieb commented May 20, 2024

I think your pypy example fails because we don't have support for detecting that as an implementation name yet (that will happen in a follow up pull request). Here, cpython is the only implementation name variant (just to reduce scope).

@zanieb zanieb force-pushed the zb/interp-request-ii branch 5 times, most recently from 6e53083 to c797422 Compare May 21, 2024 16:15
@zanieb zanieb marked this pull request as ready for review May 21, 2024 16:32
# Conflicts:
#	crates/uv/src/commands/project/run.rs
@zanieb zanieb merged commit d540d0f into main May 21, 2024
45 checks passed
@zanieb zanieb deleted the zb/interp-request-ii branch May 21, 2024 19:37
zanieb added a commit that referenced this pull request May 23, 2024
…3789)

A follow-up to #3266 addressing some awkwardness where there was no
"empty" or default interpreter request kind.
zanieb added a commit that referenced this pull request May 24, 2024
Closes #3784

The cache did not use an absolute path. I'm not sure this is actually a
new bug, as this code wasn't touched in #3266 but perhaps there was a
slight difference in the paths we were passing around. Note, just
canonicalizing the path as soon as we see it doesn't work because then
we jump out of the virtual environmnent into the system interpreter.

## Test plan

```
❯ uv venv
Using Python 3.12.3 interpreter at: /opt/homebrew/opt/python@3.12/bin/python3.12
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate
❯ uv pip install anyio
Resolved 3 packages in 81ms
Installed 3 packages in 4ms
 + anyio==4.3.0
 + idna==3.7
 + sniffio==1.3.1
❯ mkdir uv-issue-3784 && cd uv-issue-3784
❯ uv venv
Using Python 3.12.3 interpreter at: /opt/homebrew/opt/python@3.12/bin/python3.12
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate

❯ gcm
Switched to branch 'main'
Your branch is up to date with 'origin/main'.
❯ cargo run -q -- pip list -v -p .venv
DEBUG Checking for Python interpreter in directory `.venv`
TRACE Cached interpreter info for Python 3.12.3, skipping probing: .venv/bin/python3
DEBUG Using Python 3.12.3 environment at .venv/bin/python3
Package Version
------- -------
anyio   4.3.0
idna    3.7
sniffio 1.3.1
❯ cd uv-issue-3784
❯ cargo run -q -- pip list -v -p .venv
DEBUG Checking for Python interpreter in directory `.venv`
TRACE Cached interpreter info for Python 3.12.3, skipping probing: .venv/bin/python3
DEBUG Using Python 3.12.3 environment at /Users/zb/workspace/uv/.venv/bin/python3
Package Version
------- -------
anyio   4.3.0
idna    3.7
sniffio 1.3.1

❯ cd ..
❯ gco zb/fix-relative-venv
Switched to branch 'zb/fix-relative-venv'
❯ cargo run -q -- pip list -v -p .venv
DEBUG Checking for Python interpreter in directory `.venv`
TRACE Cached interpreter info for Python 3.12.3, skipping probing: .venv/bin/python3
DEBUG Using Python 3.12.3 environment at .venv/bin/python3
Package Version
------- -------
anyio   4.3.0
idna    3.7
sniffio 1.3.1
❯ cd uv-issue-3784
❯ cargo run -q -- pip list -v -p .venv
DEBUG Checking for Python interpreter in directory `.venv`
TRACE Cached interpreter info for Python 3.12.3, skipping probing: .venv/bin/python3
DEBUG Using Python 3.12.3 environment at .venv/bin/python3
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selecting a Python interpreter
3 participants