Skip to content

Commit

Permalink
Add uv tool install --force
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Jun 26, 2024
1 parent ec03575 commit 46e4225
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 22 deletions.
6 changes: 6 additions & 0 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1908,6 +1908,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
Expand Down
14 changes: 14 additions & 0 deletions crates/uv-tool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
66 changes: 58 additions & 8 deletions crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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,
Expand All @@ -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?
Expand Down Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ async fn run() -> Result<ExitStatus> {
args.python,
args.from,
args.with,
args.force,
args.settings,
globals.preview,
globals.toolchain_preference,
Expand Down
3 changes: 3 additions & 0 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -252,6 +253,7 @@ impl ToolInstallSettings {
from,
with,
installer,
force,
build,
refresh,
python,
Expand All @@ -262,6 +264,7 @@ impl ToolInstallSettings {
from,
with,
python,
force,
refresh: Refresh::from(refresh),
settings: ResolverInstallerSettings::combine(
resolver_installer_options(installer, build),
Expand Down
135 changes: 121 additions & 14 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -262,9 +262,80 @@ 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!(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]
Prepared 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 point for tool already exists: black (use `--force` to overwrite)
"###);

// We should delete the virtual environment
assert!(!tool_dir.child("black").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(), @"");

});

// 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
Expand All @@ -274,7 +345,6 @@ fn tool_install_entry_point_exists() {
----- stderr -----
warning: `uv tool install` is experimental and may change without warning.
Resolved 6 packages in [TIME]
Prepared 6 packages in [TIME]
Installed 6 packages in [TIME]
+ black==24.3.0
+ click==8.1.7
Expand All @@ -284,22 +354,59 @@ fn tool_install_entry_point_exists() {
+ platformdirs==4.2.0
"###);

// TODO(zanieb): We happily overwrite entry points by default right now
// https://github.com/astral-sh/uv/pull/4501 should resolve this
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())
"###);

// 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());
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"] }
"###);
});

// 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(), @"");
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`
Expand Down

0 comments on commit 46e4225

Please sign in to comment.