More robust check for mintty when creating winpty bash aliases#664
Conversation
|
/updpkgsums The workflow run was started. |
|
In my tests, On the other hand, in my tests, |
|
Are we talking about two different questions?
|
Correct.
Originally, all regular Console programs required Some time ago, Windows started to offer Pseudo Consoles, with the intention to provide enough functionality to make that emulated Unix pseudo terminal support obsolete. Cygwin started supporting these Pseudo Consoles, at first opt-in, then later as opt-out feature. I personally experienced so many issues with Cygwin's Pseudo Console support that I held out a lot longer than Cygwin, making the Pseudo Consoles the default in Git for Windows only much much later than Cygwin. Today, it is the default, though. Which was the reason why I tried running I also tried
So far, the only program I tried that did require was Microsoft Store's Python. I do not quite understand why that is, though. Given my understanding of Pseudo Consoles, it should work. |
@jeremyd2019 thank you so much for the pointer! I also realized that this reproducer basically demonstrates the very same issue. |
|
For the record: I have submitted patches to the Cygwin-patches mailing list to fix this thing with app execution aliases and standard handles (and it was pointed out that another patch would also fix the issue, even if the commit message might fool readers into believing that it's about something completely different, and even if v2 of that patch still shares that unfortunate trait). I also opened a draft PR over at git-for-windows/msys2-runtime#122 whose build artifact can be used to verify that it fixes the issue. All this is to say: I hope that before long, we can do away with the |
|
Another case where these aliases break things: using
|
|
@cspotcode thank you for the ping! We actually integrated a new MSYS2 runtime version (v3.6.6) into Git for Windows, and this bump includes the fix I was talking about in its commit range. All this is to say: I think we can just drop the entire |
0befbe6 to
b2ccbbe
Compare
|
@dscho done! |
|
/updpkgsums The workflow run was started. |
@cspotcode thank you! Out of curiosity: did you test it with a Store app? |
|
/updpkgsums |
|
If the change in this here PR works, we probably also want to edit the known issues item of the release notes that talks about the console program problems... |
Signed-off-by: Andrew Bradley <cspotcode@gmail.com>
09a7314 to
8d81ed3
Compare
No, I didn't test it; I unfortunately don't have any bandwidth to test this change. |
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
For a long time we used the MSYS2 runtime in a in a mode where it did not use the Pseudo Consoles that are nowadays supported by Windows. For that reason, when running in MinTTY, which is the default terminal of Git Bash, we had to use aliases that ran interpreters like Python and Ruby through WinPTY. However, going with the times, we followed the MSYS2 project, which switched to an MSYS2 runtime version that defaults to Pseudo Consoles being supported and turned on. That makes these aliases to force WinPty obsolete. Git for Windows' release notes still talk about it though, in the "Known Issues" section. Let's remove that item. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
8d81ed3 to
724d013
Compare
No worries, I just tested it, and the MSYS2 runtime works fine with the Store version of Python, without having to have the |
|
/deploy git-extra The i686/x86_64 and the arm64 workflow runs were started. |
|
/add relnote feature The shell aliases in Git Bash that ensured that interpreters such as Python and Node.JS are executed via The workflow run was started |
The shell aliases in Git Bash that ensured that interpreters such as Python and Node.JS are executed via `winpty` are no longer necessary, and have therefore [been dropped](#664). Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>
winptyaliases cause some issues (see linked issue) and aren't necessary outside of mintty, so I tweaked the conditional to detect mintty and excludes Windows Terminal, VSCode terminal, and other modern windows terminals thatexport TERM=xterm-256color.git-for-windows/git#5960
Other detection techniques I considered
I considered checking
tty -sbut it returns exit code0everywhere, in both mintty and Windows Terminal. Plus, it would require access tottyon the path (usually true but maybe sometimes not?) and would mean an extra process spawn in the profile which slows down startup.I checked all env vars in both terminals, and
TERM_PROGRAMis the only one that seems viable.MSYSTEMis set in both mintty and Windows Terminal.Code review questions
Did I write the bash syntax correctly? I'm not using any bash-specific syntax that might break in unique configurations, and I'm not going to trigger errors if the shell has
set -u?