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

Default branch, release-branch.v6 has some compile/import problems #877

Closed
snowzach opened this issue Aug 27, 2018 · 25 comments
Closed

Default branch, release-branch.v6 has some compile/import problems #877

snowzach opened this issue Aug 27, 2018 · 25 comments

Comments

@snowzach
Copy link
Contributor

snowzach commented Aug 27, 2018

I am about to switch to go modules but it appears the branch release-branch.v6 has some import problems when you just try to use it using old-fashioned GOPATH...

When you compile you get:

/go/src/github.com/olivere/elastic/client.go:24:2: cannot find package "github.com/olivere/elastic/v6/config" in any of:
        /usr/local/go/src/github.com/olivere/elastic/v6/config (from $GOROOT)
        /go/src/github.com/olivere/elastic/v6/config (from $GOPATH)
/go/src/github.com/olivere/elastic/bulk.go:14:2: cannot find package "github.com/olivere/elastic/v6/uritemplates" in any of:
        /usr/local/go/src/github.com/olivere/elastic/v6/uritemplates (from $GOROOT)
        /go/src/github.com/olivere/elastic/v6/uritemplates (from $GOPATH)

I realize you recommend to use a dependency manager however prior to your latest update it still worked okay.

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

Can you please specify what you’re doing exactly?

Tests are running fine (https://travis-ci.org/olivere/elastic/builds), and recipes are working as well. I just prepared Elastic for Go modules the other day.

@snowzach
Copy link
Contributor Author

I realize this isn't the recommended way but I just to do go get github.com/olivere/elastic and use it as a normal import github.com/olivere/elastic using GOPATH. Since I'm already working on elastic 6 this has always worked okay.

It looks like go get is checking out the default branch which is release-branch.v6 which includes a bunch of /v6/ paths that don't appear to be in the same branch.

@snowzach
Copy link
Contributor Author

Yah, that seems to be it... check out release-branch.v6 and client.go references import path "github.com/olivere/elastic/v6/config" which does not exist in that branch.

@thuandoan274
Copy link

Me too, I just glide install new package. Seem doesn't exist "github.com/olivere/elastic/v6/config". I just saw you committed change the path import

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

As Elastic is on version 6 already, the correct thing is to use Go modules as advertised and ask it to install v6 of the library. So you need to add that v6 at the end of the import path, then use go get and you should be fine. Read e.g. this blog post about that mysterious version in the import path.

I changed import paths to be github.com/olivere/elastic/v6 with version 6.2.0. If you need to stick to the old import path (or have a version before Go 1.10.4), ask e.g. dep to require version <6.2.0.

Notice that both glide and dep don't support Go modules as of now AFAIK.

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

Here's the PR in dep that's supposed to add minimal module awareness: golang/dep#1963.

@snowzach
Copy link
Contributor Author

Any chance you would consider changing the default branch to be something just less than <6.2.0 so it will still work for people using GOPATH until modules are a much more mainstream thing.

@MoritzGoeckel
Copy link

Using "github.com/olivere/elastic/v6" as import path and running go get still results for me in

package github.com/olivere/elastic/v6: cannot find package "github.com/olivere/elastic/v6" in any of:
/usr/local/go/src/github.com/olivere/elastic/v6 (from $GOROOT)
/home/moritz/Go/src/github.com/olivere/elastic/v6 (from $GOPATH)

Did I miss something? Sorry, Im quite new to Go

@snowzach
Copy link
Contributor Author

I have purposely avoided using dependency managers until now because I knew modules were coming and really only wanted to make the jump once. I've been using your library without issue for quite a while because I've kept current with elastic.

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

@snowzach Modules are supported for both Go 1.10 (with 1.10.4) and Go 1.11. That covers the release policy already used with Go itself.

You have different options. First is to vendor version 6.1.x. Second is to use something like dep and don't use Go modules at all. Third, if you really want github.com/olivere/elastic to be resolved from $GOPATH/src/github.com/olivere/elastic is to just git clone the repo and stick with version 6.1.x. The latter option is very dangerous, and I strongly advise against it from my own experience.

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

@snowzach Go modules are there, and you can use them with a recent version of Go. What's stopping you?

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

@MoritzGoeckel What version of Go are you on? What does go version print?

If you're using Go 1.10.4 or later, are you in a sub-directory of $GOPATH? If you are, please set export GO111MODULE=on and try go get ... again. Please also read this blog post.

@MoritzGoeckel
Copy link

@olivere export GO111MODULE=on did the trick for me. Go version is go1.11 linux/amd64.
Thank you!

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

@MoritzGoeckel Go 1.11 has Go modules disabled by default in $GOPATH/.... It is enabled by default outside of $GOPATH/.... So if you want to use Go modules inside $GOPATH/..., you need to manually turn it on with export GO111MODULE=on.

In the long run, Go is moving away from $GOPATH. I'm not happy with that tbh.

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

Everybody trying to figure out what's going on with Go 1.11 and Go modules, please try this recipe first:

# Go should be 1.11 (or 1.10.4)
$ go version
go1.11 linux/amd64

# Make sure to enable Go modules even inside GOPATH
$ export GO111MODULE=on

$ cd $GOPATH/src/github.com/olivere/elastic/recipes/connect_with_config
$ go run connect_with_config.go "http://127.0.0.1:9200/test-index?sniff=false&healthcheck=false"
...
Connection succeeded

Source

@snowzach
Copy link
Contributor Author

snowzach commented Aug 27, 2018

@olivere it would appear that some of my build process breaks when switching to modules. I rely on some generated code using go generate (against another package) and when doing modules it doesn't seem to allow that. I have to figure that piece out yet. I may just end up vendoring it unless someone knows about that. Obviously not related to this library though.

@olivere
Copy link
Owner

olivere commented Aug 27, 2018

@snowzach If I can help, let me know. From my experience: If you can use a recent version of Go, it will just work: You only need to change the import path.

@snowzach
Copy link
Contributor Author

Okay thanks for the offer.

@shousper
Copy link

shousper commented Aug 28, 2018

@olivere This is not an acceptable solution to our problem. The changes to the default branch paths are not backwards compatible for consumers who've not yet enabled the experimental module support.

EDIT: From the release page:

Module support is considered experimental, and there are still a few rough edges to smooth out, so please make liberal use of the issue tracker.

@olivere
Copy link
Owner

olivere commented Aug 28, 2018

I'd consider rolling back the changes. But I need to understand what's causing the issues: Changing the import path? Not being able to use Go modules with an existing dependency manager like dep? Not being able to update to a recent version of Go? I'd love to have a solution that would work for both users trying to use Go modules as well as the existing community.

@olivere olivere reopened this Aug 28, 2018
@shousper
Copy link

@olivere Thanks for your response.

My choice would be to keep the modules changes in a branch -- something go modules support 😄

Mostly I think if you can just release a v6 tag everyone can use with gopkg.in that imports from a gopkg.in path like you've done for v5, everyone should be sweet for a while.

Ideally though, if you want to support users on dep, glide, or those that just go get without dependency management, consider keeping the default branch free of any breaking changes for as long as you can stand it ;)

