$SHELL is invalid in Vim when run via `git commit` #498

benmoss · 2015-10-20T15:33:41Z

Any vimscripts that use has("unix") or has("win32") etc will incorrectly detect that the Vim bundled with this project is not running Windows.

This code in particular for the Syntastic plugin is causing issues for me: https://github.com/scrooloose/syntastic/blob/50d0315835047c35a1f6f6edf214e612d6eae3dd/autoload/syntastic/util.vim#L11-L13

The text was updated successfully, but these errors were encountered:

dscho · 2015-10-20T15:42:10Z

I guess that we should keep saying "yes" when has("unix") was called: our VIM is an MSys2 one, i.e. it knows about POSIX paths and uses POSIX functions emulated by the MSys2 runtime.

benmoss · 2015-10-20T16:19:36Z

I traced this down a bit. It seems that when vim is started by Git, as in EDITOR=vim git commit, doing :echo $SHELL reports C:/Program Files/Git/usr/bin/bash, instead of the normal /usr/bin/bash. Not sure what is unique about the way vim is launched for a commit, but will try to debug further.

As for whether has('unix') is accurate, I would say yeah it probably is, but has("win64") etc should also probably be true.

dscho · 2015-10-21T09:17:07Z

doing :echo $SHELL reports C:/Program Files/Git/usr/bin/bash, instead of the normal /usr/bin/bash.

This is most likely due to MSys2's runtime converting the SHELL variable from POSIX to Windows format before running the (non-MSys2) git.exe. Then, when git.exe calls (MSys2) vim.exe, the variable does not get converted back to POSIX format.

For the record: this issue has been mentioned before (BTW thank you for providing precise and correct title and description in this ticket, I really appreciate this).

I have a hunch that the best place to patch would be https://github.com/git-for-windows/msys2-runtime/blob/e87aaa7d59deb2f4e5bff9cb114a0c45d1ae1a5e/winsup/cygwin/environ.cc#L317-L333

To test this hypothesis, you might find https://github.com/git-for-windows/git/wiki/Building-msys2-runtime to be a useful resource.

mhoran · 2015-10-21T16:11:38Z

We added the following to environ.cc and recompiled:

diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 2ab6d28..3799cbf 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -324,6 +324,7 @@ static win_env conv_envvars[] =
   {
     {NL ("PATH="), NULL, NULL, env_PATH_to_posix, env_plist_to_win32, true},
     {NL ("HOME="), NULL, NULL, env_path_to_posix, env_path_to_win32, false},
+    {NL ("SHELL="), NULL, NULL, env_path_to_posix, env_path_to_win32, false},
     {NL ("LD_LIBRARY_PATH="), NULL, NULL,
             env_plist_to_posix, env_plist_to_win32, true},
     {NL ("TMPDIR="), NULL, NULL, env_path_to_posix, env_path_to_win32, false},

Unfortunately, the issue persists. To verify that we were indeed compiling and moving the correct library into place, we removed the conversion for TEMP and verified that it was not converted in Windows.

Perhaps there's something special about SHELL?

dscho · 2015-10-22T13:07:30Z

Perhaps there's something special about SHELL?

Oh wow. I guess it must be, but I cannot find any verbatim mention of the term SHELL in MSys2's runtime's source code...

mhoran · 2015-10-22T13:41:26Z

Here's an interesting discovery: from the SDK shell, git commit launches vim without error. This works without a modified msys-2.0.dll in place. Does this mean anything?

dscho · 2015-10-22T14:30:22Z

Does this mean anything?

Well, I was curious and replicated your findings. This is what I got when I typed :!echo $SHELL<Return> in Git for Windows vs Git for Windows SDK:

Cannot execute shell C:/Program Files/Git/usr/bin/bash

Press ENTER or type command to continue

in Git for Windows, and in the SDK:

C:/git-sdk-64/usr/bin/bash

Press ENTER or type command to continue

This leads me to believe that it happens to work if Git is installed into a path that does not contain spaces, and fails otherwise. To test this hypothesis, I called

SHELL=/c/PROGRA~1/Git/usr/bin/bash git commit

in Git for Windows and all of a sudden, :!echo $SHELL<Return> works:

C:/PROGRA~1/Git/usr/bin/bash

Press ENTER or type command to continue

This leads me more and more to the conclusion that vim is actually at fault, for treating SHELL as if it was a command-line, not a path to an executable (that can contain spaces without quoting).

dscho · 2015-10-22T14:45:53Z

And indeed, a simple test in a Linux VM proves my hypothesis: vim interprets SHELL as a command-line, not a single path: starting vim as

$ mkdir with\ spaces
$ ln -s $(which bash) with\ spaces/
$ SHELL=$(pwd)/with\ spaces/bash vi

and then executing :!echo $SHELL produces:

Cannot execute shell /home/me/with spaces/bash

Press ENTER or type command to continue

When substituting with\ spaces with without-spaces, it works.

mhoran · 2015-10-22T15:33:01Z

Great find! We've opened an issue with vim.

dscho · 2015-10-22T15:46:57Z

Better to provide a patch.

mhoran · 2015-10-22T19:25:26Z

In reading the vim documentation for help 'shell', it seems that this is expected behavior:

        If the name of the shell contains a space, you might need to enclose
        it in quotes.  Example:
                :set shell=\"c:\program\ files\unix\sh.exe\"\ -f

What I'm not sure of is whether $SHELL should be quoted, or if vim should be quoting $SHELL when setting the shell variable. Comments on the vim issue seem to lean towards current vim behavior (no quoting).

I tried to set my login shell to a shell containing spaces, but chsh rejected my attempts because the shell was not in /etc/shells. When adding the shell to /etc/shells, it was still rejected, no matter what quoting or escaping I tried. It seems not possible to set a shell on UNIX/Linux to a path containing spaces.

I'm not sure where this leaves us. It doesn't seem like a patch to vim would be well received, and I'm not certain escaping the $SHELL variable on Windows is the correct thing to do either.

dscho · 2015-10-23T09:29:11Z

A hacky work-around would be to set SHELL=C:/PROGRA~1/Git/usr/bin/bash.

Not sure I like that as a permanent "solution", though.

dscho · 2015-10-23T14:40:20Z

Well, the more I think about it, the more I am convinced that we have to find out where SHELL is transformed from POSIX to Windows, before we can hope to fix the problem for reals.

rimrul · 2015-10-23T15:00:17Z

A hacky work-around would be to set SHELL=C:/PROGRA~1/Git/usr/bin/bash.

This could be troublesome on 64bit systems because
a) I'm not sure there is a clear definition how Windows assigns 8.3 names to Program Files, Program Files (x86) and ProgramData
b) This could cause trouble with one bitness of the installation trying to access the other bitness's bash (which is probably not available because only one git is installed)

