golang / go

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

cmd/vet: wrong package version used to vet orphaned code (?) #45678

Closed bobg closed 3 years ago

bobg commented 3 years ago

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

$ go version
go version go1.16.3 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="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/bobglickstein/Library/Caches/go-build"
GOENV="/Users/bobglickstein/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/bobglickstein/go/pkg/mod"
GONOPROXY=""
GOOS="darwin"
GOPATH="/Users/bobglickstein/go"
GOPRIVATE=""
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/bobglickstein/src/bobg/repo/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 -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/mp/8ycgyr3j64s42ym9lbl9cqdh0000gn/T/go-build1894730681=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have a project that depends on code in go.uber.org/cadence at version v0.10.5. I ran go vet on my code and got impossible-seeming output. See "Speculation," below.

What did you expect to see?

Nothing.

What did you see instead?

go.uber.org/cadence/internal/common
/root/go/pkg/mod/go.uber.org/cadence@v0.10.5/internal/common/thrift_util.go:29:38: not enough arguments in call to thrift.NewTSerializer().Write
    have (thrift.TStruct)
    want (context.Context, thrift.TStruct)
/root/go/pkg/mod/go.uber.org/cadence@v0.10.5/internal/common/thrift_util.go:51:27: not enough arguments in call to t.Protocol.Flush
    have ()
    want (context.Context)
/root/go/pkg/mod/go.uber.org/cadence@v0.10.5/internal/common/thrift_util.go:55:28: not enough arguments in call to t.Transport.Flush
    have ()
    want (context.Context)

Speculation

This output seems wrong because the version of thrift that cadence v0.10.5 depends on does not include those context.Context parameters.

Here is the go.mod line from cadence v0.10.5 specifying a version of thrift: link Here is one call in cadence v0.10.5 that go vet erroneously claims is wrong: link Here is the declaration in thrift, at the version requested by cadence, showing that the call is in fact OK: link

Later versions of thrift do add a context.Context parameter to those calls, e.g. link (but without assigning a new major version number, naughty naughty).

However, the code that go vet complains about is orphaned. Nothing in the cadence module calls it. And since it's in an internal tree, nothing outside cadence can call it.

It therefore seems as if go vet is using the version of thrift that my module would want (i.e., the latest version, if my module wanted thrift) and, perhaps because the code in question is orphaned, ignoring the version of thrift that cadence has asked for.

Note, I reproduced this with clean Go mod and build caches and with various settings for GOPROXY.

seankhliao commented 3 years ago

which version of thrift is actually used? just because cadence asks for v0.10.5 doesn't mean it will be the version selected if other dependencies or you declare the need for a higher version.

output of or find the line in your go.mod

go list -mod=readonly -m github.com/apache/thrift
bobg commented 3 years ago
$ go list -mod=readonly -m github.com/apache/thrift
github.com/apache/thrift v0.13.0

Also, in case it wasn't clear, my module has no direct dependency on thrift.

$ go mod why github.com/apache/thrift
# github.com/apache/thrift
(main module does not need package github.com/apache/thrift)
seankhliao commented 3 years ago

right now everything reachable, including unused things, participates in version selection. #36460 seeks to solve that.

try go mod graph | grep github.com/apache/thrift to find which dependencies declare a need for it

bobg commented 3 years ago

Ah, interesting, thanks. Following https://github.com/golang/go/issues/36460.

$ go mod graph | grep github.com/apache/thrift
contrib.go.opencensus.io/exporter/jaeger@v0.1.0 github.com/apache/thrift@v0.12.0
go.uber.org/cadence@v0.10.5 github.com/apache/thrift@v0.0.0-20161221203622-b2a4d4ae21c7
go.uber.org/yarpc@v1.49.1 github.com/apache/thrift@v0.13.0
go.opencensus.io@v0.19.2 github.com/apache/thrift@v0.12.0
go.opencensus.io@v0.20.1 github.com/apache/thrift@v0.12.0
seankhliao commented 3 years ago

So it is correctly selecting the highest version (v0.13.0) of thrift (since a module can only exist at a single version).

Closing as working as intended.