Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
52 changes: 52 additions & 0 deletions e2e-win/exec_inactive_tool.Tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
Describe 'exec_inactive_tool' {
# Regression test for discussion #4407: `mise install <tool>` writes to no
# config file, so the tool's bin dir never joins the PATH `mise exec` builds
# and the bin fails to resolve. The Windows arm surfaced only the opaque
# `cannot find binary path`, with no hint that mise had just installed it.

BeforeAll {
$script:miseExe = (Get-Command mise).Source

# A scratch cwd so no repo-local mise.toml can activate the tool and
# make the "not activated" path unreachable.
$script:workdir = Join-Path -Path $env:TEMP -ChildPath "mise-4407-$PID"
New-Item -ItemType Directory -Force -Path $script:workdir | Out-Null
Push-Location $script:workdir

mise install usage | Out-Null

# Drop any host `usage` from PATH. The subject here is what mise says
# when the bin is genuinely unreachable; a preinstalled copy would
# resolve instead and mask the failure entirely.
$script:savedPath = $env:PATH
$env:PATH = ($env:PATH -split ';' | Where-Object {
$_ -and -not (Test-Path -Path (Join-Path -Path $_ -ChildPath 'usage.exe') -ErrorAction SilentlyContinue)
}) -join ';'
}

AfterAll {
$env:PATH = $script:savedPath
Pop-Location
Remove-Item -Recurse -Force $script:workdir -ErrorAction Ignore
}

It 'names the installed-but-inactive tool' {
$out = (& $script:miseExe exec -- usage 2>&1 | Out-String)
$LASTEXITCODE | Should -Not -Be 0
$out | Should -Match "installed but not activated"
$out | Should -Match "mise use usage"
$out | Should -Match "mise exec usage -- usage"
}

It 'keeps the original error for a bin no installed tool provides' {
$out = (& $script:miseExe exec -- not-a-tool-4407 2>&1 | Out-String)
$LASTEXITCODE | Should -Not -Be 0
$out | Should -Match "cannot find binary path"
$out | Should -Not -Match "installed but not activated"
}

It 'suggests a command that works' {
$out = (& $script:miseExe exec usage -- usage --version 2>&1 | Out-String)
$out | Should -Match "usage-cli"
}
}
22 changes: 22 additions & 0 deletions e2e/cli/test_exec_inactive_installed_tool
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env bash

# Regression test for discussion #4407: `mise install <tool>` writes to no config
# file, so the tool's bin dir never joins the PATH `mise exec` builds and the bin
# fails to resolve. That failure used to surface as a bare ENOENT ("couldn't exec
# process") with no hint that mise had just installed the tool.

mise install dummy@1.0.0

# Installed, but activated nowhere, so this still fails -- it should now say why.
assert_fail_contains "mise exec -- dummy" "installed but not activated"
assert_fail_contains "mise exec -- dummy" "mise use dummy"
assert_fail_contains "mise exec -- dummy" "mise exec dummy -- dummy"

# The command the hint suggests has to actually work.
assert_contains "mise exec dummy -- dummy" "1.0.0"

# A bin belonging to no installed tool keeps the original error untouched: the
# hint is only added when mise can name the tool providing it.
unknown_out="$(mise exec -- not-a-tool-4407 2>&1 || true)"
assert_contains_text "$unknown_out" "couldn't exec process"
assert_not_contains_text "$unknown_out" "installed but not activated"
48 changes: 43 additions & 5 deletions src/cli/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,12 @@ where
}
let args = args.into_iter().map(Into::into).collect::<Vec<_>>();
let program = program.to_executable();
// Captured before `program` is shadowed by the resolved path, so a failed
// exec can still name what the user asked for.
let program_name = program.to_string_lossy().into_owned();
// Set when PATH resolution failed and we fell back to the bare name. The
// exec below is then expected to fail, and discussion #4407's hint applies.
let mut resolution_failed = false;
let program = if program.to_string_lossy().contains('/') {
// Already a path, no need to resolve
program
Expand Down Expand Up @@ -356,7 +362,6 @@ where
.collect();
std::env::join_paths(mise_added.iter().chain(original.iter())).unwrap()
});
let program_name = program.to_string_lossy().into_owned();
let is_shim_dispatch = env::MISE_SHIM_PATH.read().unwrap().is_some();
match which::which_in_all(&program, lookup_path, cwd) {
Ok(mut candidates) => {
Expand All @@ -368,14 +373,22 @@ where
None if is_shim_dispatch => {
return Err(crate::shims::err_shim_not_found(&program_name).await);
}
None => program, // Fall back to original if resolution fails
None => {
// Fall back to original if resolution fails
resolution_failed = true;
program
}
}
}
Err(which::Error::CannotFindBinaryPath) if is_shim_dispatch => {
return Err(crate::shims::err_shim_not_found(&program_name).await);
}
Err(err) if is_shim_dispatch => return Err(err.into()),
Err(_) => program, // Fall back to original if resolution fails
Err(_) => {
// Fall back to original if resolution fails
resolution_failed = true;
program
}
}
};
if crate::file::is_active_mise_shim(std::path::Path::new(&program)) {
Expand All @@ -399,7 +412,29 @@ where
}

