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: go mod tidy reverts upgrade #38985

Closed
myitcv opened this issue May 10, 2020 · 20 comments
Closed

cmd/go: go mod tidy reverts upgrade #38985

myitcv opened this issue May 10, 2020 · 20 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented May 10, 2020

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

@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels May 10, 2020
@jayconrod
Copy link
Contributor

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
Copy link
Contributor

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

@jayconrod jayconrod added this to the Backlog milestone May 11, 2020
@myitcv
Copy link
Member Author

myitcv commented May 11, 2020

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
Copy link
Contributor

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
Copy link
Member Author

myitcv commented May 11, 2020

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member Author

myitcv commented May 11, 2020

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.

@myitcv myitcv reopened this May 11, 2020
@bcmills
Copy link
Contributor

bcmills commented May 11, 2020

@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
Copy link
Member

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

@mvdan
Copy link
Member

mvdan commented May 11, 2020

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
Copy link
Contributor

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
Copy link
Member Author

myitcv commented May 11, 2020

What else would be helpful?

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

@jayconrod
Copy link
Contributor

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

@mvdan
Copy link
Member

mvdan commented Jan 29, 2021

@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
Copy link
Member

mvdan commented Jan 29, 2021

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
Copy link
Contributor

@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
Copy link
Member

mvdan commented Feb 3, 2021

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

@tessr
Copy link

tessr commented Feb 9, 2021

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
Copy link
Member

mvdan commented Feb 9, 2021

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.

@golang golang locked and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants