Print Installation Path if not in working directory #4835
Conversation
|
Don't spend too much time on the snapshot filtering, I can futz with that part. |
Thank you! My hero - I’ve been wrestling with that and can’t quite reproduce this last failure. |
|
Sorry lost track of this one! I'll give this a look tomorrow. |
No worries and thank you!! |
| @@ -233,6 +233,7 @@ fn run_script() -> Result<()> { | |||
| Reading inline script metadata from: main.py | |||
| Resolved 1 package in [TIME] | |||
| Prepared 1 package in [TIME] | |||
| Installing to environment at [CACHE_DIR]/environments-v1/[RANDOM]/[RANDOM]/bin/python | |||
There was a problem hiding this comment.
I'm hesitant to display these as user-facing messages for these internal environments — I wonder if there's a good way to avoid that?
There was a problem hiding this comment.
Ah so revert the [RANDOM] filter and have an additional conditional to ensure we don't print in the event it's an internal environment e.g; [CACHE]/environments-v1...?
There was a problem hiding this comment.
Yeah I think so. I'm curious for @charliermarsh's opinion.
|
I think the additional filters in #4787 will be helpful here. |
| @@ -1089,6 +1089,7 @@ fn install_editable_bare_cli() { | |||
| ----- stderr ----- | |||
| Resolved 1 package in [TIME] | |||
| Prepared 1 package in [TIME] | |||
| Installing to environment at [VENV]/bin/python3 | |||
There was a problem hiding this comment.
Shouldn't we be showing [VENV] not [VENV]/bin/python3? It seems a bit weird to show the path to the interpreter executable.
There was a problem hiding this comment.
Ah yeah, we could do that - something like:
if is_outside_working_directory {
if let Some(parent_dir) = python_executable.parent().and_then(|p| p.parent()) {
writeln!(
printer.stderr(),
"{}",
format!(
"Installing to environment at {}",
parent_dir.user_display()
)
.dimmed()
)?;
}
}? This yields [VENV]/ instead of [VENV]/bin/python
There was a problem hiding this comment.
I think you just want PythonEnvironment::root(), i.e., venv.root()?
Hm are you thinking we use them before the parent dir evaluation step? |
|
Hi @zanieb ! Just wanted to follow-up here if/when you have a moment 😄 |
|
Thanks for the nudge! Sorry I'm stretched a little thin on contributions :) Let me know if you have more questions! |
|
I don't actually think you'll need the new test filters if you're not displaying the Python executable anymore. |
|
Continuing this in #7850 |
Supersedes #4835 Closes #2155 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 ```
Summary
Addressing the issue detailed here regarding the visibility of the Python path when doing
uv install.Test Plan
TBD - can't quite reproduce the errors I'm seeing in CI locally.