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

Reinstall binaries when a GOROOT change is detected #901

Closed
FiloSottile opened this issue Jun 12, 2016 · 7 comments
Closed

Reinstall binaries when a GOROOT change is detected #901

FiloSottile opened this issue Jun 12, 2016 · 7 comments

Comments

@FiloSottile
Copy link

When GOROOT changes, for example if you simply upgrade Go with Homebrew, all binaries compiled with the old compiler will end up with a stale GOROOT. This is confusingly breaks tools like gocode.

Maybe vim-go could (suggest to?) rerun InstallBinaries when a GOROOT change is detected.

See also: https://blog.filippo.io/stale-goroot-and-gorebuild/

@fatih
Copy link
Owner

fatih commented Jun 13, 2016

Hi @FiloSottile

Thanks for the feedback. This seems like a very niche problem you have. I don't know how many people compile Go on daily basis, I assume it might be very very low?

I'm always open for new features and improving the user experience for vim-go. If this is something we can incorporate in an easy way and don't require a lot of maintenance in the future, I'm more than happy to add it.

Maybe vim-go could (suggest to?) rerun InstallBinaries when a GOROOT change is detected.

How do you think to do it? I've read your blog post, and seems like you go over all binaries by using a custom Go application you wrote (gorebuild). Is it possible to detect the change without using this application?

We already have the :GoUpdateBinaries command, that goes and updates all your binaries again. Maybe you can manually call it every time you compile a new Go toolchain?

@FiloSottile
Copy link
Author

Thanks for the response!

I'm afraid I didn't explain my case well, this happens every time a new version of Go is published for all Homebrew users, and it's incredibly hard to diagnose. Empirical evidence: http://www.reddit.com/r/golang/comments/4nss0v/stale_goroot_and_gorebuild/d46ytkd

About how to do it, does vim-go have permanent storage? Or maybe detecting a special error message? Or just also install a tool that just prints the GOROOT, and execute it to check if it's consistent with the environment before running tools.

Sent from a small keyboard

@fatih
Copy link
Owner

fatih commented Jun 24, 2016

Ok now I get what you mean, thanks for the clarification 👍

About how to do it, does vim-go have permanent storage? Or maybe detecting a special error message? Or just also install a tool that just prints the GOROOT, and execute it to check if it's consistent with the environment before running tools.

There is no global storage unfortunately. We only read the settings, if any, from vimrc and that's it.

We already have a helper method to get GOROOT in vim-go: https://github.com/fatih/vim-go/blob/master/autoload/go/util.vim#L48

The issue though here is, how do we get the embedded GOROOT from the binary itself without using any external tool, such as gorebuild ? This is the hard part of this issue, and honestly I don't know if there is any solution to this. If I go and add a check (which uses a third party app), it'll be a dependency that people have to install. Also it'll increase the latency of the editor (because it'll check everytime you execute a command). Ok maybe this can be improved by not checking it and do the check once if you start Vim. But is this all worth adding ? Wouldn't this increase the complexity of vim-go just to have this?

I'm not keen to be honest, as the requirements to implement this is also high (depending on a third party app). I'm open to suggestions, maybe there is an easy way to detect the embedded GOROOT. If you have a simple solution to this, I'm all ears.

@ashb
Copy link

ashb commented Jun 30, 2016

I think I just hit this problem too - I tried to use GoRename and I got errors like this:

scanner/types.go|4| 2: could not import fmt (cannot find package "fmt" in any of:
||  /Users/ash/.homebrew/Cellar/go/1.6/libexec/src/fmt (from $GOROOT)
||  /Users/ash/code/go/src/fmt (from $GOPATH))

(My go version is now 1.6.2)

I tried running :GoInstallBinaries but it didn't help. I understand and probably agree about the complexity. It could be argued that this is a bug in the tools we are using and they they should either not hardcode the gopath, or should respect GOROOT. Edit: It turns out whatever powers :GoRename at least does respect this. After running :let $GOROOT="/Users/ash/.homebrew/Cellar/go/1.6.2/libexec" (the value of go env GOROOT) it started working.

@fatih
Copy link
Owner

fatih commented Jun 30, 2016

@ashb in your case, can you call :GoUpdateBinaries ? :GoInstallBinaries only installs it.

It's great that setting GOROOT is fixing it. Maybe I can initially set GOROOT as you did when vim-go is loaded and override the env inside the binaries. The only concern is that, we also would override the value if the user has set GOROOT already. But I can check if GOROOT has been set, and only override it if it's not set by the user already. How would that work ?

@ashb
Copy link

ashb commented Jul 7, 2016

:GoUpdateBinaries did work for me.

The main thing that was confusing to me as a user was that the tools sort just mysteriously stopped working. It would be nice if there was some way of detecting this and just telling the user "You need to :GoUpdateBinaries" -- but I'm immediately sure how we might be able to do that.

fatih added a commit that referenced this issue 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
@fatih
Copy link
Owner

fatih commented Jul 21, 2016

@FiloSottile @ashb

Can you please try the PR: #954 ? We're using a cached version of GOROOT now. It should be fast enough and also should fixed the problem we have.

fatih added a commit that referenced this issue Sep 27, 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
fatih added a commit that referenced this issue Sep 27, 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
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

No branches or pull requests

3 participants