@olivere
Copy link
Owner

olivere commented Aug 28, 2018

My primary concern has always been to limit breaking changes. I didn't expect this to hurt users so deeply. While I still think changing the import path and supporting a current version of Go would be the correct way forward, I understand that some of you are not ready to do so right now.

With regards to the default branch on for github.com/olivere/elastic: There is a huge warning at the top of the README that you will be hurt from time to time if you're using the default branch of the repository. Always use a dependency manager and tagged releases if you want any kind of stability guarantee.

So 6.2.1 will revert the changes to the import path. I'm testing the impact as I'm writing this.

I'm deeply sorry for the confusion.

@olivere
Copy link
Owner

olivere commented Aug 28, 2018

I just released 6.2.2 which tests on Go 1.10 and 1.11 both with GO111MODULE=on and GO111MODULE=off.

Changes to the import path are reverted, so keep using github.com/olivere/elastic for v6. Go modules will probably have to wait for a while until tooling is ready, e.g. dep support for module-awareness.

However, as a reminder: DO NOT use the default branch. It will break from time to time. Use tagged releases!

@olivere olivere closed this as completed Aug 28, 2018
@shousper
Copy link

Thanks @olivere, fantastic response time! I really appreciate your effort on this.

@snowzach
Copy link
Contributor Author

Yes, thanks @olivere !

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

5 participants