Fix handling of python-preference = system when managed interpreters are on the PATH
#15059
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zanieb
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
zanieb
changed the title
python-preference = system when managed interpreters are on the PATH
zanieb
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
zanieb
commented
Aug 4, 2025
Comment on lines
+781
to
+789
| // We find the unmanaged interpreter with system Python preferred | ||
| uv_snapshot!(context.filters(), context.python_find().arg("--python-preference").arg("system"), @r" | ||
| success: true | ||
| exit_code: 0 | ||
| ----- stdout ----- | ||
| [PYTHON-3.12] | ||
| ----- stderr ----- | ||
| "); |
Member
Author
There was a problem hiding this comment.
This test case failed prior to this pull request, it'd discover 3.11.
zanieb
commented
Aug 4, 2025
| } | ||
| } | ||
|
|
||
| pub(crate) fn is_system_interpreter(source: PythonSource, interpreter: &Interpreter) -> bool { |
jtfmumm
reviewed
Aug 4, 2025
| // If we only found managed installations, and the preference allows them, we should return | ||
| // the first one. | ||
| if let Some(installation) = first_managed { | ||
| return Ok(Ok(installation)); |
Contributor
There was a problem hiding this comment.
Should we log something here since we log skipping this installation earlier?
Member
Author
There was a problem hiding this comment.
We don't for first_prerelease, so I just copied that pattern. It does seem reasonable though.
jtfmumm
approved these changes
Aug 4, 2025
…s are on the PATH
zanieb
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
zanieb
added a commit
that referenced
this pull request
Aug 5, 2025
…is used (#15061) This fixes a regression from 0.8.0 from #7934 and follows #15059 The regression is from [this change](https://github.com/astral-sh/uv/pull/7934/files#diff-c7a660ac39628d5e12f388b0cacc7360affa3d7bb21191184d7ee78489675e83), which was made because we'd otherwise (with the other changes in that pull request) _filter out_ managed Python interpreters found in virtual environments. When `--system` is used we'll convert the default Python preference of `managed` to `system` which avoids things like `uv pip install --system` targeting a managed Python installation. The basic test is ``` uv python install uv pip install --system anyio ``` Prior to this change, we'd read a managed interpreter from our managed installation directory and target that. After this change, without #15059, we'd read a managed interpreter from the PATH and target that. Both of those experiences are bad, because the managed interpreters are marked as externally managed. After this change, with #15059, we properly target the system interpreter. Since we use `system` instead of `only-system`, if there is not a system interpreter we'll still retain our existing behavior and use a managed interpreter. This should limit breakage from the change. Given the source of the regression, we could probably use `only-system` here. I don't feel strongly. I think the main benefit of doing so would be that we'd omit the check for managed installations in error messages when an interpreter cannot be found? We can't really add test coverage here because the test suite always has externally managed interpreters :)
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Aug 6, 2025
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.8.4` -> `0.8.5` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.8.5`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#085) [Compare Source](astral-sh/uv@0.8.4...0.8.5) ##### Enhancements - Enable `uv run` with a GitHub Gist ([#​15058](astral-sh/uv#15058)) - Improve HTTP response caching log messages ([#​15067](astral-sh/uv#15067)) - Show wheel tag hints in install plan ([#​15066](astral-sh/uv#15066)) - Support installing additional executables in `uv tool install` ([#​14014](astral-sh/uv#14014)) ##### Preview features - Enable extra build dependencies to 'match runtime' versions ([#​15036](astral-sh/uv#15036)) - Remove duplicate `extra-build-dependencies` warnings for `uv pip` ([#​15088](astral-sh/uv#15088)) - Use "option" instead of "setting" in `pylock` warning ([#​15089](astral-sh/uv#15089)) - Respect extra build requires when reading from wheel cache ([#​15030](astral-sh/uv#15030)) - Preserve lowered extra build dependencies ([#​15038](astral-sh/uv#15038)) ##### Bug fixes - Add Python versions to markers implied from wheels ([#​14913](astral-sh/uv#14913)) - Ensure consistent indentation when adding dependencies ([#​14991](astral-sh/uv#14991)) - Fix handling of `python-preference = system` when managed interpreters are on the PATH ([#​15059](astral-sh/uv#15059)) - Fix symlink preservation in virtual environment creation ([#​14933](astral-sh/uv#14933)) - Gracefully handle entrypoint permission errors ([#​15026](astral-sh/uv#15026)) - Include wheel hashes from local Simple indexes ([#​14993](astral-sh/uv#14993)) - Prefer system Python installations over managed ones when `--system` is used ([#​15061](astral-sh/uv#15061)) - Remove retry wrapper when matching on error kind ([#​14996](astral-sh/uv#14996)) - Revert `h2` upgrade ([#​15079](astral-sh/uv#15079)) ##### Documentation - Improve visibility of copy and line separator in dark mode ([#​14987](astral-sh/uv#14987)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS41Mi4yIiwidXBkYXRlZEluVmVyIjoiNDEuNTIuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first part of fixing a 0.8.0 regression from #7934
There, we added handling for skipping managed interpreters on the PATH when
only-systemis used, but did not update the logic to prefer system interpreters over managed ones whensystemis used. Here, we fix that by skipping managed interpreters whensystemis used unless only managed interpreters are available. While this logic is applied during in a general discovery method, it's only relevant for the PATH (and the Windows registry) because we already change the order that we inspect installations in whensystemis used, so the managed installation directory is inspected last.This behavior did not regress in 0.8, it's always been this way, however, I need this change in order to fix a different bug.