search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.36k stars 17.38k forks source link

cmd/go: prefer to report mismatched module paths over mismatched major versions #34432

Open brenol opened 4 years ago

brenol commented 4 years ago

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

$ go version
go version go1.13 linux/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="/home/brenol/.cache/go-build"
GOENV="/home/brenol/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY="bitbucket.org"
GONOSUMDB="bitbucket.org"
GOOS="linux"
GOPATH="/home/brenol/goworkspace"
GOPRIVATE="bitbucket.org"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/brenol/goworkspace/src/github.com/twitchscience/kinsumer/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build321165660=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tried to run go mod tidy in github.com/twitchscience/kinsumer

What did you expect to see?

I expected it to report correctly that the project github.com/myesui/uuid is the one that uses gopkg.in/stretchr/testify.v1. However, it's reporting that twinj/uuid incorrectly is the one that is using.

What did you see instead?

go: finding gopkg.in/stretchr/testify.v1 v1.4.0
github.com/twitchscience/kinsumer imports
    github.com/twinj/uuid tested by
    github.com/twinj/uuid.test imports
    gopkg.in/stretchr/testify.v1/assert: cannot find module providing package gopkg.in/stretchr/testify.v1/assert

How the module looks like:

module github.com/twitchscience/kinsumer

go 1.13

require (
    github.com/aws/aws-sdk-go v1.24.2
    github.com/cactus/go-statsd-client/statsd v0.0.0-20190906215803-47b6058c80f5
    github.com/myesui/uuid v1.0.0 // indirect
    github.com/stretchr/testify v1.4.0
    github.com/twinj/uuid v1.0.0

    golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab // indirect
    golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
)

The strangest thing is that I see no mention of myesui/uuid in the vendored directory.

brenol commented 4 years ago

While debugging this issue I also got this output, not sure on how I did that though:

go get gopkg.in/stretchr/testify.v1: gopkg.in/stretchr/testify.v1@v1.4.0: invalid version: go.mod has non-....v1 module path "github.com/stretchr/testify" at revision v1.4.0
bcmills commented 4 years ago

The chain reported by the first error message looks accurate:

That last error is because the module path declared by the go.mod file does not match the path by which the module was imported, a fact should have been reported earlier in the error output. Was it? (If not, that's a diagnostic bug we should fix.)

bcmills commented 4 years ago

go get gopkg.in/stretchr/testify.v1: gopkg.in/stretchr/testify.v1@v1.4.0: invalid version: go.mod has non-....v1 module path "github.com/stretchr/testify" at revision v1.4.0

There are two possible errors the go command could have reported there:

  1. That the module path in general does not match the requested module path.
  2. That the major version of the module path does not match the major version implied by the gopkg.in URL.

Both are correct; (1) is probably clearer, but the go command ended up reporting (2) instead. We should fix that.

bcmills commented 4 years ago

@brenol, could you give some more detail as to why you expected to see github.com/myesui/uuid instead of github.com/twinj/uuid?

brenol commented 4 years ago

@brenol, could you give some more detail as to why you expected to see github.com/myesui/uuid instead of github.com/twinj/uuid?

I actually tried to mean that I did not expect to see github.com/myesui/uuid anywhere in the go.mod file, as I don't see any references of it in github.com/twinj/uuid source code.

brenol commented 4 years ago

The chain reported by the first error message looks accurate:

  • github.com/twitchscience/kinsumer imports github.com/twinj/uuid: link
  • tested by github.com/twinj/uuid.test imports gopkg.in/stretchr/testify.v1/assert: link
  • cannot find module providing package gopkg.in/stretchr/testify.v1/assert: link

That last error is because the module path declared by the go.mod file does not match the path by which the module was imported, a fact should have been reported earlier in the error output. Was it? (If not, that's a diagnostic bug we should fix.)

About this one, I did not see it report anywhere in the go mod tidy command, neither in the go build/go test etc.

One more thing: I feel like go mod vendor should checkout to the tag (or something like this? I'm not sure) that is actually being used. While debugging, I was looking at the go mod vendor to search for github.com/myesui/uuid but as it was nowhere to be found, I misunderstood the problem completely -- it is because go mod vendor was bringing github.com/twinj/uuid and leaving it at the master branch (which is the reason why, in my last comment, I mentioned I said "I don't see any references of it in github.com/twinj/uuid source)

bcmills commented 4 years ago

go mod vendor should checkout to the tag (or something like this? I'm not sure) that is actually being used

go mod vendor is intended to copy in the source code from the exact version in use. If you are seeing otherwise, please file a separate issue with steps to reproduce it.

brenol commented 4 years ago

Sure, opened #34435. I don't want to make this issue a conversation, however, I have a few more questions:

The other question is if go mod why should output the same error as the go mod tidy:

$ go mod why github.com/myesui/uuid                                                                                                                                                             
go: finding gopkg.in/stretchr/testify.v1 v1.4.0
go: finding gopkg.in/stretchr/testify.v1 v1.4.0
github.com/twitchscience/kinsumer imports
    github.com/twinj/uuid tested by
    github.com/twinj/uuid.test imports
    gopkg.in/stretchr/testify.v1/assert: cannot find module providing package gopkg.in/stretchr/testify.v1/assert
bcmills commented 4 years ago

I see no mention of gopkg.in in go mod graph. Is this expected?

Check the last three lines. (You have two dependencies in gopkg.in.)

bcmills commented 4 years ago

The other question is if go mod why should output the same error as the go mod tidy:

go mod why should probably be more error-tolerant than it is, and probably should not try to resolve missing imports. (#26977 is somewhat related.)

brenol commented 4 years ago

I see no mention of gopkg.in in go mod graph. Is this expected?

Check the last three lines. (You have two dependencies in gopkg.in.)

Sorry, tried to mean that I see no mention of gopkg.in/stretchr/testify.v1. This is expected then, right?

About the go mod why, I see. Thank you.

bcmills commented 4 years ago

Yep, this is expected: the reason go mod graph isn't showing the dependency is because nothing ever resolves the import successfully, so the corresponding module never gets added to the module graph. (That was one reason we focused on the import-based diagnostics for 1.13.)

bcmills commented 4 years ago

I think the module resolution is working as designed here (and the go mod vendor issue is tracked separately).

I'm going to close this issue, but we'll certainly bear this report in mind when we're thinking about how to further improve the diagnostics.

thepudds commented 4 years ago

@bcmills

There are two possible errors the go command could have reported there:

That the module path in general does not match the requested module path. That the major version of the module path does not match the major version implied by the gopkg.in URL. Both are correct; (1) is probably clearer, but the go command ended up reporting (2) instead. We should fix that.

I’ve seen that trip other people up, and I agree with your assessment that 1 is clearer (significantly clearer, I think).

What do you think about perhaps re-opening this issue and using it to track that change? (Or maybe there’s already another issue, or maybe a new one should be opened…)