Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactive MSVC Nushell doesn't work under Cygwin Mintty with disable_pcon (e.g. Git Bash) #10808

Open
CAD97 opened this issue Oct 22, 2023 · 6 comments
Labels
🐛 bug Something isn't working line editor Issues related to reedline platform-specific platform-specific issues windows A Windows specific issue

Comments

@CAD97
Copy link
Contributor

CAD97 commented Oct 22, 2023

Describe the bug

If nu is run from the Git Bash terminal (a cygwin mintty terminal, distributed as part of Git for Windows), the interactive prompt does not function and the session is inescapable; typing ENTER moves the cursor to the next line but does not cause a response from the nu prompt, and even CTRL+C has no effect. The arrow keys can be used to move the cursor around in the screen buffer arbitrarily and edit anywhere.

How to reproduce

  1. On a Windows machine, have Nushell and Git for Windows installed. (e.g. winget install Nushell.Nushell; winget install Git.Git)
  2. Launch Git Bash and from Git Bash run nu.
  3. Observe that the behavior is not correct for a shell.
  4. The only way to exit nu (or do anything other than edit the text buffer) is to close Git Bash.

Expected behavior

I expected nu to work, or at a minimum to detect the incompatible terminal/environment and error instead of softlocking the terminal.

Screenshots

image

Configuration

key value
version 0.86.0
branch
commit_hash 5d8763e
build_os windows-x86_64
build_target x86_64-pc-windows-msvc
rust_version rustc 1.71.1 (eb26296b5 2023-08-03)
rust_channel 1.71.1-x86_64-pc-windows-msvc
cargo_version cargo 1.71.1 (7f1d04c00 2023-07-29)
build_time 2023-10-18 07:39:55 -05:00
build_rust_channel release
allocator mimalloc
features default, sqlite, trash, which, zip
installed_plugins

Additional context

cmd.exe does not seem to have this issue, but pwsh has an equivalent one. An initial guess was mintty sending \n instead of \r\n, but the behavior is far weirder than could reasonably be just that.

> git --version --build-options
git version 2.42.0.windows.2
cpu: x86_64
built from commit: 2f819d1670fff9a1818f63b6722e9959405378e3
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon

> pwsh --version
PowerShell 7.3.8
@CAD97 CAD97 added the needs-triage An issue that hasn't had any proper look label Oct 22, 2023
@ChrisDenton
Copy link
Contributor

Hm, I've not yet been able to reproduce on my local (win11) desktop. But I can when repeating your steps in a sandbox. I'm not sure what the difference is. Possibly some git setting I've used as I installed it from the GUI installer awhile back.

Btw, this is probably unrelated but the git-bash window really does not like the sandbox. It seems to mess up window switching and input focus somehow.

@ChrisDenton
Copy link
Contributor

I don't have time to investigate right now but my suspicion would be it's the difference between msys2 passing asynchronous pipes that emulate a terminal (inherited from cygwin) vs. using the built-in pseudo console that appears like a regular console. I believe git allows you to change this while installing but I'm uncertain how to do it after the fact.

If anyone wants to investigate this before I get round to it then GetConsoleMode with one of stdin/stdout/stderr is a quick way to see if the Windows console is being used. There's also some code in the standard library to detect msys2 terminal. I don't love it and false positive are possible but it should be fairly robust.

@ChrisDenton
Copy link
Contributor

Ok, because this is bugging me I did find the cause. Here's how to fix it on the user's machine:

Open C:\Program Files\Git\etc\git-bash.config, delete the line MSYS=disable_pcon and save. I've confirmed this fixes the issue. Not sure why the store version of git is setting that tbh as the msys2 default is not to.

So my suggestion would be for nushell to:

  1. on startup, detect msys2 pipes from std{in,out,err}
  2. print an error and suggest the user make the above change
  3. exit

@sholderbach sholderbach added platform-specific platform-specific issues windows A Windows specific issue labels Oct 23, 2023
@CAD97 CAD97 changed the title Interactive MSVC Nushell in a Git Bash terminal (mintty, cygwin) is inescapable Interactive MSVC Nushell doesn't work under Cygwin Mintty with disable_pcon (e.g. Git Bash) Oct 24, 2023
@CAD97
Copy link
Contributor Author

CAD97 commented Oct 24, 2023

Potential env to sniff to determine terminal context:

