Display the target virtual environment path if non-default #7850

zanieb · 2024-10-01T16:47:31Z

Supersedes #4835

e.g.

❯ cargo run -q -- pip install --python ../../example httpx
Using Python 3.12.1 environment at /Users/zb/example
Resolved 7 packages in 561ms
Prepared 4 packages in 62ms
Installed 4 packages in 13ms
 + certifi==2024.8.30
 + h11==0.14.0
 + httpcore==1.0.5
 + httpx==0.27.2

❯ cargo run -q -- pip install httpx
Resolved 7 packages in 17ms
Installed 4 packages in 10ms
 + certifi==2024.8.30
 + h11==0.14.0
 + httpcore==1.0.5
 + httpx==0.27.2

zanieb · 2024-10-01T16:48:28Z

crates/uv/tests/pip_install.rs

@@ -1152,6 +1153,7 @@ fn install_editable_bare_requirements_txt() -> Result<()> {
    ----- stdout -----

    ----- stderr -----
+    Using Python 3.12.[X] environment at [VENV]/


We see this in some tests because they change the working directory.

zanieb · 2024-10-01T16:56:29Z

crates/uv/src/commands/pip/operations.rs

+    // Do not report environments in the cache
+    if target.starts_with(cache.root()) {
+        debug!("{}", message);
+        return Ok(());
+    }
+
+    // Do not report tool environments
+    if let Ok(tools) = InstalledTools::from_settings() {
+        if target.starts_with(tools.root()) {
+            debug!("{}", message);
+            return Ok(());
+        }
+    }


These guard against cases seen in #4835 but that implementation was in operations::install which is used far more broadly. In contrast, here we target the uv pip interface only and display a message way earlier, not during install. I think it's important to display the message earlier, as it's relevant for the resolution and audit. I think these guards are okay to retain here, though they're not used now. Willing to remove them if someone feels strongly though.

crates/uv/src/commands/pip/check.rs

crates/uv/src/commands/pip/operations.rs

charliermarsh · 2024-10-01T21:27:56Z

crates/uv/src/commands/pip/operations.rs

+        env.root().user_display()
+    );
+
+    let Ok(target) = env.root().canonicalize() else {


We should really avoid using canonicalize IMO. I've tried to remove as many of these calls as I can.

What is this case? The env doesn't exist?

We're canonicalizing for comparison, which I think is the correct way to compare paths? We don't display the canonicalized path and we don't fail if it fails. Why should we not here?

We're preparing for this comparison

// Do not report a default environment path if let Ok(default) = PathBuf::from(".venv").canonicalize() { if target == default { ...

as well as the other starts_with comparisons, I guess.

Does it do the right thing on Windows? For example, if this gets resolved to a ?\\ path, does target.starts_with(cache.root()) return true if cache.root() is not a ?\\ path?

(Asking genuinely, I don't know how that behaves.)

I'm not sure, I'd need to try it. Basically, I think for this sort of comparison to be "generally robust" we need to be sure we're comparing paths that have been normalized the same way, ideally canonicalized but we need to fall back to absolutized paths when the path does not exist. Maybe I should just write a utility for it? I could try not canonicalizing here and see what happens, but it feels likely to be wrong? If we're avoiding storing canonicalized paths anywhere I guess it has a decent chance of being right.

We'll see what CI says ec07503

I think since this is just a log we can easily iterate if this is wrong in practice.

Ok understood. I'm only pushing on it because every time I use canonicalize, I end up having to fix a bug later on!

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.18` -> `0.4.20` | 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.20`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0420) [Compare Source](astral-sh/uv@0.4.19...0.4.20) ##### Enhancements - Add managed downloads for CPython 3.13.0 (final) ([#8010](astral-sh/uv#8010)) - Python 3.13 is the default version for `uv python install` ([#8010](astral-sh/uv#8010)) - Hint at wrong endpoint in `uv publish` failures ([#7872](astral-sh/uv#7872)) - List available scripts when a command is not specified for `uv run` ([#7687](astral-sh/uv#7687)) - Fill in `authors` field during `uv init` ([#7756](astral-sh/uv#7756)) ##### Documentation - Add snapshot testing to contribution guide ([#7882](astral-sh/uv#7882)) - Fix and improve GitLab integration docs ([#8000](astral-sh/uv#8000)) ### [`v0.4.19`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0419) [Compare Source](astral-sh/uv@0.4.18...0.4.19) ##### Enhancements - Add managed downloads for CPython 3.13.0rc3 and 3.12.7 ([#7880](astral-sh/uv#7880)) - Display the target virtual environment path if non-default ([#7850](astral-sh/uv#7850)) - Preserve case-insensitive sorts in `uv add` ([#7864](astral-sh/uv#7864)) - Respect project upper bounds when filtering wheels on `requires-python` ([#7904](astral-sh/uv#7904)) - Add `--script` to `uv run` to treat an input as PEP 723 regardless of extension ([#7739](astral-sh/uv#7739)) - Improve legibility of build failure errors ([#7854](astral-sh/uv#7854)) - Show interpreter source during Python discovery query errors ([#7928](astral-sh/uv#7928)) ##### Configuration - Add `UV_FIND_LINKS` environment variable for `--find-links` ([#7912](astral-sh/uv#7912)) - Ignore empty string values for `UV_PYTHON` environment variable ([#7878](astral-sh/uv#7878)) ##### Bug fixes - Allow `py3x-none` tags in newer than Python 3.x ([#7867](astral-sh/uv#7867)) - Allow self-dependencies in the `dev` section ([#7943](astral-sh/uv#7943)) - Always ignore `cp2` wheels in resolution ([#7902](astral-sh/uv#7902)) - Clear the publish progress bar on retry ([#7921](astral-sh/uv#7921)) - Fix parsing of `gnueabi` libc variants in Python version requests ([#7975](astral-sh/uv#7975)) - Simplify supported environments when comparing to lockfile ([#7894](astral-sh/uv#7894)) - Trim commits when reading from Git refs ([#7922](astral-sh/uv#7922)) - Use a higher HTTP read timeout when publishing packages ([#7923](astral-sh/uv#7923)) - Remove the first empty line for `uv tree --package foo` ([#7885](astral-sh/uv#7885)) ##### Documentation - Add 3.13 support to the platform reference ([#7971](astral-sh/uv#7971)) - Clarify project environment creation ([#7941](astral-sh/uv#7941)) - Fix code block title in Gitlab integration docs ([#7861](astral-sh/uv#7861)) - Fix project guide section on adding a Git dependency ([#7916](astral-sh/uv#7916)) - Fix uninstallation command for Windows ([#7944](astral-sh/uv#7944)) - Clearly specify the minimum supported Windows Server version ([#7946](astral-sh/uv#7946)) ##### Rust API - Remove unused `Sha256Reader` ([#7929](astral-sh/uv#7929)) - Remove unnecessary `Deserialize` derives on settings ([#7856](astral-sh/uv#7856)) </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).

AndydeCleyre · 2024-10-16T17:12:40Z

Can this be disabled with a flag? In my case it creates a ton of noise.

When I use my wrapper to, for example, list installed versions of the 64 Python apps I have installed, I get 64 extra lines of this.

zanieb · 2024-10-16T17:16:07Z

@AndydeCleyre there's a -q flag, does that work for you?

AndydeCleyre · 2024-10-16T17:28:11Z

Thanks for the quick response.

No for two reasons:

pip list is just one of the bulk operations I do, and for others I want to see some output (which packages are upgraded, for example).
it completely hides the info I need:

$ uv pip list --format json
Using Python 3.12.2 environment at /home/andy/.local/share/venvs/d9521a6e560469d0860c4ddb9c9baaa9/venv
[{"name":"brotli","version":"1.1.0"},{"name":"certifi","version":"2024.8.30"},{"name":"charset-normalizer","version":"3.3.2"},{"name":"idna","version":"3.10"},{"name":"mutagen","version":"1.47.0"},{"name":"pycryptodomex","version":"3.21.0"},{"name":"requests","version":"2.32.3"},{"name":"urllib3","version":"2.2.3"},{"name":"websockets","version":"13.1"},{"name":"yt-dlp","version":"2024.9.27"}]

$ uv -q pip list --format json  # (no output here)

charliermarsh · 2024-10-16T17:31:27Z

I think it's a bug if -q is silencing the pip list output.

charliermarsh · 2024-10-16T17:31:53Z

But if you're wrapping uv I feel like you almost certainly do want -q? Who knows what we'll print in the future.

AndydeCleyre · 2024-10-16T17:32:53Z

Who knows what we'll print in the future.

Even for the json-formatted output of uv pip list? I thought it was meant to match the json-formatted output of regular pip list.

charliermarsh · 2024-10-16T17:35:08Z

Ah no, that should definitely follow an API contract! I'm more referring to the kind of output we'd suppress with -q, which wouldn't be covered by any API contract (it's just peripheral logging).

