Skip to content

Commit

Permalink
Prioritize PATH over py --list-paths in Windows selection (#2057)
Browse files Browse the repository at this point in the history
`uv --system` is failing in GitHub Actions, because `py --list-paths`
returns all the pre-cached Pythons:

```
-V:3.12 *        C:\hostedtoolcache\windows\Python\3.12.2\x64\python.exe
-V:3.12-32       C:\hostedtoolcache\windows\Python\3.12.2\x86\python.exe
-V:3.11          C:\hostedtoolcache\windows\Python\3.11.8\x64\python.exe
-V:3.11-32       C:\hostedtoolcache\windows\Python\3.11.8\x86\python.exe
-V:3.10          C:\hostedtoolcache\windows\Python\3.10.11\x64\python.exe
-V:3.10-32       C:\hostedtoolcache\windows\Python\3.10.11\x86\python.exe
-V:3.9           C:\hostedtoolcache\windows\Python\3.9.13\x64\python.exe
-V:3.9-32        C:\hostedtoolcache\windows\Python\3.9.13\x86\python.exe
-V:3.8           C:\hostedtoolcache\windows\Python\3.8.10\x64\python.exe
-V:3.8-32        C:\hostedtoolcache\windows\Python\3.8.10\x86\python.exe
-V:3.7           C:\hostedtoolcache\windows\Python\3.7.9\x64\python.exe
-V:3.7-32        C:\hostedtoolcache\windows\Python\3.7.9\x86\python.exe
```

So, our default selector returns the first entry here. But none of these
are actually in `PATH` except the one that the user installed via
`actions/setup-python@v5` -- that's the point of the action, that it
puts the correct versions in `PATH`.

It seems to me like we should prioritize `PATH` over `py --list-paths`.
Is there a good reason not to do this?

Closes: #2056
  • Loading branch information
charliermarsh authored Feb 29, 2024
1 parent 0fbfa11 commit b983ff4
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 24 deletions.
7 changes: 4 additions & 3 deletions .github/workflows/system-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ jobs:
steps:
- uses: actions/checkout@v4

# - name: "Install Python"
# run: choco install python
- uses: actions/setup-python@v5
with:
python-version: "3.10"

- name: "Install Rust toolchain"
run: rustup show
Expand All @@ -82,7 +83,7 @@ jobs:
run: echo $(which python)

- name: "Validate global Python install"
run: py -3.12 ./scripts/check_system_python.py --uv ./target/debug/uv
run: py -3.10 ./scripts/check_system_python.py --uv ./target/debug/uv

install-pyenv:
name: "Install Python using pyenv"
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ search for a Python interpreter matching that version in the following order:
- An activated virtual environment based on the `VIRTUAL_ENV` environment variable.
- An activated Conda environment based on the `CONDA_PREFIX` environment variable.
- A virtual environment at `.venv` in the current directory, or in the nearest parent directory.
- The Python interpreter available as, e.g., `python3.7` on macOS and Linux. On Windows, uv
will use the same mechanism as `py --list-paths` to discover all available Python interpreters,
and will select the first interpreter matching the requested version.
- The Python interpreter available as, e.g., `python3.7` on macOS and Linux.
- The Python interpreter available as `python3` on macOS and Linux, or `python.exe` on Windows.
- On Windows, the Python interpreter returned by `py --list-paths` that matches the requested
version.

Since uv has no dependency on Python, it can even install into virtual environments other than
its own. For example, setting `VIRTUAL_ENV=/path/to/venv` will cause uv to install into
Expand Down
34 changes: 16 additions & 18 deletions crates/uv-interpreter/src/python_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,14 @@ pub(crate) fn try_find_default_python(

/// Finds a python version matching `selector`.
/// It searches for an existing installation in the following order:
/// * (windows): Discover installations using `py --list-paths` (PEP514). Continue if `py` is not installed.
/// * Search for the python binary in `PATH` (or `UV_TEST_PYTHON_PATH` if set). Visits each path and for each path resolves the
/// files in the following order:
/// * Major.Minor.Patch: `pythonx.y.z`, `pythonx.y`, `python.x`, `python`
/// * Major.Minor: `pythonx.y`, `pythonx`, `python`
/// * Major: `pythonx`, `python`
/// * Default: `python3`, `python`
/// * (windows): For each of the above, test for the existence of `python.bat` shim (pyenv-windows) last.
/// * (windows): Discover installations using `py --list-paths` (PEP514). Continue if `py` is not installed.
///
/// (Windows): Filter out the windows store shim (Enabled in Settings/Apps/Advanced app settings/App execution aliases).
fn find_python(
Expand All @@ -114,23 +114,7 @@ fn find_python(
#[allow(non_snake_case)]
let UV_TEST_PYTHON_PATH = env::var_os("UV_TEST_PYTHON_PATH");

if cfg!(windows) && UV_TEST_PYTHON_PATH.is_none() {
// Use `py` to find the python installation on the system.
match windows::py_list_paths(selector, platform, cache) {
Ok(Some(interpreter)) => return Ok(Some(interpreter)),
Ok(None) => {
// No matching Python version found, continue searching PATH
}
Err(Error::PyList(error)) => {
if error.kind() == std::io::ErrorKind::NotFound {
debug!("`py` is not installed. Falling back to searching Python on the path");
// Continue searching for python installations on the path.
}
}
Err(error) => return Err(error),
}
}

let override_path = UV_TEST_PYTHON_PATH.is_some();
let possible_names = selector.possible_names();

#[allow(non_snake_case)]
Expand Down Expand Up @@ -197,6 +181,20 @@ fn find_python(
}
}

if cfg!(windows) && !override_path {
// Use `py` to find the python installation on the system.
match windows::py_list_paths(selector, platform, cache) {
Ok(Some(interpreter)) => return Ok(Some(interpreter)),
Ok(None) => {}
Err(Error::PyList(error)) => {
if error.kind() == std::io::ErrorKind::NotFound {
debug!("`py` is not installed");
}
}
Err(error) => return Err(error),
}
}

Ok(None)
}

Expand Down

0 comments on commit b983ff4

Please sign in to comment.