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

cmd/go: mod tidy pulls in too many dependencies #27920

Closed
alicebob opened this issue Sep 28, 2018 · 11 comments

Comments

Projects
None yet
5 participants
@alicebob
Copy link

commented Sep 28, 2018

What version of Go are you using (go version)?

go version devel +38861b5127 Fri Sep 28 09:18:14 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

I did not try.

What operating system and processor architecture are you using (go env)?

...
GOARCH="amd64"
...
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
...

What did you do?

Make simple module which imports github.com/hashicorp/consul/api and uses go.mod. go.mod is good after a go build, but go mod tidy will pull in everything the whole github.com/hashicorp/consul/ repo depends on, which is not needed.

Make a new tiny module:

$ mkdir mod
$ go mod init github.com/alicebob/mod
$ cat > example.go <<HERE
package mod

import (
    "github.com/hashicorp/consul/api"
)

func Hello() {
    _ = api.NewClient
}
HERE

now run go build. go.mod is a small list of 5 dependencies:

module github.com/alicebob/mod

require (
	github.com/hashicorp/consul v1.2.3
	github.com/hashicorp/go-cleanhttp v0.5.0 // indirect
	github.com/hashicorp/go-rootcerts v0.0.0-20160503143440-6bb64b370b90 // indirect
	github.com/hashicorp/serf v0.8.1 // indirect
	github.com/mitchellh/mapstructure v1.0.0 // indirect
)

now run go mod tidy, after that this is go.mod:

module github.com/alicebob/mod

require (
	github.com/DataDog/datadog-go v0.0.0-20180822151419-281ae9f2d895 // indirect
	github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da // indirect
	github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 // indirect
	github.com/circonus-labs/circonus-gometrics v2.2.3+incompatible // indirect
	github.com/circonus-labs/circonusllhist v0.0.0-20180430145027-5eb751da55c6 // indirect
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/hashicorp/consul v1.2.3
	github.com/hashicorp/go-cleanhttp v0.5.0 // indirect
	github.com/hashicorp/go-immutable-radix v1.0.0 // indirect
	github.com/hashicorp/go-msgpack v0.0.0-20150518234257-fa3f63826f7c // indirect
	github.com/hashicorp/go-multierror v1.0.0 // indirect
	github.com/hashicorp/go-retryablehttp v0.0.0-20180718195005-e651d75abec6 // indirect
	github.com/hashicorp/go-rootcerts v0.0.0-20160503143440-6bb64b370b90 // indirect
	github.com/hashicorp/go-sockaddr v0.0.0-20180320115054-6d291a969b86 // indirect
	github.com/hashicorp/memberlist v0.1.0 // indirect
	github.com/hashicorp/serf v0.8.1 // indirect
	github.com/hashicorp/yamux v0.0.0-20180917205041-7221087c3d28 // indirect
	github.com/kr/pretty v0.1.0 // indirect
	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
	github.com/miekg/dns v1.0.10 // indirect
	github.com/mitchellh/go-homedir v1.0.0 // indirect
	github.com/mitchellh/go-testing-interface v1.0.0 // indirect
	github.com/mitchellh/mapstructure v1.0.0 // indirect
	github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c // indirect
	github.com/pkg/errors v0.8.0 // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	github.com/prometheus/client_golang v0.8.0 // indirect
	github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910 // indirect
	github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e // indirect
	github.com/prometheus/procfs v0.0.0-20180920065004-418d78d0b9a7 // indirect
	github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 // indirect
	github.com/stretchr/testify v1.2.2 // indirect
	github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926 // indirect
	golang.org/x/crypto v0.0.0-20180927165925-5295e8364332 // indirect
	golang.org/x/net v0.0.0-20180926154720-4dfa2610cdf3 // indirect
	golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f // indirect
	golang.org/x/sys v0.0.0-20180927150500-dad3d9fb7b6e // indirect
	google.golang.org/appengine v1.2.0 // indirect
	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
	gopkg.in/vmihailenco/msgpack.v2 v2.9.1 // indirect
	labix.org/v2/mgo v0.0.0-20140701140051-000000000287 // indirect
	launchpad.net/gocheck v0.0.0-20140225173054-000000000087 // indirect
)

most of these are not needed because the module only uses the /api package from consul, and it doesn't need to build the whole server. This is especially bad when vendoring.

What did you expect to see?

The small version of go.mod

What did you see instead?

The huge version of go.mod

@myitcv

This comment has been minimized.

@myitcv myitcv closed this Sep 28, 2018

@myitcv myitcv changed the title `go mod tidy` pulls in too many dependencies cmd/go: mod tidy pulls in too many dependencies Sep 28, 2018

@myitcv myitcv added the modules label Sep 28, 2018

@alicebob

This comment has been minimized.

Copy link
Author

commented Sep 28, 2018

I'm not sure I agree. That link talks about build tags and all active modules, but what happens here is that clearly inactive modules are pulled in.

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 29, 2018

go mod why -m $module_path should be able to give you an import path to each module. If it can't, then there is a bug somewhere.

@thepudds

This comment has been minimized.

Copy link

commented Sep 29, 2018

There is more discussion of this particular issue #27920 in a recent golang-nuts thread (also started by @alicebob, I think), where that thread might have answered at least some of the questions raised in this issue:
https://groups.google.com/forum/#!topic/golang-nuts/Cl--P02fCjw

