fix(task): convert PATH to /cygdrive form for Cygwin bash tasks#10147
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for Cygwin bash on Windows by detecting Cygwin paths and converting the PATH environment variable to the /cygdrive/c/... format instead of the Git Bash /c/... format. It also allows overriding the default prefix via the MISE_CYGDRIVE_PREFIX environment variable, accompanied by documentation updates and tests. The review feedback focuses on performance optimizations, specifically suggesting allocation-free implementations for path segment matching in is_cygwin_shell and prefix resolution in msys_drive_prefix_for.
Greptile SummaryThis PR fixes Cygwin PATH corruption when
Confidence Score: 5/5Safe to merge — the change is Windows-only, narrowly scoped to PATH conversion at the bash spawn boundary, and does not touch any shared non-Windows codepaths. The detection logic is well-guarded (whole-segment matching, case-insensitive), the drive_prefix plumbing is mechanically straightforward, and MISE_CYGDRIVE_PREFIX validation correctly rejects relative values. Unit tests cover the main happy paths and edge cases. The only gap is a missing assertion for the cygwin32 segment in the detection tests, which is a minor oversight with no runtime impact. No files require special attention. Important Files Changed
Reviews (5): Last reviewed commit: "fix(task): address review feedback on Cy..." | Re-trigger Greptile |
f9eafb8 to
6898c7c
Compare
PR jdx#9547 converts a Windows-form PATH (C:\foo;D:\bar) to Unix form when mise on Windows spawns a bash task, but always emitted the MSYS2 / Git Bash `/c/foo` style. Cygwin expects `/cygdrive/c/foo`, so PATH was corrupted and tools like cygpath became "command not found". Detect Cygwin bash via a `cygwin` / `cygwin64` / `cygwin32` segment in the resolved bash path and convert PATH using the `/cygdrive` prefix. A non-default cygdrive mount (configured in /etc/fstab) can be supplied via MISE_CYGDRIVE_PREFIX; mise does not parse fstab. MSYS2 / Git Bash behaviour is unchanged. Reported in discussion jdx#9950. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- is_cygwin_shell: split on both separators with eq_ignore_ascii_case (allocation-free; drops the lowercase/replace temporaries) - msys_drive_prefix_for: trim the trailing slash in place via truncate, and reject a non-absolute MISE_CYGDRIVE_PREFIX (e.g. `mnt`) with a warning, falling back to the default `/cygdrive` instead of emitting relative PATH entries that bash silently ignores - docs: mention the `cygwin32` segment and that the prefix must be absolute - tests: relative-prefix rejection and `MISE_CYGDRIVE_PREFIX=/` -> MSYS form Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6898c7c to
e05d406
Compare
|
@JamBalaya56562 A non-default Lines 242 to 246 in 310e325 |
Problem
PR #9547 added Windows→Unix PATH conversion so
miselaunched from PowerShell can spawn a POSIX shell (bash -c) for a task with a usable PATH. It unconditionally emits the MSYS2 / Git Bash/c/fooform, but Cygwin expects/cygdrive/c/foo— so Cygwin users' PATH was corrupted and binaries likecygpathbecamecommand not found.Reported in #9950. Cygwin was intentionally out of scope in #9547; this is the follow-up.
Fix
cygwin/cygwin64/cygwin32path segment in the resolved bash path. Detection runs after bash resolution, so it sees the real exe (C:\cygwin64\bin\bash.exe, or aMISE_BASH_PATHpointing there). Segment-matching avoids false positives likemy-cygwinish-tools.drive_prefixthroughwindows_path_list_to_unix:""for MSYS2 / Git Bash (/c/..., unchanged) and/cygdrivefor Cygwin (/cygdrive/c/...)./etc/fstab) can be supplied viaMISE_CYGDRIVE_PREFIX(e.g./mnt); mise does not parse fstab. A trailing/is trimmed, soMISE_CYGDRIVE_PREFIX=/collapses to the MSYS/c/...form.Pure Rust, no subprocess (no
cygpathfork). Windows-only and bash-path-based;MSYSTEMis not consulted since PowerShell-launched mise inherits none.Out of scope (intentional)
/etc/fstab— replaced by the explicitMISE_CYGDRIVE_PREFIXescape hatch.bash_candidates()— would change bash auto-selection for users who have both Git Bash and Cygwin installed. Cygwin users pick their bash viaMISE_BASH_PATHor PATH; this fix ensures PATH is formatted correctly for whichever bash is chosen.Tests
src/path.rs: Cygwin conversion (basic, forward-slash, drive-letter case, spaces, UNC / already-unix passthrough, empty, relative), custom prefix, andis_cygwin_shelldetection (true/false incl. the false-positive guard).src/task/task_executor.rs:maybe_convert_env_for_msys_shellroutes Cygwin →/cygdrive, Git Bash →/c/(unchanged), and honorsMISE_CYGDRIVE_PREFIX.e2e-win/task.Tests.ps1: skippable Cygwin e2e asserting the task PATH contains/cygdrive/.28 unit tests pass locally on
x86_64-pc-windows-msvc.CHANGELOG.mdis git-cliff-generated, so left untouched.🤖 Generated with Claude Code