From 355fd834aa5d126dd84cc7edd7b3bb12e4a438c9 Mon Sep 17 00:00:00 2001 From: jdx <216188+jdx@users.noreply.github.com> Date: Sat, 16 Nov 2024 12:42:58 -0600 Subject: [PATCH] fix: bugs with `mise up` and prefix: versions --- e2e/cli/test_upgrade | 23 +++++ e2e/cli/test_use | 8 ++ e2e/tasks/test_task_deps | 7 +- src/backend/mod.rs | 24 +++-- src/cli/upgrade.rs | 20 ++-- src/cli/use.rs | 25 ++++- src/config/config_file/legacy_version.rs | 4 +- src/config/config_file/mise_toml.rs | 119 +++++++++++++++++++---- src/config/config_file/mod.rs | 38 +++++--- src/config/config_file/tool_versions.rs | 16 ++- src/toolset/mod.rs | 26 ++++- src/toolset/tool_version.rs | 16 ++- 12 files changed, 246 insertions(+), 80 deletions(-) diff --git a/e2e/cli/test_upgrade b/e2e/cli/test_upgrade index 4edc249aea..c702d3d2fc 100644 --- a/e2e/cli/test_upgrade +++ b/e2e/cli/test_upgrade @@ -24,3 +24,26 @@ bazelbuild_latest=$(mise latest ubi:bazelbuild/buildtools) mise up --bump ubi:bazelbuild/buildtools assert "cat mise.toml" "[tools] \"ubi:bazelbuild/buildtools\" = { version = \"$bazelbuild_latest\", exe = \"buildifier\", matching = \"buildifier\" }" + +rm -f .tool-versions +echo "tools.dummy = 'prefix:1'" >mise.toml +mise uninstall dummy --all +mkdir -p "$MISE_DATA_DIR/installs/dummy/1.0.0" +assert "mise ls dummy" "dummy 1.0.0 (outdated) ~/workdir/mise.toml prefix:1" +assert "mise up -n" "Would uninstall dummy@1.0.0 +Would install dummy@1.1.0" +mise up +assert "mise ls dummy" "dummy 1.1.0 ~/workdir/mise.toml prefix:1" +assert "mise up dummy -n --bump" "Would uninstall dummy@1.1.0 +Would install dummy@2.0.0 +Would bump dummy@prefix:2 in ~/workdir/mise.toml" +mise up dummy --bump +assert "mise ls dummy" "dummy 2.0.0 ~/workdir/mise.toml prefix:2" +echo "tools.dummy = 'prefix:1'" >mise.toml +mise uninstall dummy --all +mise i dummy@1.0.0 +assert "mise up dummy -n --bump" "Would uninstall dummy@1.0.0 +Would install dummy@2.0.0 +Would bump dummy@prefix:2 in ~/workdir/mise.toml" +mise up dummy --bump +assert "mise ls dummy" "dummy 2.0.0 ~/workdir/mise.toml prefix:2" diff --git a/e2e/cli/test_use b/e2e/cli/test_use index 85b998ebf9..aa14f9ee6c 100644 --- a/e2e/cli/test_use +++ b/e2e/cli/test_use @@ -64,3 +64,11 @@ mise use -g gh@2 assert "cat ~/.config/mise/config.toml" '[tools] gh = "2"' rm -f ~/.config/mise/config.toml + +mise uninstall dummy --all +mise use dummy@system +assert "mise ls dummy" "dummy system ~/workdir/mise.toml system" + +mkdir -p ~/workdir/mydummy +mise use "dummy@path:~/workdir/mydummy" +assert_contains "mise ls dummy" "dummy path:~/workdir/mydummy ~/workdir/mise.toml path:~/workdir/mydummy" diff --git a/e2e/tasks/test_task_deps b/e2e/tasks/test_task_deps index 59a47705b7..4a62c2f3dc 100644 --- a/e2e/tasks/test_task_deps +++ b/e2e/tasks/test_task_deps @@ -2,12 +2,13 @@ cat <mise.toml tasks.a.run = "echo a" +tasks.b.depends = ["a"] tasks.b.run = "echo b" -tasks.c.run = "echo c" tasks.c.depends = ["a"] -tasks.b.depends = ["a"] -tasks.d.run = "echo d" +tasks.c.run = "echo c" +tasks.c.wait_for = ["b"] tasks.d.depends = ["c"] +tasks.d.run = "echo d" tasks.d.wait_for = ["b"] EOF diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 99366451cd..d73c163f2c 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -235,16 +235,26 @@ pub trait Backend: Debug + Send + Sync { install_state::list_versions(&self.ba().short) } fn is_version_installed(&self, tv: &ToolVersion, check_symlink: bool) -> bool { + let check_path = |install_path: &Path| { + let is_installed = install_path.exists(); + let is_not_incomplete = !self.incomplete_file_path(tv).exists(); + let is_valid_symlink = !check_symlink || !is_runtime_symlink(install_path); + + is_installed && is_not_incomplete && is_valid_symlink + }; match tv.request { ToolRequest::System { .. } => true, + ToolRequest::Prefix { .. } => { + // can't use tv.request.install_path() in this case since it may point to a version + // that matches the prefix but is not the correct one. For example, in the following + // scenario this would not upgrade tiny because "prefix:1" would still match and + // `mise up` would think it is installed + // mise install tiny@1.0.0 + // mise use tiny@prefix:1 + // mise up tiny + check_path(&tv.install_path()) + } _ => { - let check_path = |install_path: &Path| { - let is_installed = install_path.exists(); - let is_not_incomplete = !self.incomplete_file_path(tv).exists(); - let is_valid_symlink = !check_symlink || !is_runtime_symlink(install_path); - - is_installed && is_not_incomplete && is_valid_symlink - }; if let Some(install_path) = tv.request.install_path() { if check_path(&install_path) { return true; diff --git a/src/cli/upgrade.rs b/src/cli/upgrade.rs index e3ac6393d8..3abb338be9 100644 --- a/src/cli/upgrade.rs +++ b/src/cli/upgrade.rs @@ -88,9 +88,9 @@ impl Upgrade { let config_file_updates = outdated .iter() .filter_map(|o| { - if let (Some(path), Some(bump)) = (o.source.path(), &o.bump) { + if let (Some(path), Some(_bump)) = (o.source.path(), &o.bump) { match config_file::parse(path) { - Ok(cf) => Some((o, bump.clone(), cf)), + Ok(cf) => Some((o, cf)), Err(e) => { warn!("failed to parse {}: {e}", display_path(path)); None @@ -100,7 +100,7 @@ impl Upgrade { None } }) - .filter(|(o, _bump, cf)| { + .filter(|(o, cf)| { if let Ok(trs) = cf.to_tool_request_set() { if let Some(versions) = trs.tools.get(o.tool_request.ba()) { if versions.len() != 1 { @@ -120,16 +120,16 @@ impl Upgrade { if self.dry_run { for (o, current) in &to_remove { - info!("Would uninstall {}@{}", o.name, current); + miseprintln!("Would uninstall {}@{}", o.name, current); } for o in &outdated { - info!("Would install {}@{}", o.name, o.latest); + miseprintln!("Would install {}@{}", o.name, o.latest); } - for (o, bump, cf) in &config_file_updates { - info!( + for (o, cf) in &config_file_updates { + miseprintln!( "Would bump {}@{} in {}", o.name, - bump, + o.tool_request.version(), display_path(cf.get_path()) ); } @@ -147,8 +147,8 @@ impl Upgrade { let new_versions = outdated.iter().map(|o| o.tool_request.clone()).collect(); let versions = ts.install_versions(config, new_versions, &mpr, &opts)?; - for (o, bump, mut cf) in config_file_updates { - cf.replace_versions(o.tool_request.ba(), &[(bump, o.tool_request.options())])?; + for (o, mut cf) in config_file_updates { + cf.replace_versions(o.tool_request.ba(), vec![o.tool_request.clone()])?; cf.save()?; } diff --git a/src/cli/use.rs b/src/cli/use.rs index 0973dc1b63..7a3e84545f 100644 --- a/src/cli/use.rs +++ b/src/cli/use.rs @@ -117,18 +117,33 @@ impl Use { let mut cf = self.get_config_file()?; let pin = self.pin || !self.fuzzy && (SETTINGS.pin || SETTINGS.asdf_compat); - for (fa, tvl) in &versions.iter().chunk_by(|tv| tv.ba()) { + for (ba, tvl) in &versions.iter().chunk_by(|tv| tv.ba()) { let versions: Vec<_> = tvl .into_iter() .map(|tv| { + let mut request = tv.request.clone(); if pin { - (tv.version.clone(), tv.request.options()) - } else { - (tv.request.version(), tv.request.options()) + if let ToolRequest::Version { + version: _version, + source, + os, + options, + backend, + } = request + { + request = ToolRequest::Version { + version: tv.version.clone(), + source, + os, + options, + backend, + }; + } } + request }) .collect(); - cf.replace_versions(fa, &versions)?; + cf.replace_versions(ba, versions)?; } if self.global { diff --git a/src/config/config_file/legacy_version.rs b/src/config/config_file/legacy_version.rs index 8f79722bab..b5bcf427b7 100644 --- a/src/config/config_file/legacy_version.rs +++ b/src/config/config_file/legacy_version.rs @@ -5,7 +5,7 @@ use eyre::Result; use crate::backend::{self, BackendList}; use crate::cli::args::BackendArg; use crate::config::config_file::ConfigFile; -use crate::toolset::{ToolRequest, ToolRequestSet, ToolSource, ToolVersionOptions}; +use crate::toolset::{ToolRequest, ToolRequestSet, ToolSource}; #[derive(Debug, Clone)] pub struct LegacyVersionFile { @@ -62,7 +62,7 @@ impl ConfigFile for LegacyVersionFile { fn replace_versions( &mut self, _plugin_name: &BackendArg, - _versions: &[(String, ToolVersionOptions)], + _versions: Vec, ) -> Result<()> { unimplemented!() } diff --git a/src/config/config_file/mise_toml.rs b/src/config/config_file/mise_toml.rs index f049618db6..044c1e6bb5 100644 --- a/src/config/config_file/mise_toml.rs +++ b/src/config/config_file/mise_toml.rs @@ -295,7 +295,7 @@ impl ConfigFile for MiseToml { fn replace_versions( &mut self, ba: &BackendArg, - versions: &[(String, ToolVersionOptions)], + versions: Vec, ) -> eyre::Result<()> { let existing = self.tools.entry(ba.clone()).or_default(); let output_empty_opts = |opts: &ToolVersionOptions| { @@ -309,15 +309,7 @@ impl ConfigFile for MiseToml { }; existing.0 = versions .iter() - .map(|(v, opts)| MiseTomlTool { - tt: ToolVersionType::Version(v.clone()), - os: None, // TODO: use existing os - options: if !output_empty_opts(opts) { - Some(opts.clone()) - } else { - None - }, - }) + .map(|tr| MiseTomlTool::from(tr.clone())) .collect(); let tools = self .doc_mut()? @@ -335,25 +327,26 @@ impl ConfigFile for MiseToml { } if versions.len() == 1 { - if output_empty_opts(&versions[0].1) { - tools.insert_formatted(&key, value(versions[0].0.clone())); + if output_empty_opts(&versions[0].options()) { + tools.insert_formatted(&key, value(versions[0].version())); } else { let mut table = InlineTable::new(); - table.insert("version", versions[0].0.to_string().into()); - for (k, v) in &versions[0].1 { - table.insert(k, v.clone().into()); + table.insert("version", versions[0].version().into()); + for (k, v) in versions[0].options() { + table.insert(k, v.into()); } tools.insert_formatted(&key, table.into()); } } else { let mut arr = Array::new(); - for (v, opts) in versions { - if output_empty_opts(opts) { + for tr in versions { + let v = tr.version(); + if output_empty_opts(&tr.options()) { arr.push(v.to_string()); } else { let mut table = InlineTable::new(); table.insert("version", v.to_string().into()); - for (k, v) in opts { + for (k, v) in tr.options() { table.insert(k, v.clone().into()); } arr.push(table); @@ -499,6 +492,89 @@ impl Clone for MiseToml { } } +impl From for MiseTomlTool { + fn from(tr: ToolRequest) -> Self { + match tr { + ToolRequest::Version { + version, + os, + options, + backend: _backend, + source: _source, + } => Self { + tt: ToolVersionType::Version(version), + os, + options: if options.is_empty() { + None + } else { + Some(options) + }, + }, + ToolRequest::Path { + path, + os, + backend: _backend, + source: _source, + } => Self { + tt: ToolVersionType::Path(path), + os, + options: None, + }, + ToolRequest::Prefix { + prefix, + os, + options, + backend: _backend, + source: _source, + } => Self { + tt: ToolVersionType::Prefix(prefix), + os, + options: if options.is_empty() { + None + } else { + Some(options) + }, + }, + ToolRequest::Ref { + ref_, + ref_type, + os, + options, + backend: _backend, + source: _source, + } => Self { + tt: ToolVersionType::Ref(ref_, ref_type), + os, + options: if options.is_empty() { + None + } else { + Some(options) + }, + }, + ToolRequest::Sub { + sub, + os, + orig_version, + backend: _backend, + source: _source, + } => Self { + tt: ToolVersionType::Sub { sub, orig_version }, + os, + options: None, + }, + ToolRequest::System { + os, + backend: _backend, + source: _source, + } => Self { + tt: ToolVersionType::System, + os, + options: None, + }, + } + } +} + fn deserialize_version<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, @@ -1062,6 +1138,7 @@ mod tests { use crate::dirs::CWD; use crate::test::{replace_path, reset}; + use crate::toolset::ToolRequest; use super::*; @@ -1296,9 +1373,9 @@ mod tests { let node = "node".into(); cf.replace_versions( &node, - &[ - ("16.0.1".into(), Default::default()), - ("18.0.1".into(), Default::default()), + vec![ + ToolRequest::new("node".into(), "16.0.1", ToolSource::Unknown).unwrap(), + ToolRequest::new("node".into(), "18.0.1", ToolSource::Unknown).unwrap(), ], ) .unwrap(); diff --git a/src/config/config_file/mod.rs b/src/config/config_file/mod.rs index cce34be41b..e030fe0fe0 100644 --- a/src/config/config_file/mod.rs +++ b/src/config/config_file/mod.rs @@ -21,7 +21,7 @@ use crate::errors::Error::UntrustedConfig; use crate::file::display_path; use crate::hash::{file_hash_sha256, hash_to_str}; use crate::task::Task; -use crate::toolset::{ToolRequestSet, ToolSource, ToolVersionList, ToolVersionOptions, Toolset}; +use crate::toolset::{ToolRequest, ToolRequestSet, ToolSource, ToolVersionList, Toolset}; use crate::ui::{prompt, style}; use crate::{backend, dirs, env, file}; @@ -71,11 +71,8 @@ pub trait ConfigFile: Debug + Send + Sync { Default::default() } fn remove_plugin(&mut self, _fa: &BackendArg) -> eyre::Result<()>; - fn replace_versions( - &mut self, - fa: &BackendArg, - versions: &[(String, ToolVersionOptions)], - ) -> eyre::Result<()>; + fn replace_versions(&mut self, fa: &BackendArg, versions: Vec) + -> eyre::Result<()>; fn save(&self) -> eyre::Result<()>; fn dump(&self) -> eyre::Result; fn source(&self) -> ToolSource; @@ -117,19 +114,34 @@ impl dyn ConfigFile { ts.versions.insert(fa.clone(), tvl); } ts.resolve()?; - for (fa, versions) in plugins_to_update { + for (ba, versions) in plugins_to_update { let versions = versions .into_iter() - .map(|tvr| { + .map(|tr| { + let mut tr = tr.clone(); if pin { - let tv = tvr.resolve(&Default::default())?; - Ok((tv.version, tv.request.options())) - } else { - Ok((tvr.version(), tvr.options())) + let tv = tr.resolve(&Default::default())?; + if let ToolRequest::Version { + version: _version, + source, + os, + options, + backend, + } = tr + { + tr = ToolRequest::Version { + version: tv.version, + source, + os, + options, + backend, + }; + } } + Ok(tr) }) .collect::>>()?; - self.replace_versions(&fa, &versions)?; + self.replace_versions(&ba, versions)?; } Ok(()) diff --git a/src/config/config_file/tool_versions.rs b/src/config/config_file/tool_versions.rs index 935b8b460a..1338c9eeca 100644 --- a/src/config/config_file/tool_versions.rs +++ b/src/config/config_file/tool_versions.rs @@ -13,7 +13,7 @@ use crate::config::config_file::ConfigFile; use crate::file; use crate::file::display_path; use crate::tera::{get_tera, BASE_CONTEXT}; -use crate::toolset::{ToolRequest, ToolRequestSet, ToolSource, ToolVersionOptions}; +use crate::toolset::{ToolRequest, ToolRequestSet, ToolSource}; // python 3.11.0 3.10.0 // shellcheck 0.9.0 @@ -118,10 +118,8 @@ impl ToolVersions { plugins } - fn add_version(&mut self, fa: &BackendArg, version: &str) { - self.get_or_create_plugin(fa) - .versions - .push(version.to_string()); + fn add_version(&mut self, fa: &BackendArg, version: String) { + self.get_or_create_plugin(fa).versions.push(version); } fn populate_toolset(&mut self) -> eyre::Result<()> { @@ -165,14 +163,14 @@ impl ConfigFile for ToolVersions { fn replace_versions( &mut self, fa: &BackendArg, - versions: &[(String, ToolVersionOptions)], + versions: Vec, ) -> eyre::Result<()> { self.get_or_create_plugin(fa).versions.clear(); - for (version, opts) in versions { - if !opts.is_empty() { + for tr in versions { + if !tr.options().is_empty() { warn!("tool options are not supported in .tool-versions files"); } - self.add_version(fa, version); + self.add_version(fa, tr.version()); } Ok(()) } diff --git a/src/toolset/mod.rs b/src/toolset/mod.rs index c67996fd36..f958b71db3 100644 --- a/src/toolset/mod.rs +++ b/src/toolset/mod.rs @@ -426,11 +426,10 @@ impl Toolset { let new = out.latest.strip_prefix(&prefix).unwrap_or_default(); if let Some(bumped_version) = check_semver_bump(old, new) { if bumped_version != tv.request.version() { - out.bump = Some(format!("{prefix}{bumped_version}")); - match out.tool_request.clone() { + out.bump = match out.tool_request.clone() { ToolRequest::Version { - backend, version: _version, + backend, options, source, os, @@ -439,13 +438,30 @@ impl Toolset { backend, options, source, - version: out.bump.clone().unwrap(), + version: format!("{prefix}{bumped_version}"), + os, + }; + Some(out.tool_request.version()) + } + ToolRequest::Prefix { + prefix: _prefix, + backend, + options, + source, + os, + } => { + out.tool_request = ToolRequest::Prefix { + backend, + options, + source, + prefix: format!("{prefix}{bumped_version}"), os, }; + Some(out.tool_request.version()) } _ => { warn!("upgrading non-version tool requests"); - out.bump = None; + None } } } diff --git a/src/toolset/tool_version.rs b/src/toolset/tool_version.rs index 9a89d68377..d11bf7294f 100644 --- a/src/toolset/tool_version.rs +++ b/src/toolset/tool_version.rs @@ -44,7 +44,7 @@ impl ToolVersion { } let tv = match request.clone() { ToolRequest::Version { version: v, .. } => Self::resolve_version(request, &v, opts)?, - ToolRequest::Prefix { prefix, .. } => Self::resolve_prefix(request, &prefix)?, + ToolRequest::Prefix { prefix, .. } => Self::resolve_prefix(request, &prefix, opts)?, ToolRequest::Sub { sub, orig_version, .. } => Self::resolve_sub(request, &sub, &orig_version, opts)?, @@ -147,7 +147,7 @@ impl ToolVersion { return Self::resolve_path(PathBuf::from(p), &request); } Some(("prefix", p)) => { - return Self::resolve_prefix(request, p); + return Self::resolve_prefix(request, p, opts); } Some((part, v)) if part.starts_with("sub-") => { let sub = part.split_once('-').unwrap().1; @@ -187,7 +187,7 @@ impl ToolVersion { if matches.contains(&v) { return build(v); } - Self::resolve_prefix(request, &v) + Self::resolve_prefix(request, &v, opts) } /// resolve a version like `sub-1:12.0.0` which becomes `11.0.0`, `sub-0.1:12.1.0` becomes `12.0.0` @@ -206,8 +206,14 @@ impl ToolVersion { Self::resolve_version(request, &v, opts) } - fn resolve_prefix(request: ToolRequest, prefix: &str) -> Result { - let matches = request.backend()?.list_versions_matching(prefix)?; + fn resolve_prefix(request: ToolRequest, prefix: &str, opts: &ResolveOptions) -> Result { + let backend = request.backend()?; + if !opts.latest_versions { + if let Some(v) = backend.list_installed_versions_matching(prefix)?.last() { + return Ok(Self::new(request, v.to_string())); + } + } + let matches = backend.list_versions_matching(prefix)?; let v = match matches.last() { Some(v) => v, None => prefix,