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

util: add go#util#env function #954

Merged
merged 1 commit into from
Oct 23, 2016
Merged

util: add go#util#env function #954

merged 1 commit into from
Oct 23, 2016

Conversation

fatih
Copy link
Owner

@fatih fatih commented Jul 21, 2016

Add new function to return cached version of Go environment variables.
With that set GOROOT everytime we execute gocode. This will prevent
gocode using the internal embedded GOROOT and will use the GOROOT from
the latest installed Go version.

Fixes #901

@ashb
Copy link

ashb commented Jul 21, 2016

I had to add this hunk to get it working for :GoRename too:

diff --git a/autoload/go/tool.vim b/autoload/go/tool.vim
index 487b933..fbbe12b 100644
--- a/autoload/go/tool.vim
+++ b/autoload/go/tool.vim
@@ -121,7 +121,9 @@ endfunction

 function! go#tool#ExecuteInDir(cmd) abort
   let old_gopath = $GOPATH
+  let old_goroot = $GOROOT
   let $GOPATH = go#path#Detect()
+  let $GOROOT = go#util#env("goroot")

   let cd = exists('*haslocaldir') && haslocaldir() ? 'lcd ' : 'cd '
   let dir = getcwd()
@@ -133,6 +135,7 @@ function! go#tool#ExecuteInDir(cmd) abort
   endtry

   let $GOPATH = old_gopath
+  let $GOROOT = old_goroot
   return out
 endfunction

Are there any other paths that might need it to?

Edit: :GoDoc will need something similar it looks like.

@fatih
Copy link
Owner Author

fatih commented Jul 21, 2016

I'm not sure why :GoDoc or :GoRename needs them. Do they also use compiled GOROOT information?

@ashb
Copy link

ashb commented Jul 21, 2016

I guess so - they seem to have problems if there's a syntax error. Trying to use ^] I just got this error trying to jump to the code for json.Marshal

vim-go: guru: cannot find package "encoding/json" in any of:
        /Users/ash/.homebrew/Cellar/go/1.6.2/libexec/src/encoding/json (from $GOROOT)
        /Users/ash/code/go/src/encoding/json (from $GOPATH)

However addingthe fix-up into autoload/go/guru.vim didn't seem to help in this case. So maybe this won't work for all tools :(

(My test is I last ran :GoUpdateBinaries with go 1.6.2 and this morning I upgraded to go 1.6.3)

@fatih
Copy link
Owner Author

fatih commented Jul 31, 2016

@FiloSottile can you try this PR and see if this fixes #901 ?

@fatih
Copy link
Owner Author

fatih commented Sep 4, 2016

Another ping @ashb @FiloSottile

@ashb
Copy link

ashb commented Sep 4, 2016

I think it makes sense to include the patch form #954 (comment) too - it helps for more of the tools, even if not every single one.

@fatih
Copy link
Owner Author

fatih commented Sep 6, 2016

@ashb good call, I've forget it. I'll try to add them.

Add new function to return cached version of Go environment variables.
With that set GOROOT everytime we execute gocode. This will prevent
gocode using the internal embedded GOROOT and will use the GOROOT from
the latest installed Go version.

Fixes #901
@fatih
Copy link
Owner Author

fatih commented Sep 27, 2016

@ashb sorry for the late response. But I've included your patch as well. Does this look good now? Can you try it?

@ashb
Copy link

ashb commented Sep 29, 2016

This is an improvment and fixes some of the tools, if not all of them. Lets merge it.

(guru doesn't work because it doesn't call go#tool#ExecuteInDir but uses go#uitl#System. Probably worth fixing guru in another PR. Either to use ExecuteInDir or to save/restore GOROOT itself.)

@fatih fatih merged commit fcd04ce into master Oct 23, 2016
@fatih fatih deleted the fix-gocode-goroot branch October 23, 2016 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants