From 73f519ca095c133da1414ed6a47ceafcff2004c5 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 26 Jun 2024 11:24:42 -0500 Subject: [PATCH] Add support for `--reinstall` and `--reinstall-package` in `uv tool install` --- Cargo.lock | 1 + crates/uv-tool/Cargo.toml | 1 + crates/uv-tool/src/lib.rs | 21 ++++- crates/uv/src/commands/tool/install.rs | 70 ++++++++++---- crates/uv/tests/tool_install.rs | 125 ++++++++++++++++++++++++- 5 files changed, 195 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f074e9be3e6f5..6d4198b3a98d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5027,6 +5027,7 @@ dependencies = [ "toml", "toml_edit", "tracing", + "uv-cache", "uv-fs", "uv-state", "uv-toolchain", diff --git a/crates/uv-tool/Cargo.toml b/crates/uv-tool/Cargo.toml index e565b63990f5e..0fa2adefe4ab1 100644 --- a/crates/uv-tool/Cargo.toml +++ b/crates/uv-tool/Cargo.toml @@ -21,6 +21,7 @@ uv-virtualenv = { workspace = true } uv-toolchain = { workspace = true } install-wheel-rs = { workspace = true } pep440_rs = { workspace = true } +uv-cache = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } diff --git a/crates/uv-tool/src/lib.rs b/crates/uv-tool/src/lib.rs index 3c6d8ded5299a..38d8537e8cc3e 100644 --- a/crates/uv-tool/src/lib.rs +++ b/crates/uv-tool/src/lib.rs @@ -8,6 +8,7 @@ use std::io::{self, Write}; use std::path::{Path, PathBuf}; use thiserror::Error; use tracing::debug; +use uv_cache::Cache; use uv_fs::{LockedFile, Simplified}; use uv_toolchain::{Interpreter, PythonEnvironment}; @@ -21,9 +22,9 @@ pub enum Error { #[error(transparent)] IO(#[from] io::Error), // TODO(zanieb): Improve the error handling here - #[error("Failed to update `tools.toml` at {0}")] + #[error("Failed to update `tools.toml` metadata at {0}")] TomlEdit(PathBuf, #[source] tools_toml::Error), - #[error("Failed to read `tools.toml` at {0}")] + #[error("Failed to read `tools.toml` metadata at {0}")] TomlRead(PathBuf, #[source] Box), #[error(transparent)] VirtualEnvError(#[from] uv_virtualenv::Error), @@ -33,6 +34,8 @@ pub enum Error { DistInfoMissing(String, PathBuf), #[error("Failed to find a directory for executables")] NoExecutableDirectory, + #[error(transparent)] + EnvironmentError(#[from] uv_toolchain::Error), } /// A collection of uv-managed tools installed on the current system. @@ -121,16 +124,26 @@ impl InstalledTools { Ok(()) } - pub fn create_environment( + pub fn environment( &self, name: &str, + remove_existing: bool, interpreter: Interpreter, + cache: &Cache, ) -> Result { let _lock = self.acquire_lock(); let environment_path = self.root.join(name); + if !remove_existing && environment_path.exists() { + debug!( + "Using existing environment for tool `{name}` at `{}`.", + environment_path.user_display() + ); + return Ok(PythonEnvironment::from_root(environment_path, cache)?); + } + debug!( - "Creating environment for tool `{name}` at {}.", + "Creating environment for tool `{name}` at `{}`.", environment_path.user_display() ); diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index 82d068b784663..c187f94253037 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -9,10 +9,10 @@ use itertools::Itertools; use pep508_rs::Requirement; use pypi_types::VerbatimParsedUrl; -use tracing::debug; +use tracing::{debug, trace}; use uv_cache::Cache; use uv_client::Connectivity; -use uv_configuration::{Concurrency, PreviewMode}; +use uv_configuration::{Concurrency, PreviewMode, Reinstall}; #[cfg(unix)] use uv_fs::replace_symlink; use uv_fs::Simplified; @@ -47,29 +47,44 @@ pub(crate) async fn install( if preview.is_disabled() { warn_user!("`uv tool install` is experimental and may change without warning."); } + let from = from.unwrap_or(name.clone()); let installed_tools = InstalledTools::from_settings()?; + // TODO(zanieb): Figure out the interface here, do we infer the name or do we match the `run --from` interface? + let from = Requirement::::from_str(&from)?; + let existing_tool_entry = installed_tools.find_tool_entry(&name)?; // TODO(zanieb): Automatically replace an existing tool if the request differs - if installed_tools.find_tool_entry(&name)?.is_some() { + let reinstall_entry_points = if existing_tool_entry.is_some() { if force { debug!("Replacing existing tool due to `--force` flag."); + false } else { - writeln!(printer.stderr(), "Tool `{name}` is already installed.")?; - return Ok(ExitStatus::Failure); + match settings.reinstall { + Reinstall::All => { + debug!("Replacing existing tool due to `--reinstall` flag."); + true + } + // Do not replace the entry points unless the tool is explicitly requested + Reinstall::Packages(ref packages) => packages.contains(&from.name), + // If not reinstalling... then we're done + Reinstall::None => { + writeln!(printer.stderr(), "Tool `{name}` is already installed.")?; + return Ok(ExitStatus::Failure); + } + } } - } - - // TODO(zanieb): Figure out the interface here, do we infer the name or do we match the `run --from` interface? - let from = from.unwrap_or(name.clone()); + } else { + false + }; - let requirements = [Requirement::from_str(&from)] + let requirements = [Requirement::from_str(from.name.as_ref())] .into_iter() .chain(with.iter().map(|name| Requirement::from_str(name))) .collect::>, _>>()?; // TODO(zanieb): Duplicative with the above parsing but needed for `update_environment` - let requirements_sources = [RequirementsSource::from_package(from.clone())] + let requirements_sources = [RequirementsSource::from_package(from.name.to_string())] .into_iter() .chain(with.into_iter().map(RequirementsSource::from_package)) .collect::>(); @@ -93,7 +108,13 @@ pub(crate) async fn install( // TODO(zanieb): Build the environment in the cache directory then copy into the tool directory // This lets us confirm the environment is valid before removing an existing install - let environment = installed_tools.create_environment(&name, interpreter)?; + let environment = installed_tools.environment( + &name, + // Do not remove the existing environment if we're reinstalling a subset of packages + !matches!(settings.reinstall, Reinstall::Packages(_)), + interpreter, + cache, + )?; // Install the ephemeral requirements. let environment = update_environment( @@ -115,13 +136,23 @@ pub(crate) async fn install( bail!("Expected at least one requirement") }; + // Exit early if we're not supposed to be reinstalling entry points + // e.g. `--reinstall-package` was used for some dependency + if existing_tool_entry.is_some() && !reinstall_entry_points { + writeln!(printer.stderr(), "Updated environment for tool `{name}`")?; + return Ok(ExitStatus::Success); + } + // Find a suitable path to install into // TODO(zanieb): Warn if this directory is not on the PATH let executable_directory = find_executable_directory()?; fs_err::create_dir_all(&executable_directory) .context("Failed to create executable directory")?; - debug!("Installing into {}", executable_directory.user_display()); + debug!( + "Installing tool entry points into {}", + executable_directory.user_display() + ); let entrypoints = entrypoint_paths( &environment, @@ -147,9 +178,12 @@ pub(crate) async fn install( .iter() .filter(|(_, _, target)| target.exists()) .peekable(); - if force { + + // Note we use `reinstall_entry_points` here instead of `reinstall`; requesting reinstall + // will _not_ remove existing entry points when they are not managed by uv. + if force || reinstall_entry_points { for (name, _, target) in existing_targets { - debug!("Removing existing install of `{name}`"); + debug!("Removing existing entry point `{name}`"); fs_err::remove_file(target)?; } } else if existing_targets.peek().is_some() { @@ -178,9 +212,13 @@ pub(crate) async fn install( replace_symlink(&path, &target).context("Failed to install entrypoint")?; #[cfg(windows)] fs_err::copy(&path, &target).context("Failed to install entrypoint")?; + writeln!(printer.stderr(), "Installed `{name}`")?; } - debug!("Adding `{name}` to {}", path.user_display()); + trace!( + "Tracking installed tool `{name}` in tool metadata at `{}`", + path.user_display() + ); let installed_tools = installed_tools.init()?; installed_tools.add_tool_entry(&name, &tool)?; diff --git a/crates/uv/tests/tool_install.rs b/crates/uv/tests/tool_install.rs index c5ae3362eb368..3dc357f1e5ecc 100644 --- a/crates/uv/tests/tool_install.rs +++ b/crates/uv/tests/tool_install.rs @@ -39,6 +39,8 @@ fn tool_install() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 + Installed `black` + Installed `blackd` "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -109,6 +111,7 @@ fn tool_install() { + jinja2==3.1.3 + markupsafe==2.1.5 + werkzeug==3.0.1 + Installed `flask` "###); tool_dir.child("flask").assert(predicate::path::is_dir()); @@ -156,9 +159,9 @@ fn tool_install() { }); } -/// Test installing a tool twice with `uv tool install` +/// Test installing and reinstalling an already installed tool #[test] -fn tool_install_twice() { +fn tool_install_already_installed() { let context = TestContext::new("3.12"); let tool_dir = context.temp_dir.child("tools"); let bin_dir = context.temp_dir.child("bin"); @@ -183,6 +186,8 @@ fn tool_install_twice() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 + Installed `black` + Installed `blackd` "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -250,6 +255,76 @@ fn tool_install_twice() { black = { requirements = ["black"] } "###); }); + + // Install `black` again with the `--reinstall` flag + // We should recreate the entire environment and reinstall the entry points + uv_snapshot!(context.filters(), context.tool_install() + .arg("black") + .arg("--reinstall") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv tool install` is experimental and may change without warning. + Resolved 6 packages in [TIME] + Installed 6 packages in [TIME] + + black==24.3.0 + + click==8.1.7 + + mypy-extensions==1.0.0 + + packaging==24.0 + + pathspec==0.12.1 + + platformdirs==4.2.0 + Installed `blackd` + Installed `black` + "###); + + // Install `black` again with `--reinstall-package` for `black` + // We should reinstall `black` in the environment and reinstall the entry points + uv_snapshot!(context.filters(), context.tool_install() + .arg("black") + .arg("--reinstall-package") + .arg("black") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv tool install` is experimental and may change without warning. + Resolved 6 packages in [TIME] + Uninstalled 1 package in [TIME] + Installed 1 package in [TIME] + - black==24.3.0 + + black==24.3.0 + Installed `black` + Installed `blackd` + "###); + + // Install `black` again with `--reinstall-package` for a dependency + // We should reinstall `click` in the environment but not reinstall the entry points + uv_snapshot!(context.filters(), context.tool_install() + .arg("black") + .arg("--reinstall-package") + .arg("click") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv tool install` is experimental and may change without warning. + Resolved 6 packages in [TIME] + Uninstalled 1 package in [TIME] + Installed 1 package in [TIME] + - click==8.1.7 + + click==8.1.7 + Updated environment for tool `black` + "###); } /// Test installing a tool when its entry point already exists @@ -262,7 +337,7 @@ fn tool_install_entry_point_exists() { let executable = bin_dir.child(format!("black{}", std::env::consts::EXE_SUFFIX)); executable.touch().unwrap(); - // Install `black` + // Attempt to install `black` uv_snapshot!(context.filters(), context.tool_install() .arg("black") .env("UV_TOOL_DIR", tool_dir.as_os_str()) @@ -299,6 +374,44 @@ fn tool_install_entry_point_exists() { }); + // Attempt to install `black` with the `--reinstall` flag + // Should have no effect + uv_snapshot!(context.filters(), context.tool_install() + .arg("black") + .arg("--reinstall") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + warning: `uv tool install` is experimental and may change without warning. + Resolved 6 packages in [TIME] + Installed 6 packages in [TIME] + + black==24.3.0 + + click==8.1.7 + + mypy-extensions==1.0.0 + + packaging==24.0 + + pathspec==0.12.1 + + platformdirs==4.2.0 + error: Entry point for tool already exists: black (use `--force` to overwrite) + "###); + + // We should not create a virtual environment + assert!(tool_dir.child("black").exists()); + + // We should not write a tools entry + assert!(!tool_dir.join("tools.toml").exists()); + + insta::with_settings!({ + filters => context.filters(), + }, { + // Nor should we change the `black` entry point that exists + assert_snapshot!(fs_err::read_to_string(bin_dir.join("black")).unwrap(), @""); + + }); + // Test error message when multiple entry points exist bin_dir.child("blackd").touch().unwrap(); uv_snapshot!(context.filters(), context.tool_install() @@ -425,6 +538,8 @@ fn tool_install_home() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 + Installed `blackd` + Installed `black` "###); context @@ -460,6 +575,8 @@ fn tool_install_xdg_data_home() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 + Installed `black` + Installed `blackd` "###); context @@ -495,6 +612,8 @@ fn tool_install_xdg_bin_home() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 + Installed `black` + Installed `blackd` "###); bin_dir