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: pseudo-versions for post-v1 commits use the wrong major version #29262

Closed
hyangah opened this issue Dec 14, 2018 · 8 comments

Comments

@hyangah
Copy link
Contributor

commented Dec 14, 2018

Go version:

go version devel +976bab6003 Fri Dec 14 09:10:02 2018 +0000 darwin/amd64

rsc.io/quote's latest released version is v1.5.2.
But go chooses v0.0.0-... as the pseudo version selected when using the master branch.

> go list -m -versions -json rsc.io/quote@master
go: finding rsc.io/quote master
{
	"Path": "rsc.io/quote",
	"Version": "v0.0.0-20180710144737-5d9f230bcfba",
	"Versions": [
		"v1.0.0",
		"v1.1.0",
		"v1.2.0",
		"v1.2.1",
		"v1.3.0",
		"v1.4.0",
		"v1.5.0",
		"v1.5.1",
		"v1.5.2",
		"v1.5.3-pre1"
	],
	"Time": "2018-07-10T14:47:37Z",
	"Dir": "/Users/hakim/go/pkg/mod/rsc.io/quote@v0.0.0-20180710144737-5d9f230bcfba",
	"GoMod": "/Users/hakim/go/pkg/mod/cache/download/rsc.io/quote/@v/v0.0.0-20180710144737-5d9f230bcfba.mod"
}

Possibly related to #27171 but needs investigation.

@bcmills

@hyangah hyangah added the modules label Dec 14, 2018

@bcmills bcmills added this to the Go1.12 milestone Dec 14, 2018

@bcmills bcmills self-assigned this Dec 14, 2018

@bcmills bcmills modified the milestones: Go1.12, Go1.13 Jan 17, 2019

@bcmills bcmills added the Testing label Jan 17, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Jan 17, 2019

Dup of #27171, but we should make sure the tests cover this case.

@bcmills bcmills changed the title cmd/go: pseudo version chosen when using commit hash or branch prefers v0 even when v1 module is available. cmd/go: ensure that pseudo-versions for post-v1 commits on the master branch use v1 pseudo-versions Jan 17, 2019

@bcmills bcmills added the NeedsFix label May 17, 2019

@bcmills bcmills added NeedsInvestigation and removed Testing labels May 17, 2019

@bcmills bcmills changed the title cmd/go: ensure that pseudo-versions for post-v1 commits on the master branch use v1 pseudo-versions cmd/go: pseudo-versions for post-v1 commits use the wrong major version May 17, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented May 17, 2019

This might not be an exact duplicate of #27171, since it also interacts non-trivially with v2 and +incompatible.

@gopherbot gopherbot removed the NeedsFix label May 17, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented May 17, 2019

At current head:

$ go list -f '{{.Version}}' -m github.com/sylr/prometheus@v2.9.2+sylr.2
go: finding github.com/sylr/prometheus v2.9.2+sylr.2
v0.0.0-20190510132036-085fc219838d

It should instead be something like

v2.9.3-0.20190510132036-085fc219838d+incompatible

(CC @sylr)

@sylr

This comment has been minimized.

Copy link

commented May 17, 2019

It should instead be something like

v2.9.3-0.20190510132036-085fc219838d+incompatible

For readability it would be nice if the tag would be kept in the go.mod -> v2.9.2+sylr.2+incompatible

@bcmills

This comment has been minimized.

Copy link
Member

commented May 17, 2019

For readability it would be nice if the tag would be kept in the go.mod

That's (an interesting special case of) #25898.

@thepudds

This comment has been minimized.

Copy link

commented May 22, 2019

FWIW, from looking at this somewhat briefly (and hence might not be correct), it seems this issue is specific to a v2+ module and asking for a commit (current meaning of @master in this case) where the most recent semver tag at that commit is for a different major version than the major version in the go get request. I had a slightly longer write up in #27171 (comment) (along with some links to the possibly corresponding code). Partial quote from that comment:

In #29262, rsc.io/quote at master has two different major versions following the Major Subdirectory approach. It has a v1 module in the root of the repo, and a v3 module in a /v3 subdirectory. If you ask for go get rsc.io/quote@master, that is asking for v1 of rsc.io/quote, which is a valid request, but the most recent tag on master matching that glob pattern ends up being v3.0.0, which I think is what is returned by RecentTag. The v3 is not what is desired, and the caller gives up and falls back to a v0.0.0- pseudo version (probably here). This could be viewed as a variation of 1., or could be viewed as a separate problem.

I suspect but have not confirmed that this particular problem pattern would not happen for someone following the "Major Branch" approach for a v2+ module with different branches for v1 vs v2 vs v3 with the appropriate semver tags on their respective branches.

CC @rogpeppe

@bcmills

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

It should instead be something like

v2.9.3-0.20190510132036-085fc219838d+incompatible

That commit actually has a go.mod file, and the module path it declares is github.com/prometheus/prometheus.

However, the +incompatible suffix is generally not allowed in conjunction with a go.mod file: the +incompatible suffix is intended only for projects predating modules, not as an ongoing bypass for versioned import paths. That probably means that we should use the highest tag matching the path (v1.8.2) as the base for the pseudo-version, rather than the v2.[…] tag with the +incompatible suffix.

It also means I need to find a different example for +incompatible pseudo-versions. 😞

@gopherbot

This comment has been minimized.

Copy link

commented Jun 12, 2019

Change https://golang.org/cl/181881 mentions this issue: cmd/go: validate pseudo-versions against module paths and revision metadata

@gopherbot gopherbot closed this in 1803ab1 Jun 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.