Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make tool install robust to malformed receipts #5305

Merged
merged 1 commit into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(())
}
Loading