Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions e2e/core/test_python_venv_tool_override
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/usr/bin/env bash
# Regression test: a `--tool python@X` override must reach venv creation.
#
# `_.python.venv` rebuilds a python/uv-only toolset from the config files to avoid a circular wait
# during env resolution, so on its own it cannot see a CLI override — it would build the venv from
# the first `[tools] python` entry and then silently run every task under that interpreter.
# https://github.com/jdx/mise/discussions/5281

export MISE_PYTHON_GITHUB_ATTESTATIONS=0

cat >.mise.toml <<EOF
[tools]
python = ["3.13.14", "3.12.13"]
[env._.python]
venv = {path = "{{env.HOME}}/my_venv", create=true}
[tasks]
version = "python --version"
EOF

mise i

# no override: the first [tools] entry, unchanged
assert_contains "mise run version" "Python 3.13.14"

# the override decides which interpreter the venv is built from. A venv cannot be retargeted after
# creation, so remove it first — that is the same thing a per-version venv path would achieve.
rm -rf "$HOME/my_venv"
assert_contains "mise run --tool python@3.12.13 version" "Python 3.12.13"
26 changes: 26 additions & 0 deletions src/config/env_directive/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::path_env::PathEnv;
use crate::tera::{
TeraEngine, contains_template_syntax, get_tera, render_str, tera_exec, tera1_exec,
};
use crate::toolset::Toolset;
use eyre::{Context, eyre};
use indexmap::IndexMap;
use itertools::Itertools;
Expand Down Expand Up @@ -337,6 +338,12 @@ pub(super) struct EnvDirectiveContext<'a> {
source: &'a Path,
exec_env: &'a EnvMap,
config_root: &'a Path,
/// The caller's resolved toolset, when the caller has one. `Toolset::env` does; plain
/// `Config::env` does not, because it runs before any toolset exists. Directives that need to
/// know *which* version of a tool is active — rather than just rendering `tools.*` templates —
/// read it from here, since CLI overrides such as `--tool python@3.12` only ever reach a
/// toolset and are never written back to `Config`.
toolset: Option<&'a Toolset>,
}

impl EnvDirectiveContext<'_> {
Expand Down Expand Up @@ -383,11 +390,24 @@ impl EnvResults {
}

pub async fn resolve(
config: &Arc<Config>,
ctx: tera::Context,
initial: &EnvMap,
input: Vec<(EnvDirective, PathBuf)>,
resolve_opts: EnvResolveOptions,
) -> eyre::Result<Self> {
Self::resolve_with_toolset(config, ctx, initial, input, resolve_opts, None).await
}

