cmd/go: go mod tidy reverts upgrade

[myitcv]

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

$ go version
go version devel +5d9549debb Thu May 7 02:47:46 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
GOAMD64="alignedjumps"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/tmp.LuGUD8I5CH/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-build548100936=/tmp/go-build -gno-record-gcc-switches"

What did you do?

cd $(mktemp -d)
go mod init mod.com
cat <<EOD > tools.go
// +build tools

package tools

import (
        _ "golang.org/x/tools/gopls"
        _ "honnef.co/go/tools/cmd/staticcheck"
)
EOD
go get golang.org/x/tools@v0.0.0-20200509030707-2212a7e161a5 golang.org/x/tools/gopls@v0.0.0-20200509030707-2212a7e161a5
go get honnef.co/go/tools@v0.0.1-2020.1.0.20200510173921-01541ecfc85e

At this point you should see go.mod as:

module mod.com

go 1.15

require (
        github.com/sergi/go-diff v1.1.0 // indirect
        golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5 // indirect
        honnef.co/go/tools v0.0.1-2020.1.0.20200510173921-01541ecfc85e // indirect
        mvdan.cc/xurls/v2 v2.1.0 // indirect
)

Now run go mod tidy and go.mod looks like this:

module mod.com

go 1.15

require (
        golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5 // indirect
        golang.org/x/tools/gopls v0.4.0
        honnef.co/go/tools v0.0.1-2020.1.3
)

Notice how both golang.org/x/tools/gopls and honnef.co/go/tools have reverted to earlier versions that those requested via go get above.

What did you expect to see?

As above.

What did you see instead?

As above

Is this related to #27899?

cc @bcmills @jayconrod

[jayconrod]

After the first go get command, there's a transitive dependency on honnef.co/go/tools:

$ go list -modfile=1.mod -m honnef.co/go/tools
honnef.co/go/tools v0.0.1-2020.1.3

The second go get command downgrades that module to an older version (as far as MVS is concerned).

$ go list -modfile=2.mod -m honnef.co/go/tools
honnef.co/go/tools v0.0.1-2020.1.0.20200510173921-01541ecfc85e

In order to perform the downgrade, go get will downgrade or remove modules that depend on newer versions of honnef.co/go/tools. There is no such version of golang.org/x/tools/gopls, so that module is removed.

There isn't a way to use these three versions together while keeping MVS happy. go get will complain if you try.

$ go get -d golang.org/x/tools@v0.0.0-20200509030707-2212a7e161a5 golang.org/x/tools/gopls@v0.0.0-20200509030707-2212a7e161a5 honnef.co/go/tools@v0.0.1-2020.1.0.20200510173921-01541ecfc85e
go get: inconsistent versions:
	golang.org/x/tools/gopls@v0.0.0-20200509030707-2212a7e161a5 requires honnef.co/go/tools@v0.0.1-2020.1.3 (not honnef.co/go/tools@v0.0.1-2020.1.0.20200510173921-01541ecfc85e)

go mod tidy will restore the dependency on golang.org/x/tools/gopls at the latest version, and it will upgrade honnef.co/go/tools to the minimum version required by golang.org/x/tools/gopls. That doesn't need to be written in go.mod, so the requirement is removed.

[jayconrod]

Thinking about what we should do here:

#33284 is related. Each go get command should print upgrades, downgrades, additions, and removals. That would make it more clear what's happening.

In general, the behavior of removing requirements during a downgrade seems strange to me. They'll almost always be added back by the next build command or go mod tidy. Maybe it would be better for the second go get command to fail with an error.

cc @matloob

[myitcv]

The second go get command downgrades that module to an older version (as far as MVS is concerned).

This is not a downgrade; it's a later version. Is there an issue therefore with the pseudo version calculated for the later version?

$ git log -1 v0.0.1-2020.1.3
commit 508b5eb18ee2f667ce06235ed383647038e2afc5 (tag: v0.0.1-2020.1.3, tag: 2020.1.3, origin/release.2020.1)
Author:     Dominik Honnef <dominik@honnef.co>
AuthorDate: Sat Feb 22 12:45:36 2020 +0100
Commit:     Dominik Honnef <dominik@honnef.co>
CommitDate: Sat Feb 22 12:51:38 2020 +0100

    Version 2020.1.3
$ git log -1 01541ecfc85e
commit 01541ecfc85eb4c7488ad2fac0bdf94e52782e41
Author:     Dominik Honnef <dominik@honnef.co>
AuthorDate: Sun May 10 19:39:21 2020 +0200
Commit:     Dominik Honnef <dominik@honnef.co>
CommitDate: Sun May 10 19:39:21 2020 +0200

    lint: skip work for non-initial packages

    Don't waste cycles trying to collect and filter problems from
    non-initial packages; they have none. This also fixes a bug where we
    would complain about unmatched linter directives in dependencies.

[jayconrod]

It's chronologically newer, but MVS just looks at the versions themselves, and v0.0.1-2020.1.3 is higher than that pseudo-version.

