diff --git a/e2e/backend/test_http_platform_switch b/e2e/backend/test_http_platform_switch new file mode 100644 index 0000000000..ca3b6b94c9 --- /dev/null +++ b/e2e/backend/test_http_platform_switch @@ -0,0 +1,40 @@ +#!/usr/bin/env bash + +# Test switching from single url to platform-specific urls +# Regression test for https://github.com/jdx/mise/discussions/7034 +# +# When a tool is first installed with a single `url`, the install state stores +# the options (including url) in the manifest. If the user then changes the +# config to use platform-specific urls via [tools."http:X".platforms], the +# stale cached options could shadow the new config, causing +# "Http backend requires 'url' option" error. + +# Step 1: Install with a single url +cat <mise.toml +[tools] +"http:hello-plat-switch" = { version = "1.0.0", url = "https://mise.jdx.dev/test-fixtures/hello-world-1.0.0.tar.gz", bin_path = "hello-world-1.0.0/bin", postinstall = "chmod +x \$MISE_TOOL_INSTALL_PATH/hello-world-1.0.0/bin/hello-world" } +EOF + +mise install +assert_contains "mise x -- hello-world" "hello world" + +# Step 2: Switch to platform-specific urls (same tool name, same version) +# This previously failed with "Http backend requires 'url' option" because +# the stale cached platforms data from parse_tool_options was mangled and +# shadowed the correct config values during merge. +cat <mise.toml +[tools."http:hello-plat-switch"] +version = "1.0.0" +bin_path = "hello-world-1.0.0/bin" +postinstall = "chmod +x \$MISE_TOOL_INSTALL_PATH/hello-world-1.0.0/bin/hello-world" + +[tools."http:hello-plat-switch".platforms] +linux-x64 = { url = "https://mise.jdx.dev/test-fixtures/hello-world-1.0.0.tar.gz" } +linux-arm64 = { url = "https://mise.jdx.dev/test-fixtures/hello-world-1.0.0.tar.gz" } +darwin-arm64 = { url = "https://mise.jdx.dev/test-fixtures/hello-world-1.0.0.tar.gz" } +darwin-x64 = { url = "https://mise.jdx.dev/test-fixtures/hello-world-1.0.0.tar.gz" } +EOF + +# This should succeed, not fail with "Http backend requires 'url' option" +mise install +assert_contains "mise x -- hello-world" "hello world" diff --git a/src/backend/asdf.rs b/src/backend/asdf.rs index 6f22d907e5..3baf80b077 100644 --- a/src/backend/asdf.rs +++ b/src/backend/asdf.rs @@ -170,11 +170,11 @@ impl AsdfBackend { tv: &ToolVersion, ) -> Result { let mut sm = self.plugin.script_man.clone(); - for (key, value) in tv.request.options().opts { + for (key, value) in tv.request.options().opts_as_strings() { let k = format!("RTX_TOOL_OPTS__{}", key.to_uppercase()); sm = sm.with_env(k, value.clone()); let k = format!("MISE_TOOL_OPTS__{}", key.to_uppercase()); - sm = sm.with_env(k, value.clone()); + sm = sm.with_env(k, value); } for (key, value) in tv.request.options().install_env { sm = sm.with_env(key, value.clone()); diff --git a/src/backend/cargo.rs b/src/backend/cargo.rs index 88329292a8..08474173e7 100644 --- a/src/backend/cargo.rs +++ b/src/backend/cargo.rs @@ -126,7 +126,9 @@ impl Backend for CargoBackend { }; let opts = tv.request.options(); - if let Some(bin) = lookup_platform_key(&opts, "bin").or_else(|| opts.get("bin").cloned()) { + if let Some(bin) = + lookup_platform_key(&opts, "bin").or_else(|| opts.get("bin").map(|s| s.to_string())) + { cmd = cmd.arg(format!("--bin={bin}")); } if opts @@ -177,7 +179,7 @@ impl Backend for CargoBackend { // These options affect what gets compiled/installed for key in ["features", "default-features", "bin"] { if let Some(value) = opts.get(key) { - result.insert(key.to_string(), value.clone()); + result.insert(key.to_string(), value.to_string()); } } diff --git a/src/backend/conda.rs b/src/backend/conda.rs index b6636de07f..3dd26b4bb6 100644 --- a/src/backend/conda.rs +++ b/src/backend/conda.rs @@ -56,7 +56,7 @@ impl CondaBackend { self.ba .opts() .get("channel") - .cloned() + .map(|s| s.to_string()) .unwrap_or_else(|| Settings::get().conda.channel.clone()) } diff --git a/src/backend/external_plugin_cache.rs b/src/backend/external_plugin_cache.rs index 64370f2b69..a381334c66 100644 --- a/src/backend/external_plugin_cache.rs +++ b/src/backend/external_plugin_cache.rs @@ -109,7 +109,7 @@ fn render_cache_key(config: &Config, tv: &ToolVersion, cache_key: &[String]) -> fn parse_template(config: &Config, tv: &ToolVersion, tmpl: &str) -> eyre::Result { let mut ctx = BASE_CONTEXT.clone(); ctx.insert("project_root", &config.project_root); - ctx.insert("opts", &tv.request.options().opts); + ctx.insert("opts", &tv.request.options().opts_as_strings()); get_tera( config .project_root diff --git a/src/backend/github.rs b/src/backend/github.rs index c0b0d22e09..6674f3874f 100644 --- a/src/backend/github.rs +++ b/src/backend/github.rs @@ -309,7 +309,7 @@ impl Backend for UnifiedGitBackend { // These options affect which artifact is downloaded for key in ["asset_pattern", "url", "version_prefix"] { if let Some(value) = opts.get(key) { - result.insert(key.to_string(), value.clone()); + result.insert(key.to_string(), value.to_string()); } } @@ -383,7 +383,6 @@ impl UnifiedGitBackend { fn get_api_url(&self, opts: &ToolVersionOptions) -> String { opts.get("api_url") - .map(|s| s.as_str()) .unwrap_or(if self.is_gitlab() { DEFAULT_GITLAB_API_BASE_URL } else if self.is_forgejo() { @@ -407,7 +406,7 @@ impl UnifiedGitBackend { // Check if we'll verify checksum let has_checksum = lookup_platform_key(opts, "checksum") - .or_else(|| opts.get("checksum").cloned()) + .or_else(|| opts.get("checksum").map(|s| s.to_string())) .is_some(); // Store the asset URL and digest (if available) in the tool version @@ -493,8 +492,8 @@ impl UnifiedGitBackend { /// Discovers bin paths in the installation directory fn discover_bin_paths(&self, tv: &ToolVersion) -> Result> { let opts = tv.request.options(); - if let Some(bin_path_template) = - lookup_platform_key(&opts, "bin_path").or_else(|| opts.get("bin_path").cloned()) + if let Some(bin_path_template) = lookup_platform_key(&opts, "bin_path") + .or_else(|| opts.get("bin_path").map(|s| s.to_string())) { let bin_path = template_string(&bin_path_template, tv); return Ok(vec![tv.install_path().join(&bin_path)]); @@ -599,7 +598,7 @@ impl UnifiedGitBackend { } let version = &tv.version; - let version_prefix = opts.get("version_prefix").map(|s| s.as_str()); + let version_prefix = opts.get("version_prefix"); if self.is_gitlab() { try_with_v_prefix(version, version_prefix, |candidate| async move { self.resolve_gitlab_asset_url_for_target( @@ -655,7 +654,7 @@ impl UnifiedGitBackend { // Try explicit pattern first if let Some(pattern) = lookup_platform_key_for_target(opts, "asset_pattern", target) - .or_else(|| opts.get("asset_pattern").cloned()) + .or_else(|| opts.get("asset_pattern").map(|s| s.to_string())) { // Template the pattern for the target platform let templated_pattern = template_string_for_target(&pattern, tv, target); @@ -752,7 +751,7 @@ impl UnifiedGitBackend { // Try explicit pattern first if let Some(pattern) = lookup_platform_key_for_target(opts, "asset_pattern", target) - .or_else(|| opts.get("asset_pattern").cloned()) + .or_else(|| opts.get("asset_pattern").map(|s| s.to_string())) { // Template the pattern for the target platform let templated_pattern = template_string_for_target(&pattern, tv, target); @@ -847,7 +846,7 @@ impl UnifiedGitBackend { // Try explicit pattern first if let Some(pattern) = lookup_platform_key_for_target(opts, "asset_pattern", target) - .or_else(|| opts.get("asset_pattern").cloned()) + .or_else(|| opts.get("asset_pattern").map(|s| s.to_string())) { // Template the pattern for the target platform let templated_pattern = template_string_for_target(&pattern, tv, target); @@ -1007,7 +1006,7 @@ impl UnifiedGitBackend { fn get_filter_bins(&self, tv: &ToolVersion) -> Option> { let opts = tv.request.options(); let filter_bins = lookup_platform_key(&opts, "filter_bins") - .or_else(|| opts.get("filter_bins").cloned())?; + .or_else(|| opts.get("filter_bins").map(|s| s.to_string()))?; Some( filter_bins @@ -1194,7 +1193,7 @@ impl UnifiedGitBackend { let version = &tv.version; // Try to get the release (with version prefix support) - let version_prefix = opts.get("version_prefix").map(|s| s.as_str()); + let version_prefix = opts.get("version_prefix"); let release = match try_with_v_prefix_and_repo(version, version_prefix, Some(&repo), |candidate| { let api_url = api_url.clone(); @@ -1429,8 +1428,10 @@ mod tests { // Test with custom version prefix let mut opts = ToolVersionOptions::default(); - opts.opts - .insert("version_prefix".to_string(), "release-".to_string()); + opts.opts.insert( + "version_prefix".to_string(), + toml::Value::String("release-".to_string()), + ); assert_eq!( backend.strip_version_prefix("release-1.0.0", &opts), diff --git a/src/backend/go.rs b/src/backend/go.rs index 5c22fde955..0c24ed8fff 100644 --- a/src/backend/go.rs +++ b/src/backend/go.rs @@ -163,7 +163,7 @@ impl Backend for GoBackend { // tags affect compilation if let Some(value) = opts.get("tags") { - result.insert("tags".to_string(), value.clone()); + result.insert("tags".to_string(), value.to_string()); } result diff --git a/src/backend/http.rs b/src/backend/http.rs index 58c35c2a0d..25f96c69ee 100644 --- a/src/backend/http.rs +++ b/src/backend/http.rs @@ -29,7 +29,7 @@ const METADATA_FILE: &str = "metadata.json"; /// Helper to get an option value with platform-specific fallback fn get_opt(opts: &ToolVersionOptions, key: &str) -> Option { - lookup_platform_key(opts, key).or_else(|| opts.get(key).cloned()) + lookup_platform_key(opts, key).or_else(|| opts.get(key).map(|s| s.to_string())) } /// Metadata stored alongside cached extractions @@ -571,13 +571,13 @@ impl HttpBackend { }; let url = match opts.get("version_list_url") { - Some(url) => url.clone(), + Some(url) => url.to_string(), None => return Ok(vec![]), }; - let regex = opts.get("version_regex").map(|s| s.as_str()); - let json_path = opts.get("version_json_path").map(|s| s.as_str()); - let version_expr = opts.get("version_expr").map(|s| s.as_str()); + let regex = opts.get("version_regex"); + let json_path = opts.get("version_json_path"); + let version_expr = opts.get("version_expr"); version_list::fetch_versions(&url, regex, json_path, version_expr).await } diff --git a/src/backend/pipx.rs b/src/backend/pipx.rs index edfa60619a..f4bcc1bac2 100644 --- a/src/backend/pipx.rs +++ b/src/backend/pipx.rs @@ -197,7 +197,7 @@ impl Backend for PIPXBackend { // Check if pipx is available (unless uvx is being used) let use_uvx = self.uv_is_installed(&ctx.config).await && Settings::get().pipx.uvx != Some(false) - && tv.request.options().get("uvx") != Some(&"false".to_string()); + && tv.request.options().get("uvx") != Some("false"); if !use_uvx { self.warn_if_dependency_missing( @@ -268,7 +268,7 @@ impl Backend for PIPXBackend { // These options affect what gets installed for key in ["extras", "pipx_args", "uvx_args", "uvx"] { if let Some(value) = opts.get(key) { - result.insert(key.to_string(), value.clone()); + result.insert(key.to_string(), value.to_string()); } } diff --git a/src/backend/spm.rs b/src/backend/spm.rs index be6757418e..757555773b 100644 --- a/src/backend/spm.rs +++ b/src/backend/spm.rs @@ -264,8 +264,9 @@ impl GitProvider { fn from_ba(ba: &BackendArg) -> Self { let opts = ba.opts(); - let default_provider = GitProviderKind::GitHub.as_ref().to_string(); - let provider = opts.get("provider").unwrap_or(&default_provider); + let provider = opts + .get("provider") + .unwrap_or(GitProviderKind::GitHub.as_ref()); let kind = if ba.tool_name.contains("gitlab.com") { GitProviderKind::GitLab } else { @@ -365,7 +366,7 @@ mod tests { "tool".to_string(), Some(ToolVersionOptions { opts: indexmap![ - "provider".to_string() => "gitlab".to_string() + "provider".to_string() => toml::Value::String("gitlab".to_string()) ], ..Default::default() }) @@ -381,8 +382,8 @@ mod tests { "tool".to_string(), Some(ToolVersionOptions { opts: indexmap![ - "api_url".to_string() => "https://gitlab.acme.com/api/v4".to_string(), - "provider".to_string() => "gitlab".to_string(), + "api_url".to_string() => toml::Value::String("https://gitlab.acme.com/api/v4".to_string()), + "provider".to_string() => toml::Value::String("gitlab".to_string()), ], ..Default::default() }) diff --git a/src/backend/static_helpers.rs b/src/backend/static_helpers.rs index 19c67ddf53..e87700bf55 100644 --- a/src/backend/static_helpers.rs +++ b/src/backend/static_helpers.rs @@ -247,7 +247,7 @@ pub fn lookup_platform_key(opts: &ToolVersionOptions, key_type: &str) -> Option< // Try flat format: platforms_macos_arm64_url let flat_key = format!("{prefix}_{os}_{arch}_{key_type}"); if let Some(val) = opts.get(&flat_key) { - return Some(val.clone()); + return Some(val.to_string()); } } } @@ -265,7 +265,7 @@ pub fn lookup_platform_key(opts: &ToolVersionOptions, key_type: &str) -> Option< /// * `Some(value)` if found in platform-specific or base options /// * `None` if not found pub fn lookup_with_fallback(opts: &ToolVersionOptions, key: &str) -> Option { - lookup_platform_key(opts, key).or_else(|| opts.get(key).cloned()) + lookup_platform_key(opts, key).or_else(|| opts.get(key).map(|s| s.to_string())) } /// Returns all possible aliases for a given platform target (os, arch). @@ -315,7 +315,7 @@ pub fn lookup_platform_key_for_target( // Try flat format: platforms_macos_arm64_url let flat_key = format!("{prefix}_{os}_{arch}_{key_type}"); if let Some(val) = opts.get(&flat_key) { - return Some(val.clone()); + return Some(val.to_string()); } } } @@ -414,7 +414,7 @@ pub fn install_artifact( ) -> eyre::Result<()> { let install_path = tv.install_path(); let mut strip_components = lookup_platform_key(opts, "strip_components") - .or_else(|| opts.get("strip_components").cloned()) + .or_else(|| opts.get("strip_components").map(|s| s.to_string())) .and_then(|s| s.parse().ok()); file::remove_all(&install_path)?; @@ -494,7 +494,7 @@ pub fn install_artifact( // Only do this if strip_components was not explicitly set by the user AND bin_path is not configured if strip_components.is_none() && lookup_platform_key(opts, "bin_path") - .or_else(|| opts.get("bin_path").cloned()) + .or_else(|| opts.get("bin_path").map(|s| s.to_string())) .is_none() && let Ok(should_strip) = file::should_strip_components(file_path, format) && should_strip @@ -1016,16 +1016,16 @@ mod tests { // Flat keys with os_arch_keytype naming opts.insert( "platforms_macos_x86_64_url".to_string(), - "https://example.com/macos-x86_64.tar.gz".to_string(), + toml::Value::String("https://example.com/macos-x86_64.tar.gz".to_string()), ); opts.insert( "platforms_linux_x64_url".to_string(), - "https://example.com/linux-x64.tar.gz".to_string(), + toml::Value::String("https://example.com/linux-x64.tar.gz".to_string()), ); // Different prefix variant also supported opts.insert( "platform_windows_arm64_url".to_string(), - "https://example.com/windows-arm64.zip".to_string(), + toml::Value::String("https://example.com/windows-arm64.zip".to_string()), ); let tool_opts = ToolVersionOptions { @@ -1048,7 +1048,8 @@ mod tests { let mut opts = IndexMap::new(); opts.insert( "platforms".to_string(), - r#" + toml::Value::String( + r#" [macos-x64] checksum = "blake3:abc123" size = "1024" @@ -1073,7 +1074,8 @@ size = "3072" checksum = "blake3:mno345" size = "5120" "# - .to_string(), + .to_string(), + ), ); let tool_opts = ToolVersionOptions { @@ -1105,8 +1107,11 @@ size = "5120" #[test] fn test_verify_artifact_fallback_to_generic() { let mut opts = IndexMap::new(); - opts.insert("checksum".to_string(), "blake3:generic123".to_string()); - opts.insert("size".to_string(), "512".to_string()); + opts.insert( + "checksum".to_string(), + toml::Value::String("blake3:generic123".to_string()), + ); + opts.insert("size".to_string(), toml::Value::String("512".to_string())); let tool_opts = ToolVersionOptions { opts, @@ -1115,7 +1120,7 @@ size = "5120" // Test that generic fallback works when no platform-specific values exist let checksum = lookup_platform_key(&tool_opts, "checksum") - .or_else(|| tool_opts.get("checksum").cloned()); + .or_else(|| tool_opts.get("checksum").map(|s| s.to_string())); let size = lookup_with_fallback(&tool_opts, "size"); assert_eq!(checksum, Some("blake3:generic123".to_string())); @@ -1127,7 +1132,8 @@ size = "5120" let mut opts = IndexMap::new(); opts.insert( "platform".to_string(), - r#" + toml::Value::String( + r#" [macos-arm64] bin_path = "CMake.app/Contents/bin" @@ -1137,7 +1143,8 @@ bin_path = "bin" [windows-x64] bin_path = "." "# - .to_string(), + .to_string(), + ), ); let tool_opts = ToolVersionOptions { @@ -1164,7 +1171,8 @@ bin_path = "." let mut opts = IndexMap::new(); opts.insert( "platforms".to_string(), - r#" + toml::Value::String( + r#" [macos-arm64] bin = "xmake" @@ -1174,7 +1182,8 @@ bin = "xmake" [windows-x64] bin = "xmake.exe" "# - .to_string(), + .to_string(), + ), ); let tool_opts = ToolVersionOptions { @@ -1199,14 +1208,19 @@ bin = "xmake.exe" #[test] fn test_lookup_platform_key_bin_with_fallback() { let mut opts = IndexMap::new(); - opts.insert("bin".to_string(), "generic-tool".to_string()); + opts.insert( + "bin".to_string(), + toml::Value::String("generic-tool".to_string()), + ); opts.insert( "platforms".to_string(), - r#" + toml::Value::String( + r#" [windows-x64] bin = "tool.exe" "# - .to_string(), + .to_string(), + ), ); let tool_opts = ToolVersionOptions { @@ -1232,10 +1246,16 @@ bin = "tool.exe" let mut opts = IndexMap::new(); opts.insert( "platforms_windows_x64_bin".to_string(), - "xmake.exe".to_string(), + toml::Value::String("xmake.exe".to_string()), + ); + opts.insert( + "platforms_linux_x64_bin".to_string(), + toml::Value::String("xmake".to_string()), + ); + opts.insert( + "platforms_macos_arm64_bin".to_string(), + toml::Value::String("xmake".to_string()), ); - opts.insert("platforms_linux_x64_bin".to_string(), "xmake".to_string()); - opts.insert("platforms_macos_arm64_bin".to_string(), "xmake".to_string()); let tool_opts = ToolVersionOptions { opts, diff --git a/src/backend/ubi.rs b/src/backend/ubi.rs index 6f6dc497ce..17012fe6c6 100644 --- a/src/backend/ubi.rs +++ b/src/backend/ubi.rs @@ -207,7 +207,7 @@ impl Backend for UbiBackend { let v = tv.version.to_string(); let opts = tv.request.options(); let bin_path = lookup_platform_key(&opts, "bin_path") - .or_else(|| opts.get("bin_path").cloned()) + .or_else(|| opts.get("bin_path").map(|s| s.to_string())) .unwrap_or_else(|| "bin".to_string()); let extract_all = opts.get("extract_all").is_some_and(|v| v == "true"); let bin_dir = tv.install_path(); @@ -243,7 +243,7 @@ impl Backend for UbiBackend { tv.request .options() .get("exe") - .cloned() + .map(|s| s.to_string()) .unwrap_or(tv.ba().short.to_string()), ]; if cfg!(windows) { @@ -341,8 +341,8 @@ impl Backend for UbiBackend { tv: &ToolVersion, ) -> eyre::Result> { let opts = tv.request.options(); - if let Some(bin_path) = - lookup_platform_key(&opts, "bin_path").or_else(|| opts.get("bin_path").cloned()) + if let Some(bin_path) = lookup_platform_key(&opts, "bin_path") + .or_else(|| opts.get("bin_path").map(|s| s.to_string())) { // bin_path should always point to a directory containing binaries Ok(vec![tv.install_path().join(&bin_path)]) @@ -375,7 +375,7 @@ impl Backend for UbiBackend { // These options affect which artifact is downloaded for key in ["exe", "matching", "matching_regex", "provider"] { if let Some(value) = opts.get(key) { - result.insert(key.to_string(), value.clone()); + result.insert(key.to_string(), value.to_string()); } } diff --git a/src/backend/vfox.rs b/src/backend/vfox.rs index de3c091721..b581dc817e 100644 --- a/src/backend/vfox.rs +++ b/src/backend/vfox.rs @@ -116,7 +116,7 @@ impl Backend for VfoxBackend { &tv.version, tv.install_path(), tv.download_path(), - tool_opts.opts.clone(), + tool_opts.opts_as_strings(), ) .await .wrap_err("Backend install method failed")?; @@ -296,7 +296,18 @@ impl VfoxBackend { tool_name, &tv.version, tv.install_path(), - opts.opts.clone(), + opts.opts + .iter() + .map(|(k, v)| { + ( + k.clone(), + match v { + toml::Value::String(s) => s.clone(), + _ => v.to_string(), + }, + ) + }) + .collect(), ) .await .wrap_err("Backend exec env method failed")? diff --git a/src/cli/args/backend_arg.rs b/src/cli/args/backend_arg.rs index 71e1c06397..51be078eff 100644 --- a/src/cli/args/backend_arg.rs +++ b/src/cli/args/backend_arg.rs @@ -4,7 +4,7 @@ use crate::config::Config; use crate::plugins::PluginType; use crate::registry::REGISTRY; use crate::toolset::install_state::InstallStateTool; -use crate::toolset::{ToolVersionOptions, install_state, parse_tool_options}; +use crate::toolset::{EPHEMERAL_OPT_KEYS, ToolVersionOptions, install_state, parse_tool_options}; use crate::{backend, config, dirs, lockfile, registry}; use contracts::requires; use eyre::{Result, bail}; @@ -76,7 +76,16 @@ impl> From for BackendArg { impl From for BackendArg { fn from(ist: InstallStateTool) -> Self { - let (short, tool_name, opts) = parse_backend_components(&ist.short, ist.full.as_ref()); + let (short, tool_name, mut opts) = parse_backend_components(&ist.short, ist.full.as_ref()); + + // Merge manifest opts into the parsed opts (manifest opts provide defaults) + if !ist.opts.is_empty() { + let tvo = opts.get_or_insert_with(ToolVersionOptions::default); + for (k, v) in ist.opts { + tvo.opts.entry(k).or_insert(v); + } + } + Self::new_raw( short, ist.full, @@ -87,6 +96,19 @@ impl From for BackendArg { } } +/// Split a string like `"http:hello[url=...,bin=bin]"` into `("http:hello", "url=...,bin=bin")`. +/// Returns `None` if no bracketed opts are present. +pub fn split_bracketed_opts(s: &str) -> Option<(&str, &str)> { + regex!(r"^(.+)\[(.+)\]$") + .captures(s) + .map(|c| (c.get(1).unwrap().as_str(), c.get(2).unwrap().as_str())) +} + +/// Strip trailing `[...]` opts from a string, e.g. `"foo[a=1]"` → `"foo"`. +pub(crate) fn strip_opts(s: &str) -> String { + regex!(r#"\[.+\]$"#).replace_all(s, "").to_string() +} + fn parse_backend_components( short: &str, full: Option<&String>, @@ -96,12 +118,12 @@ fn parse_backend_components( .unwrap_or(&short) .split_once(':') .unwrap_or(("", full.unwrap_or(&short))); - let short = regex!(r#"\[.+\]$"#).replace_all(&short, "").to_string(); + let short = strip_opts(&short); let mut opts = None; - if let Some(c) = regex!(r"^(.+)\[(.+)\]$").captures(tool_name) { - tool_name = c.get(1).unwrap().as_str(); - opts = Some(parse_tool_options(c.get(2).unwrap().as_str())); + if let Some((name, opts_str)) = split_bracketed_opts(tool_name) { + tool_name = name; + opts = Some(parse_tool_options(opts_str)); } (short, tool_name.to_string(), opts) @@ -357,16 +379,19 @@ impl BackendArg { pub fn full_with_opts(&self) -> String { let full = self.full(); - if regex!(r"^(.+)\[(.+)\]$").is_match(&full) { + if split_bracketed_opts(&full).is_some() { return full; } if let Some(opts) = &self.opts { let opts_str = opts .opts .iter() - // filter out global options that are only relevant for initial installation - .filter(|(k, _)| !["postinstall", "install_env"].contains(&k.as_str())) - .map(|(k, v)| format!("{k}={v}")) + .filter(|(k, _)| !EPHEMERAL_OPT_KEYS.contains(&k.as_str())) + .filter_map(|(k, v)| match v { + toml::Value::String(s) => Some(format!("{k}={s}")), + toml::Value::Table(_) | toml::Value::Array(_) => None, + _ => Some(format!("{k}={v}")), + }) .collect::>() .join(","); if !full.contains(['[', ']']) && !opts_str.is_empty() { @@ -378,15 +403,14 @@ impl BackendArg { pub fn full_without_opts(&self) -> String { let full = self.full(); - if let Some(c) = regex!(r"^(.+)\[(.+)\]$").captures(&full) { - return c.get(1).unwrap().as_str().to_string(); + if let Some((name, _)) = split_bracketed_opts(&full) { + return name.to_string(); } full } pub fn opts(&self) -> ToolVersionOptions { // Start with registry options as base (if available) - // Use backend_options to get options specific to the backend being used let full = self.full(); let mut opts = REGISTRY .get(self.short.as_str()) @@ -395,8 +419,8 @@ impl BackendArg { // Get user-provided options (from self.opts or from full string) let user_opts = self.opts.clone().unwrap_or_else(|| { - if let Some(c) = regex!(r"^(.+)\[(.+)\]$").captures(&full) { - parse_tool_options(c.get(2).unwrap().as_str()) + if let Some((_, opts_str)) = split_bracketed_opts(&full) { + parse_tool_options(opts_str) } else { ToolVersionOptions::default() } @@ -439,8 +463,8 @@ impl BackendArg { if !self.resolution.explicit { let full = self.full(); // Strip options since lockfiles have a separate options field - if let Some(c) = regex!(r"^(.+)\[(.+)\]$").captures(&full) { - return c.get(1).unwrap().as_str().to_string(); + if let Some((name, _)) = split_bracketed_opts(&full) { + return name.to_string(); } return full; } @@ -463,8 +487,8 @@ impl BackendArg { } }; // Strip options since lockfiles have a separate options field - if let Some(c) = regex!(r"^(.+)\[(.+)\]$").captures(&full) { - return c.get(1).unwrap().as_str().to_string(); + if let Some((name, _)) = split_bracketed_opts(&full) { + return name.to_string(); } full } @@ -472,8 +496,7 @@ impl BackendArg { pub fn tool_name(&self) -> String { let full = self.full(); let (_backend, tool_name) = full.split_once(':').unwrap_or(("", &full)); - let tool_name = regex!(r#"\[.+\]$"#).replace_all(tool_name, "").to_string(); - tool_name.to_string() + strip_opts(tool_name) } /// maps something like cargo:cargo-binstall to cargo-binstall and ubi:cargo-binstall, etc diff --git a/src/cli/args/mod.rs b/src/cli/args/mod.rs index c8a75d507e..42b2956ca9 100644 --- a/src/cli/args/mod.rs +++ b/src/cli/args/mod.rs @@ -1,4 +1,4 @@ -pub use backend_arg::{BackendArg, BackendResolution}; +pub use backend_arg::{BackendArg, BackendResolution, split_bracketed_opts}; pub use env_var_arg::EnvVarArg; pub use tool_arg::{ToolArg, ToolVersionType}; diff --git a/src/cli/tool_stub.rs b/src/cli/tool_stub.rs index 82a7b40037..4c0d2e7132 100644 --- a/src/cli/tool_stub.rs +++ b/src/cli/tool_stub.rs @@ -26,7 +26,7 @@ pub struct ToolStubFile { pub os: Option>, pub lock: Option, #[serde(flatten, deserialize_with = "deserialize_tool_stub_options")] - pub opts: indexmap::IndexMap, + pub opts: indexmap::IndexMap, #[serde(skip)] pub tool_name: String, } @@ -42,15 +42,13 @@ pub struct ToolStubLockPlatform { pub checksum: Option, } -// Custom deserializer that converts TOML values to strings for storage in opts +// Custom deserializer that keeps TOML values native, converting scalars to strings fn deserialize_tool_stub_options<'de, D>( deserializer: D, -) -> Result, D::Error> +) -> Result, D::Error> where D: Deserializer<'de>, { - use serde::de::Error; - let value = Value::deserialize(deserializer)?; let mut opts = indexmap::IndexMap::new(); @@ -64,20 +62,13 @@ where continue; } - // Convert TOML values to strings for storage - let string_value = match val { - Value::String(s) => s, - Value::Table(_) | Value::Array(_) => { - // For complex values (tables, arrays), serialize them as TOML strings - toml::to_string(&val).map_err(D::Error::custom)? - } - Value::Integer(i) => i.to_string(), - Value::Float(f) => f.to_string(), - Value::Boolean(b) => b.to_string(), - Value::Datetime(dt) => dt.to_string(), + let stored_value = match val { + Value::String(_) | Value::Table(_) | Value::Array(_) => val, + // Convert scalar values (ints, bools, floats) to strings + _ => Value::String(val.to_string().trim_matches('"').to_string()), }; - opts.insert(key, string_value); + opts.insert(key, stored_value); } } @@ -88,7 +79,7 @@ fn default_version() -> String { "latest".to_string() } -fn has_http_backend_config(opts: &indexmap::IndexMap) -> bool { +fn has_http_backend_config(opts: &indexmap::IndexMap) -> bool { // Check for top-level url if opts.contains_key("url") { return true; @@ -96,8 +87,21 @@ fn has_http_backend_config(opts: &indexmap::IndexMap) -> bool { // Check for platform-specific configs with urls for (key, value) in opts { - if key.starts_with("platforms") && value.contains("url") { - return true; + if key.starts_with("platforms") { + // Check if the value is a table containing url keys + if let toml::Value::Table(table) = value { + for (_, v) in table { + if let toml::Value::Table(inner) = v + && inner.contains_key("url") + { + return true; + } + } + } else if let toml::Value::String(s) = value + && s.contains("url") + { + return true; + } } } @@ -153,7 +157,12 @@ impl ToolStubFile { let tool_name = stub .tool .clone() - .or_else(|| stub.opts.get("tool").map(|s| s.to_string())) + .or_else(|| { + stub.opts + .get("tool") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + }) .unwrap_or_else(|| { if has_http_backend_config(&stub.opts) { format!("http:{stub_name}") @@ -185,13 +194,13 @@ impl ToolStubFile { // Add bin field if present if let Some(bin) = &self.bin { - opts.insert("bin".to_string(), bin.clone()); + opts.insert("bin".to_string(), toml::Value::String(bin.clone())); } let options = ToolVersionOptions { os: self.os.clone(), install_env: self.install_env.clone(), - opts: opts.clone(), + opts, }; // Set options on the BackendArg so they're available to the backend @@ -199,12 +208,12 @@ impl ToolStubFile { // For HTTP backend with "latest" version, use URL+checksum hash as version for stability let version = if self.tool_name.starts_with("http:") && self.version == "latest" { - if let Some(url) = - lookup_platform_key(&options, "url").or_else(|| opts.get("url").cloned()) + if let Some(url) = lookup_platform_key(&options, "url") + .or_else(|| options.get("url").map(|s| s.to_string())) { // Include checksum in hash calculation for better version stability let checksum = lookup_platform_key(&options, "checksum") - .or_else(|| opts.get("checksum").cloned()) + .or_else(|| options.get("checksum").map(|s| s.to_string())) .unwrap_or_default(); let hash_input = format!("{url}:{checksum}"); // Use first 8 chars of URL+checksum hash as version @@ -403,9 +412,11 @@ fn resolve_platform_specific_bin(stub: &ToolStubFile, stub_path: &Path) -> Strin let platform_key = get_current_platform_key(); // Check for platform-specific bin field: platforms.{platform}.bin - let platform_bin_key = format!("platforms.{platform_key}.bin"); - if let Some(platform_bin) = stub.opts.get(&platform_bin_key) { - return platform_bin.to_string(); + if let Some(toml::Value::Table(platforms)) = stub.opts.get("platforms") + && let Some(toml::Value::Table(platform)) = platforms.get(&platform_key) + && let Some(toml::Value::String(bin)) = platform.get("bin") + { + return bin.clone(); } // Fall back to global bin field diff --git a/src/config/config_file/mise_toml.rs b/src/config/config_file/mise_toml.rs index 66533f9469..cb0d215765 100644 --- a/src/config/config_file/mise_toml.rs +++ b/src/config/config_file/mise_toml.rs @@ -38,6 +38,37 @@ use crate::{env, file}; use super::diagnostic::toml_parse_error; use super::{ConfigFileType, min_version::MinVersionSpec}; +/// Convert a `toml::Value` to a `toml_edit::Value` for serialization. +fn toml_value_to_edit(v: toml::Value) -> Value { + match v { + toml::Value::String(s) => Value::from(s), + toml::Value::Integer(i) => Value::from(i), + toml::Value::Float(f) => Value::from(f), + toml::Value::Boolean(b) => Value::from(b), + toml::Value::Datetime(dt) => { + // Parse the datetime string back into a toml_edit datetime + dt.to_string() + .parse::() + .map(Value::from) + .unwrap_or_else(|_| Value::from(dt.to_string())) + } + toml::Value::Array(arr) => { + let mut edit_arr = Array::new(); + for item in arr { + edit_arr.push(toml_value_to_edit(item)); + } + Value::Array(edit_arr) + } + toml::Value::Table(table) => { + let mut edit_table = InlineTable::new(); + for (k, v) in table { + edit_table.insert(k, toml_value_to_edit(v)); + } + Value::InlineTable(edit_table) + } + } +} + #[derive(Default, Deserialize)] pub struct MiseToml { #[serde(rename = "_")] @@ -582,7 +613,7 @@ impl ConfigFile for MiseToml { let mut table = InlineTable::new(); table.insert("version", versions[0].version().into()); for (k, v) in options.opts { - table.insert(k, v.into()); + table.insert(k, toml_value_to_edit(v)); } if let Some(os) = options.os { let mut arr = Array::new(); @@ -610,7 +641,7 @@ impl ConfigFile for MiseToml { let mut table = InlineTable::new(); table.insert("version", v.to_string().into()); for (k, v) in tr.options().opts { - table.insert(k, v.clone().into()); + table.insert(k, toml_value_to_edit(v.clone())); } arr.push(table); } @@ -694,7 +725,9 @@ impl ConfigFile for MiseToml { let mut opts_context = context.clone(); opts_context.insert("version", "{{ version }}"); for v in options.opts.values_mut() { - *v = self.parse_template_with_context(&opts_context, v)?; + if let toml::Value::String(s) = v { + *s = self.parse_template_with_context(&opts_context, s)?; + } } let mut ba = ba.clone(); // Start with cached options but filter out install-time-only options @@ -1528,7 +1561,7 @@ impl<'de> de::Deserialize<'de> for MiseTomlToolList { toml::Value::String(s) => { // Convert {{version}} to {version} for backend templating let s = s.replace("{{version}}", "{version}"); - options.opts.insert(k, s); + options.opts.insert(k, toml::Value::String(s)); } _ => { return Err(de::Error::custom("os must be a string or array")); @@ -1558,28 +1591,25 @@ impl<'de> de::Deserialize<'de> for MiseTomlToolList { } }, _ => { - // Handle nested structures + // Store values as native toml::Value match v { - toml::Value::Table(_) => { - // Store as TOML string, will be flattened later - options.opts.insert(k, v.to_string()); - } toml::Value::String(s) => { // Convert {{version}} to {version} for backend templating let s = s.replace("{{version}}", "{version}"); - options.opts.insert(k, s); + options.opts.insert(k, toml::Value::String(s)); } - toml::Value::Boolean(b) => { - options.opts.insert(k, b.to_string()); - } - toml::Value::Integer(i) => { - options.opts.insert(k, i.to_string()); - } - toml::Value::Float(f) => { - options.opts.insert(k, f.to_string()); + toml::Value::Table(_) | toml::Value::Array(_) => { + // Keep tables and arrays as native TOML + options.opts.insert(k, v); } _ => { - return Err(de::Error::custom("invalid value type")); + // Convert scalar values (ints, bools, floats) to strings + options.opts.insert( + k, + toml::Value::String( + v.to_string().trim_matches('"').to_string(), + ), + ); } } } @@ -1648,7 +1678,7 @@ impl<'de> de::Deserialize<'de> for MiseTomlTool { toml::Value::String(s) => { // Convert {{version}} to {version} for backend templating let s = s.replace("{{version}}", "{version}"); - options.opts.insert(k, s); + options.opts.insert(k, toml::Value::String(s)); } _ => { return Err(de::Error::custom("os must be a string or array")); @@ -1678,24 +1708,27 @@ impl<'de> de::Deserialize<'de> for MiseTomlTool { } }, _ => { - // Handle nested tables (like platform.macos-arm64) - // and convert them to string representation for ToolVersionOptions - let value_str = match v { + // Store values as native toml::Value + match v { toml::Value::String(s) => { // Convert {{version}} to {version} for backend templating - s.replace("{{version}}", "{version}") + let s = s.replace("{{version}}", "{version}"); + options.opts.insert(k, toml::Value::String(s)); } toml::Value::Table(_) | toml::Value::Array(_) => { - // Serialize complex types back to TOML string - // This preserves nested structures like platform.macos-arm64.bin_path - toml::to_string(&v).map_err(de::Error::custom)? + // Keep tables and arrays as native TOML + options.opts.insert(k, v); } - toml::Value::Boolean(b) => b.to_string(), - toml::Value::Integer(i) => i.to_string(), - toml::Value::Float(f) => f.to_string(), - toml::Value::Datetime(dt) => dt.to_string(), - }; - options.opts.insert(k, value_str); + _ => { + // Convert scalar values (ints, bools, floats) to strings + options.opts.insert( + k, + toml::Value::String( + v.to_string().trim_matches('"').to_string(), + ), + ); + } + } } } } @@ -2348,12 +2381,12 @@ mod tests { .expect("ansible should be in tool request set"); let opts = ansible_requests[0].options(); assert_eq!( - opts.get("uvx").map(|s| s.as_str()), + opts.get("uvx"), Some("false"), "registry default uvx=false should be preserved with table syntax" ); assert_eq!( - opts.get("pipx_args").map(|s| s.as_str()), + opts.get("pipx_args"), Some("--include-deps"), "registry default pipx_args=--include-deps should be preserved with table syntax" ); @@ -2372,12 +2405,12 @@ mod tests { .expect("ansible should be in tool request set"); let opts2 = ansible2[0].options(); assert_eq!( - opts2.get("uvx").map(|s| s.as_str()), + opts2.get("uvx"), Some("true"), "user-provided uvx=true should override registry default uvx=false" ); assert_eq!( - opts2.get("pipx_args").map(|s| s.as_str()), + opts2.get("pipx_args"), Some("--include-deps"), "non-overridden registry default pipx_args should still be preserved" ); diff --git a/src/config/config_file/snapshots/mise__config__config_file__mise_toml__tests__fixture-3.snap b/src/config/config_file/snapshots/mise__config__config_file__mise_toml__tests__fixture-3.snap index 712323b194..86dddce94c 100644 --- a/src/config/config_file/snapshots/mise__config__config_file__mise_toml__tests__fixture-3.snap +++ b/src/config/config_file/snapshots/mise__config__config_file__mise_toml__tests__fixture-3.snap @@ -105,7 +105,9 @@ ToolRequestSet { os: None, install_env: {}, opts: { - "venv": ".venv", + "venv": String( + ".venv", + ), }, }, source: MiseToml( diff --git a/src/plugins/core/java.rs b/src/plugins/core/java.rs index 04151dfba6..6d6a3b7ee8 100644 --- a/src/plugins/core/java.rs +++ b/src/plugins/core/java.rs @@ -244,7 +244,7 @@ impl JavaPlugin { tv.request .options() .get("release_type") - .cloned() + .map(|s| s.to_string()) .unwrap_or(String::from("ga")) } @@ -304,7 +304,7 @@ impl Backend for JavaPlugin { .list_tools() .iter() .find(|ba| ba.short == "java") - .and_then(|ba| ba.opts().get("release_type").cloned()) + .and_then(|ba| ba.opts().get("release_type").map(|s| s.to_string())) .unwrap_or_else(|| "ga".to_string()); let versions = self diff --git a/src/plugins/core/python.rs b/src/plugins/core/python.rs index 20033be8e1..0e35f5fc6d 100644 --- a/src/plugins/core/python.rs +++ b/src/plugins/core/python.rs @@ -318,7 +318,7 @@ impl PythonPlugin { .map(|s| re_digits.replace(s, "").to_string()); if cfg!(unix) { if let (Some(major), Some(minor), Some(suffix)) = (major, minor, suffix) { - if tv.request.options().get("patch_sysconfig") != Some(&"false".to_string()) { + if tv.request.options().get("patch_sysconfig") != Some("false") { sysconfig::update_sysconfig(&install, major, minor, &suffix)?; } } else { diff --git a/src/plugins/core/rust.rs b/src/plugins/core/rust.rs index 77d92f4c81..5db02538ae 100644 --- a/src/plugins/core/rust.rs +++ b/src/plugins/core/rust.rs @@ -265,7 +265,7 @@ fn get_args(tv: &ToolVersion) -> (Option, Option>, Option, pub versions: Vec, pub explicit_backend: bool, + pub opts: BTreeMap, } /// Entry in the consolidated manifest file (.mise-installs.toml). @@ -45,6 +47,8 @@ struct ManifestTool { full: Option, #[serde(default = "default_true")] explicit_backend: bool, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + opts: BTreeMap, } fn default_true() -> bool { @@ -222,8 +226,36 @@ async fn init_tools() -> MutexResult { } // Get metadata: prefer manifest, fall back to legacy .mise.backend - let (short, full, explicit_backend) = if let Some(mt) = manifest.get(&dir_name) { - (mt.short.clone(), mt.full.clone(), mt.explicit_backend) + let (short, full, explicit_backend, opts) = if let Some(mt) = manifest.get(&dir_name) { + let mut full = mt.full.clone(); + let mut opts = mt.opts.clone(); + // Backward compat: if opts is empty but full contains [...], extract opts + if opts.is_empty() + && let Some(ref f) = full + && let Some((stripped_str, opts_str)) = crate::cli::args::split_bracketed_opts(f) + { + let stripped = stripped_str.to_string(); + let parsed = parse_tool_options(opts_str); + for (k, v) in &parsed.opts { + if EPHEMERAL_OPT_KEYS.contains(&k.as_str()) { + continue; + } + opts.insert(k.clone(), v.clone()); + } + full = Some(stripped); + // Schedule manifest rewrite to migrate to new format + let m = updated_manifest.get_or_insert_with(|| manifest.clone()); + m.insert( + dir_name.clone(), + ManifestTool { + short: mt.short.clone(), + full: full.clone(), + explicit_backend: mt.explicit_backend, + opts: opts.clone(), + }, + ); + } + (mt.short.clone(), full, mt.explicit_backend, opts) } else if let Some((s, full, explicit)) = read_legacy_backend_meta(&dir_name) { // Migration: absorb into manifest (clone on first migration) let m = updated_manifest.get_or_insert_with(|| manifest.clone()); @@ -233,11 +265,12 @@ async fn init_tools() -> MutexResult { short: s.clone(), full: full.clone(), explicit_backend: explicit, + opts: BTreeMap::new(), }, ); - (s, full, explicit) + (s, full, explicit, BTreeMap::new()) } else { - (dir_name.clone(), None, true) + (dir_name.clone(), None, true, BTreeMap::new()) }; let tool = InstallStateTool { @@ -245,6 +278,7 @@ async fn init_tools() -> MutexResult { full, versions, explicit_backend, + opts, }; time!("init_tools {short}"); tools.insert(short, tool); @@ -271,6 +305,7 @@ async fn init_tools() -> MutexResult { full: Some(full.clone()), versions: Default::default(), explicit_backend: true, + opts: BTreeMap::new(), }); tool.full = Some(full); } @@ -357,12 +392,19 @@ pub async fn add_plugin(short: &str, plugin_type: PluginType) -> Result<()> { /// Writes backend metadata to the consolidated manifest file. pub fn write_backend_meta(ba: &BackendArg) -> Result<()> { - let full = match ba.full() { - full if full.starts_with("core:") => ba.full(), - _ => ba.full_with_opts(), - }; + let full = ba.full_without_opts(); let explicit = ba.has_explicit_backend(); + // Store opts as native TOML values, filtering out ephemeral keys. + let mut opts_map: BTreeMap = BTreeMap::new(); + if let Some(o) = ba.opts.as_ref() { + for (k, v) in &o.opts { + if !EPHEMERAL_OPT_KEYS.contains(&k.as_str()) { + opts_map.insert(k.clone(), v.clone()); + } + } + } + let _lock = MANIFEST_LOCK.lock().expect("MANIFEST_LOCK lock failed"); let mut manifest = read_manifest(); manifest.insert( @@ -371,6 +413,7 @@ pub fn write_backend_meta(ba: &BackendArg) -> Result<()> { short: ba.short.clone(), full: Some(full), explicit_backend: explicit, + opts: opts_map, }, ); write_manifest(&manifest)?; @@ -423,6 +466,7 @@ pub fn reset() { mod tests { use super::normalize_version_for_sort; use itertools::Itertools; + use std::collections::BTreeMap; use versions::Versioning; #[test] @@ -477,6 +521,7 @@ mod tests { short: "node".to_string(), full: Some("core:node".to_string()), explicit_backend: true, + opts: BTreeMap::new(), }, ); manifest.insert( @@ -485,6 +530,7 @@ mod tests { short: "bun".to_string(), full: Some("aqua:oven-sh/bun".to_string()), explicit_backend: false, + opts: BTreeMap::new(), }, ); manifest.insert( @@ -493,6 +539,7 @@ mod tests { short: "tiny".to_string(), full: None, explicit_backend: true, + opts: BTreeMap::new(), }, ); @@ -510,4 +557,81 @@ mod tests { assert!(deserialized["tiny"].full.is_none()); assert!(deserialized["tiny"].explicit_backend); } + + #[test] + fn test_manifest_with_opts_roundtrip() { + use super::{Manifest, ManifestTool}; + + let mut opts = BTreeMap::new(); + opts.insert( + "url".to_string(), + toml::Value::String("https://example.com/tool.tar.gz".to_string()), + ); + opts.insert( + "bin_path".to_string(), + toml::Value::String("bin".to_string()), + ); + + // Nested table for platforms + let mut platforms = toml::map::Map::new(); + let mut linux = toml::map::Map::new(); + linux.insert( + "url".to_string(), + toml::Value::String("https://example.com/linux.tar.gz".to_string()), + ); + platforms.insert("linux-x64".to_string(), toml::Value::Table(linux)); + opts.insert("platforms".to_string(), toml::Value::Table(platforms)); + + let mut manifest = Manifest::new(); + manifest.insert( + "hello".to_string(), + ManifestTool { + short: "hello".to_string(), + full: Some("http:hello".to_string()), + explicit_backend: true, + opts, + }, + ); + + let serialized = toml::to_string_pretty(&manifest).unwrap(); + let deserialized: Manifest = toml::from_str(&serialized).unwrap(); + + assert_eq!(deserialized["hello"].full.as_deref(), Some("http:hello")); + assert_eq!( + deserialized["hello"].opts.get("url"), + Some(&toml::Value::String( + "https://example.com/tool.tar.gz".to_string() + )) + ); + assert_eq!( + deserialized["hello"].opts.get("bin_path"), + Some(&toml::Value::String("bin".to_string())) + ); + // Verify nested platforms table survived round-trip + let platforms = deserialized["hello"].opts.get("platforms").unwrap(); + assert!(platforms.is_table()); + let linux = platforms.get("linux-x64").unwrap(); + assert_eq!( + linux.get("url").unwrap().as_str(), + Some("https://example.com/linux.tar.gz") + ); + } + + #[test] + fn test_manifest_backward_compat_bracketed_full() { + use super::Manifest; + + // Old format: full contains bracketed opts + let toml_str = r#" +[hello] +short = "hello" +full = "http:hello[url = \"https://example.com/tool.tar.gz\", bin_path = \"bin\"]" +explicit_backend = true +"#; + let manifest: Manifest = toml::from_str(toml_str).unwrap(); + let mt = &manifest["hello"]; + // Old format should deserialize with opts empty and brackets in full + assert!(mt.opts.is_empty()); + assert!(mt.full.as_ref().unwrap().contains('[')); + } } diff --git a/src/toolset/mod.rs b/src/toolset/mod.rs index 09fa566358..50770f34c1 100644 --- a/src/toolset/mod.rs +++ b/src/toolset/mod.rs @@ -33,7 +33,7 @@ pub use tool_request_set::{ToolRequestSet, ToolRequestSetBuilder}; pub use tool_source::ToolSource; pub use tool_version::{ResolveOptions, ToolVersion}; pub use tool_version_list::ToolVersionList; -pub use tool_version_options::{ToolVersionOptions, parse_tool_options}; +pub use tool_version_options::{EPHEMERAL_OPT_KEYS, ToolVersionOptions, parse_tool_options}; mod builder; pub mod env_cache; diff --git a/src/toolset/tool_version_options.rs b/src/toolset/tool_version_options.rs index d16d2ab1a1..0f15e037d0 100644 --- a/src/toolset/tool_version_options.rs +++ b/src/toolset/tool_version_options.rs @@ -1,13 +1,23 @@ use indexmap::IndexMap; -#[derive(Debug, Default, Clone, PartialEq, Eq, serde::Deserialize, serde::Serialize)] +/// Option keys that are only relevant during initial installation and should not +/// be persisted in the manifest or included in `full_with_opts()`. +// install_env is a named field on ToolVersionOptions (serde puts it in self.install_env), +// but parse_tool_options() can still place it in opts, so we filter it here as well. +pub const EPHEMERAL_OPT_KEYS: &[&str] = &["postinstall", "install_env"]; + +#[derive(Debug, Default, Clone, PartialEq, serde::Deserialize, serde::Serialize)] pub struct ToolVersionOptions { pub os: Option>, pub install_env: IndexMap, #[serde(flatten)] - pub opts: IndexMap, + pub opts: IndexMap, } +// toml::Value doesn't implement Eq (due to floats), but we control the values +// and won't have NaN, so this is safe in practice. +impl Eq for ToolVersionOptions {} + // Implement Hash manually to ensure deterministic hashing across IndexMap impl std::hash::Hash for ToolVersionOptions { fn hash(&self, state: &mut H) { @@ -20,8 +30,31 @@ impl std::hash::Hash for ToolVersionOptions { // Hash opts in sorted order for deterministic hashing let mut opts_sorted: Vec<_> = self.opts.iter().collect(); - opts_sorted.sort_by_key(|(k, _)| *k); - opts_sorted.hash(state); + opts_sorted.sort_by_key(|(k, _)| k.as_str()); + for (k, v) in opts_sorted { + k.hash(state); + hash_toml_value(v, state); + } + } +} + +fn hash_toml_value(v: &toml::Value, state: &mut H) { + use std::hash::Hash; + match v { + toml::Value::Table(t) => { + let mut sorted: Vec<_> = t.iter().collect(); + sorted.sort_by_key(|(k, _)| k.as_str()); + for (k, v) in sorted { + k.hash(state); + hash_toml_value(v, state); + } + } + toml::Value::Array(arr) => { + for v in arr { + hash_toml_value(v, state); + } + } + _ => v.to_string().hash(state), } } @@ -30,23 +63,32 @@ impl ToolVersionOptions { self.install_env.is_empty() && self.opts.is_empty() } - pub fn get(&self, key: &str) -> Option<&String> { - // First try direct lookup - if let Some(value) = self.opts.get(key) { - return Some(value); - } + /// Get a string value for a key. Returns the str for String values, + /// or None for non-string values. + pub fn get(&self, key: &str) -> Option<&str> { + self.opts.get(key).and_then(|v| v.as_str()) + } - // We can't return references to temporarily parsed TOML values, - // so nested lookup is not possible with this API. - // For nested values, users should access the raw opts and parse themselves. - None + /// Convert opts to string values, extracting inner strings from + /// `toml::Value::String` and calling `to_string()` on other types. + pub fn opts_as_strings(&self) -> IndexMap { + self.opts + .iter() + .map(|(k, v)| { + ( + k.clone(), + match v { + toml::Value::String(s) => s.clone(), + _ => v.to_string(), + }, + ) + }) + .collect() } - pub fn merge(&mut self, other: &IndexMap) { + pub fn merge(&mut self, other: &IndexMap) { for (key, value) in other { - self.opts - .entry(key.to_string()) - .or_insert(value.to_string()); + self.opts.entry(key.to_string()).or_insert(value.clone()); } } @@ -59,7 +101,7 @@ impl ToolVersionOptions { self.get_nested_value_exists(key) } - pub fn iter(&self) -> impl Iterator { + pub fn iter(&self) -> impl Iterator { self.opts.iter() } @@ -74,19 +116,8 @@ impl ToolVersionOptions { let root_key = parts[0]; let nested_path = &parts[1..]; - // Get the root value and try to parse it as TOML if let Some(value) = self.opts.get(root_key) { - if let Ok(toml_value) = toml::de::from_str::(value) { - return Self::value_exists_at_path(&toml_value, nested_path); - } else if value.trim().starts_with('{') && value.trim().ends_with('}') { - // Try to parse as inline TOML table - if let Ok(toml_value) = - toml::de::from_str::(&format!("value = {value}")) - && let Some(table_value) = toml_value.get("value") - { - return Self::value_exists_at_path(table_value, nested_path); - } - } + return Self::value_exists_at_path(value, nested_path); } false @@ -109,9 +140,8 @@ impl ToolVersionOptions { } } - // New method to get nested values as owned Strings + /// Get nested values as owned Strings by navigating the toml::Value tree. pub fn get_nested_string(&self, key: &str) -> Option { - // Split the key by dots to navigate nested structure let parts: Vec<&str> = key.split('.').collect(); if parts.len() < 2 { return None; @@ -120,19 +150,8 @@ impl ToolVersionOptions { let root_key = parts[0]; let nested_path = &parts[1..]; - // Get the root value and try to parse it as TOML if let Some(value) = self.opts.get(root_key) { - if let Ok(toml_value) = toml::de::from_str::(value) { - return Self::get_string_at_path(&toml_value, nested_path); - } else if value.trim().starts_with('{') && value.trim().ends_with('}') { - // Try to parse as inline TOML table - if let Ok(toml_value) = - toml::de::from_str::(&format!("value = {value}")) - && let Some(table_value) = toml_value.get("value") - { - return Self::get_string_at_path(table_value, nested_path); - } - } + return Self::get_string_at_path(value, nested_path); } None @@ -163,21 +182,61 @@ impl ToolVersionOptions { } pub fn parse_tool_options(s: &str) -> ToolVersionOptions { + // Try TOML parsing first (handles nested structures like platforms={...} correctly) + if let Some(tvo) = try_parse_as_toml(s) { + return tvo; + } + // Fall back to manual parsing for legacy formats with unquoted values + parse_tool_options_manual(s) +} + +/// Try parsing an options string as a TOML inline table. +/// Returns `Some(opts)` if the string is valid TOML, `None` otherwise. +fn try_parse_as_toml(s: &str) -> Option { + let toml_str = format!("_x_ = {{ {s} }}"); + let value: toml::Value = toml::from_str(&toml_str).ok()?; + let table = value.get("_x_")?.as_table()?; + let mut tvo = ToolVersionOptions::default(); + for (k, v) in table { + match v { + toml::Value::Table(_) | toml::Value::Array(_) => { + tvo.opts.insert(k.clone(), v.clone()); + } + toml::Value::String(_) => { + tvo.opts.insert(k.clone(), v.clone()); + } + _ => { + // Convert scalar values (ints, bools, floats) to strings + tvo.opts.insert( + k.clone(), + toml::Value::String(v.to_string().trim_matches('"').to_string()), + ); + } + } + } + Some(tvo) +} + +/// Legacy manual parser for option strings with unquoted values (e.g. `exe=rg,match=musl`). +/// Splits by commas, but segments without `=` are appended to the previous key's value. +fn parse_tool_options_manual(s: &str) -> ToolVersionOptions { let mut tvo = ToolVersionOptions::default(); let mut current_key: Option = None; for opt in s.split(',') { if let Some((k, v)) = opt.split_once('=') { if !k.trim().is_empty() { - tvo.opts.insert(k.trim().to_string(), v.to_string()); + tvo.opts + .insert(k.trim().to_string(), toml::Value::String(v.to_string())); current_key = Some(k.trim().to_string()); } } else if !opt.is_empty() { // No '=' found, append to the previous value or create a new key if let Some(key) = ¤t_key && let Some(existing_value) = tvo.opts.get_mut(key) + && let toml::Value::String(s) = existing_value { - existing_value.push(','); - existing_value.push_str(opt); + s.push(','); + s.push_str(opt); } } } @@ -190,6 +249,10 @@ mod tests { use pretty_assertions::assert_eq; use test_log::test; + fn s(v: &str) -> toml::Value { + toml::Value::String(v.to_string()) + } + #[test] fn test_parse_tool_options() { let t = |input, expected| { @@ -201,10 +264,7 @@ mod tests { t( "exe=rg", ToolVersionOptions { - opts: [("exe".to_string(), "rg".to_string())] - .iter() - .cloned() - .collect(), + opts: [("exe".to_string(), s("rg"))].iter().cloned().collect(), ..Default::default() }, ); @@ -212,8 +272,8 @@ mod tests { "exe=rg,match=musl", ToolVersionOptions { opts: [ - ("exe".to_string(), "rg".to_string()), - ("match".to_string(), "musl".to_string()), + ("exe".to_string(), s("rg")), + ("match".to_string(), s("musl")), ] .iter() .cloned() @@ -225,11 +285,11 @@ mod tests { "profile=minimal,components=rust-src,llvm-tools,targets=wasm32-unknown-unknown,thumbv2-none-eabi", ToolVersionOptions { opts: [ - ("profile".to_string(), "minimal".to_string()), - ("components".to_string(), "rust-src,llvm-tools".to_string()), + ("profile".to_string(), s("minimal")), + ("components".to_string(), s("rust-src,llvm-tools")), ( "targets".to_string(), - "wasm32-unknown-unknown,thumbv2-none-eabi".to_string(), + s("wasm32-unknown-unknown,thumbv2-none-eabi"), ), ] .iter() @@ -243,8 +303,8 @@ mod tests { " exe = rg , match = musl ", ToolVersionOptions { opts: [ - ("exe".to_string(), " rg ".to_string()), - ("match".to_string(), " musl ".to_string()), + ("exe".to_string(), s(" rg ")), + ("match".to_string(), s(" musl ")), ] .iter() .cloned() @@ -257,9 +317,9 @@ mod tests { "foo=,bar=baz,baz=", ToolVersionOptions { opts: [ - ("foo".to_string(), "".to_string()), - ("bar".to_string(), "baz".to_string()), - ("baz".to_string(), "".to_string()), + ("foo".to_string(), s("")), + ("bar".to_string(), s("baz")), + ("baz".to_string(), s("")), ] .iter() .cloned() @@ -269,22 +329,67 @@ mod tests { ); } + #[test] + fn test_parse_tool_options_with_nested_braces() { + let input = r#"platforms={ linux-x64 = { url = "https://example.com/linux.tar.gz" }, macos-arm64 = { url = "https://example.com/macos.tar.gz" } }"#; + let opts = parse_tool_options(input); + assert_eq!(opts.opts.len(), 1, "should have exactly one key"); + assert!(opts.opts.get("platforms").unwrap().is_table()); + + assert_eq!( + opts.get_nested_string("platforms.linux-x64.url"), + Some("https://example.com/linux.tar.gz".to_string()) + ); + assert_eq!( + opts.get_nested_string("platforms.macos-arm64.url"), + Some("https://example.com/macos.tar.gz".to_string()) + ); + } + + #[test] + fn test_parse_tool_options_mixed_braces_and_simple() { + let input = r#"bin_path="bin",platforms={ linux-x64 = { url = "https://example.com/linux.tar.gz" } },strip_components="1""#; + let opts = parse_tool_options(input); + assert_eq!(opts.get("bin_path"), Some("bin")); + assert_eq!(opts.get("strip_components"), Some("1")); + assert!(opts.opts.get("platforms").is_some()); + } + + #[test] + fn test_parse_tool_options_integer_strip_components() { + // strip_components=1 (integer, not string) should be converted to string + let input = r#"bin_path="bin",strip_components=1"#; + let opts = parse_tool_options(input); + assert_eq!(opts.get("bin_path"), Some("bin")); + assert_eq!(opts.get("strip_components"), Some("1")); + } + #[test] fn test_nested_option_with_os_arch_dash() { let mut opts = IndexMap::new(); - opts.insert( - "platforms".to_string(), - r#" -[macos-x64] -url = "https://example.com/macos-x64.tar.gz" -checksum = "sha256:abc123" - -[linux-x64] -url = "https://example.com/linux-x64.tar.gz" -checksum = "sha256:def456" -"# - .to_string(), + let mut platforms = toml::map::Map::new(); + let mut macos = toml::map::Map::new(); + macos.insert( + "url".to_string(), + toml::Value::String("https://example.com/macos-x64.tar.gz".to_string()), ); + macos.insert( + "checksum".to_string(), + toml::Value::String("sha256:abc123".to_string()), + ); + platforms.insert("macos-x64".to_string(), toml::Value::Table(macos)); + + let mut linux = toml::map::Map::new(); + linux.insert( + "url".to_string(), + toml::Value::String("https://example.com/linux-x64.tar.gz".to_string()), + ); + linux.insert( + "checksum".to_string(), + toml::Value::String("sha256:def456".to_string()), + ); + platforms.insert("linux-x64".to_string(), toml::Value::Table(linux)); + opts.insert("platforms".to_string(), toml::Value::Table(platforms)); let tool_opts = ToolVersionOptions { opts, @@ -312,19 +417,26 @@ checksum = "sha256:def456" #[test] fn test_generic_nested_options() { let mut opts = IndexMap::new(); - opts.insert( - "config".to_string(), - r#" -[database] -host = "localhost" -port = 5432 - -[cache.redis] -host = "redis.example.com" -port = 6379 -"# - .to_string(), + let mut config = toml::map::Map::new(); + let mut database = toml::map::Map::new(); + database.insert( + "host".to_string(), + toml::Value::String("localhost".to_string()), ); + database.insert("port".to_string(), toml::Value::Integer(5432)); + config.insert("database".to_string(), toml::Value::Table(database)); + + let mut cache = toml::map::Map::new(); + let mut redis = toml::map::Map::new(); + redis.insert( + "host".to_string(), + toml::Value::String("redis.example.com".to_string()), + ); + redis.insert("port".to_string(), toml::Value::Integer(6379)); + cache.insert("redis".to_string(), toml::Value::Table(redis)); + config.insert("cache".to_string(), toml::Value::Table(cache)); + + opts.insert("config".to_string(), toml::Value::Table(config)); let tool_opts = ToolVersionOptions { opts, @@ -352,41 +464,42 @@ port = 6379 #[test] fn test_direct_and_nested_options() { let mut opts = IndexMap::new(); + let mut platforms = toml::map::Map::new(); + let mut macos = toml::map::Map::new(); + macos.insert( + "url".to_string(), + toml::Value::String("https://example.com/macos-x64.tar.gz".to_string()), + ); + platforms.insert("macos-x64".to_string(), toml::Value::Table(macos)); + opts.insert("platforms".to_string(), toml::Value::Table(platforms)); opts.insert( - "platforms".to_string(), - r#" -[macos-x64] -url = "https://example.com/macos-x64.tar.gz" -"# - .to_string(), + "simple_option".to_string(), + toml::Value::String("value".to_string()), ); - opts.insert("simple_option".to_string(), "value".to_string()); let tool_opts = ToolVersionOptions { opts, ..Default::default() }; - // Test nested option assert_eq!( tool_opts.get_nested_string("platforms.macos-x64.url"), Some("https://example.com/macos-x64.tar.gz".to_string()) ); - // Test direct option - assert_eq!(tool_opts.get("simple_option"), Some(&"value".to_string())); + assert_eq!(tool_opts.get("simple_option"), Some("value")); } #[test] fn test_contains_key_with_nested_options() { let mut opts = IndexMap::new(); - opts.insert( - "platforms".to_string(), - r#" -[macos-x64] -url = "https://example.com/macos-x64.tar.gz" -"# - .to_string(), + let mut platforms = toml::map::Map::new(); + let mut macos = toml::map::Map::new(); + macos.insert( + "url".to_string(), + toml::Value::String("https://example.com/macos-x64.tar.gz".to_string()), ); + platforms.insert("macos-x64".to_string(), toml::Value::Table(macos)); + opts.insert("platforms".to_string(), toml::Value::Table(platforms)); let tool_opts = ToolVersionOptions { opts, @@ -401,29 +514,29 @@ url = "https://example.com/macos-x64.tar.gz" #[test] fn test_merge_functionality() { let mut opts = IndexMap::new(); - opts.insert( - "platforms".to_string(), - r#" -[macos-x64] -url = "https://example.com/macos-x64.tar.gz" -"# - .to_string(), + let mut platforms = toml::map::Map::new(); + let mut macos = toml::map::Map::new(); + macos.insert( + "url".to_string(), + toml::Value::String("https://example.com/macos-x64.tar.gz".to_string()), ); + platforms.insert("macos-x64".to_string(), toml::Value::Table(macos)); + opts.insert("platforms".to_string(), toml::Value::Table(platforms)); let mut tool_opts = ToolVersionOptions { opts, ..Default::default() }; - // Verify nested option access assert!(tool_opts.contains_key("platforms.macos-x64.url")); - // Merge new options let mut new_opts = IndexMap::new(); - new_opts.insert("simple_option".to_string(), "value".to_string()); + new_opts.insert( + "simple_option".to_string(), + toml::Value::String("value".to_string()), + ); tool_opts.merge(&new_opts); - // Should be able to access both old and new options assert!(tool_opts.contains_key("platforms.macos-x64.url")); assert!(tool_opts.contains_key("simple_option")); } @@ -431,21 +544,20 @@ url = "https://example.com/macos-x64.tar.gz" #[test] fn test_non_existent_nested_paths() { let mut opts = IndexMap::new(); - opts.insert( - "platforms".to_string(), - r#" -[macos-x64] -url = "https://example.com/macos-x64.tar.gz" -"# - .to_string(), + let mut platforms = toml::map::Map::new(); + let mut macos = toml::map::Map::new(); + macos.insert( + "url".to_string(), + toml::Value::String("https://example.com/macos-x64.tar.gz".to_string()), ); + platforms.insert("macos-x64".to_string(), toml::Value::Table(macos)); + opts.insert("platforms".to_string(), toml::Value::Table(platforms)); let tool_opts = ToolVersionOptions { opts, ..Default::default() }; - // Test non-existent nested paths assert_eq!( tool_opts.get_nested_string("platforms.windows-x64.url"), None @@ -461,12 +573,10 @@ url = "https://example.com/macos-x64.tar.gz" fn test_indexmap_preserves_order() { let mut tvo = ToolVersionOptions::default(); - // Insert options in a specific order - tvo.opts.insert("zebra".to_string(), "last".to_string()); - tvo.opts.insert("alpha".to_string(), "first".to_string()); - tvo.opts.insert("beta".to_string(), "second".to_string()); + tvo.opts.insert("zebra".to_string(), s("last")); + tvo.opts.insert("alpha".to_string(), s("first")); + tvo.opts.insert("beta".to_string(), s("second")); - // Collect keys to verify order is preserved let keys: Vec<_> = tvo.opts.keys().collect(); assert_eq!(keys, vec!["zebra", "alpha", "beta"]); }