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: misleading "invalid version" error message for gopkg.in modules with mismatched module path #43809

Open
jayconrod opened this issue Jan 20, 2021 · 9 comments
Labels
Milestone

Comments

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Jan 20, 2021

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

$ go version
go version go1.16beta1 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jayconrod/Library/Caches/go-build"
GOENV="/Users/jayconrod/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/jayconrod/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jayconrod/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go/installed"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/installed/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16beta1"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/jayconrod/Code/test/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/rq/x0692kqj6ml8cvrhcqh5bswc008xj1/T/go-build892501000=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run go mod download on a module with a gopkg.in module path, when the actual go.mod file has a different module path.

go mod download gopkg.in/golang/mock.v1@v1.4.4

What did you expect to see?

go mod download should succeed. go get and anything else that actually loads the module graph should fail with a mismatched module path error message.

In general, go mod download should succeed when the module path on the command line doesn't match the actual module path in go.mod. If it didn't, module version replacements wouldn't work. For example, go mod download github.com/google/go-cloud@v0.21.0 works even though it declares itself as gocloud.dev.

It's fine that the go command checks that the version matches the major version suffix, but it should check that against the module path named on the command line, not the module path in go.mod.

What did you see instead?

go mod download: gopkg.in/golang/mock.v1@v1.4.4: invalid version: go.mod has non-....v1 module path "github.com/golang/mock" at revision v1.4.4

gopkg.in module paths must have a major version suffix, in this case, .v1. The go command is reporting that the actual module path in go.mod, github.com/golang/mock does not end with .v1.

cc @bcmills @matloob @julieqiu

@jayconrod jayconrod added the NeedsFix label Jan 20, 2021
@jayconrod jayconrod added this to the Go1.17 milestone Jan 20, 2021
@bcmills

This comment has been hidden.

@bcmills

This comment has been hidden.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 20, 2021

That error is coming from here:

if r.pathMajor != "" { // ".v1", ".v2" for gopkg.in
return "", "", nil, fmt.Errorf("%s has non-...%s module path %q%s at revision %s", file1, r.pathMajor, mpath1, suffix, rev)
}

See previously #34254.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 20, 2021

I think the relevant check is here:

if mpathMajor == "" {
// Even if pathMajor is ".v0" or ".v1", we can't be sure that a module
// without a suffix is tagged appropriately. Besides, we don't expect clones
// of non-gopkg.in modules to have gopkg.in paths, so a non-empty,
// non-gopkg.in mpath is probably the wrong module for any such pathMajor
// anyway.
return false
}

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 20, 2021

Prompted by that comment, I'm not sure what the use of the go mod download command in this example would be.

gopkg.in/golang/mock.v1@v1.4.4 cannot be used for module gopkg.in/golang/mock.v1 because its go.mod file declares a non-matching path. The only direction it could go would be:

replace github.com/golang/mock => gopkg.in/golang/mock.v1 v1.4.4

but in that case why not use the canonical path instead?

replace github.com/golang/mock => github.com/golang/mock v1.4.4

So I agree that the error message we're emitting is confusing, but I'm not sure that it should be changed to a non-error.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Jan 20, 2021

Context for this was a 404 error on pkgsite. They're trying to understand why they show errors for some pages where godoc.org succeeds. gopkg.in/golang/mock.v1 worked in GOPATH mode, so https://godoc.org/gopkg.in/golang/mock.v1 works.

Since go mod download fails here (proxy returns 410 with same message), they can't read the go.mod to find the correct module path and redirect to the right page.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 20, 2021

We could perhaps weaken the mpathMajor == "" case in isMajor to return pathMajor == "v0" || pathMajor == "v1".

That would change the semantics of isMajor from “whether the versions allowed for mpath are compatible” to “whether the versions allowed for mpath may be compatible”. I'm not sure what other implications that would carry, but I suspect they're relatively benign.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 26, 2021

See previously #34254.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Jan 26, 2021

Just to add a note: this is a bug with the way cmd/go handles gopkg.in modules that are being used as replacements for non-gopkg.in paths. So the fix here should be very narrow.

In general, a replacement module should have the original module path in its go.mod file. The go command doesn't check that in all cases. We should be careful not to loosen that restriction though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants