From b983ff4fa70fdfc8974df4d41c460a34d203f800 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 29 Feb 2024 10:06:29 -0500 Subject: [PATCH] Prioritize `PATH` over `py --list-paths` in Windows selection (#2057) `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: https://github.com/astral-sh/uv/issues/2056 --- .github/workflows/system-install.yml | 7 +++-- README.md | 6 ++-- crates/uv-interpreter/src/python_query.rs | 34 +++++++++++------------ 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/.github/workflows/system-install.yml b/.github/workflows/system-install.yml index 82172f4bf..011fb08cd 100644 --- a/.github/workflows/system-install.yml +++ b/.github/workflows/system-install.yml @@ -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 @@ -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" diff --git a/README.md b/README.md index f1cc3d7e4..4395e14f4 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/crates/uv-interpreter/src/python_query.rs b/crates/uv-interpreter/src/python_query.rs index 503d4564a..ebddb2f64 100644 --- a/crates/uv-interpreter/src/python_query.rs +++ b/crates/uv-interpreter/src/python_query.rs @@ -96,7 +96,6 @@ 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` @@ -104,6 +103,7 @@ pub(crate) fn try_find_default_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( @@ -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)] @@ -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) }