Fixing 'shell' on Windows, properly

[LunarLambda]

Problem

See #16957 for some of the existing issues with using a Unix shell in Neovim on Windows.

I've dug into it now, seeing as no workaround really managed to fully take, and here's what I was able to find.

The root of evil is the fact that CreateProcessA takes some long string as the command line. The Microsoft C runtime takes that string, runs it through splitting logic described here, then passes the processed result to main. The result of that processing is what 'shell' ultimately receives.

Vim on the other hand, wants to be able to think about running programs in terms of regular C argv arrays.

Assuming shell=bash, shellcmdflag=-c and shellxquote=", running !ls -l, produces the argument vector [bash, -c, "ls -l"] (note the quotes around ls -l). The resulting argument vector is then passed to libuv, which then reconstitutes the entire thing back to a single string so it can pass it to CreateProcessA.

This is where the issue arises. libuv expects an unquoted argument vector ([bash, -c, ls -l], no quotes), as you would pass to execve on Unix, but gets [bash, -c, "ls -l"] instead. This causes it to quote the quoted string on Windows, which ultimately causes bash to receive the command "ls -l" (one shell word, with quotes) instead of ls -l (two shell words, no quotes).

So, basically, there are two places in the chain that are trying to make the argument vector "windows-friendly": Vim's shell_build_argv, and libuv's make_program_args. Only one of the two should be done. To only use vim's behaviour, it is probably sufficient to change libuv_process_spawn to always pass UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS. Since cmd.exe already has to use this behaviour since its argument parsing does not follow that of the C runtime (i.e. all other programs), this seems like it will cause the least amount of breakage.

To only follow libuv's behaviour (an arguably more sound strategy but that may not work with cmd.exe), we'd simply need to skip shell_xescape_xquote (or set both shellxescape and shellxquote to the empty string?).

Whichever the case may be, it's pretty clear that Neovim's current behaviour on Windows is incorrect. Double quoting like this shouldn't occur, and having two completely different implementations both doing argv-sanitation depending on context is bound to lead to issues. (As it did very recently for Rust)

There's also some other issues. For example, Neovim tries to pass 2>&1 | tee <tempfile> to the shell. However, the Windows-style backslash-using tempfile path will cause the shell to misinterpret something like C:\somedir\tmp\ to contain a \t (tab) (at least in zsh) and cause it to spit out the file in the wrong place, causing errors in :make, :grep, etc. Easiest solution would be to change the path to use / as the separator (possibly in accordance with 'shellslash', which doesn't apply to the temporary file arguments currently) (though this will also not work for UNC paths), or to use something like cygpath if available (most unix shells on Windows are cygwin/msys2 based).

Steps to reproduce

nvim --clean
:set shell=C:/path/to/bash.exe shellcmdflag=-c
:!echo test <- fails
:set shellxquote=
:!echo test <- works
(and many other such cases)

Expected behavior

We should figure out a way to fix this properly, once and for all.

Neovim version (nvim -v)

master

Vim (not Nvim) behaves the same?

Have not verified, but it seems to be Neovim specific

Operating system/version

Windows 10/11

Terminal name/version

wezterm, but doesn't matter (not TUI related)

$TERM environment variable

xterm-256color

Installation

GitHub nightly releases

[clason]

For the record, being more restrictive (and explicit) about which "shells" we support on Windows is an option here (though maybe not for 0.10).

[LunarLambda]

In the sense of saying "we only support cmd.exe and powershell", or "we eventually stop supporting cmd.exe"?

I use Neovim on Windows exclusively through an MSYS2 environment, and while I understand it's not an officially supported use case/"platform", I would really like for unix shells to be supported and working. Lots of people use Git Bash on Windows for example.

I think, generally speaking:

For non-cmd.exe, we should completely ignore shellquote/shellxquote/shellxescape/etc, not do any quoting/escaping of our own, and instead let libuv handle it for us. This will work correctly with any program that uses the regular C runtime argument handling (including, I believe, powershell/pwsh).

For cmd.exe, we could continue using the existing code paths and tell libuv to pass the arguments verbatim as we do now. libuv does not handle cmd.exe specially in any way, so this seems reasonable if it's important that the edge cases are handled proper.

I could imagine a future where Neovim simply doesn't support cmd.exe as a shell anymore, and defaults to powershell. I can't think of anything that would definitely break, since users are already free to change the shell settings away from cmd.exe, so nothing inside of Neovim should really have a hard dependency on it, but I still imagine it may cause some churn.

Either way, that decision should not stand in the way of fixing the existing use cases today.

[clason]

I could imagine a future where Neovim simply doesn't support cmd.exe as a shell anymore, and defaults to powershell.

That's what I was thinking, yes, if it allows things to work better on other platforms (including win shells) and the alternatives are reasonable.

