Add uv tool install --force (#4501)

Adds detection of existing entry points, avoiding clobbering entry
points that were installed by another tool. If we see any existing entry
point collisions, we'll stop instead of overwriting them. The `--force`
flag can be used to opt-in to overwriting the files; we can't use `-f`
because it's taken by `--find-links` which is silly. The `--force` flag
also implies replacing a tool previously installed by uv (the
environment is rebuilt).

Similarly, #4504 adds support for reinstalls that _will not_ clobber
entry points managed by other tools.
This commit is contained in:
Zanie Blue 2024-06-26 16:03:01 -04:00 committed by GitHub
parent dc408146ac
commit 747ab0d9f7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 205 additions and 24 deletions

View file

@ -1912,6 +1912,12 @@ pub struct ToolInstallArgs {
#[command(flatten)]
pub refresh: RefreshArgs,
/// Force installation of the tool.
///
/// Will replace any existing entry points with the same name in the executable directory.
#[arg(long)]
pub force: bool,
/// The Python interpreter to use to build the tool environment.
///
/// By default, uv will search for a Python executable in the `PATH`. uv ignores virtual

View file

@ -107,6 +107,20 @@ impl InstalledTools {
Ok(())
}
pub fn remove_environment(&self, name: &str) -> Result<(), Error> {
let _lock = self.acquire_lock();
let environment_path = self.root.join(name);
debug!(
"Deleting environment for tool `{name}` at {}",
environment_path.user_display()
);
fs_err::remove_dir_all(environment_path)?;
Ok(())
}
pub fn create_environment(
&self,
name: &str,

View file

@ -1,10 +1,13 @@
use std::collections::BTreeSet;
use std::ffi::OsString;
use std::fmt::Write;
use std::str::FromStr;
use anyhow::{bail, Context, Result};
use distribution_types::Name;
use pep508_rs::Requirement;
use itertools::Itertools;
use pep508_rs::Requirement;
use pypi_types::VerbatimParsedUrl;
use tracing::debug;
use uv_cache::Cache;
@ -31,6 +34,7 @@ pub(crate) async fn install(
python: Option<String>,
from: Option<String>,
with: Vec<String>,
force: bool,
settings: ResolverInstallerSettings,
preview: PreviewMode,
toolchain_preference: ToolchainPreference,
@ -46,10 +50,14 @@ pub(crate) async fn install(
let installed_tools = InstalledTools::from_settings()?;
// TODO(zanieb): Allow replacing an existing tool
// TODO(zanieb): Automatically replace an existing tool if the request differs
if installed_tools.find_tool_entry(&name)?.is_some() {
writeln!(printer.stderr(), "Tool `{name}` is already installed.")?;
return Ok(ExitStatus::Failure);
if force {
debug!("Replacing existing tool due to `--force` flag.");
} else {
writeln!(printer.stderr(), "Tool `{name}` is already installed.")?;
return Ok(ExitStatus::Failure);
}
}
// TODO(zanieb): Figure out the interface here, do we infer the name or do we match the `run --from` interface?
@ -113,17 +121,59 @@ pub(crate) async fn install(
fs_err::create_dir_all(&executable_directory)
.context("Failed to create executable directory")?;
debug!("Installing into {}", executable_directory.user_display());
let entrypoints = entrypoint_paths(
&environment,
installed_dist.name(),
installed_dist.version(),
)?;
// Determine the entry points targets
let targets = entrypoints
.into_iter()
.map(|(name, path)| {
let target = executable_directory.join(
path.file_name()
.map(std::borrow::ToOwned::to_owned)
.unwrap_or_else(|| OsString::from(name.clone())),
);
(name, path, target)
})
.collect::<Vec<_>>();
// Check if they exist, before installing
let mut existing_targets = targets
.iter()
.filter(|(_, _, target)| target.exists())
.peekable();
if force {
for (name, _, target) in existing_targets {
debug!("Removing existing install of `{name}`");
fs_err::remove_file(target)?;
}
} else if existing_targets.peek().is_some() {
// Clean up the environment we just created
installed_tools.remove_environment(&name)?;
let existing_targets = existing_targets
// SAFETY: We know the target has a filename because we just constructed it above
.map(|(_, _, target)| target.file_name().unwrap().to_string_lossy())
.collect::<BTreeSet<_>>();
let (s, exists) = if existing_targets.len() == 1 {
("", "exists")
} else {
("s", "exist")
};
bail!(
"Entry point{s} for tool already {exists}: {} (use `--force` to overwrite)",
existing_targets.iter().join(", ")
)
}
// TODO(zanieb): Handle the case where there are no entrypoints
// TODO(zanieb): Better error when an entry point exists, check if they all are don't exist first
for (name, path) in entrypoints {
let target = executable_directory.join(path.file_name().unwrap());
debug!("Installing {name} to {}", target.user_display());
for (name, path, target) in targets {
debug!("Installing `{name}`");
#[cfg(unix)]
replace_symlink(&path, &target).context("Failed to install entrypoint")?;
#[cfg(windows)]

View file

@ -815,6 +815,7 @@ async fn run() -> Result<ExitStatus> {
args.python,
args.from,
args.with,
args.force,
args.settings,
globals.preview,
globals.toolchain_preference,

View file

@ -241,6 +241,7 @@ pub(crate) struct ToolInstallSettings {
pub(crate) python: Option<String>,
pub(crate) refresh: Refresh,
pub(crate) settings: ResolverInstallerSettings,
pub(crate) force: bool,
}
impl ToolInstallSettings {
@ -252,6 +253,7 @@ impl ToolInstallSettings {
from,
with,
installer,
force,
build,
refresh,
python,
@ -262,6 +264,7 @@ impl ToolInstallSettings {
from,
with,
python,
force,
refresh: Refresh::from(refresh),
settings: ResolverInstallerSettings::combine(
resolver_installer_options(installer, build),

View file

@ -252,7 +252,7 @@ fn tool_install_twice() {
});
}
/// Test installing a tool when its entry pint already exists
/// Test installing a tool when its entry point already exists
#[test]
fn tool_install_entry_point_exists() {
let context = TestContext::new("3.12");
@ -262,13 +262,20 @@ fn tool_install_entry_point_exists() {
let executable = bin_dir.child(format!("black{}", std::env::consts::EXE_SUFFIX));
executable.touch().unwrap();
// Drop executable suffixes for cross-platform snapshtos
let filters = context
.filters()
.into_iter()
.chain([(std::env::consts::EXE_SUFFIX, "")])
.collect::<Vec<_>>();
// Install `black`
uv_snapshot!(context.filters(), context.tool_install()
uv_snapshot!(filters, context.tool_install()
.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
success: false
exit_code: 2
----- stdout -----
----- stderr -----
@ -282,24 +289,124 @@ fn tool_install_entry_point_exists() {
+ packaging==24.0
+ pathspec==0.12.1
+ platformdirs==4.2.0
error: Entry point for tool already exists: black (use `--force` to overwrite)
"###);
// TODO(zanieb): We happily overwrite entry points by default right now
// https://github.com/astral-sh/uv/pull/4501 should resolve this
// We should delete the virtual environment
assert!(!tool_dir.child("black").exists());
// We should not create a virtual environment
// assert!(tool_dir.child("black").exists());
// We should not write a tools entry
assert!(!tool_dir.join("tools.toml").exists());
// // We should not write a tools entry
// assert!(!tool_dir.join("tools.toml").exists());
insta::with_settings!({
filters => context.filters(),
}, {
// Nor should we change the `black` entry point that exists
assert_snapshot!(fs_err::read_to_string(bin_dir.join(format!("black{}", std::env::consts::EXE_SUFFIX))).unwrap(), @"");
// insta::with_settings!({
// filters => context.filters(),
// }, {
// // Nor should we change the `black` entry point that exists
// assert_snapshot!(fs_err::read_to_string(bin_dir.join("black")).unwrap(), @"");
});
// });
// Test error message when multiple entry points exist
bin_dir
.child(format!("blackd{}", std::env::consts::EXE_SUFFIX))
.touch()
.unwrap();
uv_snapshot!(filters, context.tool_install()
.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: 2
----- stdout -----
----- stderr -----
warning: `uv tool install` is experimental and may change without warning.
Resolved 6 packages in [TIME]
Installed 6 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
error: Entry points for tool already exist: black, blackd (use `--force` to overwrite)
"###);
// Install `black` with `--force`
uv_snapshot!(context.filters(), context.tool_install()
.arg("black")
.arg("--force")
.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 6 packages in [TIME]
Installed 6 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
"###);
tool_dir.child("black").assert(predicate::path::is_dir());
insta::with_settings!({
filters => context.filters(),
}, {
// We write a tool entry
assert_snapshot!(fs_err::read_to_string(tool_dir.join("tools.toml")).unwrap(), @r###"
[tools]
black = { requirements = ["black"] }
"###);
});
let executable = bin_dir.child(format!("black{}", std::env::consts::EXE_SUFFIX));
assert!(executable.exists());
// On Windows, we can't snapshot an executable file.
#[cfg(not(windows))]
insta::with_settings!({
filters => context.filters(),
}, {
// Should run black in the virtual environment
assert_snapshot!(fs_err::read_to_string(executable).unwrap(), @r###"
#![TEMP_DIR]/tools/black/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from black import patched_main
if __name__ == "__main__":
sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
sys.exit(patched_main())
"###);
});
insta::with_settings!({
filters => context.filters(),
}, {
// We should have a tool entry
assert_snapshot!(fs_err::read_to_string(tool_dir.join("tools.toml")).unwrap(), @r###"
[tools]
black = { requirements = ["black"] }
"###);
});
uv_snapshot!(context.filters(), Command::new("black").arg("--version").env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
black, 24.3.0 (compiled: yes)
Python (CPython) 3.12.[X]
----- stderr -----
"###);
}
/// Test `uv tool install` when the bin directory is inferred from `$HOME`