@alicebob, in terms of your comment above about "clearly inactive modules are pulled in", the modules documentation has a particular definition for the phrase "active modules" (from https://tip.golang.org/cmd/go/#hdr-List_packages_or_modules, with emphasis added):

The main module is the module containing the current directory. The active modules are the main module and its dependencies.

In any event, setting aside for the moment if you think the go tooling should behave in a different manner, do you see anywhere that go mod why -m $module_path does not give you an explanation of why a dependency is present?

@alicebob

This comment has been minimized.

Copy link
Author

commented Sep 29, 2018

@thepudds thanks for your replies. Yeah, I posted that to get more answers.

go mod why does explain all modules in my example repo. I did manage to get a module in a go.mod which wasn't used; I think it was a sequence of running tidy, and then using go get to upgrade a module. But I didn't manage to reproduce that yet.

So I guess things work indeed mostly "as intended" :(

@alicebob

This comment has been minimized.

Copy link
Author

commented Sep 29, 2018

I see now that vendor does not include all the modules only included in tests by vendored modules. So what's in go.mod is not simply what's added in the vendor folder, and it's smarter than that.

Great! I missed that completely. It is indeed described in go help mod vendor. My bad.

To answer my own initial question: yes, go.mod is long, but since that's not the list of modules which get vendored it doesn't matter.

Thanks @thepudds and @myitcv!

@sagikazarmark

This comment has been minimized.

Copy link

commented Nov 13, 2018

Here is a case which is not clear to me and I think is related to this issue:

Whenever a dependency is added to the code I can just run go get or go build and it will automatically be added to go.mod with all the required dependencies.
The counterpart of this behavior is whenever I remove some code importing dependencies and those dependencies are not imported anywhere else in my module, they should get removed.

This is how I think the majority of projects work and worked before using tools like dep and glide.

Compared to this go mod tidy adds all transitive dependencies (even ones not required to build my project) to the go.mod file.

Now, I understand the reasons (I think) why it works like that, but as I said before, I don't think (the majority of) Go projects work or want to work like that. Let me give an example:

It was mentioned in the google groups thread (and it is my understanding as well), that in a CI all dependencies in the go.mod files will be downloaded. This can be a huge amount of dependencies making CI builds very slow. Imagine a library providing some sort of kubernetes integration (in a separate package of the module). Even if I don't use that, the kubernetes repo will be downloaded (since it's in the project's go.mod and it should be), which is friendly 2GB in size.

I think in most of the cases we need a solution that keeps the go.mod an absolute minimum with the dependencies that are directly or indirectly imported during go build or go test.

I understand that there can be cases when one would like to run tests for all the dependencies, but I also think that's probably not the majority of the projects. (Sticking to the example: I don't want to run the test suite of kubernetes)

In any case, it would be nice to have an option for the above, or at least an option to limit "tidying" to removal of unused dependencies, leaving new dependencies to go build.

I may not have all the facts right, but after a few hours of reading and playing around this is what I ended up with.

@bcmills

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

whenever I remove some code importing dependencies and those dependencies are not imported anywhere else in my module, they should get removed.

Note that “not imported anywhere else” is an expensive property to detect. That's why go mod tidy is a separate operation.

Compared to this go mod tidy adds all transitive dependencies (even ones not required to build my project) to the go.mod file.

go mod tidy adds all dependencies needed to satisfy the (transitive) imports of the packages in your module, including their tests. go mod why should be able to explain each of those dependencies. If you find an example for which that is not the case, please file a separate issue with steps to reproduce it.

I think in most of the cases we need a solution that keeps the go.mod an absolute minimum with the dependencies that are directly or indirectly imported during go build or go test.

In the general case, the go.mod file is the minimal set of dependencies required to make go test all reproducible.


in a CI all dependencies in the go.mod files will be downloaded. This can be a huge amount of dependencies making CI builds very slow.

According to the documentation:

With no arguments, download applies to all dependencies of the main module.

It's not obvious to me whether “dependencies of the main module” should refer to package dependencies or module dependencies (that's #26902), but in this case it doesn't matter, because there is an alternative that downloads only the dependencies of the main module:

go list -test -deps ./...

which tells the go command to resolve the transitive non-test dependencies of all packages and tests in the main module (and necessarily downloads them to the cache in the process).

@sagikazarmark

This comment has been minimized.

Copy link

commented Nov 14, 2018

Thanks for clarification. The only missing thing for me then is the remove functionality of tidy. Since in most of the cases I don't want to run go test all, go mod tidy is probably not for me.

To be honest go mod feels a bit fragile for this use case: one accidental go mod tidy and my go.mod is doomed forever. (Obviously I can try to revert, given I notice it in time, or remove it and start over, remove indirect deps, but I think non of those are optimal solutions)

I wonder though if I'm right about my assumption, that a majority of Go projects doesn't want to use go test all, thus go mod tidy simply won't be used, despite it having the removal of unnecessary dependencies.

@thepudds

This comment has been minimized.

Copy link

commented Jan 12, 2019

@sagikazarmark This issue was closed, and as far as I am following things here, the original problem reporter seemed to indicate they were satisfied with the discussion here:

Great! I missed that completely. It is indeed described in go help mod vendor. My bad.

To answer my own initial question: yes, go.mod is long, but since that's not the list of modules which get vendored it doesn't matter.

Thanks

Reading over again what you wrote here, it might be a bit different than the original problem report?

Would it make sense to open a new issue for what you are reporting, or do you think your open questions/issues are already covered by another set of issue(s), or is your view that this issue here should be re-opened?

Sorry, just trying to see what is pending, vs. not.

@sagikazarmark

This comment has been minimized.

Copy link

commented Jan 13, 2019

@thepudds I think I'm going to open a new issue (unless I found another one). Although I understand the explanation given in this issue, after experimenting with modules for a while I still think the behavior above is undesirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.