In any case, it would be helpful to gather some data on 1. which shells/platforms are actually used on Windows and 2. the issues each have (what works, what doesn't, what requires jumping through hoops).

[justinmk]

Nice analysis!

"we eventually stop supporting cmd.exe"

Seems like a separate topic. Based on this thread, I don't see a reason to drop support for a particular shell?

So, basically, there are two places in the chain that are trying to make the argument vector "windows-friendly": Vim's shell_build_argv, and libuv's make_program_args. Only one of the two should be done. To only use vim's behaviour, it is probably sufficient to change libuv_process_spawn to always pass UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS. Since cmd.exe already has to use this behaviour since its argument parsing does not follow that of the C runtime (i.e. all other programs), this seems like it will cause the least amount of breakage

Based on the git logs, I don't see a rationale for why f3cc843 special-cased "cmd.exe", I assume it was just conservative.

For non-cmd.exe, we should completely ignore shellquote/shellxquote/shellxescape/etc, not do any quoting/escaping of our own, and instead let libuv handle it for us.

Sure, let's try it. Existing tests are here:

neovim/test/functional/vimscript/system_spec.lua

Line 130 in 8e5c48b

describe('executes shell function', function()

[LunarLambda]

One thing I'd definitely like to fix is this

:make
:!cargo  2>&1| tee F:\msys2\tmp\nvim.0\cn0m97\2
cargo  2>&1| tee F:\msys2       mp
vim.0error
...
E40: Can't open errorfile F:\msys2\tmp\nvim.0\cn0m97\2

The weird printing seems to be zsh specific, but even with bash it still doesn't write the correct file, so quickfix is broken.

I think the temporary file path should be obeying 'shellslash':

When set, a forward slash is used when expanding file names. This is
useful when a Unix-like shell is used instead of cmd.exe. Backward
slashes can still be typed, but they are changed to forward slashes by

I'm not sure how involved of a fix that is, but seeing as Neovim doesn't currently use UNC paths, I don't see a big issue.

[clason]

I think the temporary file path should be obeying 'shellslash':

Or 'shellslash' should be removed and forward slashes be used unconditionally. The point here is that we are fine making breaking changes for legacy systems that are no longer officially supported, if that helps fixing bugs on supported systems.

[LunarLambda]

That would be a more radical if for my purposes equally workable solution.

I am slightly worried about breaking the ability to use UNC Paths, which are used for referring to files on networked devices without assigning them a drive letter (so it could be worked around within limits). After testing, it seems one can write UNC paths with forward slashes too!

The \\?\/\\.\ format for paths (which mandates backslashes) used to be the only way to get around the MAX_PATH limitations on Windows, but as of Windows 10 1607 this is no longer the case. Windows 10 1607 has been end of life since April 2019, so all currently officially supported versions of Windows support the registry setting.

There are still weird edge cases that use the "DOS device" path format, like named pipes. I don't know that we care about this, but for the sake of completeness I'm mentioning it.

[clason]

(Again, this is just opening a door to more options for a solution to these issues, not a push to do that.)

Note we claim support for Windows >= 8, but that, too, can be revisited. We need to distinguish platform tiers (:h supported) by version anyway.

[justinmk]

We use named pipes. Does it not support forward slashes? In any case I'm not sure we need to care, because we don't treat named pipes as filesystem paths.

[dundargoc]

Note we claim support for Windows >= 8, but that, too, can be revisited.

I will one-up you and claim we already don't support anything older than Windows 10 "Version 1809" as the :terminal feature does not work for those systems. It depends on how one interprets what "support" means, but I personally interpret it to mean ALL editor features are supported, rather than that some parts happen to work.

One might argue that this quasi-bump in minimum version was accidental and that it should therefore count more as a bug/accident than intentional. I personally think this is a bit weak stance as documenting this problem is already an admission from our side that we don't see this being resolved soon.

I propose we simply bite the bullet and officially only support Windows 10 "Version 1809" to simplify the question on which windows version we support (which is brought up surprisingly frequently).

[clason]

It depends on how one interprets what "support" means, but I personally interpret it to mean ALL editor features are supported, rather than that some parts happen to work.

Yes. The current wording implies that ":terminal" is not covered by "support". It's a slippery slope I would like to get off of. It strikes me as weird that we support significantly more versions than our core dependencies (libuv; would have to check tree-sitter).

[LunarLambda]

Linking #28395 since the topic of slash handling got brought up here.

Treating this as a sort of "tracking issue" while the details of what we want to change/fix aren't fully decided yet.

If/when that PR lands, I would try and take stock of what works/doesn't on Windows with various shells and try to implement the "let's have libuv alone handle non-cmd shells" strategy, unless there are strong objections or other things to try first.