-
-
Notifications
You must be signed in to change notification settings - Fork 952
feat(config): support vars in tool versions #6401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a73e0f6
031ba71
cbaabe5
0c03d18
fb76fd3
550e8b6
3411106
a4f2876
68d9e06
7c84f05
08eba8f
c9c11e4
56732e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| #!/usr/bin/env bash | ||
| # shellcheck disable=SC2016 | ||
|
|
||
| # Test that env works with vars template (this already works) | ||
| cat <<EOF >mise.toml | ||
| [env] | ||
| FOO = "{{vars.BAR}}" | ||
|
|
||
| [vars] | ||
| BAR = "test" | ||
| EOF | ||
|
|
||
| assert_contains "mise env -s bash | grep FOO" "export FOO=test" | ||
|
|
||
| # Now test using vars in tool version | ||
| cat <<EOF >mise.toml | ||
| [vars] | ||
| TINY_VERSION = "3.1.0" | ||
|
|
||
| [tools] | ||
| tiny = "{{vars.TINY_VERSION}}" | ||
| EOF | ||
|
|
||
| # Check that the tool request includes the expanded version | ||
| mise install tiny@3.1.0 >/dev/null 2>&1 || true | ||
| assert_contains "mise ls tiny" "3.1.0" | ||
|
|
||
| # Test with more complex expression | ||
| cat <<EOF >mise.toml | ||
| [vars] | ||
| NODE_MAJOR = "20" | ||
|
|
||
| [tools] | ||
| node = "{{vars.NODE_MAJOR}}" | ||
| EOF | ||
|
|
||
| # Just check that it tries to resolve it correctly | ||
| mise install node@20 >/dev/null 2>&1 || true | ||
| assert_contains "mise ls node" "20" | ||
|
|
||
| # Test using env variables in tool versions (should already work) | ||
| cat <<EOF >mise.toml | ||
| [tools] | ||
| tiny = "{{ env.MISE_TINY_VERSION | default(value='3.0.0') }}" | ||
| EOF | ||
|
|
||
| MISE_TINY_VERSION=3.1.0 mise install >/dev/null 2>&1 || true | ||
| MISE_TINY_VERSION=3.1.0 assert_contains "mise ls tiny" "3.1.0" | ||
|
|
||
| # Test vars that contain templates themselves | ||
| cat <<EOF >mise.toml | ||
| [vars] | ||
| BASE_VERSION = "3" | ||
| TINY_VERSION = "{{vars.BASE_VERSION}}.1.0" | ||
|
|
||
| [tools] | ||
| tiny = "{{vars.TINY_VERSION}}" | ||
| EOF | ||
|
|
||
| # This should resolve to 3.1.0 | ||
| assert_contains "mise ls tiny" "3.1.0" | ||
|
|
||
| # Test more complex nested templating | ||
| cat <<EOF >mise.toml | ||
| [vars] | ||
| MAJOR = "20" | ||
| MINOR = "11" | ||
| NODE_VERSION = "{{vars.MAJOR}}.{{vars.MINOR}}.0" | ||
|
|
||
| [tools] | ||
| node = "{{vars.NODE_VERSION}}" | ||
| EOF | ||
|
|
||
| # This should resolve to 20.11.0 | ||
| assert_contains "mise ls node" "20.11.0" | ||
|
|
||
| # Cleanup | ||
| rm -f mise.toml |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,7 +21,7 @@ use crate::config::config_file::{ConfigFile, TaskConfig, config_trust_root, trus | |||||||||||||||||||||||||||||||||
| use crate::config::config_file::{config_root, toml::deserialize_arr}; | ||||||||||||||||||||||||||||||||||
| use crate::config::env_directive::{EnvDirective, EnvDirectiveOptions}; | ||||||||||||||||||||||||||||||||||
| use crate::config::settings::SettingsPartial; | ||||||||||||||||||||||||||||||||||
| use crate::config::{Alias, AliasMap}; | ||||||||||||||||||||||||||||||||||
| use crate::config::{Alias, AliasMap, Config}; | ||||||||||||||||||||||||||||||||||
| use crate::file::{create_dir_all, display_path}; | ||||||||||||||||||||||||||||||||||
| use crate::hooks::{Hook, Hooks}; | ||||||||||||||||||||||||||||||||||
| use crate::redactions::Redactions; | ||||||||||||||||||||||||||||||||||
|
|
@@ -96,6 +96,10 @@ impl EnvList { | |||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| impl MiseToml { | ||||||||||||||||||||||||||||||||||
| fn contains_template_syntax(input: &str) -> bool { | ||||||||||||||||||||||||||||||||||
| input.contains("{{") || input.contains("{%") || input.contains("{#") | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| pub fn init(path: &Path) -> Self { | ||||||||||||||||||||||||||||||||||
| let mut context = BASE_CONTEXT.clone(); | ||||||||||||||||||||||||||||||||||
| context.insert( | ||||||||||||||||||||||||||||||||||
|
|
@@ -267,16 +271,22 @@ impl MiseToml { | |||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| fn parse_template(&self, input: &str) -> eyre::Result<String> { | ||||||||||||||||||||||||||||||||||
| if !input.contains("{{") && !input.contains("{%") && !input.contains("{#") { | ||||||||||||||||||||||||||||||||||
| self.parse_template_with_context(&self.context, input) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| fn parse_template_with_context( | ||||||||||||||||||||||||||||||||||
| &self, | ||||||||||||||||||||||||||||||||||
| context: &TeraContext, | ||||||||||||||||||||||||||||||||||
| input: &str, | ||||||||||||||||||||||||||||||||||
| ) -> eyre::Result<String> { | ||||||||||||||||||||||||||||||||||
| if !Self::contains_template_syntax(input) { | ||||||||||||||||||||||||||||||||||
| return Ok(input.to_string()); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| let dir = self.path.parent(); | ||||||||||||||||||||||||||||||||||
| let output = get_tera(dir) | ||||||||||||||||||||||||||||||||||
| .render_str(input, &self.context) | ||||||||||||||||||||||||||||||||||
| .wrap_err_with(|| { | ||||||||||||||||||||||||||||||||||
| let p = display_path(&self.path); | ||||||||||||||||||||||||||||||||||
| eyre!("failed to parse template {input} in {p}") | ||||||||||||||||||||||||||||||||||
| })?; | ||||||||||||||||||||||||||||||||||
| let output = get_tera(dir).render_str(input, context).wrap_err_with(|| { | ||||||||||||||||||||||||||||||||||
| let p = display_path(&self.path); | ||||||||||||||||||||||||||||||||||
| eyre!("failed to parse template {input} in {p}") | ||||||||||||||||||||||||||||||||||
| })?; | ||||||||||||||||||||||||||||||||||
| Ok(output) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
@@ -484,12 +494,27 @@ impl ConfigFile for MiseToml { | |||||||||||||||||||||||||||||||||
| let source = ToolSource::MiseToml(self.path.clone()); | ||||||||||||||||||||||||||||||||||
| let mut trs = ToolRequestSet::new(); | ||||||||||||||||||||||||||||||||||
| let tools = self.tools.lock().unwrap(); | ||||||||||||||||||||||||||||||||||
| let mut context = self.context.clone(); | ||||||||||||||||||||||||||||||||||
| if context.get("vars").is_none() { | ||||||||||||||||||||||||||||||||||
| if let Some(config) = Config::maybe_get() { | ||||||||||||||||||||||||||||||||||
| if let Some(vars_results) = config.vars_results_cached() { | ||||||||||||||||||||||||||||||||||
| let vars = vars_results | ||||||||||||||||||||||||||||||||||
| .vars | ||||||||||||||||||||||||||||||||||
| .iter() | ||||||||||||||||||||||||||||||||||
| .map(|(k, (v, _))| (k.clone(), v.clone())) | ||||||||||||||||||||||||||||||||||
| .collect::<IndexMap<_, _>>(); | ||||||||||||||||||||||||||||||||||
| context.insert("vars", &vars); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+501
to
+506
|
||||||||||||||||||||||||||||||||||
| let vars = vars_results | |
| .vars | |
| .iter() | |
| .map(|(k, (v, _))| (k.clone(), v.clone())) | |
| .collect::<IndexMap<_, _>>(); | |
| context.insert("vars", &vars); | |
| // Avoid unnecessary clones by passing a reference to the existing vars map | |
| context.insert("vars", &vars_results.vars); |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The vars collection is being cloned twice on each call (line 520). Consider caching this transformed vars map or using references where possible to avoid unnecessary allocations, especially if this method is called frequently during tool resolution.
| let vars = vars_results | |
| .vars | |
| .iter() | |
| .map(|(k, (v, _))| (k.clone(), v.clone())) | |
| .collect::<IndexMap<_, _>>(); | |
| context.insert("vars", &vars); | |
| } else if !config.vars.is_empty() { | |
| // Cache the transformed vars map to avoid double cloning | |
| let vars: IndexMap<_, _> = vars_results | |
| .vars | |
| .iter() | |
| .map(|(k, (v, _))| (k.clone(), v.clone())) | |
| .collect(); | |
| context.insert("vars", &vars); | |
| } else if !config.vars.is_empty() { | |
| // Insert a reference to the vars map directly to avoid unnecessary cloning |
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,8 @@ pub struct Config { | |
| tasks: OnceCell<BTreeMap<String, Task>>, | ||
| tool_request_set: OnceCell<ToolRequestSet>, | ||
| toolset: OnceCell<Toolset>, | ||
| vars_loader: Option<Arc<Config>>, | ||
| vars_results: OnceCell<EnvResults>, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Default)] | ||
|
|
@@ -84,6 +86,9 @@ impl Config { | |
| } | ||
| measure!("load config", { Self::load().await }) | ||
| } | ||
| pub fn maybe_get() -> Option<Arc<Self>> { | ||
| _CONFIG.read().unwrap().as_ref().cloned() | ||
| } | ||
|
Comment on lines
+89
to
+91
|
||
| pub fn get_() -> Arc<Self> { | ||
| (*_CONFIG.read().unwrap()).clone().unwrap() | ||
| } | ||
|
|
@@ -140,6 +145,8 @@ impl Config { | |
| project_root: Default::default(), | ||
| repo_urls: Default::default(), | ||
| vars: Default::default(), | ||
| vars_loader: None, | ||
| vars_results: OnceCell::new(), | ||
| }; | ||
| let vars_config = Arc::new(Self { | ||
| tera_ctx: config.tera_ctx.clone(), | ||
|
|
@@ -156,9 +163,15 @@ impl Config { | |
| project_root: config.project_root.clone(), | ||
| repo_urls: config.repo_urls.clone(), | ||
| vars: config.vars.clone(), | ||
| vars_loader: None, | ||
| vars_results: OnceCell::new(), | ||
| }); | ||
| let vars_results = measure!("config::load vars_results", { | ||
| load_vars(&vars_config).await? | ||
| let results = load_vars(&vars_config).await?; | ||
| vars_config.vars_results.set(results.clone()).ok(); | ||
| config.vars_results.set(results.clone()).ok(); | ||
| config.vars_loader = Some(vars_config.clone()); | ||
| results | ||
| }); | ||
| let vars: IndexMap<String, String> = vars_results | ||
| .vars | ||
|
|
@@ -263,6 +276,21 @@ impl Config { | |
| .get_or_try_init(|| async { self.load_env().await }) | ||
| .await | ||
| } | ||
|
|
||
| pub async fn vars_results(self: &Arc<Self>) -> Result<&EnvResults> { | ||
| if let Some(loader) = &self.vars_loader { | ||
| if let Some(results) = loader.vars_results.get() { | ||
| return Ok(results); | ||
| } | ||
| } | ||
| self.vars_results | ||
| .get_or_try_init(|| async move { load_vars(self).await }) | ||
| .await | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| pub fn vars_results_cached(&self) -> Option<&EnvResults> { | ||
| self.vars_results.get() | ||
| } | ||
| pub async fn path_dirs(self: &Arc<Self>) -> eyre::Result<&Vec<PathBuf>> { | ||
| Ok(&self.env_results().await?.env_paths) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string literal "vars" is used multiple times in this method. Consider defining it as a constant to avoid magic strings and improve maintainability.