From 1476df5251c4f35d10cb77acb0ac78c84e9c49bf Mon Sep 17 00:00:00 2001 From: Marukome0743 Date: Sun, 2 Aug 2026 11:42:17 +0900 Subject: [PATCH] fix(install): restore runtime symlinks after failures --- e2e/cli/test_install_failure_runtime_symlinks | 46 +++++++++++++ src/runtime_symlinks.rs | 64 +++++++++++++++++-- src/toolset/toolset_install.rs | 36 +++++++++-- 3 files changed, 133 insertions(+), 13 deletions(-) create mode 100644 e2e/cli/test_install_failure_runtime_symlinks diff --git a/e2e/cli/test_install_failure_runtime_symlinks b/e2e/cli/test_install_failure_runtime_symlinks new file mode 100644 index 00000000000..de9836ae427 --- /dev/null +++ b/e2e/cli/test_install_failure_runtime_symlinks @@ -0,0 +1,46 @@ +#!/usr/bin/env bash + +set -euo pipefail + +installs_dir="$MISE_DATA_DIR/installs/dummy" + +mise install dummy@1.0.0 +assert "test \"\$(readlink '$installs_dir/latest')\" = ./1.0.0" +assert "test \"\$(readlink '$installs_dir/1.0')\" = ./1.0.0" + +# Model aliases created for an install that is later cleaned up after failing. +ln -sfn ./other-dummy "$installs_dir/latest" +ln -sfn ./other-dummy "$installs_dir/1.0" + +# Planning an install must not repair or otherwise mutate runtime aliases. +assert_contains "mise install dummy@other-dummy --dry-run 2>&1" "would install" +assert "test \"\$(readlink '$installs_dir/latest')\" = ./other-dummy" +assert "test \"\$(readlink '$installs_dir/1.0')\" = ./other-dummy" + +install_error=$(quiet_assert_fail "mise install dummy@other-dummy 2>&1") +assert_contains_text "$install_error" "Failed to install asdf:dummy@other-dummy" +assert_contains_text "$install_error" "Dummy couldn't install version: other-dummy (on purpose)" +assert "test ! -e '$installs_dir/other-dummy'" +assert "test \"\$(readlink '$installs_dir/latest')\" = ./1.0.0" +assert "test \"\$(readlink '$installs_dir/1.0')\" = ./1.0.0" + +# A dependent blocked before its installer starts must not have its pre-existing +# aliases repaired as a side effect of the dependency's failure. +blocked_installs_dir="$MISE_DATA_DIR/installs/needs-dummy" +mise install needs-dummy@1.0.0 +ln -sfn ./blocked-target "$blocked_installs_dir/latest" +blocked_error=$(quiet_assert_fail "mise install dummy@other-dummy 'needs-dummy[depends=dummy]@1.0.0' 2>&1") +assert_contains_text "$blocked_error" "Skipped due to failed dependency" +assert "test \"\$(readlink '$blocked_installs_dir/latest')\" = ./blocked-target" + +# With no valid install left, a failed install should remove dangling aliases. +mise uninstall dummy@1.0.0 +mkdir -p "$installs_dir" +ln -s ./other-dummy "$installs_dir/latest" +ln -s ./other-dummy "$installs_dir/1.0" + +install_error=$(quiet_assert_fail "mise install dummy@other-dummy 2>&1") +assert_contains_text "$install_error" "Failed to install asdf:dummy@other-dummy" +assert_contains_text "$install_error" "Dummy couldn't install version: other-dummy (on purpose)" +assert "test ! -L '$installs_dir/latest'" +assert "test ! -L '$installs_dir/1.0'" diff --git a/src/runtime_symlinks.rs b/src/runtime_symlinks.rs index ec7d8e04574..74ac2094cf0 100644 --- a/src/runtime_symlinks.rs +++ b/src/runtime_symlinks.rs @@ -8,18 +8,52 @@ use crate::plugins::VERSION_REGEX; use crate::semver::split_version_prefix; use crate::toolset::{ToolRequest, Toolset}; use crate::{backend, env, file}; -use eyre::Result; +use eyre::{Result, WrapErr}; use indexmap::IndexMap; use itertools::Itertools; use versions::Versioning; pub async fn rebuild_for_toolset(config: &Config, ts: &Toolset) -> Result<()> { - for backend in ts.list_cached_and_current_backends() { - for installs_dir in install_dirs_for(&backend) { - rebuild_symlinks_in_dir(config, ts, &backend, &installs_dir)?; - } + rebuild_for_backends(config, ts, ts.list_cached_and_current_backends()).await +} + +pub(crate) async fn rebuild_for_backends( + config: &Config, + ts: &Toolset, + backends: impl IntoIterator>, +) -> Result<()> { + let rebuilds = backends.into_iter().flat_map(|backend| { + install_dirs_for(&backend) + .into_iter() + .map(move |installs_dir| (backend.clone(), installs_dir)) + }); + run_all_rebuilds(rebuilds, |(backend, installs_dir)| { + rebuild_symlinks_in_dir(config, ts, &backend, &installs_dir).wrap_err_with(|| { + format!( + "failed to rebuild runtime symlinks for {} in {}", + backend.ba().short, + installs_dir.display() + ) + }) + }) +} + +fn run_all_rebuilds( + rebuilds: impl IntoIterator, + mut rebuild: impl FnMut(T) -> Result<()>, +) -> Result<()> { + let errors = rebuilds + .into_iter() + .filter_map(|item| rebuild(item).err()) + .collect_vec(); + if errors.is_empty() { + return Ok(()); } - Ok(()) + Err(eyre::eyre!( + "{} runtime symlink repair(s) failed:\n{}", + errors.len(), + errors.iter().map(|err| format!("{err:#}")).join("\n") + )) } pub async fn migrate_real_dirs(config: &Config) -> Result<()> { @@ -266,6 +300,24 @@ mod tests { use super::*; use std::fs; + #[test] + fn run_all_rebuilds_attempts_every_item() { + let mut attempted = vec![]; + let err = run_all_rebuilds([1, 2, 3], |item| { + attempted.push(item); + if item == 1 || item == 3 { + eyre::bail!("repair {item} failed"); + } + Ok(()) + }) + .unwrap_err(); + + assert_eq!(attempted, [1, 2, 3]); + let message = format!("{err:#}"); + assert!(message.contains("repair 1 failed")); + assert!(message.contains("repair 3 failed")); + } + // https://github.com/jdx/mise/discussions/5260 — on Windows runtime // symlinks are regular files containing the target, so dangling pointers // were never detected as missing. diff --git a/src/toolset/toolset_install.rs b/src/toolset/toolset_install.rs index 4c48b01d9cc..4d0154f247b 100644 --- a/src/toolset/toolset_install.rs +++ b/src/toolset/toolset_install.rs @@ -21,7 +21,7 @@ use crate::toolset::tool_request::ToolRequest; use crate::toolset::tool_source::ToolSource; use crate::toolset::tool_version::{ResolveOptions, ToolVersion}; use crate::ui::multi_progress_report::MultiProgressReport; -use crate::{backend, config, hooks}; +use crate::{backend, config, hooks, runtime_symlinks}; impl Toolset { #[async_backtrace::framed] @@ -164,7 +164,13 @@ impl Toolset { preflight_system_deps(config, &versions, opts).await; // Build dependency graph and install using Kahn's algorithm - let (installed, failed) = self.install_with_deps(config, versions, opts).await; + let (installed, failed, attempted_failures) = + self.install_with_deps(config, versions, opts).await; + let failed_backends = attempted_failures + .iter() + .filter_map(|tr| tr.backend().ok()) + .unique_by(|backend| backend.ba().installs_path.clone()) + .collect_vec(); // Update footer for errors found before install tasks are spawned. let pre_install_error_count = disabled_backend_errors.len() + plugin_errors.len(); @@ -186,6 +192,12 @@ impl Toolset { if let Err(err) = self.resolve(config).await { debug!("error resolving versions after install: {err:#}"); } + if !failed_backends.is_empty() + && let Err(err) = + runtime_symlinks::rebuild_for_backends(config, self, failed_backends).await + { + warn!("failed to restore runtime symlinks after install failure: {err:#}"); + } } // Debug logging for successful installations @@ -314,15 +326,20 @@ impl Toolset { } /// Install tools using Kahn's algorithm for dependency ordering. - /// Returns (successful_installations, failed_installations). + /// Returns (successful_installations, failed_installations, attempted_failures). + /// The final collection excludes tools blocked before their installer was spawned. async fn install_with_deps( &self, config: &Arc, versions: Vec, opts: &InstallOptions, - ) -> (Vec, Vec<(ToolRequest, eyre::Error)>) { + ) -> ( + Vec, + Vec<(ToolRequest, eyre::Error)>, + Vec, + ) { if versions.is_empty() { - return (vec![], vec![]); + return (vec![], vec![], vec![]); } // Build index map to preserve original request order @@ -341,7 +358,7 @@ impl Toolset { .into_iter() .map(|tr| (tr, eyre::eyre!("Failed to build dependency graph: {}", e))) .collect(); - return (vec![], failed); + return (vec![], failed, vec![]); } }; @@ -358,6 +375,7 @@ impl Toolset { let mut installed = vec![]; let mut failed = vec![]; + let mut attempted_failures = vec![]; let mut jset: JoinSet<(ToolRequest, Result)> = JoinSet::new(); // Track in-flight tools to recover from task panics let mut in_flight: HashMap = HashMap::new(); @@ -380,6 +398,7 @@ impl Toolset { } Ok((tr, Err(e))) => { mpr.footer_inc(1); + attempted_failures.push(tr.clone()); failed.push((tr.clone(), e)); tool_deps.lock().await.complete_failure(&tr); } @@ -387,6 +406,7 @@ impl Toolset { // Task panicked - try to recover the tool request from in_flight tracking mpr.footer_inc(1); if let Some(tr) = in_flight.remove(&e.id()) { + attempted_failures.push(tr.clone()); failed.push((tr.clone(), eyre::eyre!("Installation task panicked: {e:#}"))); tool_deps.lock().await.complete_failure(&tr); } else { @@ -446,12 +466,14 @@ impl Toolset { } Ok((tr, Err(e))) => { mpr.footer_inc(1); + attempted_failures.push(tr.clone()); failed.push((tr.clone(), e)); tool_deps.lock().await.complete_failure(&tr); } Err(e) => { mpr.footer_inc(1); if let Some(tr) = in_flight.remove(&e.id()) { + attempted_failures.push(tr.clone()); failed.push((tr.clone(), eyre::eyre!("Installation task panicked: {e:#}"))); tool_deps.lock().await.complete_failure(&tr); } else { @@ -474,7 +496,7 @@ impl Toolset { request_order.get(&key).copied().unwrap_or(usize::MAX) }); - (installed, failed) + (installed, failed, attempted_failures) } /// Install a single tool