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

Allow discovering virtual environments from the first interpreter found on the PATH #11218

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Feb 4, 2025

Closes #11214

Special-cases the first Python executable we find on the PATH, allowing it to be considered during searches for virtual environments.

For some context, there are two stages to Python interpreter discovery

  1. We find possible Python executables in various sources
  2. We query the executables to determine canonical metadata about the interpreter

We can't really be "sure" if an executable is a complaint virtual environment during (1), we need to query the interpreter first. This means that if you're only allowed to installed into virtual environments, we'll query every interpreter on your PATH. This is not performant, and causes confusion for users. Notably, I recently improved error messaging when we can't find any valid interpreters, by showing the error message we encounter while querying an interpreter (if any). However, this is problematic when there's an error for an interpreter that is not relevant to your search. In #11143, I added filtering to avoid querying additional interpreters, but that regressed some user experiences where they were relying on us finding implicitly active virtual environments via the PATH.

@zanieb zanieb added the bug Something isn't working label Feb 4, 2025
@zanieb zanieb force-pushed the zb/python-find-first branch 3 times, most recently from e293c60 to e4489bd Compare February 4, 2025 19:50
@zanieb zanieb force-pushed the zb/python-find-first branch from e4489bd to b8e1816 Compare February 4, 2025 19:50
@@ -7608,12 +7608,48 @@ fn install_incompatible_python_version_interpreter_broken_in_path() -> Result<()
perms.set_mode(0o755);
fs_err::set_permissions(&python, perms)?;

// Request Python 3.12; which should fail
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case regresses, undoing the work from #11143 if the interpreter is at the front of the PATH. We don't see the regression in the diff because I added a new test case below which retains the previous error message, i.e., we do the right thing if the broken interpreter is not at the front of the PATH. We can do better, but the regression this pull requests fixes seems more important to address?

@zanieb
Copy link
Member Author

zanieb commented Feb 4, 2025

Looking back at #2109 and #4009, it seems likely that #11143 regressed behavior there. This sort of fixes that? It's basically a reimplementation of #4032

@zanieb
Copy link
Member Author

zanieb commented Feb 4, 2025

An alternative approach would be something like 1467172 which.. could also have problems, i.e., exclude interpreters incorrectly and requires querying more directories on the PATH when looking for virtual environments.

@zanieb zanieb marked this pull request as ready for review February 4, 2025 20:48
@zanieb zanieb merged commit ec480bd into main Feb 4, 2025
73 checks passed
@zanieb zanieb deleted the zb/python-find-first branch February 4, 2025 21:41
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Feb 6, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.5.27` -> `0.5.29` |

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.5.29`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0529)

[Compare Source](astral-sh/uv@0.5.28...0.5.29)

##### Enhancements

-   Add `--bare` option to `uv init` ([#&#8203;11192](astral-sh/uv#11192))
-   Add support for respecting `VIRTUAL_ENV` in project commands via `--active` ([#&#8203;11189](astral-sh/uv#11189))
-   Allow the project `VIRTUAL_ENV` warning to be silenced with `--no-active` ([#&#8203;11251](astral-sh/uv#11251))

##### Python

The managed Python distributions have been updated, including:

-   CPython 3.12.9
-   CPython 3.13.2
-   pkg-config files are now relocatable

See the [`python-build-standalone` release notes](https://github.com/astral-sh/python-build-standalone/releases/tag/20250205) for more details.

##### Bug fixes

-   Always use base Python discovery logic for cached environments ([#&#8203;11254](astral-sh/uv#11254))
-   Use a flock to avoid concurrent initialization of project environments ([#&#8203;11259](astral-sh/uv#11259))
-   Fix handling of `--all-groups` and `--no-default-groups` flags ([#&#8203;11224](astral-sh/uv#11224))

##### Documentation

-   Minor touchups to the Docker provenance docs ([#&#8203;11252](astral-sh/uv#11252))
-   Move content from the `mkdocs.public.yml` into the template ([#&#8203;11246](astral-sh/uv#11246))

### [`v0.5.28`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0528)

[Compare Source](astral-sh/uv@0.5.27...0.5.28)

##### Bug fixes

-   Allow discovering virtual environments from the first interpreter found on the `PATH` ([#&#8203;11218](astral-sh/uv#11218))
-   Clear ephemeral overlays when running tools ([#&#8203;11141](astral-sh/uv#11141))
-   Disable SSL in Git commands for `--allow-insecure-host` ([#&#8203;11210](astral-sh/uv#11210))
-   Fix hardlinks in tar unpacking ([#&#8203;11221](astral-sh/uv#11221))
-   Set base executable when returning virtual environment ([#&#8203;11209](astral-sh/uv#11209))
-   Use base Python for cached environments ([#&#8203;11208](astral-sh/uv#11208))

##### Documentation

-   Add documentation on verifying Docker image attestations ([#&#8203;11140](astral-sh/uv#11140))
-   Add `last updated` to documentation ([#&#8203;11164](astral-sh/uv#11164))

</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:eyJjcmVhdGVkSW5WZXIiOiIzOS4xNTguMSIsInVwZGF0ZWRJblZlciI6IjM5LjE1OC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v0.5.27 changes behavior for uv pip sync -r requirements.txt (breaking)
2 participants