Improve invalid environment warning messages by zanieb · Pull Request #7544 · astral-sh/uv

zanieb · 2024-09-19T11:51:02Z

Adds display of the target path of the link (since the link filename itself is basically static) and distinguishes between broken links and missing files.

lucab

LGTM with a side comment for your consideration.

lucab · 2024-09-19T12:47:34Z

crates/uv-tool/src/lib.rs

-                    interpreter_path.user_display()
-                );
+                if interpreter_path.is_symlink() {
+                    let target_path = fs_err::read_link(&interpreter_path)?;


Side note: there is a FS race here, as the symlink check and read are not atomic and the underlying FS may change in-between (TOCTOU). Probably not much relevant in this context, but consider whether you'd like to turn the error case into a .unwrap_or(interpreter_path) so that the execution flow is guaranteed to continue past here in all cases.

Should this be an unconditional read_link with appropriate error handling for non-symlinks?

That'd be a nice way to avoid the problem — if the IO error has the relevant details.

It looks like this would still require two checks

diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 5324ac9e3..1300f0ae5 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -401,18 +401,14 @@ impl FoundInterpreter { } } Err(uv_python::Error::Query(uv_python::InterpreterError::NotFound(path))) => { - if path.is_symlink() { - let target_path = fs_err::read_link(&path)?; - warn_user!( - "Ignoring existing virtual environment linked to non-existent Python interpreter: {} -> {}", - path.user_display().cyan(), - target_path.user_display().cyan(), - ); - } else { - warn_user!( - "Ignoring existing virtual environment with missing Python interpreter: {}", - path.user_display().cyan() - ); + match fs_err::read_link(&path) { + Ok(path) => { + dbg!(&path); + dbg!(path.exists()); + } + Err(err) => { + dbg!(err); + } } } Err(err) => return Err(err.into()), diff --git a/crates/uv/tests/sync.rs b/crates/uv/tests/sync.rs index 95ec51743..c40009e8c 100644 --- a/crates/uv/tests/sync.rs +++ b/crates/uv/tests/sync.rs @@ -2687,7 +2687,8 @@ fn sync_invalid_environment() -> Result<()> { ----- stdout ----- ----- stderr ----- - warning: Ignoring existing virtual environment linked to non-existent Python interpreter: .venv/[BIN]/python -> python + [crates/uv/src/commands/project/mod.rs:406:25] &path = "python" + [crates/uv/src/commands/project/mod.rs:407:25] path.exists() = false Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Removed virtual environment at: .venv Creating virtual environment at: .venv @@ -2705,7 +2706,18 @@ fn sync_invalid_environment() -> Result<()> { ----- stdout ----- ----- stderr ----- - warning: Ignoring existing virtual environment with missing Python interpreter: .venv/[BIN]/python + [crates/uv/src/commands/project/mod.rs:410:25] err = Custom { + kind: NotFound, + error: Error { + kind: ReadLink, + source: Os { + code: 2, + kind: NotFound, + message: "No such file or directory", + }, + path: "[VENV]/[BIN]/python", + }, + } Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Removed virtual environment at: .venv Creating virtual environment at: .venv

And if you try to match on canonicalize the error is identical in both cases.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.9` -> `0.4.13` | 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.4.13`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0413) [Compare Source](astral-sh/uv@0.4.12...0.4.13) ##### Enhancements - Add `socks` support ([#7503](astral-sh/uv#7503)) - Avoid warning about bad Python interpreter links for empty project environment directories ([#7527](astral-sh/uv#7527)) - Improve invalid environment warning messages ([#7544](astral-sh/uv#7544)) - Use more verbose spelling of "virtualenv" during creation ([#7523](astral-sh/uv#7523)) - Do not use a user-facing warning for "Waiting to acquire lock..." message ([#7502](astral-sh/uv#7502)) ##### Performance - Use a single buffer for hints on resolver errors ([#7497](astral-sh/uv#7497)) ##### Bug fixes - Allow Python pre-releases to be used if they are first on the `PATH` ([#7470](astral-sh/uv#7470)) - Avoid deleting the project environment directory if it is not a virtual environment ([#7522](astral-sh/uv#7522)) - Do not error if the `CACHEDIR.TAG` file exists but cannot be written to ([#7550](astral-sh/uv#7550)) - Treat invalid platform as more compatible than invalid Python ([#7556](astral-sh/uv#7556)) - Use portable paths when serializing sources ([#7504](astral-sh/uv#7504)) - Compute resolver hints using the final reduced derivation tree ([#7546](astral-sh/uv#7546)) - Bump the wheel and sdist cache versions ([#7560](astral-sh/uv#7560)) - Heal cache entries with missing source distributions ([#7559](astral-sh/uv#7559)) ##### Rust libraries - Bump minimum supported Rust version from 1.80 -> 1.81 ##### Documentation - Add `UV_LINK_MODE` to Docker caching example ([#7510](astral-sh/uv#7510)) - Clarify behavior of of overrides in CLI reference ([#7537](astral-sh/uv#7537)) ### [`v0.4.12`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0412) [Compare Source](astral-sh/uv@0.4.11...0.4.12) ##### Enhancements - Allow users to provide pre-defined metadata for resolution ([#7442](astral-sh/uv#7442)) - Invalidate existing tool environments on Python interpreter mismatch ([#7451](astral-sh/uv#7451)) ##### Bug fixes - Avoid fatal error when searching for egg-info with missing directory ([#7498](astral-sh/uv#7498)) ##### Documentation - Add note on cache growth for self-hosted GitHub runners ([#5757](astral-sh/uv#5757)) ### [`v0.4.11`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0411) [Compare Source](astral-sh/uv@0.4.10...0.4.11) ##### Enhancements - Add `--no-editable` support to `uv sync` and `uv export` ([#7371](astral-sh/uv#7371)) - Add support for `--only-dev` to `uv sync` and `uv export` ([#7367](astral-sh/uv#7367)) - Add support for remaining pip-supported file extensions ([#7387](astral-sh/uv#7387)) - Generate shell completion for `uvx` ([#7388](astral-sh/uv#7388)) - Include `uv export` command in `requirements.txt` output ([#7374](astral-sh/uv#7374)) - Prune unzipped source distributions in `uv cache prune --ci` ([#7446](astral-sh/uv#7446)) - Warn when trying to `uv sync` a package without build configuration ([#7420](astral-sh/uv#7420)) - Support requests for pre-releases in the `--python` option ([#7335](astral-sh/uv#7335)) ##### Bug fixes - Avoid erroneous version warning for `.dist-info` directories ([#7444](astral-sh/uv#7444)) - Avoid removing seed packages for `uv venv --seed` environments ([#7410](astral-sh/uv#7410)) - Avoid unnecessary progress bar initializations ([#7412](astral-sh/uv#7412)) - Error when `tool.uv.sources` contains duplicate package names ([#7383](astral-sh/uv#7383)) - Include `--branch` et al when resolving unnamed URLs in `uv add` ([#7447](astral-sh/uv#7447)) - Include `dev-dependencies` in `--no-sources` invocations ([#7408](astral-sh/uv#7408)) - Include the parent interpreter in Python discovery when `--system` is used ([#7440](astral-sh/uv#7440)) - Respect `--no-sources` in PEP 723 scripts ([#7409](astral-sh/uv#7409)) - Respect `pyproject.toml` credentials from user-provided requirements ([#7474](astral-sh/uv#7474)) - Use consistent PyPI cache bucket ([#7443](astral-sh/uv#7443)) - Use unambiguous relative paths in `uv export` ([#7378](astral-sh/uv#7378)) ##### Documentation - Add documentation on platform-specific dependencies ([#7411](astral-sh/uv#7411)) - Add documentation for passing installer options on Linux ([#6839](astral-sh/uv#6839)) - Separate project data from configuration settings ([#7053](astral-sh/uv#7053)) ##### Error messages - Hint at missing `project.name` ([#6803](astral-sh/uv#6803)) - Surface dedicated `project.name` error for workspaces ([#7399](astral-sh/uv#7399)) - Remove duplicate warning for settings discovery errors ([#7384](astral-sh/uv#7384)) ### [`v0.4.10`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0410) [Compare Source](astral-sh/uv@0.4.9...0.4.10) ##### Enhancements - Allow `uv tool upgrade --all` to continue on individual upgrade failure ([#7333](astral-sh/uv#7333)) - Support globs as cache keys in `tool.uv.cache-keys` ([#7268](astral-sh/uv#7268)) - Add Python package (`__main__.py`) support to `uv run` ([#7281](astral-sh/uv#7281)) - Add zip application support to `uv run` ([#7289](astral-sh/uv#7289)) - Add `--token` option to `self update` command ([#7279](astral-sh/uv#7279)) ##### Performance - Use `globwalk` for `cache-keys` matching ([#7337](astral-sh/uv#7337)) ##### Bug fixes - Always treat archive-like requirements as local files ([#7364](astral-sh/uv#7364)) - Apply `--no-install` options when constructing resolution ([#7277](astral-sh/uv#7277)) - Avoid clobbering existing `py.typed` files contents in `uv init` ([#7338](astral-sh/uv#7338)) - Avoid enforcing platform compatibility when validating lockfile ([#7305](astral-sh/uv#7305)) - Avoid installing transitive dev dependencies ([#7318](astral-sh/uv#7318)) - Avoid selecting prerelease Python installations without opt-in ([#7300](astral-sh/uv#7300)) - Fix PPC64 page size in binary builds. ([#7298](astral-sh/uv#7298)) - Include pre-release Python versions in `uv python list` ([#7290](astral-sh/uv#7290)) - Make version ID optional for source builds ([#7362](astral-sh/uv#7362)) - Support relative paths in `uv add --script` ([#7301](astral-sh/uv#7301)) ##### Documentation - Fix documentation typos for `uv build --build-constraint` flag ([#7330](astral-sh/uv#7330)) - Fix grammatical error in CLI docs ([#7353](astral-sh/uv#7353)) ##### Error messages - Add dedicated lock errors for wheel-only distributions ([#7307](astral-sh/uv#7307)) - Avoid treating `.whl` sources as source distributions ([#7303](astral-sh/uv#7303)) - Clarify Python requirement source for script incompatibilities ([#7339](astral-sh/uv#7339)) </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. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

zanieb added the cli Related to the command line interface label Sep 19, 2024

zanieb force-pushed the zb/sync-invalid branch from 468d632 to b5fa16d Compare September 19, 2024 11:52

zanieb mentioned this pull request Sep 19, 2024

uv sync: empty directories are assumed to be linked to non-existent interpreters #7212

Closed

Improve invalid environment warning messages

408c611

zanieb force-pushed the zb/sync-invalid branch from b5fa16d to 408c611 Compare September 19, 2024 12:00

zanieb requested review from charliermarsh and lucab September 19, 2024 12:08

lucab approved these changes Sep 19, 2024

View reviewed changes

zanieb merged commit 99d57ca into main Sep 19, 2024

zanieb deleted the zb/sync-invalid branch September 19, 2024 14:11

BrewTestBot mentioned this pull request Sep 19, 2024

uv 0.4.13 Homebrew/homebrew-core#191284

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Improve invalid environment warning messages#7544

Improve invalid environment warning messages#7544
zanieb merged 1 commit intomainfrom
zb/sync-invalid

zanieb commented Sep 19, 2024 •

edited

Loading

Uh oh!

lucab left a comment

Uh oh!

lucab Sep 19, 2024 •

edited

Loading

Uh oh!

charliermarsh Sep 19, 2024

Uh oh!

zanieb Sep 19, 2024

Uh oh!

zanieb Sep 19, 2024

Uh oh!

zanieb Sep 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

zanieb commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucab left a comment

Choose a reason for hiding this comment

Uh oh!

lucab Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charliermarsh Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

zanieb Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

zanieb Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

zanieb Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zanieb commented Sep 19, 2024 •

edited

Loading

lucab Sep 19, 2024 •

edited

Loading