Skip to content
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
13 changes: 13 additions & 0 deletions e2e/cli/test_latest
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 16 additions & 0 deletions e2e/cli/test_ls_remote
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
21 changes: 16 additions & 5 deletions src/cli/latest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:<base>` 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()),
};

Expand All @@ -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)?
Expand Down
7 changes: 5 additions & 2 deletions src/cli/ls_remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions src/toolset/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down
92 changes: 63 additions & 29 deletions src/toolset/tool_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{
sync::Arc,
};

use eyre::{Result, bail};
use eyre::{Result, bail, eyre};
use versions::{Chunk, Version};
use xx::file;

Expand Down Expand Up @@ -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, ..
Expand Down Expand Up @@ -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<String> {
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 {
Expand Down Expand Up @@ -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 <tool>@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]
Expand Down
55 changes: 42 additions & 13 deletions src/toolset/tool_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -697,6 +693,39 @@ impl ToolVersion {
}
}

/// Resolve the base of a `sub-N:<base>` 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 <tool>@sub-2:lts` panic.
pub(crate) async fn resolve_sub_base(
config: &Arc<Config>,
backend: &ABackend,
sub: &str,
base: &str,
before_date: Option<Timestamp>,
refresh_remote_versions: bool,
) -> Result<String> {
// 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)
Expand Down
Loading