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: 'panic: mistake: chose version "v0.0.N" instead of target' when an argument to 'go get' requires a newer version of itself #31491

Closed
raulk opened this issue Apr 16, 2019 · 13 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@raulk
Copy link

raulk commented Apr 16, 2019

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

$ go version
go version go1.12.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes, using the latest release available at this time.

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

go env Output
$ go env
❯ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="--homedir--/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/raul/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="--workspace--/go-libp2p/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/tv/ly66_c6121jg370fvnfcb_mh0000gn/T/go-build064039334=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

With this go.mod file, I tried to upgrade go-libp2p-peerstore to the latest commit in the consolidate-skeleton branch:

> go get github.com/libp2p/go-libp2p-peerstore@consolidate-skeleton;

Note that this was part of a batch update, where all go get commands succeeded except for this one, which failed both inside the batch and in isolation:

go get github.com/libp2p/go-conn-security@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-blankhost@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-discovery@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-host@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-interface-connmgr@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-interface-pnet@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-metrics@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-net@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-peer@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-peerstore@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-protocol@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-routing@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-secio@consolidate-skeleton;
go get github.com/libp2p/go-libp2p-transport@consolidate-skeleton;
go get github.com/libp2p/go-stream-muxer@consolidate-skeleton;

What did you expect to see?

go.mod successfully updated to reference the latest commit on the consolidate-skeleton branch of go-libp2p-peerstore.

What did you see instead?

❯ go get github.com/libp2p/go-libp2p-peerstore@consolidate-skeleton;
go: finding github.com/libp2p/go-libp2p-peerstore consolidate-skeleton
panic: mistake: chose version "v0.0.2" instead of target {Path:github.com/libp2p/go-libp2p-peerstore Version:v0.0.2-0.20190416173150-2ca0254625f6}

goroutine 1 [running]:
cmd/go/internal/mvs.buildList(0x7ffeefbff687, 0x25, 0xc000276630, 0x24, 0x16c8300, 0xc000448cb0, 0x0, 0x1e, 0x0, 0x0, ...)
	/usr/local/Cellar/go/1.12.4/libexec/src/cmd/go/internal/mvs/mvs.go:125 +0xc88
cmd/go/internal/mvs.BuildList(...)
	/usr/local/Cellar/go/1.12.4/libexec/src/cmd/go/internal/mvs/mvs.go:73
cmd/go/internal/modget.runGet(0x1a48c60, 0xc000020050, 0x1, 0x1)
	/usr/local/Cellar/go/1.12.4/libexec/src/cmd/go/internal/modget/get.go:487 +0x1283
main.main()
	/usr/local/Cellar/go/1.12.4/libexec/src/cmd/go/main.go:219 +0x7a4
@jayconrod jayconrod added modules NeedsFix The path to resolution is known, but the work has not been done. labels Apr 16, 2019
@jayconrod jayconrod added this to the Go1.13 milestone Apr 16, 2019
@jayconrod jayconrod self-assigned this Apr 16, 2019
@jayconrod
Copy link
Contributor

Thanks for reporting!

Reproduced with 1.12.4 and tip.

@raulk
Copy link
Author

raulk commented Apr 16, 2019

Thanks for the quick turnaround, @jayconrod! I tried setting the upstream version manually inside go.mod, but performing a build resets it back to v0.0.2. Curious to understand what’s special about this module, and if there’s a workaround I can try. Thanks!

@cuonglm
Copy link
Member

cuonglm commented Apr 16, 2019

Sounds like we currently generate wrong pseudo version. For branch consolidate-skeleton, the pseudo version is 0.0.2-0.20190416173150-2ca0254625f6, which is less than 0.0.2, so minimum version set wrongly:

