diff --git a/Cargo.lock b/Cargo.lock index 75bbcbd0ca40..a515c9dc939c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4490,7 +4490,6 @@ dependencies = [ "regex", "reqwest", "rustc-hash", - "same-file", "serde", "serde_json", "similar", diff --git a/Cargo.toml b/Cargo.toml index e52fee9d3645..41ea24236fe5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -175,7 +175,7 @@ xz2 = { version = "0.1.7" } zip = { version = "0.6.6", default-features = false, features = ["deflate"] } [workspace.metadata.cargo-shear] -ignored = ["flate2", "xz2", "same-file"] +ignored = ["flate2", "xz2"] [patch.crates-io] # For pyproject-toml diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index fb181f3e0c0a..aa887444dff8 100644 --- a/crates/uv-python/src/environment.rs +++ b/crates/uv-python/src/environment.rs @@ -300,4 +300,26 @@ impl PythonEnvironment { pub fn into_interpreter(self) -> Interpreter { Arc::unwrap_or_clone(self.0).interpreter } + + /// Returns `true` if the [`PythonEnvironment`] uses the same underlying [`Interpreter`]. + pub fn uses(&self, interpreter: &Interpreter) -> bool { + // TODO(zanieb): Consider using `sysconfig.get_path("stdlib")` instead, which + // should be generally robust. + if cfg!(windows) { + // On Windows, we can't canonicalize an interpreter based on its executable path + // because the executables are separate shim files (not links). Instead, we + // compare the `sys.base_prefix`. + let old_base_prefix = self.interpreter().sys_base_prefix(); + let selected_base_prefix = interpreter.sys_base_prefix(); + old_base_prefix == selected_base_prefix + } else { + // On Unix, we can see if the canonicalized executable is the same file. + self.interpreter().sys_executable() == interpreter.sys_executable() + || same_file::is_same_file( + self.interpreter().sys_executable(), + interpreter.sys_executable(), + ) + .unwrap_or(false) + } + } } diff --git a/crates/uv-python/src/interpreter.rs b/crates/uv-python/src/interpreter.rs index 42b0d34c58cc..30cdc37d37cf 100644 --- a/crates/uv-python/src/interpreter.rs +++ b/crates/uv-python/src/interpreter.rs @@ -26,8 +26,7 @@ use crate::implementation::LenientImplementationName; use crate::platform::{Arch, Libc, Os}; use crate::pointer_size::PointerSize; use crate::{ - Prefix, PythonEnvironment, PythonInstallationKey, PythonVersion, Target, VersionRequest, - VirtualEnvironment, + Prefix, PythonInstallationKey, PythonVersion, Target, VersionRequest, VirtualEnvironment, }; /// A Python executable and its associated platform markers. @@ -498,28 +497,6 @@ impl Interpreter { } } - /// Whether or not this interpreter is used by the given environment - pub fn is_interpreter_used_by(&self, environment: &PythonEnvironment) -> bool { - // TODO(zanieb): Consider using `sysconfig.get_path("stdlib")` instead, which - // should be generally robust. - if cfg!(windows) { - // On Windows, we can't canonicalize an interpreter based on its executable path - // because the executables are separate shim files (not links). Instead, we - // compare the `sys.base_prefix`. - let old_base_prefix = environment.interpreter().sys_base_prefix(); - let selected_base_prefix = self.sys_base_prefix(); - old_base_prefix == selected_base_prefix - } else { - // On Unix, we can see if the canonicalized executable is the same file. - environment.interpreter().sys_executable() == self.sys_executable() - || same_file::is_same_file( - environment.interpreter().sys_executable(), - self.sys_executable(), - ) - .unwrap_or(false) - } - } - /// Whether or not this Python interpreter is from a default Python executable name, like /// `python`, `python3`, or `python.exe`. pub(crate) fn has_default_executable_name(&self) -> bool { diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index a96f46faa36b..793ed11d5bba 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -72,7 +72,6 @@ owo-colors = { workspace = true } rayon = { workspace = true } regex = { workspace = true } rustc-hash = { workspace = true } -same-file = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } tempfile = { workspace = true } diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index 59bb08ff31ac..1723cb8697d7 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -277,7 +277,7 @@ pub(crate) async fn install( installed_tools .get_environment(&from.name, &cache)? .filter(|environment| { - if interpreter.is_interpreter_used_by(environment) { + if environment.uses(&interpreter) { trace!( "Existing interpreter matches the requested interpreter for `{}`: {}", from.name, diff --git a/crates/uv/src/commands/tool/upgrade.rs b/crates/uv/src/commands/tool/upgrade.rs index d37b38d94239..2296c62f88b6 100644 --- a/crates/uv/src/commands/tool/upgrade.rs +++ b/crates/uv/src/commands/tool/upgrade.rs @@ -1,7 +1,6 @@ use std::{collections::BTreeSet, fmt::Write}; use anyhow::Result; -use itertools::Itertools; use owo_colors::OwoColorize; use tracing::debug; @@ -29,12 +28,6 @@ use crate::commands::{tool::common::install_executables, ExitStatus, SharedState use crate::printer::Printer; use crate::settings::ResolverInstallerSettings; -enum ToolUpgradeResult { - ToolOrDependency, - Environment, - NoOp, -} - /// Upgrade a tool. pub(crate) async fn upgrade( name: Vec, @@ -52,6 +45,7 @@ pub(crate) async fn upgrade( let installed_tools = InstalledTools::from_settings()?.init()?; let _lock = installed_tools.lock().await?; + // Collect the tools to upgrade. let names: BTreeSet = { if name.is_empty() { installed_tools @@ -96,14 +90,17 @@ pub(crate) async fn upgrade( }; // Determine whether we applied any upgrades. - let mut did_upgrade = false; + let mut did_upgrade_tool = vec![]; + + // Determine whether we applied any upgrades. + let mut did_upgrade_environment = vec![]; // Determine whether any tool upgrade failed. let mut failed_upgrade = false; for name in &names { debug!("Upgrading tool: `{name}`"); - let upgrade_tool_result = upgrade_tool( + let result = upgrade_tool( name, interpreter.as_ref(), printer, @@ -117,12 +114,15 @@ pub(crate) async fn upgrade( ) .await; - match upgrade_tool_result { - Ok(ToolUpgradeResult::ToolOrDependency | ToolUpgradeResult::Environment) => { - did_upgrade |= true; + match result { + Ok(UpgradeOutcome::UpgradeEnvironment) => { + did_upgrade_environment.push(name); + } + Ok(UpgradeOutcome::UpgradeDependencies | UpgradeOutcome::UpgradeTool) => { + did_upgrade_tool.push(name); } - Ok(ToolUpgradeResult::NoOp) => { - debug!("Request for upgrade of {name} was a no-op"); + Ok(UpgradeOutcome::NoOp) => { + debug!("Upgrading `{name}` was a no-op"); } Err(err) => { // If we have a single tool, return the error directly. @@ -144,26 +144,40 @@ pub(crate) async fn upgrade( return Ok(ExitStatus::Failure); } - if !did_upgrade { + if did_upgrade_tool.is_empty() && did_upgrade_environment.is_empty() { writeln!(printer.stderr(), "Nothing to upgrade")?; } - if let (true, Some(python)) = (did_upgrade, python) { + if let Some(python_request) = python_request { + let tools = did_upgrade_environment + .iter() + .map(|name| format!("`{}`", name.cyan())) + .collect::>(); + let s = if tools.len() > 1 { "s" } else { "" }; writeln!( printer.stderr(), - "Upgraded build environment for {} to Python {}", - if let [name] = names.into_iter().collect_vec().as_slice() { - name.to_string() - } else { - "all tools".bold().to_string() - }, - python + "Upgraded tool environment{s} for {} to {}", + conjunction(tools), + python_request.cyan(), )?; } Ok(ExitStatus::Success) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum UpgradeOutcome { + /// The tool itself was upgraded. + UpgradeTool, + /// The tool's dependencies were upgraded, but the tool itself was unchanged. + UpgradeDependencies, + /// The tool's environment was upgraded. + UpgradeEnvironment, + /// The tool was already up-to-date. + NoOp, +} + +/// Upgrade a specific tool. async fn upgrade_tool( name: &PackageName, interpreter: Option<&Interpreter>, @@ -175,7 +189,7 @@ async fn upgrade_tool( connectivity: Connectivity, concurrency: Concurrency, native_tls: bool, -) -> Result { +) -> Result { // Ensure the tool is installed. let existing_tool_receipt = match installed_tools.get_tool_receipt(name) { Ok(Some(receipt)) => receipt, @@ -197,7 +211,7 @@ async fn upgrade_tool( } }; - let mut environment = match installed_tools.get_environment(name, cache) { + let environment = match installed_tools.get_environment(name, cache) { Ok(Some(environment)) => environment, Ok(None) => { let install_command = format!("uv tool install {name}"); @@ -231,54 +245,50 @@ async fn upgrade_tool( // Initialize any shared state. let state = SharedState::default(); - let mut tool_or_env_upgraded = false; - let mut changelog = None; - // Check if we need to create a new environment — if so, resolve it first, then // install the requested tool - if let Some(interpreter) = interpreter { - if !interpreter.is_interpreter_used_by(&environment) { - let resolution = resolve_environment( - RequirementsSpecification::from_requirements(requirements.to_vec()).into(), - interpreter, - settings.as_ref().into(), - &state, - Box::new(SummaryResolveLogger), - connectivity, - concurrency, - native_tls, - cache, - printer, - ) - .await?; - - environment = installed_tools.create_environment(name, interpreter.clone())?; - - environment = sync_environment( - environment, - &resolution.into(), - settings.as_ref().into(), - &state, - Box::new(DefaultInstallLogger), - connectivity, - concurrency, - native_tls, - cache, - printer, - ) - .await?; + let (environment, outcome) = if let Some(interpreter) = + interpreter.filter(|interpreter| !environment.uses(interpreter)) + { + // If we're using a new interpreter, re-create the environment for each tool. + let resolution = resolve_environment( + RequirementsSpecification::from_requirements(requirements.to_vec()).into(), + interpreter, + settings.as_ref().into(), + &state, + Box::new(SummaryResolveLogger), + connectivity, + concurrency, + native_tls, + cache, + printer, + ) + .await?; - tool_or_env_upgraded = true; - } - } + let environment = installed_tools.create_environment(name, interpreter.clone())?; - // If we didn't need to upgrade the environment, ensure the tool is updated - if !tool_or_env_upgraded { + let environment = sync_environment( + environment, + &resolution.into(), + settings.as_ref().into(), + &state, + Box::new(DefaultInstallLogger), + connectivity, + concurrency, + native_tls, + cache, + printer, + ) + .await?; + + (environment, UpgradeOutcome::UpgradeEnvironment) + } else { + // Otherwise, upgrade the existing environment. // TODO(zanieb): Build the environment in the cache directory then copy into the tool // directory. let EnvironmentUpdate { - environment: env_update, - changelog: changelog_update, + environment, + changelog, } = update_environment( environment, spec, @@ -294,18 +304,21 @@ async fn upgrade_tool( ) .await?; - environment = env_update; - if changelog_update.includes(name) { - tool_or_env_upgraded = true; - } - changelog = if changelog_update.is_empty() { - None + let outcome = if changelog.includes(name) { + UpgradeOutcome::UpgradeTool + } else if changelog.is_empty() { + UpgradeOutcome::NoOp } else { - Some(changelog_update) - } - } + UpgradeOutcome::UpgradeDependencies + }; - if tool_or_env_upgraded { + (environment, outcome) + }; + + if matches!( + outcome, + UpgradeOutcome::UpgradeEnvironment | UpgradeOutcome::UpgradeTool + ) { // At this point, we updated the existing environment, so we should remove any of its // existing executables. remove_entrypoints(&existing_tool_receipt); @@ -323,11 +336,32 @@ async fn upgrade_tool( )?; } - if changelog.is_some() { - Ok(ToolUpgradeResult::ToolOrDependency) - } else if tool_or_env_upgraded { - Ok(ToolUpgradeResult::Environment) - } else { - Ok(ToolUpgradeResult::NoOp) + Ok(outcome) +} + +/// Given a list of names, return a conjunction of the names (e.g., "Alice, Bob and Charlie"). +fn conjunction(names: Vec) -> String { + let mut names = names.into_iter(); + let first = names.next(); + let last = names.next_back(); + match (first, last) { + (Some(first), Some(last)) => { + let mut result = first; + let mut comma = false; + for name in names { + result.push_str(", "); + result.push_str(&name); + comma = true; + } + if comma { + result.push_str(", and "); + } else { + result.push_str(" and "); + } + result.push_str(&last); + result + } + (Some(first), None) => first, + _ => String::new(), } } diff --git a/crates/uv/tests/tool_upgrade.rs b/crates/uv/tests/tool_upgrade.rs index 744ece70e933..92537534cfd5 100644 --- a/crates/uv/tests/tool_upgrade.rs +++ b/crates/uv/tests/tool_upgrade.rs @@ -625,7 +625,7 @@ fn test_tool_upgrade_python() { + babel==2.6.0 + pytz==2018.5 Installed 1 executable: pybabel - Upgraded build environment for babel to Python 3.12 + Upgraded tool environment for `babel` to Python 3.12 "### ); @@ -710,7 +710,7 @@ fn test_tool_upgrade_python_with_all() { Installed [N] packages in [TIME] + python-dotenv==0.10.2.post2 Installed 1 executable: dotenv - Upgraded build environment for all tools to Python 3.12 + Upgraded tool environments for `babel` and `python-dotenv` to Python 3.12 "### );