charliermarsh · 2024-10-16T18:16:14Z

(Like, anything that's machine readable should still be shown under -q.)

AndydeCleyre · 2024-10-18T15:12:57Z

FWIW I found I can solve my issue by directing stderr to /dev/null. 👍🏼

zanieb added the cli Related to the command line interface label Oct 1, 2024

zanieb commented Oct 1, 2024

View reviewed changes

zanieb force-pushed the zb/print-env-path branch from c7f81f9 to 7396d9b Compare October 1, 2024 16:49

zanieb mentioned this pull request Oct 1, 2024

Print Installation Path if not in working directory #4835

Closed

zanieb commented Oct 1, 2024

View reviewed changes

zanieb requested a review from charliermarsh October 1, 2024 21:01

charliermarsh reviewed Oct 1, 2024

View reviewed changes

crates/uv/src/commands/pip/check.rs Outdated Show resolved Hide resolved

charliermarsh reviewed Oct 1, 2024

View reviewed changes

crates/uv/src/commands/pip/operations.rs Outdated Show resolved Hide resolved

charliermarsh reviewed Oct 1, 2024

View reviewed changes

zanieb force-pushed the zb/print-env-path branch from 7396d9b to 364cfc1 Compare October 1, 2024 21:42

zanieb added 2 commits October 1, 2024 21:57

Display the target virtual environment path if non-default

93a2c7c

Use absolute instead of canonical paths

ec07503

zanieb force-pushed the zb/print-env-path branch from 364cfc1 to ec07503 Compare October 2, 2024 03:06

charliermarsh approved these changes Oct 2, 2024

View reviewed changes

zanieb merged commit b001483 into main Oct 2, 2024
61 checks passed

zanieb deleted the zb/print-env-path branch October 2, 2024 14:38

BrewTestBot mentioned this pull request Oct 8, 2024

uv 0.4.19 Homebrew/homebrew-core#193156

Merged

AndydeCleyre mentioned this pull request Oct 16, 2024

uv backend spams bulk operations output with Python version information AndydeCleyre/zpy#42

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display the target virtual environment path if non-default #7850

Display the target virtual environment path if non-default #7850

zanieb commented Oct 1, 2024 •

edited

Loading

zanieb Oct 1, 2024

zanieb Oct 1, 2024

charliermarsh Oct 1, 2024

charliermarsh Oct 1, 2024

zanieb Oct 1, 2024

zanieb Oct 1, 2024 •

edited

Loading

charliermarsh Oct 2, 2024

charliermarsh Oct 2, 2024

zanieb Oct 2, 2024

zanieb Oct 2, 2024

charliermarsh Oct 2, 2024

AndydeCleyre commented Oct 16, 2024

zanieb commented Oct 16, 2024

AndydeCleyre commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

AndydeCleyre commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

AndydeCleyre commented Oct 18, 2024

Display the target virtual environment path if non-default #7850

Display the target virtual environment path if non-default #7850

Conversation

zanieb commented Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zanieb Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndydeCleyre commented Oct 16, 2024

zanieb commented Oct 16, 2024

AndydeCleyre commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

AndydeCleyre commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

charliermarsh commented Oct 16, 2024

AndydeCleyre commented Oct 18, 2024

zanieb commented Oct 1, 2024 •

edited

Loading

zanieb Oct 1, 2024 •

edited

Loading