[WIP] Add a --suffix option to uv tool install
#7095
Conversation
| #[derive(Debug, Clone)] | ||
| pub struct ToolName { | ||
| pub name: PackageName, | ||
| pub suffix: Option<String>, |
There was a problem hiding this comment.
This may be a problematic structure for uv tool upgrade etc., e.g., if the user specifies a tool name via the command line we won't know what's the package name and what's a suffix until sometime later?
crates/uv-tool/src/lib.rs
Outdated
| let environment_path = self.tool_dir(name); | ||
|
|
||
| debug!( | ||
| "Deleting environment for tool `{name}` at {}", | ||
| "Deleting environment for tool `{}` at {}", | ||
| name.name, |
There was a problem hiding this comment.
We'll generally probably want to see the both the name and suffix that we're performing operations on.
There was a problem hiding this comment.
I'm a bit curious why you did this instead of implementing Display for ToolName
There was a problem hiding this comment.
Yes, didn't change to showing the whole tool name to simplify for now.
Of course, I complicated myself by not implementing Display, I blame my late little use of Rust, will change to that
There was a problem hiding this comment.
I added a TODO for changing how ToolNames are displayed, since it needs discussion
| if let Some(suffix) = suffix { | ||
| file_name.push(suffix); | ||
| } |
There was a problem hiding this comment.
Does this handle file extensions on Windows?
There was a problem hiding this comment.
Added the suffix between the stem and the extension, thanks for noticing
More thought and testing is needed for this bit (as in, what happens with tools that have a . in their name)
| [tool] | ||
| requirements = [{ name = "black" }] |
There was a problem hiding this comment.
Do we need to write the tool package name somewhere now?
There was a problem hiding this comment.
That's a good question, I think it depends a bit on what's the point of the receipt
If the goal is to differentiate package installations, I think not, since a package should behave exactly the same if it was installed with a given suffix or not. (ie, could we want to have a cache of package installations? if so, are the receipts the keys of the cache?)
However, if the receipt is just general information used by uv then yes, specially since upgrading a package may add entry points, and we may need to use the suffix there.
I lean towards adding it
There was a problem hiding this comment.
Yeah, I presumed we would need the package name during future operations, e.g. during uv tool upgrade to retrieve the entry points for the package as you mentioned.
Summary
Ongoing attempt to close #6365
So far:
{name}{suffix}uv tool installcommand thread and process suffixesTODO:
upgrade,uninstallToolNameafter discussionsAandA_Bcan collide ifAis installed with suffix_B){package name}{suffix}as an id, we need to test thatTest Plan
For now, tests that the same package can be installed with different suffixes