Skip to content

Commit

Permalink
fix: bugs with mise up and prefix: versions
Browse files Browse the repository at this point in the history
  • Loading branch information
jdx committed Nov 16, 2024
1 parent d3d8dc0 commit 355fd83
Show file tree
Hide file tree
Showing 12 changed files with 246 additions and 80 deletions.
23 changes: 23 additions & 0 deletions e2e/cli/test_upgrade
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected]
Would install [email protected]"
mise up
assert "mise ls dummy" "dummy 1.1.0 ~/workdir/mise.toml prefix:1"
assert "mise up dummy -n --bump" "Would uninstall [email protected]
Would install [email protected]
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 [email protected]
assert "mise up dummy -n --bump" "Would uninstall [email protected]
Would install [email protected]
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"
8 changes: 8 additions & 0 deletions e2e/cli/test_use
Original file line number Diff line number Diff line change
Expand Up @@ -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"
7 changes: 4 additions & 3 deletions e2e/tasks/test_task_deps
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

cat <<EOF >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

Expand Down
24 changes: 17 additions & 7 deletions src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected]
// 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;
Expand Down
20 changes: 10 additions & 10 deletions src/cli/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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())
);
}
Expand All @@ -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()?;
}

Expand Down
25 changes: 20 additions & 5 deletions src/cli/use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/config/config_file/legacy_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -62,7 +62,7 @@ impl ConfigFile for LegacyVersionFile {
fn replace_versions(
&mut self,
_plugin_name: &BackendArg,
_versions: &[(String, ToolVersionOptions)],
_versions: Vec<ToolRequest>,
) -> Result<()> {
unimplemented!()
}
Expand Down
119 changes: 98 additions & 21 deletions src/config/config_file/mise_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ impl ConfigFile for MiseToml {
fn replace_versions(
&mut self,
ba: &BackendArg,
versions: &[(String, ToolVersionOptions)],
versions: Vec<ToolRequest>,
) -> eyre::Result<()> {
let existing = self.tools.entry(ba.clone()).or_default();
let output_empty_opts = |opts: &ToolVersionOptions| {
Expand All @@ -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()?
Expand All @@ -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);
Expand Down Expand Up @@ -499,6 +492,89 @@ impl Clone for MiseToml {
}
}

impl From<ToolRequest> 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<Option<Versioning>, D::Error>
where
D: Deserializer<'de>,
Expand Down Expand Up @@ -1062,6 +1138,7 @@ mod tests {

use crate::dirs::CWD;
use crate::test::{replace_path, reset};
use crate::toolset::ToolRequest;

use super::*;

Expand Down Expand Up @@ -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();
Expand Down
Loading

0 comments on commit 355fd83

Please sign in to comment.