fix(windows): treat powershell as not-Unix-like in Windows #21175

akriese · 2022-11-24T12:22:31Z

This takes part of vim patch v8.2.3071 to make pwsh and powershell return the same exepath()s (including extensions) as cmd.exe.

justinmk · 2022-12-09T17:38:37Z

cc @3N4N

src/nvim/os/fs.c

3N4N · 2022-12-12T17:01:46Z

Do we not want extensionless files to be found with :h exepath()?

After this patch, the binary files 'text.exe', 'test.bat', 'test.com' will be found, but the file 'test' (without any extension) will not be found. But both where and Get-Command find the extensionless 'test' file.

But neovim with ':set shell=cmd', doesn't find 'test' either (without applying this patch), even though where test returns correct path in cmd. So I don't know what should be the expected behavior.

Edit: where is a program to find files in general, not just executable files. And Get-Command returning extensionless (and thus non-executable) file is puzzling even to Chris Dent, the resident PowerShell expert (I asked him on Discord). So I say exepath() should not find extensionless files, even if it doesn't coincide with what where and Get-Command do.

akriese · 2022-12-20T18:09:35Z

So, as I understand it, if exepath() is supposed to only return executables and Windows does not define extension-less files as executable, this would be the desired behavior, right? Even if the result diverges from Get-Command...

3N4N · 2022-12-20T20:17:49Z

this would be the desired behavior, right?

Right. As it stands now, exepath in this PR behaves correctly.

musjj · 2022-12-28T19:36:06Z

Would it be a good idea if bash.exe (MSYS2, Git Bash, etc.) also receive the same kind of treatment?
My use-case is that I'm using exepath in order to find *.bat/*.cmd files, so that I can pass it to vim.loop.spawn. But with shell set to bash.exe, the correct path isn't returned.
Actually, why does exepath depend on your shell option in the first place? Why not just query the user's OS?

justinmk · 2022-12-30T19:46:43Z

But with shell set to bash.exe, the correct path isn't returned.
Actually, why does exepath depend on your shell option in the first place? Why not just query the user's OS?

in Git bash on windows, files without ".exe" suffix can be executable, right?

After this patch, the binary files 'text.exe', 'test.bat', 'test.com' will be found, but the file 'test' (without any extension) will not be found. But both where and Get-Command find the extensionless 'test' file.

But neovim with ':set shell=cmd', doesn't find 'test' either (without applying this patch), even though where test returns correct path in cmd. So I don't know what should be the expected behavior.

Edit: where is a program to find files in general, not just executable files. And Get-Command returning extensionless (and thus non-executable) file is puzzling even to Chris Dent, the resident PowerShell expert (I asked him on Discord). So I say exepath() should not find extensionless files, even if it doesn't coincide with what where and Get-Command do.

Great info, thanks! So what is the 3-line conclusion here? Please include it in the commit message.

musjj · 2022-12-31T12:19:38Z

In Git bash on windows, files without ".exe" suffix can be executable, right?

Not completely sure of the specs, but I think only script (bash, sh, etc.) executables can have no extension in Git Bash. Binary executables require .exe just like usual.

akriese · 2023-01-09T15:48:31Z

Oh, yeah I misunderstood that 😅

src/nvim/os/fs.c

justinmk · 2023-01-16T08:52:28Z

ported vim-patch 8.2.3071 and related patches to avoid needing to revisit this in the future. 8.2.3071 is mostly N/A because all the magic special-case bullshit (changing slash behavior depending on shell, changing option defaults depending on x, y, z and bloating the docs) is not something we want.

I was hoping 8.2.3071 added test coverage for what this PR fixes, but doesn't look like it.

We need a test case in test/functional/vimscript/exepath_spec.lua .

zeertzjq · 2023-01-16T08:58:04Z

Some other related patches: #19313

PROBLEM: exepath("test") should prefer ".bat" when shell=powershell. Current behavior differs from Vim 8.2.3071. TEST CASE: 1. in a folder which is in $PATH, create files "test" "test.bat". - "(Get-Command test).Path test" returns "test.bat". 2. compare nvim: nvim --clean :set shell=powershell :echo exepath("test") 3. should returns "path\to\test.bat" (before this patch it returns "path\to\test"). SOLUTION: After this patch, the binary files "text.exe", "test.bat", "test.com" will be found, but the file "test" (without any extension) will not be found (matches Vim 8.2.3071). But powershell's `where` and `Get-Command` _do_ find the extensionless 'test' file. But Nvim with ":set shell=cmd.exe", doesn't find "test" either (before and after this patch), even though `where test` returns correct path in cmd. - `where` is a program to find files in general, not just executable files. -`Get-Command` returning extensionless (and thus non-executable) file is puzzling even to Chris Dent, [PowerShell expert][1] (asked on Discord). [1]: https://www.amazon.com/Mastering-PowerShell-Scripting-Automate-environment-ebook/dp/B0971MG88X Co-authored-by: Enan Ajmain <3nan.ajmain@gmail.com> Helped-by: erw7 <erw7.github@gmail.com> Fixes neovim#21045

