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

Cannot create temp file for executing external commands over a range when shell is set to powershell #15913

Closed
3N4N opened this issue Oct 5, 2021 · 10 comments · Fixed by #16271 or #19438
Labels
environment user system environment (terminal, shell, tmux) platform:windows
Milestone

Comments

@3N4N
Copy link
Contributor

3N4N commented Oct 5, 2021

Neovim version (nvim -v)

NVIM v0.5.1

Vim (not Nvim) behaves the same?

Can't say. Do not have vim installed.

Operating system/version

Windows 10 21H1

Terminal name/version

Windows Terminal, Conhost

$TERM environment variable

Nil

Installation

Put the zip folder from release page and added the location in Windows path variable

How to reproduce the issue

nvim -u NONE
:let &shell = 'pwsh'
:let &shellcmdflag = '-NoLogo -NoProfile -ExecutionPolicy RemoteSigned -Command [Console]::InputEncoding=[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;'
:let &shellredir = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
:let &shellpipe = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
:set shellquote= shellxquote=
:e random-nonempty.file
:1,5!sort

Expected behavior

The lines from 1 to 5 should be sorted with the external sort command (which exists in my path).

Actual behavior

I get the following error.

E485: Can't read file C:\Users\ACER\AppData\Local\Temp\nvim6UJx04\7

I checked with Process Monitor and saw that neovim is failing to create the file/subdirectory ...\nvim6UJx04\7 when the :h 'shell' is set to pwsh (PowerShell Core). With shell set to cmd, it works alright. Note that in my full config, I set up the shell according to :h 'shell-powershell' but the same issue persists.

@3N4N 3N4N added the bug issues reporting wrong behavior label Oct 5, 2021
@3N4N
Copy link
Contributor Author

3N4N commented Oct 5, 2021

Here is a snap of the process with shell=cmd:
image

And here is one with shell=pwsh:
image

@justinmk
Copy link
Member

justinmk commented Oct 5, 2021

:let &shell = 'pwsh'

There are many shell-related options in vim, it is not a simple matter of setting 'shell' only. See :help shell-powershell for the correct settings (which is a section of :help 'shell'... read the help before opening issues).

@justinmk justinmk closed this as completed Oct 5, 2021
@justinmk justinmk added closed:question issues that are closed as usage questions environment user system environment (terminal, shell, tmux) and removed bug issues reporting wrong behavior labels Oct 5, 2021
@justinmk
Copy link
Member

justinmk commented Oct 5, 2021

Note that in my full config, I set up the shell according to :h 'shell-powershell' but the same issue persists.

Wait, why isn't that part of the repro steps?

@justinmk justinmk reopened this Oct 5, 2021
@3N4N
Copy link
Contributor Author

3N4N commented Oct 5, 2021

@justinmk I will update now. Thought I should reduce the steps as much as possible.

Edit: Issue updated.

@justinmk justinmk added platform:windows and removed closed:question issues that are closed as usage questions labels Oct 5, 2021
@justinmk
Copy link
Member

justinmk commented Oct 5, 2021

Thought I should reduce the steps as much as possible.

Yes :) But in this case setting only 'shell' is incorrect so it is not a sufficient repro.

BTW, latest stable release is Nvim 0.5.1

@3N4N
Copy link
Contributor Author

3N4N commented Oct 5, 2021

Just checked with the newest release (0.5.1) and the issue remains.

@3N4N
Copy link
Contributor Author

3N4N commented Oct 7, 2021

I checked with Ubuntu 20.04 in WSL. Using powershell as shell produces the same bug in Linux as well.

@3N4N
Copy link
Contributor Author

3N4N commented Oct 7, 2021

@justinmk, the problem may be because of permissions. I was debugging with gdb. It says the perm (of the temp files) with powershell is -2 while with cmd it's 33188. I would guess it's because the spawned powershell process doesn't have the required permission. But then how does cmd/bash spawned processes get the permission? Dunno.

Also, the do_filter function reads/writes two files. One for storing the original text, and another I guess for storing the processed text. Powershell can write the infile, but not the outfile. That seems weird.