dscho · 2015-10-24T08:44:24Z

@rimrul oh, I was thinking about GetShortNameW(), of course, but I described the effect for better understanding.

Having said that, in the meantime I came to the conclusion that we need to find the gateway that is responsible for transforming the SHELL variable and add code to convert it back close to that code (when the MSys2 realm is reentered).

When calling a non-MSys2 binary, all of the environment is converted from POSIX to Win32, including the SHELL environment variable. In Git for Windows, for example, `SHELL=/usr/bin/bash` is converted to `SHELL=C:\Program Files\Git\usr\bin\bash.exe` when calling the `git.exe` binary. This is appropriate because non-MSys2 binaries would not handle POSIX paths correctly. Under certain circumstances, however, `git.exe` calls an *MSys2* binary in turn, such as `git config --edit` calling `vim.exe` unless Git is configured to use another editor specifically. Now, when this "improved vi" calls shell commands, it uses that $SHELL variable *without quoting*, resulting in a nasty error: C:\Program: No such file or directory Many other programs behave in the same manner, assuming that $SHELL does not contain spaces and hence needs no quoting, unfortunately including some of Git's own scripts. Therefore let's make sure that $SHELL gets "posified" again when entering MSys2 programs. This fixes git-for-windows/git#542, git-for-windows/git#498, and git-for-windows/git#468. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

When calling a non-MSys2 binary, all of the environment is converted from POSIX to Win32, including the SHELL environment variable. In Git for Windows, for example, `SHELL=/usr/bin/bash` is converted to `SHELL=C:\Program Files\Git\usr\bin\bash.exe` when calling the `git.exe` binary. This is appropriate because non-MSys2 binaries would not handle POSIX paths correctly. Under certain circumstances, however, `git.exe` calls an *MSys2* binary in turn, such as `git config --edit` calling `vim.exe` unless Git is configured to use another editor specifically. Now, when this "improved vi" calls shell commands, it uses that $SHELL variable *without quoting*, resulting in a nasty error: C:\Program: No such file or directory Many other programs behave in the same manner, assuming that $SHELL does not contain spaces and hence needs no quoting, unfortunately including some of Git's own scripts. Therefore let's make sure that $SHELL gets "posified" again when entering MSys2 programs. Earlier attempts by Git for Windows contributors claimed that adding `SHELL` to the `conv_envvars` array does not have the intended effect. These reports just missed that the `conv_start_chars` array (which makes the code more performant) needs to be adjusted, too. Note that we set the `immediate` flag to `true` so that the environment variable is set immediately by the MSys2 runtime, i.e. not only spawned processes will see the POSIX-ified `SHELL` variable, but the MSys2 runtime *itself*, too. This fixes git-for-windows/git#542, git-for-windows/git#498, and git-for-windows/git#468. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

