From be7e3f287a8187358130debea41e26c9687b7934 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 31 Oct 2024 13:27:49 -0500 Subject: [PATCH] Improve interactions with existing Python executables during install --- Cargo.lock | 2 +- crates/uv-cli/src/lib.rs | 10 +- crates/uv-python/src/managed.rs | 31 +++- crates/uv/Cargo.toml | 2 +- crates/uv/src/commands/python/install.rs | 146 +++++++++++++++---- crates/uv/src/lib.rs | 1 + crates/uv/src/settings.rs | 13 +- crates/uv/tests/it/help.rs | 8 ++ crates/uv/tests/it/python_install.rs | 171 ++++++++++++++++++++++- docs/reference/cli.md | 6 + 10 files changed, 353 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 03fdd3cd80748..725db641b3ed2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4208,7 +4208,6 @@ dependencies = [ "regex", "reqwest", "rustc-hash", - "same-file", "serde", "serde_json", "similar", @@ -4257,6 +4256,7 @@ dependencies = [ "uv-shell", "uv-static", "uv-tool", + "uv-trampoline-builder", "uv-types", "uv-version", "uv-virtualenv", diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs index 1d5d57c46a9d1..e4780f5c6b667 100644 --- a/crates/uv-cli/src/lib.rs +++ b/crates/uv-cli/src/lib.rs @@ -3905,8 +3905,16 @@ pub struct PythonInstallArgs { /// /// By default, uv will exit successfully if the version is already /// installed. - #[arg(long, short, alias = "force")] + #[arg(long, short)] pub reinstall: bool, + + /// Replace existing Python executables during installation. + /// + /// By default, uv will refuse to replace executables that it does not manage. + /// + /// Implies `--reinstall`. + #[arg(long, short)] + pub force: bool, } #[derive(Args)] diff --git a/crates/uv-python/src/managed.rs b/crates/uv-python/src/managed.rs index c1d215b6411cd..9cf6f5cf7662e 100644 --- a/crates/uv-python/src/managed.rs +++ b/crates/uv-python/src/managed.rs @@ -88,7 +88,7 @@ pub enum Error { LibcDetection(#[from] LibcDetectionError), } /// A collection of uv-managed Python installations installed on the current system. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq)] pub struct ManagedPythonInstallations { /// The path to the top-level directory of the installed Python versions. root: PathBuf, @@ -542,6 +542,35 @@ impl ManagedPythonInstallation { unreachable!("Only Windows and Unix are supported") } } + + /// Returns `true` if self is a suitable upgrade of other. + pub fn is_upgrade_of(&self, other: &ManagedPythonInstallation) -> bool { + // Require matching implementation + if self.key.implementation != other.key.implementation { + return false; + } + // Require a default variant + if self.key.variant != PythonVariant::Default { + return false; + } + // Require matching minor version + if (self.key.major, self.key.minor) != (other.key.major, other.key.minor) { + return false; + } + // Require a newer, or equal patch version (for pre-release upgrades) + if self.key.patch <= other.key.patch { + return false; + } + if let Some(other_pre) = other.key.prerelease { + if let Some(self_pre) = self.key.prerelease { + return self_pre > other_pre; + } + // Do not upgrade from non-prerelease to prerelease + return false; + } + // Do not upgrade if the patch versions are the same + self.key.patch != other.key.patch + } } /// Generate a platform portion of a key from the environment. diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index 4572de35f6f32..87c7dd9a23012 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -48,6 +48,7 @@ uv-settings = { workspace = true, features = ["schemars"] } uv-shell = { workspace = true } uv-static = { workspace = true } uv-tool = { workspace = true } +uv-trampoline-builder = { workspace = true } uv-types = { workspace = true } uv-virtualenv = { workspace = true } uv-version = { workspace = true } @@ -78,7 +79,6 @@ rayon = { workspace = true } regex = { workspace = true } reqwest = { 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/python/install.rs b/crates/uv/src/commands/python/install.rs index 6e7f8902d7e73..c718566ce0507 100644 --- a/crates/uv/src/commands/python/install.rs +++ b/crates/uv/src/commands/python/install.rs @@ -8,7 +8,6 @@ use futures::StreamExt; use itertools::{Either, Itertools}; use owo_colors::OwoColorize; use rustc_hash::{FxHashMap, FxHashSet}; -use same_file::is_same_file; use tracing::{debug, trace}; use uv_client::Connectivity; @@ -20,6 +19,7 @@ use uv_python::managed::{ }; use uv_python::{PythonDownloads, PythonInstallationKey, PythonRequest, PythonVersionFile}; use uv_shell::Shell; +use uv_trampoline_builder::{Launcher, LauncherKind}; use uv_warnings::warn_user; use crate::commands::python::{ChangeEvent, ChangeEventKind}; @@ -73,7 +73,6 @@ struct Changelog { installed: FxHashSet, uninstalled: FxHashSet, installed_executables: FxHashMap>, - uninstalled_executables: FxHashSet, } impl Changelog { @@ -104,10 +103,12 @@ impl Changelog { } /// Download and install Python versions. +#[allow(clippy::fn_params_excessive_bools)] pub(crate) async fn install( project_dir: &Path, targets: Vec, reinstall: bool, + force: bool, python_downloads: PythonDownloads, native_tls: bool, connectivity: Connectivity, @@ -291,42 +292,102 @@ pub(crate) async fn install( .or_default() .push(target.clone()); } - Err(uv_python::managed::Error::LinkExecutable { from, to, err }) + Err(uv_python::managed::Error::LinkExecutable { from: _, to, err }) if err.kind() == ErrorKind::AlreadyExists => { - // TODO(zanieb): Add `--force` - if reinstall { - fs_err::remove_file(&to)?; - installation.create_bin_link(&target)?; - debug!( - "Updated executable at {} to {}", - target.user_display(), - installation.key(), - ); - changelog.installed.insert(installation.key().clone()); - changelog - .installed_executables - .entry(installation.key().clone()) - .or_default() - .push(target.clone()); - changelog.uninstalled_executables.insert(target); - } else { - if !is_same_file(&to, &from).unwrap_or_default() { - errors.push(( + debug!( + "Inspecting existing executable at {}", + target.user_display() + ); + + // Figure out what installation it references, if any + let existing = find_matching_bin_link(&existing_installations, &target); + + match existing { + None => { + // There's an existing executable we don't manage, require `--force` + if !force { + errors.push(( + installation.key(), + anyhow::anyhow!( + "Executable already exists at `{}` but is not managed by uv; use `--force` to replace it.", + to.user_display() + ), + )); + continue; + } + debug!( + "Replacing existing executable at {} due to `--force`", + target.user_display() + ); + } + Some(existing) if existing == installation => { + // The existing link points to the same installation, so we're done unless + // they requested we reinstall + if !(reinstall || force) { + debug!( + "Executable at {} is already for {}", + target.user_display(), + installation.key(), + ); + continue; + } + debug!( + "Replacing existing executable for {} at {}", installation.key(), - anyhow::anyhow!( - "Executable already exists at `{}`. Use `--reinstall` to force replacement.", - to.user_display() - ), - )); + target.user_display(), + ); + } + Some(existing) => { + // The existing link points to a different installation, check if it + // is reasonable to replace + if force { + debug!( + "Replacing existing executable for {} at {} with executable for {} due to `--force` flag", + existing.key(), + target.user_display(), + installation.key(), + ); + } else { + if installation.is_upgrade_of(existing) { + debug!( + "Replacing existing executable for {} at {} with executable for {} since it is an upgrade", + existing.key(), + target.user_display(), + installation.key(), + ); + } else { + debug!( + "Executable already exists at `{}` for `{}`. Use `--force` to replace.", + existing.key(), + to.user_display() + ); + continue; + } + } } } + + // Replace the existing link + fs_err::remove_file(&to)?; + installation.create_bin_link(&target)?; + debug!( + "Updated executable at {} to {}", + target.user_display(), + installation.key(), + ); + changelog.installed.insert(installation.key().clone()); + changelog + .installed_executables + .entry(installation.key().clone()) + .or_default() + .push(target.clone()); } Err(err) => return Err(err.into()), } } - if changelog.installed.is_empty() { + if changelog.installed.is_empty() && errors.is_empty() { if is_default_install { writeln!( printer.stderr(), @@ -483,3 +544,32 @@ fn warn_if_not_on_path(bin: &Path) { } } } + +/// Find the [`ManagedPythonInstallation`] corresponding to an executable link installed at the +/// given path, if any. +/// +/// Like [`ManagedPythonInstallation::is_bin_link`], but this method will only resolve the +/// given path one time. +fn find_matching_bin_link<'a>( + installations: &'a [ManagedPythonInstallation], + path: &Path, +) -> Option<&'a ManagedPythonInstallation> { + let target = if cfg!(unix) { + if !path.is_symlink() { + return None; + } + path.read_link().ok()? + } else if cfg!(windows) { + let launcher = Launcher::try_from_path(path).ok()??; + if !matches!(launcher.kind, LauncherKind::Python) { + return None; + } + launcher.python_path + } else { + unreachable!("Only Windows and Unix are supported") + }; + + installations + .iter() + .find(|installation| installation.executable() == target) +} diff --git a/crates/uv/src/lib.rs b/crates/uv/src/lib.rs index addb2e6792557..60eec76118960 100644 --- a/crates/uv/src/lib.rs +++ b/crates/uv/src/lib.rs @@ -1053,6 +1053,7 @@ async fn run(mut cli: Cli) -> Result { &project_dir, args.targets, args.reinstall, + args.force, globals.python_downloads, globals.native_tls, globals.connectivity, diff --git a/crates/uv/src/settings.rs b/crates/uv/src/settings.rs index d1a4fbc68fc0b..6f06a1e4e13d3 100644 --- a/crates/uv/src/settings.rs +++ b/crates/uv/src/settings.rs @@ -620,15 +620,24 @@ impl PythonDirSettings { pub(crate) struct PythonInstallSettings { pub(crate) targets: Vec, pub(crate) reinstall: bool, + pub(crate) force: bool, } impl PythonInstallSettings { /// Resolve the [`PythonInstallSettings`] from the CLI and filesystem configuration. #[allow(clippy::needless_pass_by_value)] pub(crate) fn resolve(args: PythonInstallArgs, _filesystem: Option) -> Self { - let PythonInstallArgs { targets, reinstall } = args; + let PythonInstallArgs { + targets, + reinstall, + force, + } = args; - Self { targets, reinstall } + Self { + targets, + reinstall, + force, + } } } diff --git a/crates/uv/tests/it/help.rs b/crates/uv/tests/it/help.rs index faf837a7a3588..f7464569ba836 100644 --- a/crates/uv/tests/it/help.rs +++ b/crates/uv/tests/it/help.rs @@ -441,6 +441,13 @@ fn help_subsubcommand() { By default, uv will exit successfully if the version is already installed. + -f, --force + Replace existing Python executables during installation. + + By default, uv will refuse to replace executables that it does not manage. + + Implies `--reinstall`. + Cache options: -n, --no-cache Avoid reading from or writing to the cache, instead using a temporary directory for the @@ -646,6 +653,7 @@ fn help_flag_subsubcommand() { Options: -r, --reinstall Reinstall the requested Python version, if it's already installed + -f, --force Replace existing Python executables during installation Cache options: -n, --no-cache Avoid reading from or writing to the cache, instead using a temporary diff --git a/crates/uv/tests/it/python_install.rs b/crates/uv/tests/it/python_install.rs index ce647abc12022..41f752a2a0116 100644 --- a/crates/uv/tests/it/python_install.rs +++ b/crates/uv/tests/it/python_install.rs @@ -1,7 +1,11 @@ -use std::process::Command; +use std::{path::Path, process::Command}; -use assert_fs::{assert::PathAssert, prelude::PathChild}; +use assert_fs::{ + assert::PathAssert, + prelude::{FileTouch, PathChild}, +}; use predicates::prelude::predicate; +use uv_fs::Simplified; use crate::common::{uv_snapshot, TestContext}; @@ -87,7 +91,9 @@ fn python_install() { #[test] fn python_install_preview() { - let context: TestContext = TestContext::new_with_versions(&[]).with_filtered_python_keys(); + let context: TestContext = TestContext::new_with_versions(&[]) + .with_filtered_python_keys() + .with_filtered_exe_suffix(); // Install the latest version uv_snapshot!(context.filters(), context.python_install().arg("--preview"), @r###" @@ -147,6 +153,50 @@ fn python_install_preview() { // The executable should still be present in the bin directory bin_python.assert(predicate::path::exists()); + // You can also force replacement of the executables + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("--force"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed Python 3.13.0 in [TIME] + + cpython-3.13.0-[PLATFORM] + "###); + + // The executable should still be present in the bin directory + bin_python.assert(predicate::path::exists()); + + // If an unmanaged executable is present, `--force` is required + fs_err::remove_file(bin_python.path()).unwrap(); + bin_python.touch().unwrap(); + + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.13"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + error: Failed to install cpython-3.13.0-[PLATFORM] + Caused by: Executable already exists at `bin/python3.13` but is not managed by uv; use `--force` to replace it. + "###); + + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("--force").arg("3.13"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed Python 3.13.0 in [TIME] + + cpython-3.13.0-[PLATFORM] + "###); + + bin_python.assert(predicate::path::exists()); + + // On Unix, it should be a link + #[cfg(unix)] + bin_python.assert(predicate::path::is_symlink()); + // Uninstallation requires an argument uv_snapshot!(context.filters(), context.python_uninstall(), @r###" success: false @@ -177,6 +227,103 @@ fn python_install_preview() { bin_python.assert(predicate::path::missing()); } +#[test] +fn python_install_preview_upgrade() { + let context = TestContext::new_with_versions(&[]).with_filtered_python_keys(); + + let bin_python = context + .temp_dir + .child("bin") + .child(format!("python3.12{}", std::env::consts::EXE_SUFFIX)); + + // Install 3.12.5 + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.12.5"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed Python 3.12.5 in [TIME] + + cpython-3.12.5-[PLATFORM] + "###); + + // Installing 3.12.4 should not replace the executable, but also shouldn't fail + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.12.4"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed Python 3.12.4 in [TIME] + + cpython-3.12.4-[PLATFORM] + "###); + + insta::with_settings!({ + filters => context.filters(), + }, { + insta::assert_snapshot!( + read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.5-[PLATFORM]" + ); + }); + + // Using `--reinstall` is not sufficient to replace it either + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.12.4").arg("--reinstall"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed Python 3.12.4 in [TIME] + ~ cpython-3.12.4-[PLATFORM] + "###); + + insta::with_settings!({ + filters => context.filters(), + }, { + insta::assert_snapshot!( + read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.5-[PLATFORM]" + ); + }); + + // But `--force` is + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.12.4").arg("--force"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed Python 3.12.4 in [TIME] + + cpython-3.12.4-[PLATFORM] + "###); + + insta::with_settings!({ + filters => context.filters(), + }, { + insta::assert_snapshot!( + read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.4-[PLATFORM]" + ); + }); + + // But installing 3.12.6 should upgrade automatically + uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.12.6"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed Python 3.12.6 in [TIME] + + cpython-3.12.6-[PLATFORM] + "###); + + insta::with_settings!({ + filters => context.filters(), + }, { + insta::assert_snapshot!( + read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.6-[PLATFORM]" + ); + }); +} + #[test] fn python_install_freethreaded() { let context: TestContext = TestContext::new_with_versions(&[]).with_filtered_python_keys(); @@ -283,3 +430,21 @@ fn python_install_invalid_request() { error: No download found for request: cpython-3.8.0-[PLATFORM] "###); } + +fn read_link_path(path: &Path) -> String { + if cfg!(unix) { + path.read_link() + .unwrap_or_else(|_| panic!("{} should be readable", path.display())) + .simplified_display() + .to_string() + } else if cfg!(windows) { + let launcher = uv_trampoline_builder::Launcher::try_from_path(path) + .ok() + .unwrap_or_else(|| panic!("{} should be readable", path.display())) + .unwrap_or_else(|| panic!("{} should be a valid launcher", path.display())); + let path = launcher.python_path.simplified_display().to_string(); + path + } else { + unreachable!() + } +} diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 057d220d74dfa..d76efdb8b4476 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -4358,6 +4358,12 @@ uv python install [OPTIONS] [TARGETS]...

See --project to only change the project root directory.

+
--force, -f

Replace existing Python executables during installation.

+ +

By default, uv will refuse to replace executables that it does not manage.

+ +

Implies --reinstall.

+
--help, -h

Display the concise help for this command

--native-tls

Whether to load TLS certificates from the platform’s native certificate store.