cmd/go: 'go mod -sync' adds test dependencies of dependencies, while 'go build' does not

[fzipp]

Please answer these questions before submitting your issue. Thanks!

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

go version devel +bd98a81 Thu Jul 26 17:13:12 2018 +0000 darwin/amd64

What did you do?

$ mkdir demo
$ cd demo
$ cat >demo.go <<EOF
package main

import _ "github.com/gdamore/tcell"

func main() {
}
EOF
$ go mod -init -module example.org/demo
$ go build
go: finding github.com/gdamore/encoding latest
go: finding github.com/lucasb-eyer/go-colorful latest
go: finding golang.org/x/text/encoding latest
go: finding golang.org/x/text/transform latest
$ cat go.mod 
module example.org/demo

require (
	github.com/gdamore/encoding v0.0.0-20151215212835-b23993cbb635 // indirect
	github.com/gdamore/tcell v1.1.0
	github.com/lucasb-eyer/go-colorful v0.0.0-20180709185858-c7842319cf3a // indirect
	github.com/mattn/go-runewidth v0.0.2 // indirect
	golang.org/x/text v0.3.0 // indirect
)
$ go mod -sync
go: finding github.com/smartystreets/goconvey/convey latest
go: finding github.com/smartystreets/goconvey latest
go: finding github.com/smartystreets/assertions latest
go: finding github.com/gopherjs/gopherjs/js latest
go: finding github.com/gopherjs/gopherjs latest
$ cat go.mod
module example.org/demo

require (
	github.com/gdamore/encoding v0.0.0-20151215212835-b23993cbb635 // indirect
	github.com/gdamore/tcell v1.1.0
	github.com/gopherjs/gopherjs v0.0.0-20180628210949-0892b62f0d9f // indirect
	github.com/jtolds/gls v4.2.1+incompatible // indirect
	github.com/lucasb-eyer/go-colorful v0.0.0-20180709185858-c7842319cf3a // indirect
	github.com/mattn/go-runewidth v0.0.2 // indirect
	github.com/smartystreets/assertions v0.0.0-20180725160413-e900ae048470 // indirect
	github.com/smartystreets/goconvey v0.0.0-20180222194500-ef6db91d284a // indirect
	golang.org/x/text v0.3.0 // indirect
	gopkg.in/DATA-DOG/go-sqlmock.v1 v1.3.0 // indirect
)

What did you expect to see?

I expected "go mod -sync" to add no more dependencies after "go build".

What did you see instead?

"go mod -sync" added dependencies of _test.go files (goconvey, gopherjs) of my dependency, which are unnecessary to build my module.

[andybons]

@bcmills @rsc

[thepudds]

Possibly related:

#26571 "cmd/go: clarify doc that go commands like 'go build' are not always sufficient to update go.mod and 'go mod -sync' is sometimes required to handle variability due to GOOS/GOARCH/build tags"

(Above issue #26571 is related in the sense that 'go mod -sync' is currently expected to do more than 'go build').

Separately, I think I have also seen reference to test dependencies getting picked up in go.mod and people saying it is expected? I think that makes sense in that you could imagine wanting to have control over your test dependencies in go.mod, but I don't personally know the intent.

[fzipp]

I think that makes sense in that you could imagine wanting to have control over your test dependencies in go.mod

Note that these are not the test dependencies of my module, but test dependencies of its dependencies. I don't see why I would need them in my go.mod.

[bcmills]

If you run go test all within your module (in all possible build tag configurations), you should find that it adds the same test dependencies as go mod -sync.

[bcmills]

More generally: you should usually run your dependencies' tests before cutting a release. If the combination of versions that your program ends up with exposes a bug in the interaction of two or more dependencies, you want to find out about it.

(The number of possible version combinations in general is exponential in the number of modules, so you cannot expect your dependencies to test against all possible combinations of their dependencies up-front.)

[fzipp]

If you run go test all within your module (in all possible build tag configurations), you should find that it adds the same test dependencies as go mod -sync.

Ok, then it probably works as intended.

[thepudds]

I think a common suggested workflow is doing 'go test ...' (or maybe 'go test all' -- not sure what the final syntax will be) to test your module (including your direct dependencies and your indirect dependencies) as a way of validating that the selected packages versions are compatible.

Related snippet from https://research.swtch.com/vgo-cmd:

In the vgo prototype, we have redefined all to have a single, consistent meaning: all the packages in the current module, plus all the packages they depend on through one a sequence of one or more imports.

The new all is exactly the packages a developer would need to test in order to sanity check that a particular combination of dependency versions work together, but it leaves out nearby packages that don't matter in the current module. For example, in the overview post, our hello module imported rsc.io/quote but not any other packages, and in particular not the buggy package rsc.io/quote/buggy. Running go test all in the hello module tests all packages in that module and then also rsc.io/quote. It omits rsc.io/quote/buggy, because that one is not needed, even indirectly, by the hello module, so it's irrelevant to test. This definition of all restores repeatability, and combined with Go 1.10's test caching, it should make go test all more useful than it ever has been.

edit: couple minor typos

[bcmills]

Ok, then it probably works as intended.

In that case I'll close out this issue, but please reopen if you find that go test all and go mod -sync disagree.

[thepudds]

Hi @bcmills, while this is fresh, is 'go test all' considered preferable to 'go test ...'?

[bcmills]

is 'go test all' considered preferable to 'go test ...'?

No idea. I think (?) that they're equivalent, but they're implemented separately at the moment.

I suspect that resolving this TODO will make them completely equivalent, but when I tried I ended up breaking tests. (Not sure if it's a bug in my change or a fundamental difference that I'm missing.)

go/src/cmd/go/internal/modload/load.go

Line 106 in bd98a81

// TODO: Don't we need to reevaluate this one last time once the build list stops changing?