… out Problem: Testing the shell option is incomplete and spread out. Solution: Move shell tests to one file and increase coverage. (Yegappan Lakshmanan, closes vim/vim#8464) vim/vim@054794c Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>

Problem: Not enough testing for shell use. Solution: Add a bit more testing. (Yegappan Lakshmanan, closes vim/vim#8469) vim/vim@ffec6dd Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>

(Most of this patch is intentionally dropped, it adds far too much special-cases for almost zero purpose: ":help shell-powershell" makes it easy to choose powershell without spreading special-cases throughout the codebase, randomly changing slash behavior, etc.) Problem: Shell options are not set properly for PowerShell. Solution: Use better option defaults. (Mike Willams, closes vim/vim#8459) vim/vim@1279502 Co-authored-by: Mike Williams <mikew@globalgraphics.com>

'shellxquote' Nvim default was adjusted in: 131aad9 The use of "/s" is different than Vim, and may avoid the need for `shellxquote="&|<>()@^`. For the other shells, Nvim intentionally does not fiddle with the various "shell*" options if 'shell' is set by the user: if the user sets 'shell', they are expected to set other "shell*" options correctly.

justinmk · 2023-01-16T10:29:41Z

We need a test case in test/functional/vimscript/exepath_spec.lua .

Merged to unblock, but it would still be helpful to have this test @akriese

windows shells This test covers the changes from neovim#21175 where exepath() is set to prefer file extensions in powershell.exe aswell as in cmd.exe. In both shells, the file with a valid extension should be returned instead of the extensionless file.

akriese · 2023-01-19T16:45:16Z

I just created this test. First time writing a test for neovim, so forgive me, if I did something wrong. Unfortunately, I don't know how to run the test on Windows with is_os('win') evaluating to true. I am running with mingw32-make functionaltest and it fails because it thinks, that I am on a not-Windows machine.

Edit: Also, the test for set shell=pwsh.exe will probably fail, if that version of Powershell is not installed on the system (don't know if it is installed by default)

3N4N · 2023-01-19T16:57:24Z

I am running with mingw32-make functionaltest and it fails because it thinks, that I am on a not-Windows machine.

I suggest using cmake for everything. Neovim is a cmake project; the makefile provided with it is only for convenience, and I don't think it's catered too much to Windows devs. What I do is use something like these:

PS> cmake -E env DEPS_BUILD_DIR=E:/projects/neovim/deps cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=C:/apps/nvim -DCMAKE_C_COMPILER=clang -DCMAKE_C_FLAGS=-w -B .\build\debug

PS> cmake --build .\build\debug

PS> cmake -E env TEST_FILE=.\test\functional\path\to\your\test\file.lua cmake --build .\build\debug --target functionaltest

I'm not gonna explain what's happening in the commands above. You should be able to figure out with a little bit of time.

Edit: Also, the test for set shell=pwsh.exe will probably fail, if that version of Powershell is not installed on the system (don't know if it is installed by default)

There is a helper function in the test suite called set_shell_powershell. Grep and see how it's used.

3N4N · 2023-01-19T16:59:27Z

Also, @akriese you should do this in a new PR.

Windows shells This test covers the changes from neovim#21175 where exepath() is set to prefer file extensions in powershell.exe aswell as in cmd.exe. In both shells, the file with a valid extension should be returned instead of the extensionless file.

justinmk · 2023-01-23T11:02:27Z

What I do is use something like these:

PS> cmake -E env DEPS_BUILD_DIR=E:/projects/neovim/deps cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=C:/apps/nvim -DCMAKE_C_COMPILER=clang -DCMAKE_C_FLAGS=-w -B .\build\debug

PS> cmake --build .\build\debug

PS> cmake -E env TEST_FILE=.\test\functional\path\to\your\test\file.lua cmake --build .\build\debug --target functionaltest

see also https://github.com/neovim/neovim/wiki/Building-Neovim#windows--msvc-powershell (@3N4N any improvements to make there?)

akriese · 2023-01-23T13:19:15Z

see also https://github.com/neovim/neovim/wiki/Building-Neovim#windows--msvc-powershell (@3N4N any improvements to make there?)

I used the instructions in the README aswell as @3N4N's commands. Didn't work out of the box, maybe my environment has some issue. It eventually worked out for me when explicitly passing the LUAJIT_LIB path to the cmake command (my build script)

Windows shells This test covers the changes from neovim#21175 where exepath() is set to prefer file extensions in powershell.exe aswell as in cmd.exe. In both shells, the file with a valid extension should be returned instead of the extensionless file.

test(exepath): test if exepath returns correct path with multiple Windows shells This test covers the changes from #21175 where exepath() is set to prefer file extensions in powershell.exe aswell as in cmd.exe. In both shells, the file with a valid extension should be returned instead of the extensionless file.

…21928 test(exepath): test if exepath returns correct path with multiple Windows shells This test covers the changes from neovim#21175 where exepath() is set to prefer file extensions in powershell.exe aswell as in cmd.exe. In both shells, the file with a valid extension should be returned instead of the extensionless file.

akriese force-pushed the exepath_pwsh_fix branch 2 times, most recently from bb65cc7 to 49f9cb1 Compare November 24, 2022 14:00

akriese changed the title ~~treat powershell as not-Unix-like in Windows~~ fix(windows): treat powershell as not-Unix-like in Windows Nov 24, 2022

akriese force-pushed the exepath_pwsh_fix branch from 49f9cb1 to 25bba25 Compare December 9, 2022 15:54

3N4N reviewed Dec 10, 2022

View reviewed changes

src/nvim/os/fs.c Outdated Show resolved Hide resolved

williamboman mentioned this pull request Dec 21, 2022

NPM can't be found. mason-org/mason.nvim#781

Closed

1 task

akriese force-pushed the exepath_pwsh_fix branch from 8c55d9b to a7d6602 Compare December 21, 2022 14:32

akriese mentioned this pull request Dec 24, 2022

[Bug]: Windows related issue starting flutter nvim-flutter/flutter-tools.nvim#187

Closed

This comment was marked as resolved.

Sign in to view

zeertzjq added the platform:windows label Jan 3, 2023

akriese force-pushed the exepath_pwsh_fix branch from a7d6602 to 6f9c138 Compare January 4, 2023 17:42

This comment was marked as off-topic.

Sign in to view

justinmk force-pushed the exepath_pwsh_fix branch from 6f9c138 to 5f927aa Compare January 16, 2023 08:08

justinmk reviewed Jan 16, 2023

View reviewed changes

src/nvim/os/fs.c Show resolved Hide resolved

justinmk force-pushed the exepath_pwsh_fix branch from c199a69 to 5faec60 Compare January 16, 2023 09:57

akriese and others added 5 commits January 16, 2023 11:26

vim-patch:8.2.3070: not enough testing for shell use

26d6e27

Problem: Not enough testing for shell use. Solution: Add a bit more testing. (Yegappan Lakshmanan, closes vim/vim#8469) vim/vim@ffec6dd Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>

justinmk force-pushed the exepath_pwsh_fix branch from 5faec60 to e5b9485 Compare January 16, 2023 10:27

justinmk merged commit 2773685 into neovim:master Jan 16, 2023

akriese mentioned this pull request Jan 20, 2023

test(exepath): test if exepath returns correct path with cmd and powershell #21928

Merged

jose-elias-alvarez mentioned this pull request Jan 23, 2023

fix(command_resolver): fix resolve executable on Windows system jose-elias-alvarez/null-ls.nvim#1341

Open

musjj mentioned this pull request Feb 4, 2023

Npm (installed through Volta) can't be found mason-org/mason.nvim#879

Open

1 task

williamboman mentioned this pull request Feb 19, 2025

fix(win32): resolve executable paths on windows mason-org/mason.nvim#1882

Closed

Uh oh!

fix(windows): treat powershell as not-Unix-like in Windows #21175

fix(windows): treat powershell as not-Unix-like in Windows #21175

Uh oh!

Conversation

akriese commented Nov 24, 2022

Uh oh!

justinmk commented Dec 9, 2022

Uh oh!

Uh oh!

3N4N commented Dec 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akriese commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

3N4N commented Dec 20, 2022

Uh oh!

musjj commented Dec 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmk commented Dec 30, 2022

Uh oh!

musjj commented Dec 31, 2022

Uh oh!

This comment was marked as resolved.

This comment was marked as off-topic.

akriese commented Jan 9, 2023

Uh oh!

Uh oh!

justinmk commented Jan 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeertzjq commented Jan 16, 2023

Uh oh!

justinmk commented Jan 16, 2023

Uh oh!

akriese commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

3N4N commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

3N4N commented Jan 19, 2023

Uh oh!

justinmk commented Jan 23, 2023

Uh oh!

akriese commented Jan 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

3N4N commented Dec 12, 2022 •

edited

Loading

akriese commented Dec 20, 2022 •

edited

Loading

musjj commented Dec 28, 2022 •

edited

Loading

justinmk commented Jan 16, 2023 •

edited

Loading

akriese commented Jan 19, 2023 •

edited

Loading

3N4N commented Jan 19, 2023 •

edited

Loading

akriese commented Jan 23, 2023 •

edited

Loading