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

all: update dependencies #1155

Merged
merged 2 commits into from
Jan 23, 2019
Merged

all: update dependencies #1155

merged 2 commits into from
Jan 23, 2019

Conversation

vangent
Copy link
Contributor

@vangent vangent commented Jan 22, 2019

This was a pretty frustrating process. I expected to run go get -u and go mod tidy and be done, but it didn't work that way.

go get -u doesn't work very well. When I originally ran it, I got an error in a module we don't even use: testcontainers/testcontainers-go#47. I ended up working around that by deleting everything in go.mod and letting go build rebuilt it.

Even now, when I run go get -u, a whole bunch of things change in go.mod that I don't expect given that I just basically got the latest of everything. Many of them are implicit dependencies; I don't understand why those need to be added given that everything seems to work fine without them.

The experience with a couple of our dependencies is not great either, for example coreos. Rebuilding everything from a fresh go.mod file results in weird compile errors. I compared the resulting go.mod/go.sum to the original one, and figured out the right incantations of manual go get commands that fix it, but this was painful and manual, sometimes involving inspections of the code to see what the compile error was about and what library it might come from (e.g., to get hashicorp to compile, I had to manually fetch a specific version of SermoDigital, whatever that is).

  1. Delete everything in the require section of go.mod
  2. go build ./... && go test ./... # ctrl-c after gcppubsub test hangs
  3. go get google.golang.org/grpc@v1.17.0 due to pubsub/gcppubsub: tests hang after upgrade to grpc v1.18.0 #1151
  4. go get github.com/coreos/bbolt@v1.3.1-coreos.6 # etc.d madness
  5. go get github.com/coreos/etcd@v3.3.10 # etc.d madness
  6. go get github.com/ugorji/go/codec@v0.0.0-20181012064053-8333dd449516 # etc.d madness
  7. go get github.com/SermoDigital/jose@v0.9.2-0.20161205224733-f6df55f235c2 # hashicorp madness
  8. rm go.sum
  9. go build ./... && go test ./...
  10. Repeat the above (minus the individual go get commands) for ./internal/contributebot and ./samples/appengine.

@vangent vangent requested a review from jba January 22, 2019 23:58
@googlebot googlebot added the cla: yes Google CLA has been signed! label Jan 22, 2019
@jba
Copy link
Contributor

jba commented Jan 23, 2019

@bcmills Consider this a friction report for module-mode go get -u. Any tips to avoid this pain in the future?

@vangent vangent merged commit b7bcbaf into google:master Jan 23, 2019
@vangent vangent deleted the deps branch January 23, 2019 16:48
@bcmills
Copy link

bcmills commented Jan 24, 2019

Any tips to avoid this pain in the future?

@bcmills
Copy link

bcmills commented Jan 24, 2019

Can you give some more detail on the “etc.d madness”?

@vangent
Copy link
Contributor Author

vangent commented Jan 24, 2019

Sure. When I deleted everything in go.mod, some coreos stuff (for etcd) didn't compile, with errors like this:

# github.com/coreos/etcd/mvcc/backend
../../../../pkg/mod/github.com/coreos/etcd@v3.3.11+incompatible/mvcc/backend/config_linux.go:31:2: unknown field 'NoFreelistSync' in struct literal of type bolt.Options

There were also errors like this:

panic: codecgen version mismatch: current: 8, need 10. Re-generate file: /usr/local/google/home/rvangent/go/pkg/mod/github.com/coreos/etcd@v3.3.10+incompatible/client/keys.generated.go

Through trial and error of manually doing "go get" of the previous versions of various coreos modules, and one of its dependencies (github.com/ugorji/go/codec), I got things to work.

More recently I was able to update to the latest of etcd itself (#1173). I'm not sure what the difference is between deleting everything in go.mod and starting from scratch, vs doing go get -u xxx@latest.

@thepudds
Copy link

thepudds commented Jan 24, 2019

@vangent

One question: when you say you want to run go get -u, are you:
(a) looking for the latest version of all of your direct and indirect dependencies?
(b) looking for the latest version of your direct dependencies (and then hoping to use whatever version of their dependencies that they might have used with their last release)?
(c) something else entirely?

If you are interested in (a), as you have seen, that can present some challenges, especially given this is a transitional time period.

If we take the example of github.com/coreos/etcd, if you run go get -u, you are upgrading dependencies of github.com/coreos/etcd beyond the latest set of versions used by the latest stable github.com/coreos/etcd, which likely explains some of the version mismatches you hit.

If you are instead interested in (b), one approach you can use is to bootstrap your versions to get a consistent set of recent github.com/coreos/etcd dependencies by cloning github.com/coreos/etcd, then try to use the dependency information from https://github.com/etcd-io/etcd/blob/v3.3.11/glide.lock, where you might be able to have go mod init do most of the work for you.

Sorry if that is not a great explanation, but the following FAQ covers the technique in a bit more detail:

"FAQ: I have a problem with a complex dependency that has not opted in to modules. Can I use information from its current dependency manager?"

And here is a concrete example in golang/go#28489 (comment) of following that technique (for docker, in that example).

@bcmills
Copy link

bcmills commented Jan 24, 2019

I'm not sure what the difference is between deleting everything in go.mod and starting from scratch, vs doing go get -u xxx@latest.

If you delete everything in go.mod and start from scratch, you end up adding concentric rings of dependencies at their latest versions. If any of those dependencies has a go.mod file (as coreos does), then as soon as you pick up that go.mod file you also pick up its transitive dependencies at the version declared in that go.mod file, unless some other module specifies newer versions of those dependencies.

In contrast, go get -u xxx@latest upgrades all of the transitive dependencies of xxx to their latest versions.

Perhaps what you wanted instead was go get github.com/coreos/etcd@latest, without -u. That would upgrade github.com/coreos/etcd pointwise, without upgrading its dependencies further than necessary.

@thepudds
Copy link

thepudds commented Jan 24, 2019

If any of those dependencies has a go.mod file (as coreos does),

github.com/coreos/etcd has a go.mod file in master, but not in the latest tagged release (as far as I noticed in quick look, anyway).

Things likely could have been smoother here if github.com/coreos/etcd did have a go.mod file in a released version.

edit: part of the reason I say that is because the technique described in the FAQ two comments back #1155 (comment) would not be useful if there was a go.mod file. That technique described there gives you similar benefits, but requires a couple of manual steps to get the dependency information out of the older dependency manager (glide, in this case).

@vangent
Copy link
Contributor Author

vangent commented Jan 25, 2019

I want to run go get -u and get all of my direct dependencies update to the latest, and have it work. So, b), but I don't want to have to do some manual stuff for each individual dependency (since there are dozens).