By the way, the source of problem, the source inside nvim, is this call to uv_fs_stat(...). Looks like a call do libuv. I couldn't go deeper than that.

Any thoughts?

EDIT: The stack of function calls:

[#0] 0x55bfb030d843 → os_stat(name=0x55bfb1594910 "/tmp/nvimE8wDVQ/10", statbuf=0x7fff84d2f920)
[#1] 0x55bfb030d9b3 → os_getperm(name=0x55bfb1594910 "/tmp/nvimE8wDVQ/10")
[#2] 0x55bfb0246061 → readfile(fname=0x55bfb1594910 "/tmp/nvimE8wDVQ/10", sfname=0x55bfb1594910 "/tmp/nvimE8wDVQ/10", from=0x3, lines_to_skip=0x0, lines_to_read=0x7fffffff, eap=0x7fff84d2fff0, flags=0x2)
[#3] 0x55bfb01fdb51 → do_filter(line1=0x1, line2=0x3, eap=0x7fff84d2fff0, cmd=0x55bfb1491850 "sort", do_in=0x1, do_out=0x1)
[#4] 0x55bfb01fd72b → do_bang(addr_count=0x2, eap=0x7fff84d2fff0, forceit=0x0, do_in=0x1, do_out=0x1)
[#5] 0x55bfb0223d2f → ex_bang(eap=0x7fff84d2fff0)
[#6] 0x55bfb0216325 → do_one_cmd(cmdlinep=0x7fff84d30200, flags=0x0, cstack=0x7fff84d30320, fgetline=0x55bfb02329ed <getexline>, cookie=0x0)
[#7] 0x55bfb02135d1 → do_cmdline(cmdline=0x0, fgetline=0x55bfb02329ed <getexline>, cookie=0x0, flags=0x0)
[#8] 0x55bfb02db6d4 → nv_colon(cap=0x7fff84d30990)
[#9] 0x55bfb02d3803 → normal_execute(state=0x7fff84d30900, key=0x3a)

@3N4N 3N4N changed the title Cannot create temporary files for executing external commands over a range when shell is set to powershell Cannot read temp files for executing external commands over a range when shell is set to powershell Nov 9, 2021
@3N4N
Copy link
Contributor Author

3N4N commented Nov 9, 2021

Changing the title because nvim can create both in and out temp files, but just can't read the out temp file. I was wrong. Nvim with powershell actually cannot even create the out temp file. So the problem is not with the file permission, but with the powershell redirection.

Need some insight from devs, though, to make sense of what's happening.

@3N4N 3N4N changed the title Cannot read temp files for executing external commands over a range when shell is set to powershell Cannot create temp file for executing external commands over a range when shell is set to powershell Nov 9, 2021
@3N4N
Copy link
Contributor Author

3N4N commented Nov 9, 2021

Okay, the problem was with powershell redirection. The one suggested in :h shell-powershell gives cmd_buf to be --

gdb> p cmd_buf
$1 = (char_u *) 0x2513025da80 "sort.exe < C:/Users/ACER/AppData/Local/Temp/nvimnr4hM4/0 2>&1 | Out-File -Encoding UTF8 C:/Users/ACER/AppData/Local/Temp/nvimnr4hM4/1; exit $LastExitCode"

But PowerShell doesn't use < as file input redirection. It has a special argument. When I change cmd_buf like in the following, the filter works (the lines are sorted, I mean).

gdb> set cmd_buf = "Start-Process sort.exe -RedirectStandardInput C:/Users/ACER/AppData/Local/Temp/nvimnr4hM4/0 -RedirectStandardOutput C:/Users/ACER/AppData/Local/Temp/nvimnr4hM4/1  -NoNewWindow -Wait"
gdb> p cmd_buf
$2 = (char_u *) 0x2513026b0b0 "Start-Process sort.exe -RedirectStandardInput C:/Users/ACER/AppData/Local/Temp/nvimnr4hM4/0 -RedirectStandardOutput C:/Users/ACER/AppData/Local/Temp/nvimnr4hM4/1  -NoNewWindow -Wait"

I guess the easy workaround would be to force-use cmd.exe whatever the shell is set to. I don't think we users would care who is doing our work, as long as the work is done. Then again, someone may want to use powershell-specific command. Then you'd have to create a powershell-specific command for make_filter_cmd function. But judging by how no one except me found this bug, I'm gonna bet no one actually cares.

@justinmk justinmk added this to the 0.8 milestone Jun 25, 2022
clason pushed a commit that referenced this issue Jun 25, 2022
Problem:
Nvim fails to create tempfile "…/nvim6UJx04/7" when 'shell' is set to
pwsh (PowerShell Core). This breaks filtered shell commands ":{range}!".
With shell set to cmd, it works.

Solution:
PowerShell doesn't use "<" for stdin redirection. Instead, use
"-RedirectStandardInput".

Closes #15913
@justinmk justinmk modified the milestones: 0.8, 0.7.1 Jun 25, 2022
justinmk added a commit to justinmk/neovim that referenced this issue Jun 26, 2022
BREAKING CHANGES:
fdd5178 neovim#18986 introduce $NVIM, unset $NVIM_LISTEN_ADDRESS

VIM PATCHES:
git log --oneline --grep "vim-patch" v0.7.0..v0.7.1

FEATURES:
0b8facc neovim#18440 feat(api): add `group_name` to `nvim_get_autocmds`
b7a5278 neovim#18440 feat(api): add `group_name` to `nvim_get_autocmds`
9e5cef9 neovim#18264 feat(tui): query terminal for CSI u support

FIXES:
b477afa neovim#19088
    71e2c49 refactor(tests): introduce testprg()
    175892f neovim#15913 fix: make_filter_cmd for :! powershell
f2b4652 neovim#18331 fix(lsp): make sure to always reset active codelens refreshes
b3d754c neovim#18869 fix(lsp): fix multi client handling in code action
e820c6d neovim#19069
    cf9c097 fix(api): check for inclusive buffer line index out of bounds correctly
a05da1c neovim#19053
    17299b3 fix(input): use correct grid when restoring cursor for <expr> mapping
7266150 neovim#19025
    40e13c8 neovim#19010 fix(decorations): nvim_buf_set_extmark breaks conceal
38928b5 neovim#19023
    18e0d64 fix(tui): fix piping output make nvim unusable
cd7ac0e neovim#19014
    433f306 fix: use ctermbg/fg instead of bg/fg when use_rgb is false
30ae06c neovim#19009
    777d415 fix(highlight): let winhighlight use cursor
    f70e083 neovim#18981 fix(hl): return cterm fg/bg even if they match Normal
    ee210b0 fix(hl): DRY set_hl_group() sg_attr set
    512a819 test(hl): Add Normal group set_hl/get_hl_by_name check
    79ca64a neovim#18024) fix(hl): set Normal hl group sg_attr value (fixes
2ebc76b neovim#19001
    4170983 fix(startup): nvim with --clean should not load user rplugins
ed9e6d1 neovim#18990 fix(treesitter): new iter if folded
05ce04e neovim#18984
    fe42dea neovim#18976 fix(lua): highlight.on_yank can close timer in twice
    f15d609 neovim#18824 fix(lua): stop pending highlight.on_yank timer, if any
5243cb8 neovim#18975
    bf4df2a fix(ui): do not call showmode() when setting window height
3cea4d6 neovim#18942
    05f6883 fix(buffer): disable buffer-updates before removing buffer from window
1dba6cf neovim#18918
    0c9d666 fix(input): fix macro recording with ALT and special key
fc4e4b3 neovim#18905
    bd3bb12 test(ts): skip test if C parser is not available
d317cb2 neovim#18886
    1496f42 fix(input): allow Ctrl-C to interrupt a recursive mapping even if mapped
0dc5bcd neovim#18680
    3a1e8ef neovim#18617 fix(autocmds): separate command from desc
cb1b4bb neovim#18655 fix(lsp): only send diagnostics from current buffer in code_action()
01d0d2c neovim#18517 build(deps): bump Luv to HEAD - c51e7052e
f3ffb73 neovim#18612 fix(health): return 0 if file not exists
496e786 neovim#18534
    0377973 fix(runtime/genvimvim): omit s[ubstitute] from vimCommand
35075dc neovim#18469 fix(lsp): fix unnecessary buffers being added on empty diagnostics
9e040ac neovim#18468 fix(lsp): unify progress message handling
e28799f neovim#18436 fix: display global statusline correctly with ext_messages
1e28068 neovim#18415 fix: ensure has() does not change v:shell_error
203b088 neovim#18411 build(deps): bump LuaJIT to HEAD - 91bc6b8ad
e502e81 neovim#18400 fix(treesitter): bump match limit up
7567d21 neovim#18390
    d165289 fix(lsp): add missing bufnr argument
f3587ba neovim#18367
    631393a fix(filetype.lua): escape expansion of ~ when used as a pattern
08cd391 neovim#18360 fix(shared): avoid indexing unindexable values in vim.tbl_get()
508c8a5 neovim#18362 fix(ftdetect): source plugins in autogroup
ffd46b7 neovim#18351
    2a61983 fix(mac): use same $LANG fallback mechanism as Vim
8d4fbcb neovim#18324
    b80ef0d fix(tui): disable extended keys before exiting alternate screen
14a5b6b neovim#18312
    89260ea fix(input): only disable mapped CTRL-C interrupts when getting input
ef43e7d neovim#18298 fix: suppress "is a directory" messages with shortmess 'F'
aff05c5 neovim#18256 fix: show autocmd output when F is in shortmess
fa7d8c3 neovim#18229
    d916d2f neovim#18227 fix(lua): don't mutate opts parameter of vim.keymap.del
f7e2ad7 neovim#18220 fix(treesitter): create new parser if language is not the same as cached parser
7f8faac neovim#18205 fix(diagnostic): use nvim_exec_autocmds to trigger DiagnosticChanged
3ee089e neovim#18167
    0f811af fix(tui): update modifyOtherKeys reporting
justinmk added a commit that referenced this issue Jun 26, 2022
BREAKING CHANGES:
fdd5178 #18986 introduce $NVIM, unset $NVIM_LISTEN_ADDRESS

VIM PATCHES:
git log --oneline --grep "vim-patch" v0.7.0..v0.7.1

FEATURES:
0b8facc #18440 feat(api): add `group_name` to `nvim_get_autocmds`
b7a5278 #18440 feat(api): add `group_name` to `nvim_get_autocmds`
9e5cef9 #18264 feat(tui): query terminal for CSI u support

FIXES:
b477afa #19088
    71e2c49 refactor(tests): introduce testprg()
    175892f #15913 fix: make_filter_cmd for :! powershell
f2b4652 #18331 fix(lsp): make sure to always reset active codelens refreshes
b3d754c #18869 fix(lsp): fix multi client handling in code action
e820c6d #19069
    cf9c097 fix(api): check for inclusive buffer line index out of bounds correctly
a05da1c #19053
    17299b3 fix(input): use correct grid when restoring cursor for <expr> mapping
7266150 #19025
    40e13c8 #19010 fix(decorations): nvim_buf_set_extmark breaks conceal
38928b5 #19023
    18e0d64 fix(tui): fix piping output make nvim unusable
cd7ac0e #19014
    433f306 fix: use ctermbg/fg instead of bg/fg when use_rgb is false
30ae06c #19009
    777d415 fix(highlight): let winhighlight use cursor
    f70e083 #18981 fix(hl): return cterm fg/bg even if they match Normal
    ee210b0 fix(hl): DRY set_hl_group() sg_attr set
    512a819 test(hl): Add Normal group set_hl/get_hl_by_name check
    79ca64a #18024) fix(hl): set Normal hl group sg_attr value (fixes
2ebc76b #19001
    4170983 fix(startup): nvim with --clean should not load user rplugins
ed9e6d1 #18990 fix(treesitter): new iter if folded
05ce04e #18984
    fe42dea #18976 fix(lua): highlight.on_yank can close timer in twice
    f15d609 #18824 fix(lua): stop pending highlight.on_yank timer, if any
5243cb8 #18975
    bf4df2a fix(ui): do not call showmode() when setting window height
3cea4d6 #18942
    05f6883 fix(buffer): disable buffer-updates before removing buffer from window
1dba6cf #18918
    0c9d666 fix(input): fix macro recording with ALT and special key
fc4e4b3 #18905
    bd3bb12 test(ts): skip test if C parser is not available
d317cb2 #18886
    1496f42 fix(input): allow Ctrl-C to interrupt a recursive mapping even if mapped
0dc5bcd #18680
    3a1e8ef #18617 fix(autocmds): separate command from desc
cb1b4bb #18655 fix(lsp): only send diagnostics from current buffer in code_action()
01d0d2c #18517 build(deps): bump Luv to HEAD - c51e7052e
f3ffb73 #18612 fix(health): return 0 if file not exists
496e786 #18534
    0377973 fix(runtime/genvimvim): omit s[ubstitute] from vimCommand
35075dc #18469 fix(lsp): fix unnecessary buffers being added on empty diagnostics
9e040ac #18468 fix(lsp): unify progress message handling
e28799f #18436 fix: display global statusline correctly with ext_messages
1e28068 #18415 fix: ensure has() does not change v:shell_error
203b088 #18411 build(deps): bump LuaJIT to HEAD - 91bc6b8ad
e502e81 #18400 fix(treesitter): bump match limit up
7567d21 #18390
    d165289 fix(lsp): add missing bufnr argument
f3587ba #18367
    631393a fix(filetype.lua): escape expansion of ~ when used as a pattern
08cd391 #18360 fix(shared): avoid indexing unindexable values in vim.tbl_get()
508c8a5 #18362 fix(ftdetect): source plugins in autogroup
ffd46b7 #18351
    2a61983 fix(mac): use same $LANG fallback mechanism as Vim
8d4fbcb #18324
    b80ef0d fix(tui): disable extended keys before exiting alternate screen
14a5b6b #18312
    89260ea fix(input): only disable mapped CTRL-C interrupts when getting input
ef43e7d #18298 fix: suppress "is a directory" messages with shortmess 'F'
aff05c5 #18256 fix: show autocmd output when F is in shortmess
fa7d8c3 #18229
    d916d2f #18227 fix(lua): don't mutate opts parameter of vim.keymap.del
f7e2ad7 #18220 fix(treesitter): create new parser if language is not the same as cached parser
7f8faac #18205 fix(diagnostic): use nvim_exec_autocmds to trigger DiagnosticChanged
3ee089e #18167
    0f811af fix(tui): update modifyOtherKeys reporting

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 769a93aa2901..9b5563be0d1b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -136,7 +136,7 @@ set_property(CACHE CMAKE_BUILD_TYPE PROPERTY
 set(NVIM_VERSION_MAJOR 0)
 set(NVIM_VERSION_MINOR 7)
 set(NVIM_VERSION_PATCH 1)
-set(NVIM_VERSION_PRERELEASE "-dev") # for package maintainers
+set(NVIM_VERSION_PRERELEASE "") # for package maintainers

 # API level
 set(NVIM_API_LEVEL 9)         # Bump this after any API change.
diff --git a/runtime/nvim.appdata.xml b/runtime/nvim.appdata.xml
index 1464c2769443..893023db8298 100644
--- a/runtime/nvim.appdata.xml
+++ b/runtime/nvim.appdata.xml
@@ -26,6 +26,7 @@
   </screenshots>

   <releases>
+    <release date="2022-06-26" version="0.7.1"/>
     <release date="2022-04-15" version="0.7.0"/>
     <release date="2021-12-31" version="0.6.1"/>
     <release date="2021-11-30" version="0.6.0"/>
kraftwerk28 pushed a commit to kraftwerk28/neovim that referenced this issue Jul 6, 2022
Problem:
Nvim fails to create tempfile "…/nvim6UJx04/7" when 'shell' is set to
pwsh (PowerShell Core). This breaks filtered shell commands ":{range}!".
With shell set to cmd, it works.

Solution:
PowerShell doesn't use "<" for stdin redirection. Instead, use
"-RedirectStandardInput".

Closes neovim#15913
justinmk added a commit that referenced this issue Oct 1, 2022
Reverts #16271
Fixs #15913

Problem:
Since #16271, `make_filter_cmd` uses `Start-Process` cmdlet to execute the user
provided shell command for `:%!`. `Start-Process` requires the command to be
split into the shell command and its arguments. This was implemented in #19268
by parsing (splitting the user-provided command at the first space) which didn't
handle cases such as --
  - commands with escaped space in their filepath
  - quoted commands with space in their filepath

Solution: Use piping.

The total shell command formats (excluding noise of unimportant parameters):

1. Before #16271
    ```powershell
    pwsh -C "(shell_cmd) < tmp.in | 2>&1 Out-File -Encoding UTF8 <tmp.out>"
    # not how powershell commands work
    ```
2. Since #16271
    ```powershell
    pwsh -C "Start-Process shell_cmd -RedirectStandardInput <tmp.in> -RedirectStandardOutput <tmp.out>"
    # doesn't handle executable path with space in it
    # doesn't write error to <tmp.out>
    ```
3. This PR
    ```powershell
    pwsh -C "& { Get-Content <tmp.in> | & 'path\with space\to\shell_cmd.exe' arg1 arg2 } 2>&1 | Out-File -Encoding UTF8 <tmp.out>"
    # also works with forward slash in the filepath
    # also works with double quotes around shell command
    ```

After this PR, the user can use the following formats:

    :%!c:\Program` Files\Git\usr\bin\sort.exe
    :%!'c:\Program Files\Git\usr\bin\sort.exe'
    :%!"c:\Program Files\Git\usr\bin\sort.exe"
    :%!"c:\Program` Files\Git\usr\bin\sort.exe"

They can even chain different commands:

    :%!"c:\Program` Files\Git\usr\bin\sort.exe" | sort.exe -r

But if they want to call a stringed executable path, they have to provide the
Invoke-Command operator (&). In fact, the first stringed executable path also
needs this & operator, but this PR adds that behind the scene.

    :%!"c:\Program` Files\Git\usr\bin\sort.exe" | sort.exe -r | & 'c:\Program Files\Git\usr\bin\sort.exe'

## What this PR solves

- Having to parse the user-provided bang ex-command (for splitting into shell
  cmd and its args).
- Removes a lot of human-unreadable `#ifdef` blocks.
- Accepting escaped spaces in executable path.
- Accepting quoted string of executable path.
- Redirects error and exception to tmp.out (exception for when `wrong_cmd.exe
  not found`)

## What this PR doesn't solve

- Handling wrongly escaped path to executable, which the user may pass because
  of cmdline tab-completion. #18592

## Edge cases
- (Not handled) If the user themself provides the `&` sign (means `call
  this.exe` in powershell)
- (Not handled) Use `-Encoding utf8` parameter for `Get-Content`?
- (Handled) Doesn't write to tmp.out if shell command is not found.
    - fix: use anonymous function (`{wrong_cmd.exe}`).

## Changes other than `make_filter_cmd()` function

- Encoding for piping to external executables. See BOM-less UTF8:
  PowerShell/PowerShell#4681
smjonas pushed a commit to smjonas/neovim that referenced this issue Dec 31, 2022
Problem:
Nvim fails to create tempfile "…/nvim6UJx04/7" when 'shell' is set to
pwsh (PowerShell Core). This breaks filtered shell commands ":{range}!".
With shell set to cmd, it works.

Solution:
PowerShell doesn't use "<" for stdin redirection. Instead, use
"-RedirectStandardInput".

Closes neovim#15913
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
environment user system environment (terminal, shell, tmux) platform:windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants