Conversation
- if $MSYSTEM is set and cygpath is available, convert those path variables to MSYS Unix-style paths - Ensure binary is invoked even if a function/alias named mise is overridden elsewhere. Addresses jdx#3961 Similar adaptions could be done for ZSH and other shells in MSYS2, though Git Bash is most popular
There was a problem hiding this comment.
Pull Request Overview
This PR adds Windows/MSYS2 Git Bash compatibility by converting Windows paths to Unix-style paths when running in MSYS environments. The changes ensure that the mise binary can be properly invoked with Unix-style paths and that the binary is called directly rather than potentially conflicting with aliases or functions.
- Uses MSYS environment detection and cygpath to convert paths to Unix format
- Replaces hardcoded binary references with a MISE_BIN variable for consistent invocation
- Adds path conversion logic specifically for Git Bash environments
Comments suppressed due to low confidence (2)
src/shell/bash.rs:1
- Add a comment explaining what this line does. The colon command syntax for setting default values may not be immediately clear to all maintainers.
#![allow(unknown_lints)]
src/shell/bash.rs:1
- Add a comment explaining that this converts Windows paths to MSYS Unix-style paths for Git Bash compatibility. The purpose of this MSYS-specific logic should be documented.
#![allow(unknown_lints)]
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
If there's interest in making Mise work in MSYS2 (in particular Git Bash), then tests can be adapted: |
|
I don't understand how this fixed the issue If it does, add some e2e to prove it but I don't see how this would. |
|
You are right. This only fixes the very first problem of the wrong path of the // src/cli/hook_env.rs (lines 146-180)
/// modifies the PATH and optionally DIRENV_DIFF env var if it exists
fn build_path_operations(
&self,
installs: &Vec<PathBuf>,
to_remove: &Vec<PathBuf>,
) -> Result<Vec<EnvDiffOperation>> {
let full = join_paths(&*env::PATH)?.to_string_lossy().to_string();
let (pre, post) = match &*env::__MISE_ORIG_PATH {
Some(orig_path) => match full.split_once(&format!("{PATH_ENV_SEP}{orig_path}")) {
Some((pre, post)) if !Settings::get().activate_aggressive => (
split_paths(pre).collect_vec(),
split_paths(&format!("{orig_path}{post}")).collect_vec(),
),
_ => (vec![], split_paths(&full).collect_vec()),
},
None => (vec![], split_paths(&full).collect_vec()),
};
let new_path = join_paths(pre.iter().chain(installs.iter()).chain(post.iter()))?
.to_string_lossy()
.into_owned();
let mut ops = vec![EnvDiffOperation::Add(PATH_KEY.to_string(), new_path)];
if let Some(input) = env::DIRENV_DIFF.deref() {
match self.update_direnv_diff(input, installs, to_remove) {
Ok(Some(op)) => {
ops.push(op);
}
Err(err) => warn!("failed to update DIRENV_DIFF: {:#}", err),
_ => {}
}
}
Ok(ops)
}Maybe @zartc could give details on where mise
|
| export __MISE_ORIG_PATH="$PATH" | ||
|
|
||
| if [ -n "${{MSYSTEM:-}}" ] && command -v cygpath >/dev/null 2>&1; then | ||
| MISE_BIN="$(cygpath -au "$MISE_BIN")" |
There was a problem hiding this comment.
Cygpath is probably not necessary for $MISE_BIN. Instead, if mise emits the path with forward slashes instead of backslashes, then both git bash and busybox.exe bash can understand it. Git Bash can understand C:/Users/me/bin/foo.exe and will spawn foo.exe, for example. busybox.exe bash does not use cygpath's /c/-style paths.
Cygpath is definitely still necessary for $PATH.
| _mise_hook() {{ | ||
| local previous_exit_status=$?; | ||
| eval "$(mise hook-env{flags} -s bash)"; | ||
| eval "$("$MISE_BIN" hook-env{flags} -s bash)"; |
There was a problem hiding this comment.
Immediately after anywhere that mise sets a new $PATH, we'll need to add the $MSYSTEM conditional and call PATH="$(cygpath -up "$PATH")" Should probably store the absolute path to cygpath in a variable, since at the time it is spawned, PATH will be corrupted, and bash will be unable to find any executables.
Alternatively, this $MSYSTEM conditional could be included in the output of mise hook-env -s bash so the value is reliably converted before being assigned to $PATH.
There was a problem hiding this comment.
Additionally, to support busybox.exe bash, we don't need to call cygpath -- it doesn't exist in that environment -- but we do need to ensure all paths use forward slashes, not backslashes. I feel fairly confident mise hook-env could do this indiscriminately on Windows without issue, since Windows can handle forward slashes in the PATH. But maybe it's better to only normalize to forward-slashes for the output of -s bash, or do the conversion within the bash functions.
I was wrong; busybox can handle a mix of forward slashes and backslashes in $PATH. Mise doesn't need to do anything different for $PATH to support busybox. It still needs to use forward slashes in the hardcoded paths to EDIT: I'm wrong again! It only needs single-quotes to avoid interpreting the mise.exe, because bash syntax requires that backslashes be interpreted as escape sequences.\s as escapes. I fixed this in #7002
|
Here is how python The key is |
|
Thank you @cspotcode for your useful hints; it'll take a while till I can come back to this more involved feature implementation and this PR has been closed; I or better we'd be glad if you could go ahead with implementing your suggestions on a successor PR |
|
For what it's worth, $MSYSTEM exists in both Git Bash and MSYS2 whereas MINGW_PREFIX only in MSYS2 |
…on Windows
When mise on Windows spawns a POSIX-style shell (`bash -c`, `sh -c`, ...)
for a task, the child receives PATH in Windows form (`C:\foo;D:\bar`).
bash uses `:` as the path separator, so it fails to resolve any command —
including tools mise has just installed for the task via per-task
`tools = { ... }`. PowerShell-launched mise inherits no MSYSTEM, so the
prior `MSYSTEM`-based detection (PR jdx#5581) cannot help here.
Convert PATH to Unix form (`/c/foo:/d/bar`) at the spawn boundary in
`task_executor::exec_program`, gated on the target program being a
recognized POSIX shell name. Pure Rust, no subprocess, no `cygpath`
fallback — narrow scope per maintainer guidance from jdx#3961 / jdx#5581 /
jdx#6633. Linux/macOS and Windows-native-shell tasks are unaffected.
Closes a long-standing class of bugs around per-task `tools = { ... }` +
`shell = "bash -c"` on Windows.
…on Windows (#9547) ## Maintainer context — scope and prior decisions This PR addresses the long-standing class of bugs around mise + Windows + POSIX subshells discussed in [#3961](#3961), [#5581](#5581), [#6633](#6633). Per maintainer feedback on those threads: **Accepted:** - Pure Rust, no `cygpath` subprocess (`Command::new("cygpath")`). - Narrow scope: only the conversion mise actually needs (PATH list direction). - 80% of common cases covered; edge cases pass-through or error explicitly. **Rejected on past PRs (and avoided here):** - Subprocess to `cygpath`. - Hardcoded `MSYSTEM` env trigger (the PowerShell→mise→bash path has no `MSYSTEM`). - Full cygpath compat (mount table, UNC, reserved names, etc.). - "Land now, fix perf later" — *"We need it right the first time."* **Distinct from already-merged [#4048](#4048 that PR fixed `mise activate bash` *output* when mise itself runs inside Git Bash. This PR fixes the orthogonal case — mise spawning a POSIX subshell from any host shell. --- ## Summary - Add `crate::path::windows_path_list_to_unix` (pure-Rust `;`-separated → `:`-separated, drive-letter `C:\` → `/c/`, UNC pass-through). - Add `crate::path::is_posix_shell_program` (basename match against `bash`/`sh`/`zsh`/`fish`/`ksh`/`dash`, case-insensitive, `.exe` stripped). - In `task_executor::exec_program`, just before `CmdLineRunner::envs(...)`, call `maybe_convert_env_for_msys_shell(program, env)`. On Windows + recognized POSIX shell only, clone the env and rewrite the `PATH` entry; otherwise return `Cow::Borrowed` (zero allocation). The cfg-gated `Cow` pattern keeps the call site OS-agnostic and adds zero cost to Linux/macOS or to Windows-native-shell tasks. ## Design choices | Question | Choice | Why | |---|---|---| | Mount prefix | `/c/` only (Git Bash style) | Cygwin's `/cygdrive/c/` users typically don't go through `bash -c` from PowerShell. Configurable setting would be scope creep. | | Where to convert | `task_executor::exec_program`, env-passing site only. `src/shell/*.rs` and `hook_env.rs` untouched. | The bug is in the `mise run` path, not `mise activate` (that's PR #5581's scope). Single chokepoint, no shell-escape concerns. | | MSYS detection | Target program basename (`bash`/`sh`/`zsh`/...), **not** `MSYSTEM` env | `MSYSTEM`-trigger misses the PowerShell→mise→bash chain entirely. Program-name trigger covers shebang dispatch (`#!/usr/bin/env bash`) too. | | PowerShell→mise→bash chain | Resolved by item 3 above (target-program detection at the spawn boundary). | `task.shell()` resolves to a program path before reaching `exec_program`; we read it there. | ## Out of scope (intentional) - Cygwin `/etc/fstab` mount table. - UNC paths (`\?\C:\...`, `\server\share\...`) — passed through verbatim, bash will fail to use them, matches no-conversion behavior. - Cygwin `/cygdrive/c/` prefix. - Git Bash `/usr` magic mount — `/c/Program Files/Git/usr/bin` resolves to the same exe via PATH search, no remap needed. - `windows_path_list_to_unix` does not provide the reverse direction (`-wp`); not currently needed. ## Tests **Unit tests** (`src/path.rs` + `src/task/task_executor.rs`, all platforms): - `windows_path_list_to_unix`: basic, forward-slash input, mixed separators, Unix entry pass-through, UNC pass-through, empty entries (leading/trailing/sole `;`), drive-letter case folding, `Program Files` with spaces, bare `C:` / `C:foo` pass-through, single entry — **11 cases**. - `is_posix_shell_program`: bare names, `.exe` suffix, full Windows paths, Unix paths, `BASH.EXE` uppercase, negative cases (`cmd`, `powershell`, `pwsh`, `rustc`, empty) — **1 case with all assertions**. - `maybe_convert_env_for_msys_shell` (Windows-only): converts for `bash.exe`, skips for `cmd.exe`, full path to `bash.exe`, plus a Unix-side no-op test — **3 Windows + 1 Unix**. **Integration test** (`e2e-win/task.Tests.ps1`): a Pester case that writes a `mise.toml` with `shell = "bash -c"`, runs the task from PowerShell, and asserts the PATH the task observes contains `:` (Unix-style) rather than `;` (Windows-style). Skips gracefully if `bash.exe` isn't on PATH. ## Local verification | Check | Result | |---|---| | `cargo test -p mise --bin mise -- path::tests:: task::task_executor::tests::` | **14/14 pass** | | `cargo check --workspace --message-format=short` | exit 0, no new warnings | | `cargo fmt -p mise --check` | clean | | `cargo clippy -- -Dwarnings`, `cargo build --bin mise`, `pwsh` repro, e2e-win Pester | not run locally — Rust 1.95 + `getrandom v0.3` `raw-dylib` requires `dlltool` that the local MSVC toolchain didn't provide. Deferred to CI. | ## Acceptance criterion The repro from [#3961](#3961): ```toml [tasks.repro_build_rust] tools = { rust = "1.84.0" } shell = "bash -c" run = ''' set -euo pipefail rustup target add wasm32-wasip1 || true cargo build --release --target wasm32-wasip1 ''' ``` …run from PowerShell as `mise run repro_build_rust` should resolve `rustup` and `cargo` inside the bash subshell. Bug-source code path verified by unit tests; final repro to be confirmed by CI on this PR. --- *This PR description was authored with the assistance of an AI coding assistant.* --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: jdx <[email protected]>
Addresses #3961
Similar adaptions could be done for ZSH and other shells in MSYS2, though Git Bash is most popular