From 4ab21b84ff94b9f9b6d7bdb9d4f4be1dfef03028 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 22 Jul 2024 14:32:30 -0400 Subject: [PATCH] Make tool install robust to malformed receipts --- crates/uv/src/commands/tool/install.rs | 34 +++++++++--- crates/uv/tests/tool_install.rs | 75 +++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 8 deletions(-) diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index 309b9e5d58a8..e13558c282de 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -160,7 +160,34 @@ pub(crate) async fn install( let installed_tools = InstalledTools::from_settings()?.init()?; let _lock = installed_tools.acquire_lock()?; - let existing_tool_receipt = installed_tools.get_tool_receipt(&from.name)?; + + // Find the existing receipt, if it exists. If the receipt is present but malformed, we'll + // remove the environment and continue with the install. + // + // Later on, we want to replace entrypoints if the tool already exists, regardless of whether + // the receipt was valid. + // + // (If we find existing entrypoints later on, and the tool _doesn't_ exist, we'll avoid removing + // the external tool's entrypoints (without `--force`).) + let (existing_tool_receipt, reinstall_entry_points) = + match installed_tools.get_tool_receipt(&from.name) { + Ok(None) => (None, false), + Ok(Some(receipt)) => (Some(receipt), true), + Err(_) => { + // If the tool is not installed properly, remove the environment and continue. + match installed_tools.remove_environment(&from.name) { + Ok(()) => { + warn_user!("Removed existing `{}` with invalid receipt", from.name); + } + Err(uv_tool::Error::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => { + return Err(err.into()); + } + } + (None, true) + } + }; + let existing_environment = installed_tools .get_environment(&from.name, cache)? @@ -200,11 +227,6 @@ pub(crate) async fn install( } } - // Replace entrypoints if the tool already exists (and we made it this far). If we find existing - // entrypoints later on, and the tool _doesn't_ exist, we'll avoid removing the external tool's - // entrypoints (without `--force`). - let reinstall_entry_points = existing_tool_receipt.is_some(); - // Resolve the requirements. let state = SharedState::default(); let spec = RequirementsSpecification::from_requirements(requirements.clone()); diff --git a/crates/uv/tests/tool_install.rs b/crates/uv/tests/tool_install.rs index e2f58a18890b..a33303f538d0 100644 --- a/crates/uv/tests/tool_install.rs +++ b/crates/uv/tests/tool_install.rs @@ -2,9 +2,10 @@ use std::process::Command; +use anyhow::Result; use assert_fs::{ assert::PathAssert, - fixture::{FileTouch, PathChild}, + fixture::{FileTouch, FileWriteStr, PathChild}, }; use insta::assert_snapshot; use predicates::prelude::predicate; @@ -443,7 +444,6 @@ fn tool_install_already_installed() { sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0]) sys.exit(patched_main()) "###); - }); insta::with_settings!({ @@ -1609,3 +1609,74 @@ fn tool_install_warn_path() { warning: [TEMP_DIR]/bin is not on your PATH. To use installed tools, run export PATH="[TEMP_DIR]/bin:$PATH" or uv tool update-shell. "###); } + +/// Test installing and reinstalling with an invalid receipt. +#[test] +fn tool_install_bad_receipt() -> Result<()> { + let context = TestContext::new("3.12") + .with_filtered_counts() + .with_filtered_exe_suffix(); + let tool_dir = context.temp_dir.child("tools"); + let bin_dir = context.temp_dir.child("bin"); + + // Install `black` + uv_snapshot!(context.filters(), context.tool_install() + .arg("black") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()) + .env("PATH", 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 [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] 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 + Installed 2 executables: black, blackd + "###); + + tool_dir + .child("black") + .child("uv-receipt.toml") + .assert(predicate::path::exists()); + + // Override the `uv-receipt.toml` file with an invalid receipt. + tool_dir + .child("black") + .child("uv-receipt.toml") + .write_str("invalid")?; + + // Reinstall `black`, which should remove the invalid receipt. + uv_snapshot!(context.filters(), context.tool_install() + .arg("black") + .env("UV_TOOL_DIR", tool_dir.as_os_str()) + .env("XDG_BIN_HOME", bin_dir.as_os_str()) + .env("PATH", bin_dir.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv tool install` is experimental and may change without warning + warning: Removed existing `black` with invalid receipt + Resolved [N] packages in [TIME] + Installed [N] 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 + Installed 2 executables: black, blackd + "###); + + Ok(()) +}