if v, ok := min[m.Path]; !ok || reqs.Max(v, m.Version) != v {

@cuonglm
Copy link
Member

cuonglm commented Apr 17, 2019

@raulk The issue is that your consolidate-skeleton does not contains v0.0.2 tag commit, so recent tag returns v0.0.1 instead, causing pseudo version wrong:

➜  go-libp2p-peerstore git:(consolidate-skeleton) git log --oneline --decorate=short | grep '(tag:' | head -n3
d2a21b6 (tag: v0.0.1) Merge pull request #61 from libp2p/b32migrate
6295e61 (tag: gx/v2.0.6) gx publish 2.0.6
98ce915 (tag: gx/v2.0.5) gx publish 2.0.5

If you try rebasing the branch, go get will works.

@jayconrod should we returns the most recent tag of repo, instead of compare with current revision?

@raulk
Copy link
Author

raulk commented Apr 17, 2019

@cuonglm Thanks for the diagnosis! Mystery solved.

IMO go mod should not make assumptions about what commits/tags are present when using a branch reference.

It's a valid use case to want to switch a required module to an entirely different branch that has diverged enough to not contain the latest repo semver tag.

@cuonglm
Copy link
Member

cuonglm commented Apr 17, 2019

cc @bcmills How do you think about

should we returns the most recent tag of repo, instead of compare with current revision?

@bcmills
Copy link
Contributor

bcmills commented Apr 17, 2019

We should certainly ensure that the pseudo-version of a commit on a branch sorts after any tag applied to any of the (transitive) parents of that commit. (That's #27171.)

If two branches run parallel without periodically merging, then there is no well-defined ordering between them. We don't know whether the branch is, say, a release branch for ongoing maintenance of an older minor version, or a feature-development branch intended for release three more versions from now. So we stick to a simple rule: the pseudo-version is semantically after whatever comes before it, but only by a minimal degree.

The problem here, I suspect, is that mvs.BuildList expects the target it is passed to not have a transitive dependency on a newer version of itself. We should at least make that condition an explicit error rather than a panic.

@jayconrod
Copy link
Contributor

I tried to reproduce this again to figure out what the bug was, but it doesn't panic anymore. @raulk @cuonglm, what exactly did the consolidate-skeleton branch point to? I'm not able to fetch 2ca0254625f6 from the origin anymore. Any chance you could push a branch somewhere with this commit?


To give some explanation of why the the pseudoversion v0.0.2-0.20190416173150-2ca0254625f6 was being selected, when we fetch a commit, branch, or tag, we try to convert it into a semantic version that can be listed in the go.mod file. If it doesn't actually refer to a semver tag, we'll try and find a recent semver tag using git describe, bump the patch version, then add the date and commit as a prerelease segment. It sounds like v0.0.1 was the most recent tag on this branch, so the pseudoversion is a prerelease of v0.0.2.

should we returns the most recent tag of repo, instead of compare with current revision?

This would break the use case where different branches are used to maintain minor versions. For example, if you run go get example.com/module@version-1.5, the most recent tag on that branch might be v1.5.6, but there might be a v1.6.0 tag on the master branch.

IMO go mod should not make assumptions about what commits/tags are present when using a branch reference.

It's a valid use case to want to switch a required module to an entirely different branch that has diverged enough to not contain the latest repo semver tag.

The version that appears in the go.mod file is a minimum version: you may get a later version if another module in the requirement graph requires a later version. In order for this to work, we need a total ordering on versions. Git commits are only partially ordered, which means we can't always produce correct semantic versions. It's up to you to tag these in a way that Go will understand.

@bcmills
Copy link
Contributor

bcmills commented Apr 17, 2019

Here's the self-edge to the semantically-higher tag:

~/go-libp2p-peerstore$ git checkout consolidate-skeleton
Already on 'consolidate-skeleton'
Your branch is up to date with 'origin/consolidate-skeleton'.

~/go-libp2p-peerstore$ go1.12.4 mod graph | grep 'go-libp2p-peerstore@v0.0.2$'
github.com/libp2p/go-libp2p-core@v0.0.0-20190416150442-54a1b70f07da github.com/libp2p/go-libp2p-peerstore@v0.0.2

@jayconrod
Copy link
Contributor

Got it to reproduce again: go get github.com/libp2p/go-libp2p-peerstore@v0.0.2-0.20190417163308-393cab16de63.

@raulk
Copy link
Author

raulk commented Apr 17, 2019

@jayconrod – glad you reproduced it. I've pushed the errored commit onto the debug-go-issue-31491 branch. This should allow you to continue reproducing even in the event of a future rebase.


@bcmills, @jayconrod – Let me repeat the situation how I understood it. The repo contained a v0.0.2 tag which wasn't in the lineage of the branch. When issuing a go get for that branch, go calculated a pseudoversion by bumping up the patch number of the latest tag found in the branch, resulting in v0.0.2-0.20190416173150-2ca0254625f6. Alas, the repo contained a v0.0.2 tag and MVS ended up selecting v0.0.2 instead of the desired version, therefore causing an assertion to fail and thus the panic.

^^ is my narration correct?


Maintain long-lived branches for feature development, hotfixing, testing, etc. is a legit situation in software development. It's improper for go to force rebasing those branches atop the freshest tag in the repo, as that defeats the purpose of branching in the first place.

The way I see it, this is a scoping problem introduced by seeking linear, unambiguous total ordering atop of a VCS that's a DAG.

@jayconrod
Copy link
Contributor

Your summary of the situation is accurate. At this point, go get has determined the requested downgrade to v0.0.2-0.20190416173150-2ca0254625f6 was lost due to another requirement that upgrades it back to v0.0.2. It panics as it attempts to explain why the downgrade was lost.

The way I see it, this is a scoping problem introduced by seeking linear, unambiguous total ordering atop of a VCS that's a DAG.

If two modules require a common module at different versions, there must be a way to resolve the conflict to a single version. Without a total ordering, this cannot generally be done automatically.

For development, testing, and hotfixing, replace directives are probably the right tool for the job. They will let you select a specific version of a required module (or a fork).

@bcmills bcmills changed the title cmd/go: (gomod) panic: mistake: chose version "v0.0.N" instead of target cmd/go: 'panic: mistake: chose version "v0.0.N" instead of target' when an argument to 'go get' requires a newer version of itself Apr 30, 2019
@gopherbot
Copy link

Change https://golang.org/cl/177602 mentions this issue: cmd/go: don't panic when explaining lost upgrades due to downgrades

@golang golang locked and limited conversation to collaborators May 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants