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

[WIP] Add a --suffix option to uv tool install #7095

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3156,6 +3156,10 @@ pub struct ToolInstallArgs {
help_heading = "Python options"
)]
pub python: Option<String>,

/// The suffix to install the package and executables with
#[arg(long, help_heading = "Installer options")]
pub suffix: Option<String>,
}

#[derive(Args)]
Expand Down
41 changes: 31 additions & 10 deletions crates/uv-tool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@ pub enum Error {
Serialization(#[from] toml_edit::ser::Error),
}

/// A Tool name
///
/// TODO: This representation works for installing, but could be problematic for upgrading and uninstalling
/// Consider changing it to either a single String (with the resolved name), or an enum with variants for different identifiers
#[derive(Debug, Clone)]
pub struct ToolName {
pub name: PackageName,
pub suffix: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

Agree, thanks for pointing it out

}

impl fmt::Display for ToolName {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
// TODO Decide how Tools are displayed
write!(f, "{}", self.name)
}
}

/// A collection of uv-managed tools installed on the current system.
#[derive(Debug, Clone)]
pub struct InstalledTools {
Expand Down Expand Up @@ -87,8 +104,12 @@ impl InstalledTools {
}

/// Return the expected directory for a tool with the given [`PackageName`].
pub fn tool_dir(&self, name: &PackageName) -> PathBuf {
self.root.join(name.to_string())
pub fn tool_dir(&self, name: &ToolName) -> PathBuf {
if let Some(suffix) = &name.suffix {
self.root.join(name.name.to_string() + suffix)
} else {
self.root.join(name.name.to_string())
}
}

/// Return the metadata for all installed tools.
Expand Down Expand Up @@ -130,7 +151,7 @@ impl InstalledTools {
/// error.
///
/// Note it is generally incorrect to use this without [`Self::acquire_lock`].
pub fn get_tool_receipt(&self, name: &PackageName) -> Result<Option<Tool>, Error> {
pub fn get_tool_receipt(&self, name: &ToolName) -> Result<Option<Tool>, Error> {
let path = self.tool_dir(name).join("uv-receipt.toml");
match ToolReceipt::from_path(&path) {
Ok(tool_receipt) => Ok(Some(tool_receipt.tool)),
Expand All @@ -149,7 +170,7 @@ impl InstalledTools {
/// Any existing receipt will be replaced.
///
/// Note it is generally incorrect to use this without [`Self::acquire_lock`].
pub fn add_tool_receipt(&self, name: &PackageName, tool: Tool) -> Result<(), Error> {
pub fn add_tool_receipt(&self, name: &ToolName, tool: Tool) -> Result<(), Error> {
let tool_receipt = ToolReceipt::from(tool);
let path = self.tool_dir(name).join("uv-receipt.toml");

Expand All @@ -175,7 +196,7 @@ impl InstalledTools {
/// # Errors
///
/// If no such environment exists for the tool.
pub fn remove_environment(&self, name: &PackageName) -> Result<(), Error> {
pub fn remove_environment(&self, name: &ToolName) -> Result<(), Error> {
let environment_path = self.tool_dir(name);

debug!(
Expand All @@ -196,7 +217,7 @@ impl InstalledTools {
/// Note it is generally incorrect to use this without [`Self::acquire_lock`].
pub fn get_environment(
&self,
name: &PackageName,
name: &ToolName,
cache: &Cache,
) -> Result<Option<PythonEnvironment>, Error> {
let environment_path = self.tool_dir(name);
Expand Down Expand Up @@ -228,7 +249,7 @@ impl InstalledTools {
/// Note it is generally incorrect to use this without [`Self::acquire_lock`].
pub fn create_environment(
&self,
name: &PackageName,
name: &ToolName,
interpreter: Interpreter,
) -> Result<PythonEnvironment, Error> {
let environment_path = self.tool_dir(name);
Expand Down Expand Up @@ -271,15 +292,15 @@ impl InstalledTools {
}

/// Return the [`Version`] of an installed tool.
pub fn version(&self, name: &PackageName, cache: &Cache) -> Result<Version, Error> {
pub fn version(&self, name: &ToolName, cache: &Cache) -> Result<Version, Error> {
let environment_path = self.tool_dir(name);
let environment = PythonEnvironment::from_root(&environment_path, cache)?;
let site_packages = SitePackages::from_environment(&environment)
.map_err(|err| Error::EnvironmentRead(environment_path.clone(), err.to_string()))?;
let packages = site_packages.get_packages(name);
let packages = site_packages.get_packages(&name.name);
let package = packages
.first()
.ok_or_else(|| Error::MissingToolPackage(name.clone()))?;
.ok_or_else(|| Error::MissingToolPackage(name.name.clone()))?;
Ok(package.version().clone())
}

Expand Down
38 changes: 28 additions & 10 deletions crates/uv/src/commands/tool/common.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::fmt::Write;
use std::path::Path;
use std::{collections::BTreeSet, ffi::OsString};

use anyhow::{bail, Context};
Expand All @@ -16,7 +17,9 @@ use uv_installer::SitePackages;
use uv_python::PythonEnvironment;
use uv_settings::ToolOptions;
use uv_shell::Shell;
use uv_tool::{entrypoint_paths, find_executable_directory, InstalledTools, Tool, ToolEntrypoint};
use uv_tool::{
entrypoint_paths, find_executable_directory, InstalledTools, Tool, ToolEntrypoint, ToolName,
};
use uv_warnings::warn_user;

use crate::commands::ExitStatus;
Expand Down Expand Up @@ -71,13 +74,19 @@ pub(crate) fn install_executables(
python: Option<String>,
requirements: Vec<Requirement>,
printer: Printer,
suffix: &Option<String>,
) -> anyhow::Result<ExitStatus> {
let site_packages = SitePackages::from_environment(environment)?;
let installed = site_packages.get_packages(name);
let Some(installed_dist) = installed.first().copied() else {
bail!("Expected at least one requirement")
};

let pkg = ToolName {
name: name.clone(),
suffix: suffix.clone(),
};

// Find a suitable path to install into
let executable_directory = find_executable_directory()?;
fs_err::create_dir_all(&executable_directory)
Expand All @@ -98,12 +107,21 @@ pub(crate) fn install_executables(
let target_entry_points = entry_points
.into_iter()
.map(|(name, source_path)| {
let target_path = executable_directory.join(
source_path
.file_name()
.map(std::borrow::ToOwned::to_owned)
.unwrap_or_else(|| OsString::from(name.clone())),
);
let mut file_stem = source_path
.file_stem()
.map(std::borrow::ToOwned::to_owned)
.unwrap_or_else(|| OsString::from(name.clone()));
if let Some(suffix) = suffix {
file_stem.push(suffix);
}

let target_path = if let Some(extension) = source_path.extension() {
let path = Path::new(&file_stem).with_extension(extension);
executable_directory.join(path)
} else {
let path = Path::new(&file_stem);
executable_directory.join(path)
};
(name, source_path, target_path)
})
.collect::<BTreeSet<_>>();
Expand All @@ -118,7 +136,7 @@ pub(crate) fn install_executables(
hint_executable_from_dependency(name, &site_packages, printer)?;

// Clean up the environment we just created.
installed_tools.remove_environment(name)?;
installed_tools.remove_environment(&pkg)?;

return Ok(ExitStatus::Failure);
}
Expand All @@ -138,7 +156,7 @@ pub(crate) fn install_executables(
}
} else if existing_entry_points.peek().is_some() {
// Clean up the environment we just created
installed_tools.remove_environment(name)?;
installed_tools.remove_environment(&pkg)?;

let existing_entry_points = existing_entry_points
// SAFETY: We know the target has a filename because we just constructed it above
Expand Down Expand Up @@ -190,7 +208,7 @@ pub(crate) fn install_executables(
.map(|(name, _, target_path)| ToolEntrypoint::new(name, target_path)),
options,
);
installed_tools.add_tool_receipt(name, tool)?;
installed_tools.add_tool_receipt(&pkg, tool)?;

// If the executable directory isn't on the user's PATH, warn.
if !Shell::contains_path(&executable_directory) {
Expand Down
53 changes: 30 additions & 23 deletions crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use uv_python::{
};
use uv_requirements::{RequirementsSource, RequirementsSpecification};
use uv_settings::{ResolverInstallerOptions, ToolOptions};
use uv_tool::InstalledTools;
use uv_tool::{InstalledTools, ToolName};
use uv_warnings::warn_user;

use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger};
Expand All @@ -42,6 +42,7 @@ pub(crate) async fn install(
force: bool,
options: ResolverInstallerOptions,
settings: ResolverInstallerSettings,
suffix: Option<String>,
python_preference: PythonPreference,
python_downloads: PythonDownloads,
connectivity: Connectivity,
Expand Down Expand Up @@ -241,6 +242,11 @@ pub(crate) async fn install(
let installed_tools = InstalledTools::from_settings()?.init()?;
let _lock = installed_tools.lock().await?;

let pkg = ToolName {
name: from.name.clone(),
suffix: suffix.clone(),
};

// Find the existing receipt, if it exists. If the receipt is present but malformed, we'll
// remove the environment and continue with the install.
//
Expand All @@ -249,31 +255,31 @@ pub(crate) async fn install(
//
// (If we find existing entrypoints later on, and the tool _doesn't_ exist, we'll avoid removing
// the external tool's entrypoints (without `--force`).)
let (existing_tool_receipt, invalid_tool_receipt) =
match installed_tools.get_tool_receipt(&from.name) {
Ok(None) => (None, false),
Ok(Some(receipt)) => (Some(receipt), false),
Err(_) => {
// If the tool is not installed properly, remove the environment and continue.
match installed_tools.remove_environment(&from.name) {
Ok(()) => {
warn_user!(
"Removed existing `{from}` with invalid receipt",
from = from.name.cyan()
);
}
Err(uv_tool::Error::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => {
return Err(err.into());
}
let (existing_tool_receipt, invalid_tool_receipt) = match installed_tools.get_tool_receipt(&pkg)
{
Ok(None) => (None, false),
Ok(Some(receipt)) => (Some(receipt), false),
Err(_) => {
// If the tool is not installed properly, remove the environment and continue.
match installed_tools.remove_environment(&pkg) {
Ok(()) => {
warn_user!(
"Removed existing `{from}` with invalid receipt",
from = from.name.cyan()
);
}
Err(uv_tool::Error::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => {
return Err(err.into());
}
(None, true)
}
};
(None, true)
}
};

let existing_environment =
installed_tools
.get_environment(&from.name, &cache)?
.get_environment(&pkg, &cache)?
.filter(|environment| {
python_request.as_ref().map_or(true, |python_request| {
if python_request.satisfied(environment.interpreter(), &cache) {
Expand Down Expand Up @@ -308,7 +314,7 @@ pub(crate) async fn install(
if *tool_receipt.options() != options {
// ...but the options differ, we need to update the receipt.
installed_tools
.add_tool_receipt(&from.name, tool_receipt.clone().with_options(options))?;
.add_tool_receipt(&pkg, tool_receipt.clone().with_options(options))?;
}

// We're done, though we might need to update the receipt.
Expand Down Expand Up @@ -378,7 +384,7 @@ pub(crate) async fn install(
)
.await?;

let environment = installed_tools.create_environment(&from.name, interpreter)?;
let environment = installed_tools.create_environment(&pkg, interpreter)?;

// At this point, we removed any existing environment, so we should remove any of its
// executables.
Expand Down Expand Up @@ -411,5 +417,6 @@ pub(crate) async fn install(
python,
requirements,
printer,
&suffix,
)
}
10 changes: 7 additions & 3 deletions crates/uv/src/commands/tool/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use owo_colors::OwoColorize;

use uv_cache::Cache;
use uv_fs::Simplified;
use uv_tool::InstalledTools;
use uv_tool::{InstalledTools, ToolName};
use uv_warnings::warn_user;

use crate::commands::ExitStatus;
Expand Down Expand Up @@ -46,9 +46,13 @@ pub(crate) async fn list(
);
continue;
};
let pkg = ToolName {
name: name.clone(),
suffix: None, // TODO Add suffix option
};

// Output tool name and version
let version = match installed_tools.version(&name, cache) {
let version = match installed_tools.version(&pkg, cache) {
Ok(version) => version,
Err(e) => {
writeln!(printer.stderr(), "{e}")?;
Expand All @@ -74,7 +78,7 @@ pub(crate) async fn list(
printer.stdout(),
"{} ({})",
format!("{name} v{version}{version_specifier}").bold(),
installed_tools.tool_dir(&name).simplified_display().cyan(),
installed_tools.tool_dir(&pkg).simplified_display().cyan(),
)?;
} else {
writeln!(
Expand Down
8 changes: 7 additions & 1 deletion crates/uv/src/commands/tool/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use uv_python::{
PythonPreference, PythonRequest,
};
use uv_requirements::{RequirementsSource, RequirementsSpecification};
use uv_tool::ToolName;
use uv_tool::{entrypoint_paths, InstalledTools};
use uv_warnings::warn_user;

Expand Down Expand Up @@ -436,9 +437,14 @@ async fn get_or_create_environment(
let installed_tools = InstalledTools::from_settings()?.init()?;
let _lock = installed_tools.lock().await?;

let pkg = ToolName {
name: from.name.clone(),
suffix: None, // TODO add suffix support
};

let existing_environment =
installed_tools
.get_environment(&from.name, cache)?
.get_environment(&pkg, cache)?
.filter(|environment| {
python_request.as_ref().map_or(true, |python_request| {
python_request.satisfied(environment.interpreter(), cache)
Expand Down
Loading
Loading