Repository navigation
Conversation
Inside Git Bash, git.exe is mingw64\bin\git.exe, so looking two levels up for bin\bash.exe misses and the shell tool falls back to PowerShell.
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Looks right. In a simulated Git for Windows layout on Linux (the #57310 script, with process.platform forced to win32), gitbash() returns undefined on the base when git comes from Git/mingw64/bin or MSYS2 ucrt64/bin. With this PR it returns Git/bin/bash.exe and msys64/usr/bin/bash.exe. Git/cmd still works. Core shell tests and typecheck pass. The new tests only run on Windows, so on Linux they check nothing.
Public API unchanged.
One behaviour concern: when no bash.exe is found, the failure isn't cached, so git --exec-path runs again on every lookup (see inline). Small test cleanup too, also inline.
Related: #54104
| const exec = gitExecPath(git) | ||
| if (exec) file = bashNear(exec) | ||
| } | ||
| if (!file) return |
There was a problem hiding this comment.
Only a successful lookup goes into bashForGit. When git is a real .exe but no bash.exe is found near it, every gitbash() call runs Bun.spawnSync([git, "--exec-path"]) again. That call blocks. gitbash() runs from both win() and full(), so one resolve() or list() can start git several times, and it happens again on every later call. Could you also remember the "not found" result per git path, or cache the --exec-path output?
There was a problem hiding this comment.
Cached. A miss is stored on that git path, so later lookups return without spawning git --exec-path.
| expect(path.normalize(ShellSelect.gitbash() ?? "")).toBe(path.normalize(path.join(msys, "usr/bin/bash.exe"))) | ||
| }) | ||
|
|
||
| test("finds the installed Git Bash when mingw64/bin is first on PATH", () => { |
There was a problem hiding this comment.
This test depends on whether C:\Program Files\Git exists on the machine running it, and it passes silently when it doesn't. The fake-layout test above already covers the fix. Please drop this one, and the explicit gitbash path test (behaviour this PR doesn't change), so the test file stays small.
There was a problem hiding this comment.
Removed both. The fake Git for Windows and MSYS2 layouts are the ones that stay.
A missed lookup was spawning git --exec-path on every call. Cache that miss, and drop the tests that depend on a local Git install or on unchanged behavior.
Issue for this PR
Closes #57310
Type of change
What does this PR do?
gitbash()assumedgit.exelived inGit\cmdand looked two directories up forbin\bash.exe. Inside Git Bash,gitisGit\mingw64\bin\git.exe, so that path misses and the shell tool falls back to PowerShell.It now walks up from the
gitthat is actually on PATH and checksbin\bash.exeandusr\bin\bash.exe. That coversGit\cmd,Git\mingw64\bin, and MSYS2'sucrt64\bin. If bash is not next to that git, it asksgit --exec-pathonce and walks from there. An explicitgitbashconfig path is unchanged.How did you verify your code works?
From
packages/core:bun test test/shell-select.test.ts— 4 passed, including a fakeGit\cmd,Git\mingw64\bin, and MSYS2ucrt64layout, plus this machine's real install withGit\mingw64\binfirst on PATH (gitbash()returnedC:\Program Files\Git\bin\bash.exe)bun test test/shell.test.ts— 10 passedScreenshots / recordings
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #54104