let err = exec::Command::new(program.clone()).args(&args).exec();
bail!("{:?} {err}", program.to_string_lossy())
let mut msg = format!("{:?} {err}", program.to_string_lossy());
// The bin never resolved on PATH. If an installed-but-unconfigured tool
// would have provided it, say so instead of leaving the user with a bare
// ENOENT (discussion #4407).
if resolution_failed && let Some(hint) = crate::shims::exec_resolution_hint(&program_name).await
{
msg.push_str("\n\n");
msg.push_str(&hint);
}
bail!("{msg}")
}

/// The opaque `cannot find binary path`, plus an explanation when an
/// installed-but-unconfigured tool would have provided the bin. `mise install`
/// writes to no config file, so its tool dirs never join the PATH `mise exec`
/// builds (discussion #4407).
#[cfg(all(windows, not(test)))]
async fn err_cannot_find_binary_path(program_name: &str) -> eyre::Report {
let base: eyre::Report = which::Error::CannotFindBinaryPath.into();
match crate::shims::exec_resolution_hint(program_name).await {
Some(hint) => eyre!("{base}\n\n{hint}"),
None => base,
}
}

#[cfg(all(windows, not(test)))]
Expand Down Expand Up @@ -471,6 +506,9 @@ where
Err(which::Error::CannotFindBinaryPath) if is_shim_dispatch => {
return Err(crate::shims::err_shim_not_found(&program_name).await);
}
Err(which::Error::CannotFindBinaryPath) => {
return Err(err_cannot_find_binary_path(&program_name).await);
}
Err(err) => return Err(err.into()),
};
let program = match resolved {
Expand All @@ -481,7 +519,7 @@ where
None if is_shim_dispatch => {
return Err(crate::shims::err_shim_not_found(&program_name).await);
}
None => return Err(which::Error::CannotFindBinaryPath.into()),
None => return Err(err_cannot_find_binary_path(&program_name).await),
};
env::remove_var(env::MISE_SHIM_PATH_ENV);
let args: Vec<OsString> = args.into_iter().map(Into::into).collect();
Expand Down
105 changes: 105 additions & 0 deletions src/shims.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::{
};

use crate::backend::Backend;
use crate::cli::args::BackendArg;
use crate::cli::exec::Exec;
use crate::config::{Config, Settings};
use crate::file::display_path;
Expand Down Expand Up @@ -843,6 +844,72 @@ pub(crate) fn unavailable_configured_tool_message(
Some(msg.trim().to_string())
}

/// `mise install <tool>` deliberately writes to no config file, so the tool's
/// bin dir never joins the PATH `mise exec` builds and resolution fails with an
/// opaque error (`cannot find binary path` on Windows, `couldn't exec process`
/// on unix). When an installed-but-unconfigured tool would have supplied the
/// bin, name it and say how to activate it. See discussion #4407.
///
/// Returns `None` when a configured tool already matches the bin: that failure
/// has a different cause, and `err_no_version_set` /
/// `unavailable_configured_tool_message` already explain it.
///
/// Matching is by tool name, so a bin that shares no name with the tool
/// providing it (`npm` from `node`) is not recognized. Those callers fall back
/// to the existing message rather than getting a wrong one.
pub(crate) fn inactive_installed_tool_message(
ts: &Toolset,
installed_shorts: &[String],
bin_name: &str,
) -> Option<String> {
if ts
.list_current_versions()
.iter()
.any(|(_, tv)| tv.ba().matches_bin_name(bin_name))
{
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved
return None;
}
let shorts = installed_shorts
.iter()
.filter(|short| BackendArg::from(short.as_str()).matches_bin_name(bin_name))
.collect_vec();
if shorts.is_empty() {
return None;
}
let mut msg =
format!("{bin_name} is installed but not activated β€” it is not in any config file.\n");
msg.push_str("To activate it, run:\n");
for short in &shorts {
msg.push_str(&format!(" mise use {short}\n"));
}
msg.push_str("To run it without changing any config file, run:\n");
for short in &shorts {
msg.push_str(&format!(" mise exec {short} -- {bin_name}\n"));
}
Some(msg.trim().to_string())
}

