Why :GoSetup ignores user's GOPATH?

1. Include the output from `:GoDiag`.

VERSION
    NVIM v0.4.0-1374-g66149ecff; Build type: RelWithDebInfo
    go version go1.12.7 linux/amd64; GOPATH=/home/yuki/.local; GOROOT=/usr/lib/go; GO111MODULE=on
    gopher.vim version 9e0733b 2019-08-21 17:19:48 +0100 (2 hours ago) Don't move view on :GoImport
 
VARIABLES
    gopher_build_flags    []
    gopher_debug          []
    gopher_tag_transform   snakecase
    gopher_map            {'_default': 1, 'return': 'r', 'implement': 'm', 'error': 'e', '_imap_ctrl': 1, '_popup': 0, '_nmap_prefix': ';', '_imap_prefix': '<C-k>', 'if': 'i'}
    gopher_tag_default     json
    gopher_build_tags     []
    gopher_highlight      ['string-spell', 'string-fmt']
 
COMMAND HISTORY (newest on top)

2. Include a *minimal* vimrc to reproduce the issue, with other plugins disabled.

Hi! Thank you for awesome plugin. I really love this.

:GoSetup creates ~/go regardless of user's GOPATH definition, because it unsets GOPATH before go install.

gopher.vim/autoload/gopher/system.vim

Line 265 in 9e0733b

let $GOPATH = '' " TODO(8.0.1832): use unlet; this is also okay.

Is this expected behavior?
Thanks!

The expected behaviour is that GOPATH isn't used at all, :GoSetup installs all dependencies (as Go module) in gopher.vim's tools/bin directory.

It's certainly not expected that a ~/go directory is created!

I'm not entirely sure why GOPATH is being reset here to be honest, or if that's truly needed. I think just GO111MODULE=on is enough? I wrote this almost a year ago, so it's not 100% fresh in my mind at the moment.

Wow. Thank you for quick reply!

I see. The specifications around Go Modules have changed significantly over the past year so it may need to be updated.

I tried to simply remove lines unsetting/restoring GOPATH and it worked for me, and I can submit PR but should I? Because I'm not familiar with Vim Script so maybe I cannot write test..

diff --git a/autoload/gopher/system.vim b/autoload/gopher/system.vim
index 4526948..12e9f25 100644
--- a/autoload/gopher/system.vim
+++ b/autoload/gopher/system.vim
@@ -260,9 +260,7 @@ fun! s:tool(name) abort
   try
     let l:old_gobin =  exists('$GOBIN')       ? $GOBIN       : -1
     let l:old_gomod =  exists('$GO111MODULE') ? $GO111MODULE : -1
-    let l:old_gopath = exists('$GOPATH')      ? $GOPATH      : -1

-    let $GOPATH = ''   " TODO(8.0.1832): use unlet; this is also okay.
     let $GOBIN = s:gobin
     let $GO111MODULE = 'on'  " In case user set to 'off'

@@ -277,7 +275,6 @@ fun! s:tool(name) abort
   finally
     call gopher#system#restore_env('GOBIN', l:old_gobin)
     call gopher#system#restore_env('GO111MODULE', l:old_gomod)
-    call gopher#system#restore_env('GOPATH', l:old_gopath)
   endtry

   return a:name

I was just writing a patch with exactly those lines :-) See 1634b99

Like I said in the commit message, I don't know why I did that. Probably just ignorance on my part? Either way, I did some tests and works well for me without resetting GOPATH.

Great. Thanks! 🎉