In this case, it doesn't look like it's possible to construct a valid pseudo-version for the same commit that compares higher than v0.0.1-2020.1.3. The base version of a pseudo-version must correspond to a tagged release or pre-release that's an ancestor of the pseudo-version commit. That would be written as v0.0.1-2020.1.3.0.20200510173921-01541ecfc85e. But it doesn't look like v0.0.1-2020.1.3 is an ancestor of 01541ecfc85e.

go: honnef.co/go/tools@v0.0.1-2020.1.3.0.20200510173921-01541ecfc85e: invalid pseudo-version: revision 01541ecfc85e is not a descendent of preceding tag (v0.0.1-2020.1.3)

[myitcv]

Ah thank you for that analysis. In which case I think this is an issue to raise on @dominikh's side because the situation is a function of the versioning strategy for that project.

Thanks very much @jayconrod - would you like me to leave this open for the other points you raised above?

[jayconrod]

Sounds good, let's close this then.

I'd still be curious what folks think about go get reporting an error instead of removing dependencies during a downgrade. If that sounds like a good idea, I can open a new issue for that specifically.

[dominikh]

This seems to make it difficult to use release branches? All my tagged releases are on release branches, at commits that are specific to the release branch. No tagged release will be an ancestor of the master branch. How would a user create a pseudo-version for a commit referring to master in that case?

[myitcv]

Re-opening on the basis @dominikh's comment seems related to the issue as reported. @jayconrod - please close/redirect as appropriate, just didn't want you to miss the response.

[bcmills]

@dominikh, if you apply tags only to release branches, then you should also tag corresponding pre-release tags to the development branch to inform the go command of the relative ordering between the two branches.

For example, if you cut a v1.2 release branch and tag it v1.2.0, then you could tag the commit following it on the main branch as v1.3.0-0.dev, so that all pseudo-versions after that point are treated as higher than any v1.2 but lower than v1.3.0-alpha.1.

[dominikh]

Thank you @bcmills, I will explore that and plan fixing my tags :-)

[mvdan]

Maybe we should document this somewhere clearly. I've done bugfix releases in release branches via cherry-picking in a few projects (just like the Go repo does), so I probably have the same issue - I just never noticed.

[jayconrod]

CL 229678 adds some documentation about pseudo-versions. Most of the detail is already in go help modules though.

https://tip.golang.org/ref/mod#go-get will be the reference on go get. Perhaps that should say more about downgrades. There are some TODOs to provide more examples and diagrams in there.

What else would be helpful?

[myitcv]

What else would be helpful?

The advice in #38985 (comment) for module authors regarding release branches I suspect?

[jayconrod]

Sounds good. I've added a note to #33637 to document this.

[mvdan]

@jayconrod golang/mock has this same bug today; master is newer software, but it gets the version v1.4.4-0.20210128224731-b9a8584115bc since the v1.4.4 tag was done in a separate branch.

Have the authoritative docs for this been written yet? I saw the downgrade section, but that's not what I want here. I want something like #38985 (comment) :)

I can use that link for now, but I just want to ensure this is being tracked. I could not find a reference to "branch" or "downgrade" in #33637 (other than the section I already linked to).

[mvdan]

I'm also just realising that this is a more widespread problem than we think. my mvdan.cc/sh/v3 module uses branches for backporting into stable versions, so it has the same out-of-order @master problem. github.com/tendermint/tendermint is another such module where v0.34.x releases come from a backporting branch, and @master is most likely incorrectly behind.

For those two modules, it probably doesn't cause actual bugs in practice, since it is not common for many modules to indirectly depend on either of them at the same time, forcing a MVS upgrade. I imagine it happened with golang/mock since it's a common test dependency; if you want an "older but actually newer" v1.4.4-0.20210128224731-b9a8584115bc and one of your dependencies wants the "newer but actually older" v1.4.4, you get the bug.

[jayconrod]

@mvdan I don't think this is explicitly called out in https://golang.org/ref/mod yet. I need to go through comments in #33637 and make sure everything is covered before closing that issue out.

This seems like something that should be called out in Pseudo-versions with cross-references in a couple other relevant sections.

[mvdan]

Ah, thanks. I had entirely missed #33637 (comment) as it was hidden by GitHub.

[tessr]

Thanks @mvdan for bringing this to our (Tendermint's) attention as an example team causing problems with our release tags :)

One thought, from Tendermint's perspective: In an ideal world, people wouldn't actually be able to automatically "upgrade" to master at all, because we really want people to use releases. Although master is stable in the sense that we expect it to work, pass CI, etc, it is not stable in the sense that the APIs change somewhat willy-nilly and we often make other breaking changes. If someone inadvertently upgrades to master from a release tag, they will be in for a bad time!

[mvdan]

In an ideal world, people wouldn't actually be able to automatically "upgrade" to master at all

I think this already happens if your tags and branches follow semver. If a user is on the latest v0.34.2 tag, any automatic upgrades or go get -d github.com/tendermint/tendermint@latest would do nothing, as it's already the latest tag. At most, it would upgrade the user to a v0.34.3 or 0.35.0 stable tag. It's only the explicit go get -d .../tendermint@master that would pick up an unstable master version.