alexhardwicke · 2015-12-10T08:24:59Z

Not sure if the above commits that reference this have fixed it, but as the ticket's open I thought I'd include a better workaround (that doesn't depend on program files and so will always work on x64):

You can just put

set shell=/usr/bin/bash

in your .vimrc (you don't need to include program files).

When calling a non-MSys2 binary, all of the environment is converted from POSIX to Win32, including the SHELL environment variable. In Git for Windows, for example, `SHELL=/usr/bin/bash` is converted to `SHELL=C:\Program Files\Git\usr\bin\bash.exe` when calling the `git.exe` binary. This is appropriate because non-MSys2 binaries would not handle POSIX paths correctly. Under certain circumstances, however, `git.exe` calls an *MSys2* binary in turn, such as `git config --edit` calling `vim.exe` unless Git is configured to use another editor specifically. Now, when this "improved vi" calls shell commands, it uses that $SHELL variable *without quoting*, resulting in a nasty error: C:\Program: No such file or directory Many other programs behave in the same manner, assuming that $SHELL does not contain spaces and hence needs no quoting, unfortunately including some of Git's own scripts. Therefore let's make sure that $SHELL gets "posified" again when entering MSys2 programs. Earlier attempts by Git for Windows contributors claimed that adding `SHELL` to the `conv_envvars` array does not have the intended effect. These reports just missed that the `conv_start_chars` array (which makes the code more performant) needs to be adjusted, too. Note that we set the `immediate` flag to `true` so that the environment variable is set immediately by the MSys2 runtime, i.e. not only spawned processes will see the POSIX-ified `SHELL` variable, but the MSys2 runtime *itself*, too. This fixes git-for-windows/git#542, git-for-windows/git#498, and git-for-windows/git#468. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

benmoss changed the title ~~Vim incorrectly reports running on unix~~ $SHELL is invalid in Vim when run via git commit Oct 20, 2015

dscho added bug msys2 labels Oct 21, 2015

mhoran mentioned this issue Oct 22, 2015

vim cannot execute SHELL if it contains spaces vim/vim#459

Closed

dscho added unclear and removed msys2 labels Oct 23, 2015

dscho mentioned this issue Nov 9, 2015

vimdiff as mergetool causes E810 and E97 #468

Closed

alexhardwicke mentioned this issue Jan 22, 2016

Cygwin shell interfering on Windows airblade/vim-gitgutter#285

Closed

git-for-windows-ci mentioned this issue Sep 6, 2022

[New msys2-runtime version] 19 new items #4009

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

$SHELL is invalid in Vim when run via `git commit` #498

$SHELL is invalid in Vim when run via `git commit` #498

benmoss commented Oct 20, 2015

dscho commented Oct 20, 2015

benmoss commented Oct 20, 2015

dscho commented Oct 21, 2015

mhoran commented Oct 21, 2015

dscho commented Oct 22, 2015

mhoran commented Oct 22, 2015

dscho commented Oct 22, 2015

dscho commented Oct 22, 2015

mhoran commented Oct 22, 2015

dscho commented Oct 22, 2015

mhoran commented Oct 22, 2015

dscho commented Oct 23, 2015

dscho commented Oct 23, 2015

rimrul commented Oct 23, 2015

dscho commented Oct 24, 2015

alexhardwicke commented Dec 10, 2015

$SHELL is invalid in Vim when run via git commit #498

$SHELL is invalid in Vim when run via git commit #498

Comments

benmoss commented Oct 20, 2015

dscho commented Oct 20, 2015

benmoss commented Oct 20, 2015

dscho commented Oct 21, 2015

mhoran commented Oct 21, 2015

dscho commented Oct 22, 2015

mhoran commented Oct 22, 2015

dscho commented Oct 22, 2015

dscho commented Oct 22, 2015

mhoran commented Oct 22, 2015

dscho commented Oct 22, 2015

mhoran commented Oct 22, 2015

dscho commented Oct 23, 2015

dscho commented Oct 23, 2015

rimrul commented Oct 23, 2015

dscho commented Oct 24, 2015

alexhardwicke commented Dec 10, 2015

$SHELL is invalid in Vim when run via `git commit` #498

$SHELL is invalid in Vim when run via `git commit` #498