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: one new module error is backwards, another seemed backwards #33879

Open
thepudds opened this issue Aug 27, 2019 · 6 comments

Comments

@thepudds
Copy link

commented Aug 27, 2019

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

go1.13rc1

Does this issue reproduce with the latest release?

Not with Go 1.12, but yes with Go 1.13 rc1.

Issue 1

I think this new Go 1.13 error message is incorrectly swapping the two module paths in the final two lines of the error message:

        github.com/Sirupsen/logrus: github.com/Sirupsen/logrus@v1.4.2: parsing go.mod:
        module declares its path as: github.com/Sirupsen/logrus
                but was required as: github.com/sirupsen/logrus

Given the v1.4.2 version of that module declares its path as github.com/sirupsen/logrus, I think the correct form of the error would be:

        github.com/Sirupsen/logrus: github.com/Sirupsen/logrus@v1.4.2: parsing go.mod:
        module declares its path as: github.com/sirupsen/logrus
                but was required as: github.com/Sirupsen/logrus

Example from scratch:

$ cd $(mktemp -d)
$ go1.13rc1  mod init tempmod
$ cat <<EOF > main.go
package tempmod
import _ "github.com/docker/libcompose/docker"
EOF

$ go1.13rc1 build
[...]
go: tempmod imports
        github.com/docker/libcompose/docker imports
        github.com/Sirupsen/logrus: github.com/Sirupsen/logrus@v1.4.2: parsing go.mod:
        module declares its path as: github.com/Sirupsen/logrus
                but was required as: github.com/sirupsen/logrus

Issue 2

When I first saw this next error message in the wild, at first I thought it was backwards, but later concluded it was just potentially ambiguous (I think?):

$ go1.13rc1 get github.com/iiinsomnia/yiigo@v3.0.0
go: finding github.com v3.0.0
go: finding github.com/iiinsomnia v3.0.0
go: finding github.com/iiinsomnia/yiigo v3.0.0
go: finding github.com/iiinsomnia/yiigo v3.0.0
go get github.com/iiinsomnia/yiigo@v3.0.0: github.com/iiinsomnia/yiigo@v3.0.0: 
 invalid version: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v3

In particular, it is not immediately clear which major version should be v0 or v1, not v3 -- the major version after the @, or the major version in the module path on the module line in the go.mod. Because it leads off with module contains a go.mod file immediately before talking about the problematic major version, it seems like it is suggesting the major version in the go.mod file should be v0 or v1 (which is not the correct resolution, and you can't even have a v0 or v1 in the module line for a go.mod unless it is a corner case like gopkg.in).

In this example, there is a go.mod at v3.0.0, but that go.mod is missing the required /v3 at the end of the module path on the module line. As it stands, I think the error message is trying to say it would be acceptable to have a @v0.x.x or @v1.x.x in the go get for that go.mod? But that was not the way it read at first read, and moreover, one could at least argue that is not the fundamental error, which is really that the major version after the @ does not agree with the (missing) major version in the module path on the module line (and depending on how you look at it, that the go get is also missing the /v3 before the @).

I have seen other people read this second error message backwards as well. (Side note: I'm not sure why it emits go: finding github.com/iiinsomnia/yiigo v3.0.0 twice).

Finally, sorry -- this probably should have been two separate issues.

CC @jayconrod @bcmills

@thepudds thepudds changed the title cmd/go: one new module error message is backwards, another initially seemed backwards cmd/go: one new module error is backwards, another seemed backwards Aug 27, 2019

@thepudds thepudds added the modules label Aug 27, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

🤦‍♂️

@bcmills bcmills self-assigned this Aug 27, 2019

@bcmills bcmills added this to the Go1.14 milestone Aug 27, 2019

@bcmills bcmills added the NeedsFix label Aug 27, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

I think I can justify cherry-picking the fix for the swapped error message into the 1.13 release, but I'm not so sure about the compatibility error.

(That should be […] text comes from module.MatchPathMajor.)

@gopherbot

This comment has been minimized.

Copy link

commented Aug 27, 2019

Change https://golang.org/cl/191997 mentions this issue: cmd/go/internal/modload: fix swapped paths in error message

gopherbot pushed a commit that referenced this issue Aug 27, 2019
cmd/go/internal/modload: fix swapped paths in error message
Updates #33879

Change-Id: Ifc91490b1cb791fdf5ffe69ef81c0ec0e6cbecc3
Reviewed-on: https://go-review.googlesource.com/c/go/+/191997
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

@gopherbot, please backport to 1.13: we should at least fix the reversed error message.

@gopherbot

This comment has been minimized.

Copy link

commented Aug 27, 2019

Backport issue(s) opened: #33885 (for 1.13).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot

This comment has been minimized.

Copy link

commented Aug 27, 2019

Change https://golang.org/cl/191972 mentions this issue: [release-branch.go1.13] cmd/go/internal/modload: fix swapped paths in error message

gopherbot pushed a commit that referenced this issue Aug 27, 2019
[release-branch.go1.13] cmd/go/internal/modload: fix swapped paths in…
… error message

Cherry-picked from CL 191997.

Updates #33879
Fixes #33885

Change-Id: Ifc91490b1cb791fdf5ffe69ef81c0ec0e6cbecc3
Reviewed-on: https://go-review.googlesource.com/c/go/+/191997
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
(cherry picked from commit 8f5353f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/191972
TryBot-Result: Gobot Gobot <gobot@golang.org>
tomocy added a commit to tomocy/go that referenced this issue Sep 1, 2019
cmd/go/internal/modload: fix swapped paths in error message
Updates golang#33879

Change-Id: Ifc91490b1cb791fdf5ffe69ef81c0ec0e6cbecc3
Reviewed-on: https://go-review.googlesource.com/c/go/+/191997
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
t4n6a1ka added a commit to t4n6a1ka/go that referenced this issue Sep 5, 2019
cmd/go/internal/modload: fix swapped paths in error message
Updates golang#33879

Change-Id: Ifc91490b1cb791fdf5ffe69ef81c0ec0e6cbecc3
Reviewed-on: https://go-review.googlesource.com/c/go/+/191997
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.