> # in wt
> let env_wt = (nu -c "$env | to json" | str replace -a "\u{1b}" '(char esc)'  | from json)
> # Git Bash: nu -c "\$env | to json" | clip
> let env_git = (pwsh -c `Get-Clipboard` | str replace -a "\u{1b}" '(char esc)' | from json)
> $env_wt | transpose key val | filter { ($in.key | str upcase) not-in ($env_git | transpose key val | get key | str upcase) }
value
key val
CMD_DURATION_MS 0
WSLENV WT_SESSION:WT_PROFILE_ID:
WT_PROFILE_ID {aab79973-318f-43b6-a9bc-b4096493753f}
WT_SESSION f56383d8-ee2d-4c93-8b4e-5f42369267ac
> $env_git | transpose key val | filter { ($in.key | str upcase) not-in ($env_wt | transpose key val | get key | str upcase) }
value
key val
ACLOCAL_PATH [str length 81]
CONFIG_SITE C:/Program Files/Git/etc/config.site
DISPLAY needs-to-be-defined
EFC_9704 1
EXEPATH C:\Program Files\Git
HOME C:\Users\CAD
HOSTNAME CAD-Desktop
INFOPATH [str length 213]
LC_CTYPE en_US.UTF-8
MANPATH [str length 207]
MINGW_CHOST x86_64-w64-mingw32
MINGW_PACKAGE_PREFIX mingw-w64-x86_64
MINGW_PREFIX C:/Program Files/Git/mingw64
MSYS disable_pcon
MSYSTEM MINGW64
MSYSTEM_CARCH x86_64
MSYSTEM_CHOST x86_64-w64-mingw32
MSYSTEM_PREFIX C:/Program Files/Git/mingw64
ORIGINAL_PATH [str length 1244]
ORIGINAL_TEMP C:/Users/CAD/AppData/Local/Temp
ORIGINAL_TMP C:/Users/CAD/AppData/Local/Temp
PKG_CONFIG_PATH [str length 87]
PKG_CONFIG_SYSTEM_INCLUDE_PATH C:/Program Files/Git/mingw64/include
PKG_CONFIG_SYSTEM_LIBRARY_PATH C:/Program Files/Git/mingw64/lib
PLINK_PROTOCOL ssh
PS1 [str length 125]
SHELL C:\Program Files\Git\usr\bin\bash.exe
SHLVL 1
SSH_ASKPASS C:/Program Files/Git/mingw64/bin/git-askpass.exe
TERM xterm
TERM_PROGRAM mintty
TERM_PROGRAM_VERSION 3.6.4
TMPDIR C:\Users\CAD\AppData\Local\Temp
_ C:/Program Files/nu/bin/nu

So it looks like a reasonably precise test for "mintty without pseudo console" could be

$env.TERM_PROGRAM == mintty and ($env.MSYS? =~ \bdisable_pcon\b or $env.CYGWIN? =~ \bdisable_pcon\b)

I don't really know anything about pty / conPTY support. I think is_terminal sniffing for (and returning true for) non-console pty is probably correct/desired behavior, but there should probably also be a std::os::windows::io::IsConsole which doesn't. Then nu's interactive startup would be roughly

if !stdout.is_terminal() {
    eprintln!("refusing to run interactive shell outside terminal");
    exit(1);
}
#[cfg(windows)]
if !stdout.is_console() {
    eprintln!("It looks like you're running in a mingw environment without conPTY support.");
    if let Ok(mingw_prefix) = env::var("MINGW_PREFIX") && mingw_prefix.contains("Git") {
        eprintln!("Git Bash sets MSYS=disable_pcon by default, disabling conPTY; consider not doing that.");
    }
    exit(1);
}

...assuming that when conPTY support is used, is_console() would return true. Which I think it would? I honestly have no clue how any of this actually fits together.

@ChrisDenton
Copy link
Contributor

ChrisDenton commented Oct 25, 2023

I think is_terminal sniffing for (and returning true for) non-console pty is probably correct/desired behavior

Yes, this was considered a requirement for bringing it into Rust's std, otherwise popular crates wouldn't use it. Most people just want to roughly know, in the absence of arguments, a) should I use colour and b) is this interactive. Tbh, I'd love to eventually remove the test for msys/cygwin from std once conPTY becomes widespread enough because it's not great and it's very specific to two terminal emulators (albeit popular ones). But we're not there yet.

but there should probably also be a std::os::windows::io::IsConsole which doesn't

We do have this internally but exposing platform specific stuff always comes with the question "why isn't this just a crate". And in this particular case it does also have the issue of what happens if we do remove the msys check from is_terminal. We'd be left with two functions that do that same thing. Still, this is probably a conversation for a library ACP.

For what it's worth, is_console is at least fairly easy to implement:

let mut mode = 0;
let is_console = GetConsoleMode(handle, &mut mode) != 0;

Of course a third option would be for nushell to support msys/cygwin's pipes. But that doesn't solve the immediate problem and it probably requires dependencies to be updated to support it, which is a whole ecosystem problem. It's also maybe not possible if msys is missing support for some console thing nushell requires. Idk the details here.

@sholderbach
Copy link
Member

sholderbach commented Nov 14, 2023

I was looking into the crossterm source how it tries to do its Windows terminal capability sniffing (for #10998 / #11045 I stumbeled accross the same weirdness):

As a proxy they seem to look for the presence of $TERM as git bash does in its mintty. If not it proceeds using the separate Windows APIs (no-op for some crossterm Commands).
https://github.com/crossterm-rs/crossterm/blob/08762b3ef4519e7f834453bf91e3fe36f4c63fe7/src/ansi_support.rs#L29-L46

A related issue on their side specifically for the Windows Git bash:
crossterm-rs/crossterm#768

@sholderbach sholderbach added 🐛 bug Something isn't working line editor Issues related to reedline and removed needs-triage An issue that hasn't had any proper look labels Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working line editor Issues related to reedline platform-specific platform-specific issues windows A Windows specific issue
Projects
None yet
Development

No branches or pull requests

3 participants