From 21b87bc69ac8395db5a178d13ead6321e553de06 Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Sun, 9 Aug 2026 16:03:37 +0900 Subject: [PATCH] fix(cli): resolve the base of `sub-N:` before subtracting `sub-N:` accepts `latest` and aliases as its base, but `version_sub()` can only subtract from a concrete version. `ToolVersion::resolve_sub` resolves the base first, which is why `mise use`/`mise install` handle `sub-2:lts`; two other call sites did not. `mise ls-remote node@sub-2:lts` passed the raw string through and panicked: called `Option::unwrap()` on a `None` value src/toolset/tool_request.rs:592 `mise latest node@sub-2:lts` had no `Sub` arm at all and bailed with "invalid version" for every `sub-N:` spec, numeric base included. Extracts the base resolution into `resolve_sub_base()` and routes all three callers through it, and makes `version_sub()` return a `Result` so an unresolved base can no longer abort the process. --- e2e/cli/test_latest | 13 ++++++ e2e/cli/test_ls_remote | 16 +++++++ src/cli/latest.rs | 21 +++++++-- src/cli/ls_remote.rs | 7 ++- src/toolset/mod.rs | 1 + src/toolset/tool_request.rs | 92 +++++++++++++++++++++++++------------ src/toolset/tool_version.rs | 55 ++++++++++++++++------ 7 files changed, 156 insertions(+), 49 deletions(-) diff --git a/e2e/cli/test_latest b/e2e/cli/test_latest index 0880eda1950..379b4078c9c 100644 --- a/e2e/cli/test_latest +++ b/e2e/cli/test_latest @@ -25,3 +25,16 @@ assert "mise latest ruby | grep -E '^[0-9]+(\\.[0-9]+)*$'" assert "mise latest ruby@truffleruby | grep -E '\\-[0-9]+(\\.[0-9]+)*$'" # assert vendor 22 version ends with -22.N.N assert "mise latest ruby@truffleruby-22 | grep -E '\\-22(\\.[0-9]+)*$'" + +# sub-N: is accepted by use/install/ls-remote, so it has to work here too — it used to +# be rejected as "invalid version". dummy publishes 1.0.0, 1.1.0 and 2.0.0. +assert "mise latest dummy@sub-1:2" "1.1.0" +# the base may be an alias, which has to be resolved before subtracting +mise alias set dummy lts 2 +assert "mise latest dummy@sub-1:lts" "1.1.0" +mise alias unset dummy lts +# ...and the base may be `latest`, directly or through an alias +assert "mise latest dummy@sub-1:latest" "1.1.0" +mise alias set dummy stable latest +assert "mise latest dummy@sub-1:stable" "1.1.0" +mise alias unset dummy stable diff --git a/e2e/cli/test_ls_remote b/e2e/cli/test_ls_remote index 5e7b46e7855..f8e489a2aac 100644 --- a/e2e/cli/test_ls_remote +++ b/e2e/cli/test_ls_remote @@ -12,6 +12,22 @@ assert "mise ls-remote dummy@2" "2.0.0" assert "mise ls-remote dummy@sub-1:2" "1.0.0 1.1.0" +# The base of a `sub-N:` spec may be an alias. It has to be resolved before subtracting — +# passing it through raw used to panic here while `use`/`install` handled it fine. +mise alias set dummy lts 2 +assert "mise ls-remote dummy@sub-1:lts" "1.0.0 +1.1.0" +mise alias unset dummy lts + +# `latest` is not subtractable either, and an alias may resolve to it, so the alias has to +# be resolved before the `latest` check rather than after it. dummy's latest is 2.0.0. +assert "mise ls-remote dummy@sub-1:latest" "1.0.0 +1.1.0" +mise alias set dummy stable latest +assert "mise ls-remote dummy@sub-1:stable" "1.0.0 +1.1.0" +mise alias unset dummy stable + # Date filtering should happen after reading the cached full remote version list. # Running two cutoffs back-to-back catches regressions where the first filtered # result is cached and reused for the second query. diff --git a/src/cli/latest.rs b/src/cli/latest.rs index 03f23ab00cc..0f15bb1b2bd 100644 --- a/src/cli/latest.rs +++ b/src/cli/latest.rs @@ -4,7 +4,7 @@ use jiff::Timestamp; use crate::cli::args::ToolArg; use crate::config::Config; use crate::install_before::resolve_cli_minimum_release_age; -use crate::toolset::ToolRequest; +use crate::toolset::{ToolRequest, resolve_sub_base}; use crate::ui::multi_progress_report::MultiProgressReport; /// Gets the latest available version for a plugin @@ -50,9 +50,12 @@ impl Latest { installed, minimum_release_age: _, } = self; - let mut prefix = match &tool.tvr { + let prefix = match &tool.tvr { None => asdf_version, Some(ToolRequest::Version { version, .. }) => Some(version.clone()), + // `sub-N:` resolves its base against the backend, so it is handled + // below once the backend (and its plugin) is ready. + Some(ToolRequest::Sub { .. }) => None, _ => bail!("invalid version: {}", tool.style()), }; @@ -62,9 +65,17 @@ impl Latest { plugin.ensure_installed(&config, &mpr, false, false).await?; backend = tool.ba.backend()?; } - if let Some(v) = prefix { - prefix = Some(config.resolve_alias(&backend, &v).await?); - } + let prefix = match &tool.tvr { + Some(ToolRequest::Sub { + sub, orig_version, .. + }) => Some( + resolve_sub_base(&config, &backend, sub, orig_version, before_date, false).await?, + ), + _ => match prefix { + Some(v) => Some(config.resolve_alias(&backend, &v).await?), + None => None, + }, + }; let latest_version = if installed { backend.latest_installed_version(prefix)? diff --git a/src/cli/ls_remote.rs b/src/cli/ls_remote.rs index 290b55cb9f5..1694b334d0f 100644 --- a/src/cli/ls_remote.rs +++ b/src/cli/ls_remote.rs @@ -8,7 +8,7 @@ use crate::backend::{Backend, VersionInfo}; use crate::cli::args::ToolArg; use crate::config::Settings; use crate::install_before::{resolve_before_date_for_backend, resolve_cli_minimum_release_age}; -use crate::toolset::{ToolRequest, tool_request}; +use crate::toolset::{ToolRequest, resolve_sub_base}; use crate::ui::multi_progress_report::MultiProgressReport; use crate::{backend, config::Config}; @@ -111,7 +111,10 @@ impl LsRemote { Some(ToolRequest::Version { version: v, .. }) => Some(v.clone()), Some(ToolRequest::Sub { sub, orig_version, .. - }) => Some(tool_request::version_sub(orig_version, sub)), + }) => Some( + resolve_sub_base(config, &plugin, sub, orig_version, before_date, false) + .await?, + ), _ => self.prefix.clone(), }, _ => self.prefix.clone(), diff --git a/src/toolset/mod.rs b/src/toolset/mod.rs index 801af65ec87..d93cbd49bf0 100644 --- a/src/toolset/mod.rs +++ b/src/toolset/mod.rs @@ -37,6 +37,7 @@ pub use tool_request_set::{ ToolRequestSet, ToolRequestSetBuilder, tool_env_var_name, tool_env_vars, tool_from_env_var_name, }; pub use tool_source::ToolSource; +pub(crate) use tool_version::resolve_sub_base; pub use tool_version::{ResolveOptions, ToolVersion}; pub use tool_version_list::ToolVersionList; pub use tool_version_options::{ diff --git a/src/toolset/tool_request.rs b/src/toolset/tool_request.rs index 5498a8247d6..c97a8959e2f 100644 --- a/src/toolset/tool_request.rs +++ b/src/toolset/tool_request.rs @@ -5,7 +5,7 @@ use std::{ sync::Arc, }; -use eyre::{Result, bail}; +use eyre::{Result, bail, eyre}; use versions::{Chunk, Version}; use xx::file; @@ -299,10 +299,18 @@ impl ToolRequest { .local_resolve(config, orig_version) .inspect_err(|e| warn!("ToolRequest.local_resolve: {e:#}")) .unwrap_or_default() - .map(|v| { - let pathname = version_sub(&v, sub.as_str()); + .and_then(|v| { + // A version that cannot be subtracted from has no install path to look + // for, which is the same answer as "nothing installed matches". + let pathname = version_sub(&v, sub.as_str()) + .inspect_err(|e| warn!("ToolRequest.version_sub: {e:#}")) + .ok()?; let path = backend.installs_path.join(&pathname); - env::find_in_shared_installs(path, &backend.tool_dir_name(), &pathname) + Some(env::find_in_shared_installs( + path, + &backend.tool_dir_name(), + &pathname, + )) }), Self::Prefix { backend, prefix, .. @@ -581,32 +589,45 @@ fn normalize_arch(arch: &str) -> &str { /// e.g. version_sub("18.2.3", "2") -> "16" /// e.g. version_sub("18.2.3", "0.1") -> "18.1" /// e.g. version_sub("2.79.0", "0.0.1") -> "2.78" (underflow, returns prefix) -pub fn version_sub(orig: &str, sub: &str) -> String { - let mut orig = Version::new(orig).unwrap(); - let sub = Version::new(sub).unwrap(); - while orig.chunks.0.len() > sub.chunks.0.len() { - orig.chunks.0.pop(); - } - for i in 0..orig.chunks.0.len() { - let m = sub.nth(i).unwrap(); - let orig_val = orig.chunks.0[i].single_digit().unwrap(); +/// +/// `orig` must already be a concrete version. `latest` and aliases like `lts` have to be +/// resolved by the caller first — there is nothing here to subtract from — which is what +/// `tool_version::resolve_sub_base` exists for. +pub fn version_sub(orig: &str, sub: &str) -> Result { + fn not_numeric(orig: &str, sub: &str) -> eyre::Report { + eyre!("cannot subtract {sub} from {orig}: {orig} is not a numeric version") + } + let mut version = Version::new(orig).ok_or_else(|| eyre!("invalid version: {orig}"))?; + let sub_version = Version::new(sub).ok_or_else(|| eyre!("invalid version: {sub}"))?; + while version.chunks.0.len() > sub_version.chunks.0.len() { + version.chunks.0.pop(); + } + for i in 0..version.chunks.0.len() { + let m = sub_version + .nth(i) + .ok_or_else(|| eyre!("invalid version: {sub}"))?; + let orig_val = version.chunks.0[i] + .single_digit() + .ok_or_else(|| not_numeric(orig, sub))?; if orig_val < m { // Handle underflow with borrowing from higher digits for j in (0..i).rev() { - let prev_val = orig.chunks.0[j].single_digit().unwrap(); + let prev_val = version.chunks.0[j] + .single_digit() + .ok_or_else(|| not_numeric(orig, sub))?; if prev_val > 0 { - orig.chunks.0[j] = Chunk::Numeric(prev_val - 1); - orig.chunks.0.truncate(j + 1); - return orig.to_string(); + version.chunks.0[j] = Chunk::Numeric(prev_val - 1); + version.chunks.0.truncate(j + 1); + return Ok(version.to_string()); } } - return "0".to_string(); + return Ok("0".to_string()); } - orig.chunks.0[i] = Chunk::Numeric(orig_val - m); + version.chunks.0[i] = Chunk::Numeric(orig_val - m); } - orig.to_string() + Ok(version.to_string()) } impl Display for ToolRequest { @@ -796,20 +817,33 @@ mod tests { #[test] fn test_version_sub() { - assert_str_eq!(version_sub("18.2.3", "2"), "16"); - assert_str_eq!(version_sub("18.2.3", "0.1"), "18.1"); - assert_str_eq!(version_sub("18.2.3", "0.0.1"), "18.2.2"); + assert_str_eq!(version_sub("18.2.3", "2").unwrap(), "16"); + assert_str_eq!(version_sub("18.2.3", "0.1").unwrap(), "18.1"); + assert_str_eq!(version_sub("18.2.3", "0.0.1").unwrap(), "18.2.2"); } #[test] fn test_version_sub_underflow() { // Test cases that would cause underflow return prefix for higher digit - assert_str_eq!(version_sub("2.0.0", "0.0.1"), "1"); - assert_str_eq!(version_sub("2.79.0", "0.0.1"), "2.78"); - assert_str_eq!(version_sub("1.0.0", "0.1.0"), "0"); - assert_str_eq!(version_sub("0.1.0", "1"), "0"); - assert_str_eq!(version_sub("1.2.3", "0.2.4"), "0"); - assert_str_eq!(version_sub("1.3.3", "0.2.4"), "1.0"); + assert_str_eq!(version_sub("2.0.0", "0.0.1").unwrap(), "1"); + assert_str_eq!(version_sub("2.79.0", "0.0.1").unwrap(), "2.78"); + assert_str_eq!(version_sub("1.0.0", "0.1.0").unwrap(), "0"); + assert_str_eq!(version_sub("0.1.0", "1").unwrap(), "0"); + assert_str_eq!(version_sub("1.2.3", "0.2.4").unwrap(), "0"); + assert_str_eq!(version_sub("1.3.3", "0.2.4").unwrap(), "1.0"); + } + + #[test] + fn test_version_sub_rejects_non_numeric_base() { + // An unresolved alias must not reach here, but if it does it has to be an + // error rather than a panic: this used to abort the process from + // `mise ls-remote @sub-2:lts`. + let err = version_sub("lts", "2").unwrap_err().to_string(); + assert!(err.contains("lts"), "unexpected error: {err}"); + + assert!(version_sub("latest", "1").is_err()); + assert!(version_sub("1.2.3", "x").is_err()); + assert!(version_sub("", "1").is_err()); } #[test] diff --git a/src/toolset/tool_version.rs b/src/toolset/tool_version.rs index d2eccba001c..231f21c9d50 100644 --- a/src/toolset/tool_version.rs +++ b/src/toolset/tool_version.rs @@ -612,19 +612,15 @@ impl ToolVersion { let version = request.version(); return Ok(Self::new(request, version)); } - let v = match v { - "latest" => backend - .latest_version_with_refresh( - config, - None, - opts.before_date, - opts.refresh_remote_versions, - ) - .await? - .ok_or_else(|| Self::no_versions_found(&backend, opts.before_date))?, - _ => config.resolve_alias(&backend, v).await?, - }; - let v = tool_request::version_sub(&v, sub); + let v = resolve_sub_base( + config, + &backend, + sub, + v, + opts.before_date, + opts.refresh_remote_versions, + ) + .await?; Box::pin(Self::resolve_version(config, request, &v, opts)).await } @@ -697,6 +693,39 @@ impl ToolVersion { } } +/// Resolve the base of a `sub-N:` request and subtract from it. +/// +/// `base` may be `latest` or an alias such as `lts`, neither of which +/// `tool_request::version_sub` can subtract from — it needs a concrete version. Every +/// command that accepts `sub-N:` must go through here; skipping this step is what made +/// `mise ls-remote @sub-2:lts` panic. +pub(crate) async fn resolve_sub_base( + config: &Arc, + backend: &ABackend, + sub: &str, + base: &str, + before_date: Option, + refresh_remote_versions: bool, +) -> Result { + // An alias can point at `latest`, so the alias is resolved first and the result is + // then treated exactly like a literal `latest` — otherwise the string `latest` + // reaches the subtraction, which cannot do anything with it. + let base = if base == "latest" { + base.to_string() + } else { + config.resolve_alias(backend, base).await? + }; + let v = if base == "latest" { + backend + .latest_version_with_refresh(config, None, before_date, refresh_remote_versions) + .await? + .ok_or_else(|| ToolVersion::no_versions_found(backend, before_date))? + } else { + base + }; + tool_request::version_sub(&v, sub) +} + impl Display for ToolVersion { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { write!(f, "{}@{}", self.ba().full(), self.version)