@thepudds
Copy link

thepudds commented Jan 25, 2019 via email

@vangent
Copy link
Contributor Author

vangent commented Jan 25, 2019

Thanks @thepudds .

@bcmills
Copy link

bcmills commented Jan 25, 2019

A few more diagnostics on github.com/coreos/etcd:

All of that suggests to me that github.com/coreos/etcd is not a stable dependency, and you very much don't want to rely on it. So you probably want to figure how how it's getting in and remove it.

Unfortunately, the latest tagged version of the go.etcd.io repository is v3.3.11, which was at the old import path (and has internal imports assuming that path). The repo (and import path) probably should have moved to go.etcd.io/etcd/v4 at the time of the import-path change, since a new import path is most decidedly a breaking change. (See etcd-io/etcd#10398.)

You can observe this problem at HEAD using go mod why:

~/src/gocloud.dev$ go mod why -m github.com/coreos/etcd
# github.com/coreos/etcd
gocloud.dev/runtimevar/etcdvar
go.etcd.io/etcd/clientv3
github.com/coreos/etcd/auth/authpb

@bcmills
Copy link

bcmills commented Jan 25, 2019

If you upgrade go.etcd.io/etcd/clientv3 past v3.3.11+incompatible to master, you end up with another problem: it depends on github.com/ugorji/go v1.1.1, but github.com/ugorji/go has since added a go.mod file that breaks out codec into its own module.

That results in a build break, which I have filed as ugorji/go#279.

The workaround in the interim is to upgrade the root module past the break manually:

go get github.com/ugorji/go@v1.1.2-0.20180831062425-e253f1f20942

@bcmills
Copy link

bcmills commented Jan 25, 2019

That, unfortunately, leaves some indirect dependencies in play:

~/src/gocloud.dev$ go mod why -m github.com/coreos/etcd
# github.com/coreos/etcd
gocloud.dev/secrets/vault
github.com/hashicorp/vault/api
github.com/hashicorp/vault/api.test
github.com/hashicorp/vault/helper/builtinplugins
github.com/hashicorp/vault/builtin/logical/mongodb
github.com/hashicorp/vault/builtin/logical/mongodb.test
gopkg.in/ory-am/dockertest.v2
gopkg.in/ory-am/dockertest.v2.test
github.com/coreos/etcd/clientv3

However, I'm not sure whether that dependency actually causes any problems in practice: it might not break the packages that happen to be in the transitive import graph of the test.

@bcmills
Copy link

bcmills commented Jan 25, 2019

Unfortunately, with those updates go build all still fails:

~/src/gocloud.dev$ go build all
../../pkg/mod/github.com/gotestyourself/gotestyourself@v2.2.0+incompatible/assert/assert.go:74:2: use of internal package gotest.tools/internal/format not allowed
../../pkg/mod/github.com/gotestyourself/gotestyourself@v2.2.0+incompatible/assert/assert.go:75:2: use of internal package gotest.tools/internal/source not allowed
../../pkg/mod/github.com/gotestyourself/gotestyourself@v2.2.0+incompatible/assert/cmp/compare.go:11:2: use of internal package gotest.tools/internal/format not allowed
../../pkg/mod/github.com/gotestyourself/gotestyourself@v2.2.0+incompatible/assert/cmp/result.go:9:2: use of internal package gotest.tools/internal/source not allowed

~/src/gocloud.dev$ go mod graph | grep gotestyourself
github.com/influxdata/platform@v0.0.0-20190117200541-d500d3cf5589 github.com/gotestyourself/gotestyourself@v2.2.0+incompatible

~/src/gocloud.dev$ go mod why -m github.com/gotestyourself/gotestyourself
# github.com/gotestyourself/gotestyourself
gocloud.dev/secrets/vault
github.com/hashicorp/vault/api
github.com/hashicorp/vault/api.test
github.com/ory/dockertest
github.com/ory/dockertest/docker
github.com/ory/dockertest/docker/opts
github.com/ory/dockertest/docker/types
github.com/ory/dockertest/docker/types/filters
github.com/ory/dockertest/docker/types/filters.test
github.com/gotestyourself/gotestyourself/assert

~/src/gocloud.dev$ go mod why -m github.com/influxdata/platform
# github.com/influxdata/platform
gocloud.dev/secrets/vault
github.com/hashicorp/vault/api
github.com/hashicorp/vault/api.test
github.com/hashicorp/vault/helper/builtinplugins
github.com/hashicorp/vault/plugins/database/influxdb
github.com/influxdata/influxdb/client/v2
github.com/influxdata/influxdb/models
github.com/influxdata/platform/models

Honestly, as far as I can tell the (transitive) dependencies of github.com/hashicorp/vault are just a mess. (Perhaps hashicorp/vault#6088 would help?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA has been signed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants