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
Closed
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah 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

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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 An issue that has been verified to require only test changes, not just a test failure. label Jan 17, 2019
@bcmills
Copy link
Contributor

bcmills 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 The path to resolution is known, but the work has not been done. label May 17, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 17, 2019
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Testing An issue that has been verified to require only test changes, not just a test failure. 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
Copy link
Contributor

bcmills 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 The path to resolution is known, but the work has not been done. label May 17, 2019
@bcmills
Copy link
Contributor

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

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

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

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

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

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

@golang golang locked and limited conversation to collaborators Jun 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules 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

5 participants