cmd/go: mod download and go mod tidy conflict (1.16beta1 -> 1.16rc1 regression)

[howardjohn]

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

$ go version
go1.16rc1

Does this issue reproduce with the latest release?

Yes, it is only present in go1.16rc1, and not go1.16beta1

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

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/howardjohn/.cache/go-build"
GOENV="/home/howardjohn/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/howardjohn/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/howardjohn/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/howardjohn/sdk/go1.16rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/howardjohn/sdk/go1.16rc1/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16rc1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/howardjohn/go/src/istio.io/api/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1862906774=/tmp/go-build -gno-record-gcc-switches"

What did you do?

On 1.16beta1:

$ git status
On branch master
Your branch is up to date with 'origin/master'.

$ ~/go/bin/go1.16beta1 mod download
$ git status
On branch master
Your branch is up to date with 'origin/master'.

On 1.16rc1:

$ git status
On branch master
Your branch is up to date with 'origin/master'.
$ ~/go/bin/go1.16rc1 mod download
$ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   go.sum

$ ~/go/bin/go1.16rc1 mod tidy
$ git status
On branch master
Your branch is up to date with 'origin/master'.

Repo: https://github.com/istio/api

What did you expect to see?

Either go mod download does not add to go.sum, OR go mod tidy keeps what is added by go mod download. In either case, the two tools should be consistent

What did you see instead?

go mod download adds to go.sum, which go mod tidy removes

[bcmills]

go mod tidy downloads source code only for those modules that are needed to provide packages transitively imported by the packages and tests in the main module (go list -deps -test ./...). go mod tidy reduces the go.sum file to contain checksums for only those modules.

In contrast, go mod download downloads source code for all modules in the module graph (go list -m all; compare #41431). As a side-effect, it should also record the checksums for those modules.

If go mod download fails spuriously due to missing checksums (or due to an error writing the go.sum file), that's probably a bug we should fix, but if it is just recording more checksums than go mod tidy does, that's an unfortunate but intentional difference between the current behavior of the two commands.

CC @jayconrod @matloob

[bcmills]

(Note that lazy loading — #36460 — should bring go mod tidy and go mod download closer together, but even then go mod download will still potentially download more than go mod tidy.)

[howardjohn]

For more details on the impact: all of our CI enforces go mod tidy is run. Some of our CI was actually running go mod tidy then go mod download, resulting in it generating a git diff which is rejected. The fix there is easy - just run go mod tidy last.

However, in day to day use its pretty annoying that if I run go mod download my go.sum file is suddenly modified, and if I accidentally check it into git my PR will be rejected. I think this is a common practice in projects as well, and not unique to us to reject spurious go.sum changes.

but even then go mod download will still potentially download more than go mod tidy.

I am not sure there is a distinction in practice, but my concern is only about modifying/reverting go.sum - I don't mind if its downloading more or not.

[bcmills]

However, in day to day use its pretty annoying that if I run go mod download my go.sum file is suddenly modified, and if I accidentally check it into git my PR will be rejected.

Could you give some more detail about when you run go mod download in day-to-day use? I'm a little surprised that it's needed, and that may indicate that either we don't understand how people are using the tool or we need better documentation on the expected day-to-day workflow.

(As part of #36460 I've been mulling over whether we should add a go mod download flag to download only what's relevant to the main module, but today that's a little redundant with go list -test all.)

I think this is a common practice in projects as well, and not unique to us to reject spurious go.sum changes.

Agreed: we expect that a lot of projects will enforce that go mod tidy is a no-op for PRs. (See also #27005.)

[howardjohn]

Using go mod download in docker seems common for better layer caching.

There are cases where I want my IDE completion to work, but need the modules first. I suppose I could just run go mod tidy instead, but go mod download worked before so I've just used that. Perhaps its not a great reason, but I am a bit confused what go mod download is intended for if not this.

[bcmills]

For IDE completion, gopls should already be downloading the dependencies of the main module as needed. (If not, I'd suggest filing a separate issue for that.)

[bcmills]

I am a bit confused what go mod download is intended for if not this.

The short answer, I think, is that go mod download is not really the right tool for anything. 😅
(It is optimized for “give me a short command that does at least what I probably need and is easy to remember”.)

go list all is probably the thing you want to run to prime a Docker cache, and perhaps to prime a non-gopls IDE too.

[bcmills]

FWIW, the behavior change here was almost certainly CL 282692, for #41103.

[jayconrod]

+1 to Bryan's comments. The behavior has changed, but I think the new behavior is correct.

go mod download without arguments fetches every module in the build list (the modules printed by go list -m all). That tends to be much more than what you need to build and test the packages in the main module because it includes things like test dependencies of test dependencies. If you're also running go mod tidy, that already fetches the modules you need. go list -test all is another good option.

In earlier versions, go mod download was inconsistent with how it updated go.sum. It would add sums needed to load the module graph (that is, it would add /go.mod hashes), but it would not add sums for module content. In 1.16, we've fixed it so that go mod download saves sums for contents of modules in the build list. That's important for resolving some errors with -mod=readonly (which is the default in 1.16): there are some cases where you need to run go mod download somemod@someversion to add a specific sum.

I think that's the new behavior you're seeing here. I checked out github.com/istio/api, ran go mod download, then ran go mod why -m on each new sum in go.sum. None of those modules provide packages imported by the main module, so go mod tidy removes the sums.

[jeffwidman]

Note that this behavior was reverted in #45332 which was backported to 1.16.5... now go mod download doesn't fight go mod tidy for go.sum changes...