From 75d9c7e7e01277dc75dd7e449f04483979da90d0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 2 Jul 2024 18:23:30 -0400 Subject: [PATCH] Replace tool environments on updated Python request --- crates/uv-tool/src/lib.rs | 42 ++++++++++--- crates/uv/src/commands/tool/install.rs | 54 +++++++++++------ crates/uv/tests/tool_install.rs | 84 +++++++++++++++++++++++--- 3 files changed, 147 insertions(+), 33 deletions(-) diff --git a/crates/uv-tool/src/lib.rs b/crates/uv-tool/src/lib.rs index 117a7111a93..404d3217d35 100644 --- a/crates/uv-tool/src/lib.rs +++ b/crates/uv-tool/src/lib.rs @@ -156,28 +156,54 @@ impl InstalledTools { environment_path.user_display() ); - fs_err::remove_dir_all(environment_path)?; + match fs_err::remove_dir_all(environment_path) { + Ok(()) => {} + Err(err) if err.kind() == io::ErrorKind::NotFound => (), + Err(err) => return Err(err.into()), + } Ok(()) } - /// Return the [`PythonEnvironment`] for a given tool. - pub fn environment( + /// Return the [`PythonEnvironment`] for a given tool, if it exists. + pub fn get_environment( &self, name: &PackageName, - remove_existing: bool, - interpreter: Interpreter, cache: &Cache, - ) -> Result { + ) -> Result, Error> { let _lock = self.acquire_lock(); let environment_path = self.root.join(name.to_string()); - if !remove_existing && environment_path.exists() { + if environment_path.is_dir() { debug!( "Using existing environment for tool `{name}` at `{}`.", environment_path.user_display() ); - return Ok(PythonEnvironment::from_root(environment_path, cache)?); + Ok(Some(PythonEnvironment::from_root(environment_path, cache)?)) + } else { + Ok(None) + } + } + + /// Create the [`PythonEnvironment`] for a given tool, removing any existing environments. + pub fn create_environment( + &self, + name: &PackageName, + interpreter: Interpreter, + ) -> Result { + let _lock = self.acquire_lock(); + let environment_path = self.root.join(name.to_string()); + + // Remove any existing environment. + match fs_err::remove_dir_all(&environment_path) { + Ok(()) => { + debug!( + "Removed existing environment for tool `{name}` at `{}`.", + environment_path.user_display() + ); + } + Err(err) if err.kind() == io::ErrorKind::NotFound => (), + Err(err) => return Err(err.into()), } debug!( diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index f9446180a75..f5daeec9c81 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -56,8 +56,12 @@ pub(crate) async fn install( .connectivity(connectivity) .native_tls(native_tls); + let python_request = python.as_deref().map(ToolchainRequest::parse); + + // Pre-emptively identify a Python interpreter. We need an interpreter to resolve any unnamed + // requirements, even if we end up using a different interpreter for the tool install itself. let interpreter = Toolchain::find_or_fetch( - python.as_deref().map(ToolchainRequest::parse), + python_request.clone(), EnvironmentPreference::OnlySystem, toolchain_preference, toolchain_fetch, @@ -147,21 +151,31 @@ pub(crate) async fn install( let installed_tools = InstalledTools::from_settings()?; let existing_tool_receipt = installed_tools.get_tool_receipt(&from.name)?; + let existing_environment = + installed_tools + .get_environment(&from.name, cache)? + .filter(|environment| { + python_request.as_ref().map_or(true, |python_request| { + python_request.satisfied(environment.interpreter(), cache) + }) + }); // If the requested and receipt requirements are the same... - if let Some(tool_receipt) = existing_tool_receipt.as_ref() { - let receipt = tool_receipt - .requirements() - .iter() - .cloned() - .map(Requirement::from) - .collect::>(); - if requirements == receipt { - // And the user didn't request a reinstall or upgrade... - if !force && settings.reinstall.is_none() && settings.upgrade.is_none() { - // We're done. - writeln!(printer.stderr(), "Tool `{from}` is already installed")?; - return Ok(ExitStatus::Failure); + if existing_environment.is_some() { + if let Some(tool_receipt) = existing_tool_receipt.as_ref() { + let receipt = tool_receipt + .requirements() + .iter() + .cloned() + .map(Requirement::from) + .collect::>(); + if requirements == receipt { + // And the user didn't request a reinstall or upgrade... + if !force && settings.reinstall.is_none() && settings.upgrade.is_none() { + // We're done. + writeln!(printer.stderr(), "Tool `{from}` is already installed")?; + return Ok(ExitStatus::Failure); + } } } } @@ -171,9 +185,15 @@ pub(crate) async fn install( // entrypoints (without `--force`). let reinstall_entry_points = existing_tool_receipt.is_some(); - // 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.environment(&from.name, force, interpreter, cache)?; + // 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. However, + // entrypoints always contain an absolute path to the relevant Python interpreter, which would + // be invalidated by moving the environment. + let environment = if let Some(environment) = existing_environment { + environment + } else { + installed_tools.create_environment(&from.name, interpreter)? + }; // Install the ephemeral requirements. let spec = RequirementsSpecification::from_requirements(requirements.clone()); diff --git a/crates/uv/tests/tool_install.rs b/crates/uv/tests/tool_install.rs index f081b183a72..d71e9d89aa5 100644 --- a/crates/uv/tests/tool_install.rs +++ b/crates/uv/tests/tool_install.rs @@ -648,14 +648,6 @@ fn tool_install_entry_point_exists() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Resolved [N] packages in [TIME] - Installed [N] 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: black, blackd "###); @@ -1329,3 +1321,79 @@ fn tool_install_upgrade() { "###); }); } + +/// Test reinstalling tools with varying `--python` requests. +#[test] +fn tool_install_python_request() { + let context = TestContext::new_with_versions(&["3.11", "3.12"]) + .with_filtered_counts() + .with_filtered_exe_suffix(); + let tool_dir = context.temp_dir.child("tools"); + let bin_dir = context.temp_dir.child("bin"); + + // Install `black`. + uv_snapshot!(context.filters(), context.tool_install() + .arg("-p") + .arg("3.12") + .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 [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] 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: black, blackd + "###); + + // Install with Python 3.12 (compatible). + uv_snapshot!(context.filters(), context.tool_install() + .arg("-p") + .arg("3.12") + .arg("black") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + warning: `uv tool install` is experimental and may change without warning. + Tool `black` is already installed + "###); + + // Install with Python 3.11 (incompatible). + uv_snapshot!(context.filters(), context.tool_install() + .arg("-p") + .arg("3.11") + .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 [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] 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: black, blackd + "###); +}