Skip to content

Commit 4513bd8

Browse files
Clean up partial tool entrypoint installs (astral-sh#19966)
## Summary Prior to this change, `uv tool install` removed a newly created tool environment when entrypoint finalization failed, but left executables that had already been installed from additional packages. A missing root distribution also surfaced as an internal context error without cleaning up the environment or those entrypoints. This tracks installed entrypoint paths independently of the final tool receipt and removes them on the affected fatal paths before deleting the environment. Missing root distributions now use the existing no-executables diagnostic and the same cleanup behavior. The regression coverage uses `exclude-dependencies` to trigger the missing-root path after installing executables from an additional package, then verifies that both the environment and executables are removed. This fix was split from astral-sh#18937 because it is independent of tool locks.
1 parent 1b27b9c commit 4513bd8

2 files changed

Lines changed: 97 additions & 9 deletions

File tree

crates/uv/src/commands/tool/common.rs

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,16 @@ pub(super) fn matching_packages(name: &str, site_packages: &SitePackages) -> Vec
123123

124124
/// Remove any entrypoints attached to the [`Tool`].
125125
pub(crate) fn remove_entrypoints(tool: &Tool) {
126-
for executable in tool
127-
.entrypoints()
128-
.iter()
129-
.map(|entrypoint| &entrypoint.install_path)
130-
{
126+
remove_entrypoint_paths(
127+
tool.entrypoints()
128+
.iter()
129+
.map(|entrypoint| entrypoint.install_path.as_path()),
130+
);
131+
}
132+
133+
/// Remove the entrypoints at the given paths.
134+
fn remove_entrypoint_paths<'a>(entrypoints: impl IntoIterator<Item = &'a Path>) {
135+
for executable in entrypoints {
131136
debug!("Removing executable: `{}`", executable.simplified_display());
132137
if let Err(err) = fs_err::remove_file(executable) {
133138
warn!(
@@ -372,7 +377,7 @@ pub(crate) fn finalize_tool_install(
372377
executable_directory.user_display()
373378
);
374379

375-
let mut installed_entrypoints = Vec::new();
380+
let mut installed_entrypoints: Vec<ToolEntrypoint> = Vec::new();
376381
let site_packages = SitePackages::from_environment(environment)?;
377382
let ordered_packages = entrypoints
378383
// Install dependencies first
@@ -391,9 +396,29 @@ pub(crate) fn finalize_tool_install(
391396
}
392397

393398
let installed = site_packages.get_packages(package);
394-
let dist = installed
395-
.first()
396-
.context("Expected at least one requirement")?;
399+
let Some(dist) = installed.first() else {
400+
if package != name {
401+
bail!("Expected package `{package}` to be installed");
402+
}
403+
404+
writeln!(
405+
printer.stdout(),
406+
"No executables are provided by package `{}`; removing tool",
407+
package.cyan()
408+
)?;
409+
remove_entrypoint_paths(
410+
installed_entrypoints
411+
.iter()
412+
.map(|entrypoint| entrypoint.install_path.as_path()),
413+
);
414+
installed_tools.remove_environment(name)?;
415+
416+
return Err(NoExecutablesError::Root {
417+
package: package.clone(),
418+
matching_dependency_packages: Vec::new(),
419+
}
420+
.into());
421+
};
397422
let dist_entrypoints = entrypoint_paths(&site_packages, dist.name(), dist.version())?;
398423

399424
// Determine the entry points targets. Use a sorted collection for deterministic output.
@@ -446,6 +471,11 @@ pub(crate) fn finalize_tool_install(
446471
)?;
447472

448473
// Clean up the environment we just created.
474+
remove_entrypoint_paths(
475+
installed_entrypoints
476+
.iter()
477+
.map(|entrypoint| entrypoint.install_path.as_path()),
478+
);
449479
installed_tools.remove_environment(name)?;
450480

451481
return Err(err.into());
@@ -459,6 +489,11 @@ pub(crate) fn finalize_tool_install(
459489
.peekable();
460490
if existing_entrypoints.peek().is_some() {
461491
// Clean up the environment we just created
492+
remove_entrypoint_paths(
493+
installed_entrypoints
494+
.iter()
495+
.map(|entrypoint| entrypoint.install_path.as_path()),
496+
);
462497
installed_tools.remove_environment(name)?;
463498

464499
let existing_entrypoints = existing_entrypoints

crates/uv/tests/tool/tool_install.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3297,6 +3297,59 @@ fn tool_install_no_entrypoints() {
32973297
.assert(predicate::path::missing());
32983298
}
32993299

3300+
/// Test that a failed tool installation removes entrypoints installed from additional packages.
3301+
#[test]
3302+
fn tool_install_failure_removes_additional_entrypoints() -> Result<()> {
3303+
let context = uv_test::test_context!("3.12").with_filtered_exe_suffix();
3304+
let tool_dir = context.temp_dir.child("tools");
3305+
let bin_dir = context.temp_dir.child("bin");
3306+
3307+
context.temp_dir.child("uv.toml").write_str(
3308+
r#"
3309+
exclude-dependencies = ["iniconfig"]
3310+
"#,
3311+
)?;
3312+
3313+
uv_snapshot!(context.filters(), context.tool_install()
3314+
.arg("--with-executables-from")
3315+
.arg("black")
3316+
.arg("iniconfig")
3317+
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
3318+
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str())
3319+
.env(EnvVars::PATH, bin_dir.as_os_str()), @"
3320+
success: false
3321+
exit_code: 2
3322+
----- stdout -----
3323+
No executables are provided by package `iniconfig`; removing tool
3324+
3325+
----- stderr -----
3326+
Resolved 7 packages in [TIME]
3327+
Prepared 7 packages in [TIME]
3328+
Installed 7 packages in [TIME]
3329+
+ black==24.3.0
3330+
+ click==8.1.7
3331+
+ iniconfig==2.0.0
3332+
+ mypy-extensions==1.0.0
3333+
+ packaging==24.0
3334+
+ pathspec==0.12.1
3335+
+ platformdirs==4.2.0
3336+
Installed 2 executables from `black`: black, blackd
3337+
error: Failed to install entrypoints for `iniconfig`
3338+
");
3339+
3340+
tool_dir
3341+
.child("iniconfig")
3342+
.assert(predicate::path::missing());
3343+
bin_dir
3344+
.child(format!("black{}", std::env::consts::EXE_SUFFIX))
3345+
.assert(predicate::path::missing());
3346+
bin_dir
3347+
.child(format!("blackd{}", std::env::consts::EXE_SUFFIX))
3348+
.assert(predicate::path::missing());
3349+
3350+
Ok(())
3351+
}
3352+
33003353
#[test]
33013354
fn tool_install_no_binary_package_env_var() {
33023355
let context = uv_test::test_context!("3.12").with_filtered_exe_suffix();

0 commit comments

Comments
 (0)