/// [`Self::resolve`] for callers that already have a resolved toolset. See
/// [`EnvDirectiveContext::toolset`] for why a directive would want it.
pub async fn resolve_with_toolset(
config: &Arc<Config>,
mut ctx: tera::Context,
initial: &EnvMap,
input: Vec<(EnvDirective, PathBuf)>,
resolve_opts: EnvResolveOptions,
toolset: Option<&Toolset>,
) -> eyre::Result<Self> {
// trace!("resolve: input: {:#?}", &input);
let mut env = initial
Expand Down Expand Up @@ -640,6 +660,7 @@ impl EnvResults {
source: &source,
exec_env: &env_vars,
config_root: &config_root,
toolset,
};
let path = Self::path(&mut directive_ctx, input_str).await?;
paths.push((path.clone(), source.clone()));
Expand All @@ -656,6 +677,7 @@ impl EnvResults {
source: &source,
exec_env: &env_vars,
config_root: &config_root,
toolset,
};
let files = Self::file(&mut directive_ctx, input, opts.expand).await?;
for (f, new_env) in files {
Expand Down Expand Up @@ -686,6 +708,7 @@ impl EnvResults {
source: &source,
exec_env: &env_vars,
config_root: &config_root,
toolset,
};
let files = Self::source(&mut directive_ctx, &mut paths, input)?;
for (f, new_env) in files {
Expand Down Expand Up @@ -723,6 +746,7 @@ impl EnvResults {
source: &source,
exec_env: &env_vars,
config_root: &config_root,
toolset,
};
Self::venv(
&mut directive_ctx,
Expand All @@ -731,6 +755,8 @@ impl EnvResults {
create,
venv::PythonVenvOptions {
python,
// filled in by `venv()` from the caller's toolset
active_python: None,
uv_create_args,
python_create_args,
require_uv: false,
Expand Down
37 changes: 36 additions & 1 deletion src/config/env_directive/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ pub struct Venv {

#[derive(Default)]
pub(crate) struct PythonVenvOptions {
/// `_.python.venv.python` — the user asked for this version by name, so a miss is an error
/// they should see rather than something to paper over.
pub(crate) python: Option<String>,
/// The python the caller's toolset has active, filled in by [`EnvResults::venv`]. A
/// *preference*, not a request: if the config-derived toolset cannot offer it we fall back to
/// the previous behaviour instead of failing, because the user never named this version.
pub(crate) active_python: Option<String>,
pub(crate) uv_create_args: Option<Vec<String>>,
pub(crate) python_create_args: Option<Vec<String>>,
pub(crate) require_uv: bool,
Expand Down Expand Up @@ -109,6 +115,7 @@ pub(crate) async fn create_python_venv(
) -> Result<bool> {
let PythonVenvOptions {
python,
active_python,
uv_create_args,
python_create_args,
require_uv,
Expand All @@ -120,6 +127,18 @@ pub(crate) async fn create_python_venv(
// otherwise use the first since that's what `python3` will refer to
if let Some(v) = python {
tv.versions.iter().find(|t| t.version.starts_with(v))
} else if let Some(v) = &active_python {
// the caller's active python, which this toolset may not list at all — it was rebuilt
// from the config files. Falling back keeps a `--tool` version that is absent from
// `[tools]` working exactly as it did before (#5281).
//
// Matched exactly, unlike the branch above: that one compares against whatever partial
// version the user wrote in `_.python.venv.python`, while this is already resolved on
// both sides. A prefix match here would let `3.12.0` select a configured `3.12.0a1`.
tv.versions
.iter()
.find(|t| t.version == *v)
.or_else(|| tv.versions.first())
Comment thread
greptile-apps[bot] marked this conversation as resolved.
} else {
tv.versions.first()
}
Expand Down Expand Up @@ -179,19 +198,35 @@ pub(crate) async fn create_python_venv(
Ok(true)
}

/// The version of the python the caller's toolset has active, if any.
///
/// `create_python_venv` resolves its own python/uv-only toolset to avoid a circular wait (see
/// below), and that toolset is built from the config files — so it lists every `[tools] python`
/// entry in config order and knows nothing about `--tool`. Feeding this back in as the `python`
/// option makes it select the same interpreter the rest of the run is using, and costs nothing
/// when there is no override: the caller's toolset then holds the same first entry.
fn active_python_version(toolset: Option<&Toolset>) -> Option<String> {
let tvl = toolset?.versions.get(&BackendArg::from("python"))?;
Some(tvl.versions.first()?.version.clone())
}

impl EnvResults {
pub(super) async fn venv(
ctx: &mut EnvDirectiveContext<'_>,
env: &mut IndexMap<String, (String, Option<PathBuf>)>,
path: String,
create: bool,
options: PythonVenvOptions,
mut options: PythonVenvOptions,
) -> Result<()> {
trace!("python venv: {} create={create}", display_path(&path));
trust_check(ctx.source)?;
let venv = ctx.parse_template(&path)?;
let venv = ctx.normalize_path(venv.into());
let venv_lock = LockFile::new(&venv).lock()?;
// Record whichever python the caller actually has active. The toolset rebuilt below comes
// from the config files, so on its own it cannot see a CLI override — `mise run --tool
// python@3.12` would silently build the venv from the first `[tools] python` entry (#5281).
options.active_python = active_python_version(ctx.toolset);
if !venv.exists() && create {
// TODO: the toolset stuff doesn't feel like it's in the right place here
// TODO: in fact this should probably be moved to execute at the same time as src/uv.rs runs in ts.env() instead of config.env()
Expand Down
5 changes: 4 additions & 1 deletion src/toolset/toolset_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ impl Toolset {
.flatten()
.collect();
// trace!("load_env: entries: {:#?}", entries);
let env_results = EnvResults::resolve(
let env_results = EnvResults::resolve_with_toolset(
config,
ctx,
env,
Expand All @@ -461,6 +461,9 @@ impl Toolset {
tools: tools_filter,
warn_on_missing_required: *WARN_ON_MISSING_REQUIRED_ENV,
},
// `_.python.venv` needs the *active* python, which is only knowable here: a
// `--tool python@3.12` override lives in this toolset and never reaches `Config`.
Some(self),
)
.await?;
if log::log_enabled!(log::Level::Trace) {
Expand Down
Loading