/// Gather what [`inactive_installed_tool_message`] needs. Only called once a
/// binary has definitively failed to resolve, so the config/toolset load lands
/// on a path that is about to abort anyway.
#[cfg(not(test))]
pub async fn exec_resolution_hint(bin_name: &str) -> Option<String> {
// Windows invokes binaries as `<tool>.exe`; name `<tool>` in the message.
let bin_stem = bin_name
.strip_suffix(std::env::consts::EXE_SUFFIX)
.unwrap_or(bin_name);
let config = Config::get().await.ok()?;
let ts = ToolsetBuilder::new().build(&config).await.ok()?;
// try_list_tools rather than list_tools: an error path must not panic
// because install state was never initialized.
let installed_shorts = crate::toolset::install_state::try_list_tools()?
.values()
.filter(|t| !t.versions.is_empty())
.map(|t| t.short.clone())
.collect_vec();
inactive_installed_tool_message(&ts, &installed_shorts, bin_stem)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -874,6 +941,44 @@ mod tests {
assert!(!msg.contains("node@1.0.0"));
}

#[tokio::test]
async fn inactive_tool_message_names_the_installed_tool() {
let _config = Config::get().await.unwrap();
let ts = Toolset::new(ToolSource::Argument);

let msg = inactive_installed_tool_message(&ts, &["gh".to_string()], "gh").unwrap();
assert!(msg.contains("installed but not activated"));
assert!(msg.contains("mise use gh"));
assert!(msg.contains("mise exec gh -- gh"));
}

#[tokio::test]
async fn inactive_tool_message_is_none_for_configured_tool() {
let _config = Config::get().await.unwrap();
let mut ts = Toolset::new(ToolSource::Argument);
let ba = Arc::new(BackendArg::from("gh"));
let request = ToolRequest::new(ba.clone(), "1.0.0", ToolSource::Argument).unwrap();
let tv = ToolVersion::new(request.clone(), "1.0.0".into());
let mut tvl = ToolVersionList::new(ba.clone(), ToolSource::Argument);
tvl.requests.push(request);
tvl.versions.push(tv);
ts.versions.insert(ba, tvl);

// A configured tool that still fails to resolve has a different cause;
// err_no_version_set/unavailable_configured_tool_message own that case.
assert!(inactive_installed_tool_message(&ts, &["gh".to_string()], "gh").is_none());
}

#[tokio::test]
async fn inactive_tool_message_is_none_when_bin_does_not_name_the_tool() {
let _config = Config::get().await.unwrap();
let ts = Toolset::new(ToolSource::Argument);

// Matching is by tool name, so a bin like npm (provided by node) is not
// recognized and the caller keeps its existing message.
assert!(inactive_installed_tool_message(&ts, &["node".to_string()], "npm").is_none());
}

#[cfg(windows)]
#[test]
fn bash_shim_script_includes_wsl_guard() {
Expand Down
10 changes: 8 additions & 2 deletions src/toolset/install_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,12 +469,18 @@ pub fn get_plugin_type(short: &str) -> Option<PluginType> {
}

pub fn list_tools() -> Arc<BTreeMap<String, InstallStateTool>> {
try_list_tools().expect("INSTALL_STATE_TOOLS is None")
}

/// Non-panicking counterpart to [`list_tools`], mirroring [`try_list_plugins`].
/// Callers on error paths must not panic just because install state was never
/// initialized.
pub fn try_list_tools() -> Option<Arc<BTreeMap<String, InstallStateTool>>> {
INSTALL_STATE_TOOLS
.lock()
.expect("INSTALL_STATE_TOOLS lock failed")
.as_ref()
.expect("INSTALL_STATE_TOOLS is None")
.clone()
.cloned()
}

pub fn backend_type(short: &str) -> Result<Option<BackendType>> {
Expand Down