Improve interactions with existing Python executables during install (#8733)

Previously, we'd use the `--reinstall` flag to determine if we should
replace existing Python executables in the bin directory during an
install. There are a few problems with this:

- We replace executables we don't manage
- We can replace executables from other uv Python installations during
reinstall (surprising)
- We don't do the "right" thing when installing patch versions e.g.
installing `3.12.4` then `3.12.6` would fail without the reinstall flag

In `uv tool`, we have separate `--force` and `--reinstall` concepts.
Here we separate the flags (`--force` was previously just a
`--reinstall` alias) and add inspection of the existing executables to
inform a decision on replacement.

In brief, we will:

- Replace any executables with `--force`
- Replace executables for the same installation with `--reinstall`
- Replace executables for an older patch version by default
This commit is contained in:
Zanie Blue 2024-11-04 14:22:44 -06:00 committed by GitHub
parent 6a6b9af466
commit fb1d679f69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 354 additions and 38 deletions

2
Cargo.lock generated
View File

@ -4205,7 +4205,6 @@ dependencies = [
"regex", "regex",
"reqwest", "reqwest",
"rustc-hash", "rustc-hash",
"same-file",
"serde", "serde",
"serde_json", "serde_json",
"similar", "similar",
@ -4254,6 +4253,7 @@ dependencies = [
"uv-shell", "uv-shell",
"uv-static", "uv-static",
"uv-tool", "uv-tool",
"uv-trampoline-builder",
"uv-types", "uv-types",
"uv-version", "uv-version",
"uv-virtualenv", "uv-virtualenv",

View File

@ -3946,8 +3946,16 @@ pub struct PythonInstallArgs {
/// ///
/// By default, uv will exit successfully if the version is already /// By default, uv will exit successfully if the version is already
/// installed. /// installed.
#[arg(long, short, alias = "force")] #[arg(long, short)]
pub reinstall: bool, 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)] #[derive(Args)]

View File

@ -88,7 +88,7 @@ pub enum Error {
LibcDetection(#[from] LibcDetectionError), LibcDetection(#[from] LibcDetectionError),
} }
/// A collection of uv-managed Python installations installed on the current system. /// A collection of uv-managed Python installations installed on the current system.
#[derive(Debug, Clone)] #[derive(Debug, Clone, Eq, PartialEq)]
pub struct ManagedPythonInstallations { pub struct ManagedPythonInstallations {
/// The path to the top-level directory of the installed Python versions. /// The path to the top-level directory of the installed Python versions.
root: PathBuf, root: PathBuf,
@ -542,6 +542,35 @@ impl ManagedPythonInstallation {
unreachable!("Only Windows and Unix are supported") 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 matching variant
if self.key.variant != other.key.variant {
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. /// Generate a platform portion of a key from the environment.

View File

@ -48,6 +48,7 @@ uv-settings = { workspace = true, features = ["schemars"] }
uv-shell = { workspace = true } uv-shell = { workspace = true }
uv-static = { workspace = true } uv-static = { workspace = true }
uv-tool = { workspace = true } uv-tool = { workspace = true }
uv-trampoline-builder = { workspace = true }
uv-types = { workspace = true } uv-types = { workspace = true }
uv-virtualenv = { workspace = true } uv-virtualenv = { workspace = true }
uv-version = { workspace = true } uv-version = { workspace = true }
@ -79,7 +80,6 @@ rayon = { workspace = true }
regex = { workspace = true } regex = { workspace = true }
reqwest = { workspace = true } reqwest = { workspace = true }
rustc-hash = { workspace = true } rustc-hash = { workspace = true }
same-file = { workspace = true }
serde = { workspace = true } serde = { workspace = true }
serde_json = { workspace = true } serde_json = { workspace = true }
tempfile = { workspace = true } tempfile = { workspace = true }

View File

@ -8,7 +8,6 @@ use futures::StreamExt;
use itertools::{Either, Itertools}; use itertools::{Either, Itertools};
use owo_colors::OwoColorize; use owo_colors::OwoColorize;
use rustc_hash::{FxHashMap, FxHashSet}; use rustc_hash::{FxHashMap, FxHashSet};
use same_file::is_same_file;
use tracing::{debug, trace}; use tracing::{debug, trace};
use uv_client::Connectivity; use uv_client::Connectivity;
@ -20,6 +19,7 @@ use uv_python::managed::{
}; };
use uv_python::{PythonDownloads, PythonInstallationKey, PythonRequest, PythonVersionFile}; use uv_python::{PythonDownloads, PythonInstallationKey, PythonRequest, PythonVersionFile};
use uv_shell::Shell; use uv_shell::Shell;
use uv_trampoline_builder::{Launcher, LauncherKind};
use uv_warnings::warn_user; use uv_warnings::warn_user;
use crate::commands::python::{ChangeEvent, ChangeEventKind}; use crate::commands::python::{ChangeEvent, ChangeEventKind};
@ -73,7 +73,6 @@ struct Changelog {
installed: FxHashSet<PythonInstallationKey>, installed: FxHashSet<PythonInstallationKey>,
uninstalled: FxHashSet<PythonInstallationKey>, uninstalled: FxHashSet<PythonInstallationKey>,
installed_executables: FxHashMap<PythonInstallationKey, Vec<PathBuf>>, installed_executables: FxHashMap<PythonInstallationKey, Vec<PathBuf>>,
uninstalled_executables: FxHashSet<PathBuf>,
} }
impl Changelog { impl Changelog {
@ -104,10 +103,12 @@ impl Changelog {
} }
/// Download and install Python versions. /// Download and install Python versions.
#[allow(clippy::fn_params_excessive_bools)]
pub(crate) async fn install( pub(crate) async fn install(
project_dir: &Path, project_dir: &Path,
targets: Vec<String>, targets: Vec<String>,
reinstall: bool, reinstall: bool,
force: bool,
python_downloads: PythonDownloads, python_downloads: PythonDownloads,
native_tls: bool, native_tls: bool,
connectivity: Connectivity, connectivity: Connectivity,
@ -281,7 +282,7 @@ pub(crate) async fn install(
Ok(()) => { Ok(()) => {
debug!( debug!(
"Installed executable at {} for {}", "Installed executable at {} for {}",
target.user_display(), target.simplified_display(),
installation.key(), installation.key(),
); );
changelog.installed.insert(installation.key().clone()); changelog.installed.insert(installation.key().clone());
@ -291,42 +292,102 @@ pub(crate) async fn install(
.or_default() .or_default()
.push(target.clone()); .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 => if err.kind() == ErrorKind::AlreadyExists =>
{ {
// TODO(zanieb): Add `--force` debug!(
if reinstall { "Inspecting existing executable at {}",
fs_err::remove_file(&to)?; target.simplified_display()
installation.create_bin_link(&target)?; );
debug!(
"Updated executable at {} to {}", // Figure out what installation it references, if any
target.user_display(), let existing = find_matching_bin_link(&existing_installations, &target);
installation.key(),
); match existing {
changelog.installed.insert(installation.key().clone()); None => {
changelog // There's an existing executable we don't manage, require `--force`
.installed_executables if !force {
.entry(installation.key().clone()) errors.push((
.or_default() installation.key(),
.push(target.clone()); anyhow::anyhow!(
changelog.uninstalled_executables.insert(target); "Executable already exists at `{}` but is not managed by uv; use `--force` to replace it",
} else { to.simplified_display()
if !is_same_file(&to, &from).unwrap_or_default() { ),
errors.push(( ));
continue;
}
debug!(
"Replacing existing executable at `{}` due to `--force`",
target.simplified_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.simplified_display(),
installation.key(),
);
continue;
}
debug!(
"Replacing existing executable for `{}` at `{}`",
installation.key(), installation.key(),
anyhow::anyhow!( target.simplified_display(),
"Executable already exists at `{}`. Use `--reinstall` to force replacement.", );
to.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.simplified_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.simplified_display(),
installation.key(),
);
} else {
debug!(
"Executable already exists at `{}` for `{}`. Use `--force` to replace it.",
existing.key(),
to.simplified_display()
);
continue;
}
}
} }
} }
// Replace the existing link
fs_err::remove_file(&to)?;
installation.create_bin_link(&target)?;
debug!(
"Updated executable at `{}` to `{}`",
target.simplified_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()), Err(err) => return Err(err.into()),
} }
} }
if changelog.installed.is_empty() { if changelog.installed.is_empty() && errors.is_empty() {
if is_default_install { if is_default_install {
writeln!( writeln!(
printer.stderr(), 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)
}

View File

@ -1054,6 +1054,7 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
&project_dir, &project_dir,
args.targets, args.targets,
args.reinstall, args.reinstall,
args.force,
globals.python_downloads, globals.python_downloads,
globals.native_tls, globals.native_tls,
globals.connectivity, globals.connectivity,

View File

@ -629,15 +629,24 @@ impl PythonDirSettings {
pub(crate) struct PythonInstallSettings { pub(crate) struct PythonInstallSettings {
pub(crate) targets: Vec<String>, pub(crate) targets: Vec<String>,
pub(crate) reinstall: bool, pub(crate) reinstall: bool,
pub(crate) force: bool,
} }
impl PythonInstallSettings { impl PythonInstallSettings {
/// Resolve the [`PythonInstallSettings`] from the CLI and filesystem configuration. /// Resolve the [`PythonInstallSettings`] from the CLI and filesystem configuration.
#[allow(clippy::needless_pass_by_value)] #[allow(clippy::needless_pass_by_value)]
pub(crate) fn resolve(args: PythonInstallArgs, _filesystem: Option<FilesystemOptions>) -> Self { pub(crate) fn resolve(args: PythonInstallArgs, _filesystem: Option<FilesystemOptions>) -> Self {
let PythonInstallArgs { targets, reinstall } = args; let PythonInstallArgs {
targets,
reinstall,
force,
} = args;
Self { targets, reinstall } Self {
targets,
reinstall,
force,
}
} }
} }

View File

@ -441,6 +441,13 @@ fn help_subsubcommand() {
By default, uv will exit successfully if the version is already installed. 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: Cache options:
-n, --no-cache -n, --no-cache
Avoid reading from or writing to the cache, instead using a temporary directory for the Avoid reading from or writing to the cache, instead using a temporary directory for the
@ -646,6 +653,7 @@ fn help_flag_subsubcommand() {
Options: Options:
-r, --reinstall Reinstall the requested Python version, if it's already installed -r, --reinstall Reinstall the requested Python version, if it's already installed
-f, --force Replace existing Python executables during installation
Cache options: Cache options:
-n, --no-cache Avoid reading from or writing to the cache, instead using a temporary -n, --no-cache Avoid reading from or writing to the cache, instead using a temporary

View File

@ -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 predicates::prelude::predicate;
use uv_fs::Simplified;
use crate::common::{uv_snapshot, TestContext}; use crate::common::{uv_snapshot, TestContext};
@ -87,7 +91,9 @@ fn python_install() {
#[test] #[test]
fn python_install_preview() { 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 // Install the latest version
uv_snapshot!(context.filters(), context.python_install().arg("--preview"), @r###" 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 // The executable should still be present in the bin directory
bin_python.assert(predicate::path::exists()); 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 `[TEMP_DIR]/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 // Uninstallation requires an argument
uv_snapshot!(context.filters(), context.python_uninstall(), @r###" uv_snapshot!(context.filters(), context.python_uninstall(), @r###"
success: false success: false
@ -177,6 +227,103 @@ fn python_install_preview() {
bin_python.assert(predicate::path::missing()); 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] #[test]
fn python_install_freethreaded() { fn python_install_freethreaded() {
let context: TestContext = TestContext::new_with_versions(&[]).with_filtered_python_keys(); 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] 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!()
}
}

View File

@ -4384,6 +4384,12 @@ uv python install [OPTIONS] [TARGETS]...
<p>See <code>--project</code> to only change the project root directory.</p> <p>See <code>--project</code> to only change the project root directory.</p>
</dd><dt><code>--force</code>, <code>-f</code></dt><dd><p>Replace existing Python executables during installation.</p>
<p>By default, uv will refuse to replace executables that it does not manage.</p>
<p>Implies <code>--reinstall</code>.</p>
</dd><dt><code>--help</code>, <code>-h</code></dt><dd><p>Display the concise help for this command</p> </dd><dt><code>--help</code>, <code>-h</code></dt><dd><p>Display the concise help for this command</p>
</dd><dt><code>--native-tls</code></dt><dd><p>Whether to load TLS certificates from the platform&#8217;s native certificate store.</p> </dd><dt><code>--native-tls</code></dt><dd><p>Whether to load TLS certificates from the platform&#8217;s native certificate store.</p>