From d6c960359439af68bf8ee6da8209260468b62276 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Fri, 20 Sep 2024 14:01:14 -0500 Subject: [PATCH] Fix handling of `sys.base_prefix` collision in interpreter identity check during tool installs (#7596) Closes https://github.com/astral-sh/uv/issues/7586 Extends https://github.com/astral-sh/uv/pull/7593 (thanks @lucab!) --------- Co-authored-by: Luca BRUNO --- Cargo.lock | 1 + crates/uv/Cargo.toml | 1 + crates/uv/src/commands/tool/install.rs | 24 +++++++++++++++++------- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3164f0b72..c3ecec78d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4482,6 +4482,7 @@ dependencies = [ "regex", "reqwest", "rustc-hash", + "same-file", "serde", "serde_json", "similar", diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index 32af21d9b..c711da74d 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -66,6 +66,7 @@ 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 590958162..ef30de6eb 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -276,13 +276,23 @@ pub(crate) async fn install( installed_tools .get_environment(&from.name, &cache)? .filter(|environment| { - // NOTE(lucab): this compares `base_prefix` paths as a proxy for - // detecting interpreters mismatches. Directly comparing interpreters - // (by paths or binaries on-disk) would result in several false - // positives on Windows due to file-copying and shims. - let old_base_prefix = environment.interpreter().sys_base_prefix(); - let selected_base_prefix = interpreter.sys_base_prefix(); - if old_base_prefix == selected_base_prefix { + // TODO(zanieb): Consider using `sysconfig.get_path("stdlib")` instead, which + // should be generally robust. + // TODO(zanieb): Move this into a utility on `Interpreter` since it's non-trivial. + let same_interpreter = 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 = interpreter.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() == interpreter.sys_executable() + || same_file::is_same_file(environment.interpreter().sys_executable(), interpreter.sys_executable()).unwrap_or(false) + }; + + if same_interpreter { trace!( "Existing interpreter matches the requested interpreter for `{}`: {}", from.name,