vim-signify icon indicating copy to clipboard operation
vim-signify copied to clipboard

NUL files written by Perforce vcs command when running vim from git-bash on Windows

Open baskus opened this issue 3 years ago • 1 comments

The problem

I am getting NUL files sprinkled here and there when using vim since updating vim-signify. Most annoyingly in the .git/rebase-merge/ directory. A NUL file in there prevents git from removing the rebase-merge directory when rebasing is finished, locking git into a weird state. This is git's error message in that case: error: could not remove '.git/rebase-merge'.

Note

I have a git repository inside a Perforce repository. We're working on some code separate from the larger Perforce code base. This is copied into a directory in the Perforce code base. This is weird and might muck things up.

The cause

The reason for this odd behaviour of vim-signify seems to be the Perforce vcs command. Specifically when the logic is tripped and both cmd and bash things get combined into the command.

The offending command comes from autoload/sy/repo.vim:647: \ 'perforce': 'p4 info '. sy#util#shell_redirect('%n') . (has('win32') ? ' &&' : ' && env P4DIFF= P4COLORS=') .' p4 diff -du0 %f',

Because I'm running on Windows %n is replaced by NUL in that command.

At the same time, because I'm running git, and thus vim, from git-bash, $SHELL is set to /usr/bin/bash. This sets vim's &shell to C:\Program Files\Git\usr\bin\bash.exe. This leads wrap_cmd to construct the command "C:\Program Files\Git\usr\bin\bash.exe" -c " p4 info > NUL && p4 diff -du0 'E:\src\perforce_root_here\some\path\git_repo_here\.git\rebase-merge\git-rebase-todo' ".

Now we're in trouble. Bash will pipe p4 info to the file NUL and not to something like /dev/null since bash doesn't understand that NUL should be something special.

Hacky solution

I can solve this by replacing the Perforce vcs command with this

\ 'perforce': 'p4 info '. '> /dev/null ' . (has('win32') ? ' &&' : ' && env P4DIFF= P4COLORS=') .' p4 diff -du0 %f',

but that's not something one could commit. With that it works for my setup though.

baskus avatar Apr 08 '21 10:04 baskus

Seems like the way we set s:devnull needs to be improved. Something like this?

diff --git i/autoload/sy/repo.vim w/autoload/sy/repo.vim
index 78a6626..fc77cb2 100644
--- i/autoload/sy/repo.vim
+++ w/autoload/sy/repo.vim
@@ -667,4 +667,4 @@ else
 endif
 
 let s:difftool = sy#util#escape(get(g:, 'signify_difftool', 'diff'))
-let s:devnull  = has('win32') || has ('win64') ? 'NUL' : '/dev/null'
+let s:devnull  = ((has('win32') || has ('win64')) && &shell !~ 'sh.exe$') ? 'NUL' : '/dev/null'

jamessan avatar Apr 10 '21 18:04 jamessan