Fixed issue Vim-Go broken on freebsd #1224

[creaky]

vim-go system calls fails when using tcsh.

Having the [t]csh shells set as the login shell results in
temporary file access errors when calling ksh or dash system
shells. /bin/sh is often aliased to these shells on modern
Linux and BSD systems including VoidLinux, Debian and FreeBSD.

As more Linux systems move away from bash to dash for the system
shell, this issue will occur more often.

[arp242]

I'm confused. This will not reset the shell when using csh, right?

This will break commands that assume Bourne shell syntax. Resetting the shell to /bin/sh is a feature (I added this in #988).

[arp242]

By the way, another way to do this is to introduce a new g:go_shell setting or some such. That's what Syntastic does (see vim-syntastic/syntastic#1355).

[arp242]

Also related: junegunn/vim-plug#159 – looks like you may ran in to the shellredir problem described there?

[creaky]

Correct, this change will NOT reset the shell when running under *csh family. So far this has not caused any compatibility problems.

The feature added in #988 breaks functionality and limits the portability of vim-go. The existing Windows bypass is evidence of this, and in the same vein, a bypass for *csh users (on systems using dash, zsh or ksh as /bin/sh) is needed until something more sophisticated is coded.

An option is to allow the user to specify an appropriate bourne like shell to drop down to ala g:go_shell specifying bash however do note FreeBSD and Solaris does not have bash installed by default. Further, this requires the user to do more configuring and really impacts the user experience of "it just works".

Alternatively, if someone has time, a very deep dive into why temporary file creation fails under dash/zsh/ksh as subshell to *csh and fix it would address a root cause of needing this workaround.

Best option is to keep command system calls simple, compatible with common shells and not be dependent on any shell specific behaviour, thus removing any need to override the user's shell.

So far, commands observed using this system call falls into the best option category and thus using the *csh skip is better than the alternative of doing nothing and breaking vim-go on a number of systems which is what is happening today.

[arp242]

This patch will almost certainly break things. I didn't add it just for fun, I added it because stuff broke ;-) I don't remember what exactly though, and unfortunately I neglected to mention what exactly in the commit/PR description.

What problems are you having? Did you try setting the shellredir option as mentioned? Because I think this will fix it, and should be what we're changing?

The feature added in #988 breaks functionality and limits the portability of vim-go.

?

It does exactly the opposite, by setting a known shell we ensure that all commands always work.

[creaky]

What problems are you having?

The regression introduced by #988 causes the following problems in a *csh, zsh login environment on a system with /bin/sh pointing to dash, ksh or BSD sh.

Problem 1. Running 'vim test.go' results in errors and no template.
Problem 2. Unable to save a modified *.go file. Errors in save process prevents successful write.
Problem 3. Most vim-go commands resulting in calling the sub-shell fail with error.

Example error message produced from 'vim test.go':
Error detected while processing function <SNR>27_template_autocreate[3]..go#temp late#create[8]..go#tool#PackageName[2]..go#tool#ExecuteInDir[4]..go#util#env[7]. .go#util#goroot[1]..go#util#System: line 17: E484: Can't open file /tmp/vQAaQRG/1 Error detected while processing function <SNR>27_template_autocreate[3]..go#temp late#create[8]..go#tool#PackageName[2]..go#tool#ExecuteInDir[4]..go#util#env: line 7: E171: Missing :endif Error detected while processing function <SNR>27_template_autocreate: line 3: E171: Missing :endif

[creaky]

The shellredir is the right way to go. Expanding on the deep dive option mentioned above:

The 'shellredir' option controls what is captured by system(). From Vim help page: "The default is ">". For Unix, if the 'shell' option is "csh", "tcsh" or "zsh" during initializations, the default becomes ">&". If the 'shell' option is "sh", "ksh" or "bash" the default becomes ">%s 2>&1". This means that stderr is also included.

The initialization of this option is done after reading the ".vimrc" and the other initializations, so that when the 'shell' option is set there, the 'shellredir' option changes automatically unless it was explicitly set before."

So from #988 the shell was changed without updating shellredir to match which causes the error for the csh/tcsh/zsh users.

I have tested this successfully and will update the pull request.

[fatih]

Thanks @creaky @Carpetsmoker