Skip to content

Commit

Permalink
Make tool install robust to malformed receipts
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 22, 2024
1 parent 2d9df48 commit 4ab21b8
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 8 deletions.
34 changes: 28 additions & 6 deletions crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand Down Expand Up @@ -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());
Expand Down
75 changes: 73 additions & 2 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!({
Expand Down Expand Up @@ -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(())
}

0 comments on commit 4ab21b8

Please sign in to comment.