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

Fix best-interpreter lookups when there is an invalid interpreter in the PATH #11030

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Jan 28, 2025

Closes #10978

The root cause is the same as #10908 — I should have been more careful with the original change.

@zanieb zanieb added the bug Something isn't working label Jan 28, 2025
@zanieb zanieb force-pushed the zb/fix-best-interp branch from 20f5843 to b35faad Compare January 28, 2025 18:31
@zanieb zanieb force-pushed the zb/fix-best-interp branch from b35faad to 77630b8 Compare January 28, 2025 18:36
@zanieb zanieb force-pushed the zb/fix-interp branch 2 times, most recently from de13f02 to 8834092 Compare January 28, 2025 19:07
Base automatically changed from zb/fix-interp to main January 28, 2025 19:15
@zanieb zanieb force-pushed the zb/fix-best-interp branch from 77630b8 to 9842f17 Compare January 28, 2025 19:21
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

What's the nested Result? What do the two levels indicate?

@zanieb
Copy link
Member Author

zanieb commented Jan 28, 2025

This was a design decision in Python discovery a while back (which I'm sort of regretting as it was the source of this bug)

The outer error is if we fail during discovery for some reason.
The inner error is we don't find a Python interpreter.

/// The result of an Python installation search.
///
/// Returned by [`find_python_installation`].
type FindPythonResult = Result<PythonInstallation, PythonNotFound>;
/// The result of failed Python installation discovery.
///
/// See [`FindPythonResult`].
#[derive(Clone, Debug, Error)]
pub struct PythonNotFound {
pub request: PythonRequest,
pub python_preference: PythonPreference,
pub environment_preference: EnvironmentPreference,
}

The bug is that I changed things so a deferred discovery failure is sometimes raised instead of the inner error to improve error messages when there's an invalid interpreter on the path. However, this broke some downstream code where we take actions after failing to find an interpreter and counted the function to only return fatal outer errors.

@zanieb
Copy link
Member Author

zanieb commented Jan 28, 2025

I was worried about breakage here when I made the change, but foolishly trusted the test coverage (which it seems didn't exist for the case of a broken interpreter)

@zanieb zanieb merged commit e26affd into main Jan 28, 2025
73 checks passed
@zanieb zanieb deleted the zb/fix-best-interp branch January 28, 2025 19:44
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jan 29, 2025
This MR contains the following updates:

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

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

[Compare Source](astral-sh/uv@0.5.24...0.5.25)

##### Enhancements

-   Allow installation of manylinux wheels on loongarch64 ([#&#8203;10927](astral-sh/uv#10927))
-   Allow optional `=` for editables in `requirements.txt` ([#&#8203;10954](astral-sh/uv#10954))
-   Add Windows aarch64 to the release binaries ([#&#8203;10885](astral-sh/uv#10885))

##### Bug fixes

-   Use spec-compliant (`128+n`) exit codes for `uv run` and `uv tool run` on Unix ([#&#8203;10781](astral-sh/uv#10781))
-   Fix best-interpreter lookups when there is an invalid interpreter in the `PATH` ([#&#8203;11030](astral-sh/uv#11030))
-   Guard against concurrent cache writes on Windows ([#&#8203;11007](astral-sh/uv#11007))
-   Prioritize package preferences with greater package versions ([#&#8203;10963](astral-sh/uv#10963))
-   Reject `--editable` flag on non-directory requirements ([#&#8203;10994](astral-sh/uv#10994))
-   Respect `--no-sources` for `uv pip install` workspace discovery ([#&#8203;11003](astral-sh/uv#11003))
-   Set `JEMALLOC_SYS_WITH_LG_PAGE=16` in ARM Docker builds ([#&#8203;10943](astral-sh/uv#10943))
-   Update `riscv64` Python downloads to allow install on `riscv64gc` ([#&#8203;10937](astral-sh/uv#10937))
-   Fix file persist retries on Windows ([#&#8203;11008](astral-sh/uv#11008))
-   Fix incorrect error message when specifying `tool.uv.sources.(package).workspace` with other options ([#&#8203;11013](astral-sh/uv#11013))
-   Improve SIGINT handling in `uv run` ([#&#8203;11009](astral-sh/uv#11009))

##### Documentation

-   Add `SECURITY` policy ([#&#8203;11035](astral-sh/uv#11035))
-   Add `Requires-Python` upper bound behavior to the docs ([#&#8203;10964](astral-sh/uv#10964))
-   Add a troubleshooting section and reproducible example guide ([#&#8203;10947](astral-sh/uv#10947))
-   Add documentation for `uv add -r` ([#&#8203;10926](astral-sh/uv#10926))
-   Amend `requires-python` rules in resolver documentation ([#&#8203;10993](astral-sh/uv#10993))
-   Reference workspaces in `--no-sources` documentation ([#&#8203;10995](astral-sh/uv#10995))
-   Update documentation for activating virtual environments in different shell ([#&#8203;11000](astral-sh/uv#11000))
-   Add Docker SHA pinning tip ([#&#8203;10955](astral-sh/uv#10955))

</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:eyJjcmVhdGVkSW5WZXIiOiIzOS4xMzcuMiIsInVwZGF0ZWRJblZlciI6IjM5LjEzNy4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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.

New version of uv fails to build universal requirements for python version system does not have
2 participants