fix(task): honor MISE_CYGDRIVE_PREFIX for Git Bash, not only Cygwin#10190
Conversation
The cygdrive automount root is configurable via /etc/fstab in MSYS2 / Git Bash too, not just Cygwin (Git Bash is a Cygwin derivative). The previous msys_drive_prefix_for returned an empty prefix immediately for any non-Cygwin bash and never read MISE_CYGDRIVE_PREFIX, so a Git Bash user with a custom automount root got a corrupted /c/... PATH with no escape hatch. is_cygwin_shell now only selects the default prefix when no override is set (empty for Git Bash/MSYS2, /cygdrive for Cygwin); MISE_CYGDRIVE_PREFIX is honored for both shells. Follow-up to jdx#10147 (review feedback). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR enhances Windows PATH conversion for POSIX shells by making cygdrive prefix detection shell-aware. The ChangesWindows PATH Conversion for POSIX Shells
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge — the change is a targeted, backwards-compatible extension of existing Windows PATH conversion logic with no behaviour change for current users. The old early-return for non-Cygwin shells is cleanly replaced by a shell-aware default, No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "test(task): clarify per-shell fallback c..." | Re-trigger Greptile |
Address Greptile P2: the comment in test_maybe_convert_env_for_msys_shell_rejects_relative_cygdrive_prefix described an unconditional fall back to `/cygdrive`, which is only true for the Cygwin binary the test exercises. Git Bash falls back to an empty prefix (the `/c/...` form). Reworded to reflect the per-shell default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Good catch — fixed. The comment now states that the fallback is the shell's default: |
Problem
PR #10147 added Cygwin support to the Windows→Unix PATH conversion, with
MISE_CYGDRIVE_PREFIXas an escape hatch for a non-defaultcygdriveautomount root configured in/etc/fstab.As pointed out in review feedback (thanks @lewis-yeung), the
cygdriveautomount mechanism is also present in MSYS2 / Git Bash (a Cygwin derivative), where the mount root is likewise configurable in/etc/fstab(default/, i.e./c/...).msys_drive_prefix_forreturned an empty prefix immediately for any non-Cygwin bash and never readMISE_CYGDRIVE_PREFIX, so a Git Bash user with a custom automount root got a corrupted/c/...PATH with no way to fix it — the same class of bug #10147 set out to address, still present for Git Bash.Fix
is_cygwin_shellnow only selects the default prefix when no override is set: empty for Git Bash / MSYS2 (/c/...),/cygdrivefor Cygwin (/cygdrive/c/...).MISE_CYGDRIVE_PREFIXis now honored for both shells. The relative-value warning reports the shell-appropriate default.No behavior change for existing users: Git Bash without an override still emits
/c/..., Cygwin without an override still emits/cygdrive/c/.... The only new behavior is that a Git Bash user can now set a custom prefix.Tests
test_maybe_convert_env_for_msys_shell_honors_cygdrive_prefix_for_git_bash(Git Bash +MISE_CYGDRIVE_PREFIX=/mnt→/mnt/c/...)./c/...) and Cygwin (/cygdrive/...) tests still guard the defaults.path::unit tests pass; the#[cfg(windows)]prefix logic was exercised standalone across all input combinations.Follow-up to #10147.
This PR was generated by an AI coding assistant.
Summary by CodeRabbit
Documentation
Bug Fixes