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 init does not correctly resolve pseudo-versions #32161

Closed
rogpeppe opened this issue May 21, 2019 · 9 comments
Closed

cmd/go: go mod init does not correctly resolve pseudo-versions #32161

rogpeppe opened this issue May 21, 2019 · 9 comments
Assignees
Milestone

Comments

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented May 21, 2019

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

$ go version
go version devel +ab724d43ef Mon May 20 20:58:14 2019 +0000 linux/amd64

What did you do?

Extract the following files:

-- glide.lock --
imports:
- name: github.com/Shopify/sarama
  version: 4602b5a8c6e826f9e0737865818dd43b2339a092
  subpackages:
  - mocks
- name: github.com/eapache/queue
  version: 093482f3f8ce946c05bcba64badd2c82369e084d
-- glide.yaml --
package: example

-- main.go --
package main

import (
	_ "github.com/Shopify/sarama"
	_ "github.com/eapache/queue"
)

func main() {}

Run go mod init example.

What did you expect to see?

I would expect to see the go.mod file reflect the requirements listed in the glide.lock file. Specifically, I'd expect to see this go.mod file:

module example

require (
	github.com/Shopify/sarama v1.21.0
	github.com/eapache/queue v1.1.1-0.20180227141424-093482f3f8ce
)

go 1.13

which is what you get if you start with this go.mod file, which exactly reflects the glide.lock file:

module example

go 1.12

require (
	github.com/eapache/queue 093482f3f8ce946c05bcba64badd2c82369e084d
	github.com/Shopify/sarama 4602b5a8c6e826f9e0737865818dd43b2339a092
)

What did you see instead?

I see this go.mod file:

module example

go 1.13

require (
	github.com/Shopify/sarama v1.21.0
	github.com/eapache/queue v0.0.0-20180227141424-093482f3f8ce
)

The github.com/eapache/queue dependency has the wrong pseudo-version, which means that when dependencies are later resolved, the dependency in sarama (which depends on epache/queue v1.1.0) supercedes the dependency in the go.mod file, which leads to a version regression.

@rogpeppe rogpeppe changed the title cmd/go: go mod init does not correctly resolve pseudo=versions cmd/go: go mod init does not correctly resolve pseudo-versions May 21, 2019
@myitcv

This comment has been minimized.

Copy link
Member

@myitcv myitcv commented May 21, 2019

@cuonglm

This comment has been minimized.

Copy link
Contributor

@cuonglm cuonglm commented May 21, 2019

Seems duplicated of #29262

@rogpeppe

This comment has been minimized.

Copy link
Contributor Author

@rogpeppe rogpeppe commented May 21, 2019

@cuonglm I don't think it's a duplicate of that #29262. With that bug, I can do go get rsc.io/quote@5d9f230bcfba and version stays at v0.0.0-20180710144737-5d9f230bcfba, whereas with this bug, if I do go get github.com/eapache/queue@093482f3f8ce, the version changes to the correct v1.1.1-0.20180227141424-093482f3f8ce.

@bcmills bcmills added this to the Go1.13 milestone May 21, 2019
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented May 21, 2019

Neat example. I can confirm the observed behavior: go mod init produces the wrong pseudo-version, but go get finds the correct one.

@bcmills bcmills self-assigned this May 21, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 22, 2019

Change https://golang.org/cl/178497 mentions this issue: cmd/go: fix go mod init resolve wrong pseudo-version

@thepudds

This comment has been minimized.

Copy link

@thepudds thepudds commented May 30, 2019

https://golang.org/cl/178497 I think is trying to get go mod init to create the proper pseudoversion.

If that turns out to be tricky to get right or a bigger change than would work for 1.13, I wonder if an alternative approach might be to simplify what go mod init tries to do in this case.

Instead of go mod init creating the proper pseudoversion, perhaps instead go mod init could just leave the commit hash in the resulting go.mod file?

That should then be automatically converted to the right pseudoversion by the first go list -m all, go build or similar because the commit hash would be treated as a "module query".

However, perhaps that is no simpler than the current https://golang.org/cl/178497 approach.

@cuonglm

This comment has been minimized.

Copy link
Contributor

@cuonglm cuonglm commented May 30, 2019

@thepudds I don't know why that change is not sure or hard to understand. When the convert try to make pseudoversion, it does use statVers to query the most recent tag:

// Otherwise make a pseudo-version.
if info2.Version == "" {
        tag, _ := r.code.RecentTag(statVers, p)
        v = tagToVersion(tag)
        // TODO: Check that v is OK for r.pseudoMajor or else is OK for incompatible.
        info2.Version = PseudoVersion(r.pseudoMajor, v, info.Time, info.Short)
}

we passed statVers as empty string, so most recent tag from HEAD was returned instead of most recent tag from commit.

@gopherbot gopherbot closed this in 9e31b17 May 31, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 31, 2019

Change https://golang.org/cl/179857 mentions this issue: cmd/go/internal/modfetch: use the resolved version to search for tags in (*codeRepo).convert

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented May 31, 2019

It is not correct for convert to use statVers in the RecentTag lookup, but passing in the correct rev moves things in the right direction regardless.

gopherbot pushed a commit that referenced this issue May 31, 2019
… in (*codeRepo).convert

Previously, we used the passed-in statVers as the basis for tag search,
but it is not always valid.
Instead, use info.Name, which (by precondition) must be valid.

Updates #32161
Updates #27171

Change-Id: Iaecb5043bdf2fefd26fbe3f8e3714b07d22f580f
Reviewed-on: https://go-review.googlesource.com/c/go/+/179857
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.