search

golang / go

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

cmd/go: implement lazy module loading in workspace mode to avoid `ambiguous import` errors #60126

Open pellared opened 1 year ago

pellared commented 1 year ago

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

$ go version
go version go1.20.3 linux/amd64

Also tested on the latest:

$ go version
go version go1.20.4 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
O111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/rpajak/.cache/go-build"
GOENV="/home/rpajak/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/rpajak/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/rpajak/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/rpajak/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod"
GOWORK="/home/rpajak/repos/opentelemetry-go-contrib/go.work"
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3679794853=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I added go.work and it looks like it caused the go build tooling to differently handle dependency versions. Reference: https://github.com/open-telemetry/opentelemetry-go-contrib/pull/3805

You can checkout this repository https://github.com/pellared/opentelemetry-go-contrib/tree/5a8a020149031cf531deada3c13e83d79c39ed3a and run: go test ./instrumentation/github.com/gin-gonic/gin/otelgin/... to reproduce the bug.

What did you expect to see?

Build and tests are passing.

Using go.work does not affect the build

What did you see instead?

~/repos/opentelemetry-go-contrib$ go test ./instrumentation/github.com/gin-gonic/gin/otelgin/...
../../go/pkg/mod/github.com/gin-gonic/gin@v1.8.2/binding/msgpack.go:15:2: ambiguous import: found package github.com/ugorji/go/codec in multiple modules:
        github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83 (/home/rpajak/go/pkg/mod/github.com/ugorji/go@v0.0.0-20171122102828-84cb69a8af83/codec)
        github.com/ugorji/go/codec v1.2.9 (/home/rpajak/go/pkg/mod/github.com/ugorji/go/codec@v1.2.9)

I tried even to clear Go modules cache hoping that it will fix the issue. However, for some reason it keeps downloading github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83 even though github.com/ugorji/go/codec v1.2.9 is in go.mod:

~/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin$ go clean -modcache

~/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin$ go test
go: downloading github.com/gin-gonic/gin v1.8.2
go: downloading go.opentelemetry.io/otel v1.15.1
go: downloading github.com/stretchr/testify v1.8.2
go: downloading go.opentelemetry.io/otel/trace v1.15.1
go: downloading golang.org/x/net v0.9.0
go: downloading github.com/gin-contrib/sse v0.1.0
go: downloading github.com/mattn/go-isatty v0.0.17
go: downloading github.com/ugorji/go/codec v1.2.9
go: downloading github.com/go-playground/validator/v10 v10.11.1
go: downloading google.golang.org/protobuf v1.30.0
go: downloading github.com/pelletier/go-toml/v2 v2.0.6
go: downloading gopkg.in/yaml.v2 v2.4.0
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading gopkg.in/yaml.v3 v3.0.1
go: downloading golang.org/x/sys v0.7.0
go: downloading github.com/go-logr/logr v1.2.4
go: downloading github.com/go-logr/stdr v1.2.2
go: downloading github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83
go: downloading golang.org/x/text v0.9.0
go: downloading github.com/go-playground/universal-translator v0.18.1
go: downloading golang.org/x/crypto v0.6.0
go: downloading github.com/leodido/go-urn v1.2.1
go: downloading github.com/pelletier/go-toml v1.9.5
go: downloading github.com/go-playground/locales v0.14.1
/home/rpajak/go/pkg/mod/github.com/gin-gonic/gin@v1.8.2/binding/msgpack.go:15:2: ambiguous import: found package github.com/ugorji/go/codec in multiple modules:
        github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83 (/home/rpajak/go/pkg/mod/github.com/ugorji/go@v0.0.0-20171122102828-84cb69a8af83/codec)
        github.com/ugorji/go/codec v1.2.9 (/home/rpajak/go/pkg/mod/github.com/ugorji/go/codec@v1.2.9)

~/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin$ go mod why github.com/ugorji/go
go: downloading k8s.io/apimachinery v0.27.1
go: downloading k8s.io/client-go v0.27.1
go: downloading github.com/pierrec/lz4/v4 v4.1.17
go: downloading honnef.co/go/tools v0.4.3
# github.com/ugorji/go
(main module does not need package github.com/ugorji/go)

I found a hacky workaround:

~/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin$ go get github.com/ugorji/go@v1.2.9
go: downloading github.com/ugorji/go v1.2.9
go: added github.com/ugorji/go v1.2.9

~/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin$ go test
PASS
ok      go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin    0.016s

However, go mod tidy correctly clears the github.com/ugorji/go v1.2.9 // indirect entry from go.mod and the issue is back again:

~/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin$ go mod tidy

~/repos/opentelemetry-go-contrib/instrumentation/github.com/gin-gonic/gin/otelgin$ go test
/home/rpajak/go/pkg/mod/github.com/gin-gonic/gin@v1.8.2/binding/msgpack.go:15:2: ambiguous import: found package github.com/ugorji/go/codec in multiple modules:
        github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83 (/home/rpajak/go/pkg/mod/github.com/ugorji/go@v0.0.0-20171122102828-84cb69a8af83/codec)
        github.com/ugorji/go/codec v1.2.9 (/home/rpajak/go/pkg/mod/github.com/ugorji/go/codec@v1.2.9)

I find this issue frightening. I am not sure if I can confidently use Go workspaces in monorepos.

bcmills commented 1 year ago

I found a hacky workaround

That isn't a “hacky workaround”. It's how you fix an ambiguous import.

However, go mod tidy correctly clears the github.com/ugorji/go v1.2.9 // indirect entry from go.mod and the issue is back again

That sounds like a bug that was supposed to have been fixed in https://go.dev/cl/344572. I'll investigate.

bcmills commented 1 year ago

Here is the path leading to the dependency that causes the ambiguous import:

~/src/go.opentelemetry.io/contrib$ go mod graph | grep ' github.com/ugorji/go@'
github.com/ledisdb/ledisdb@v0.0.0-20200510135210-d35789ec47e6 github.com/ugorji/go@v0.0.0-20171122102828-84cb69a8af83

~/src/go.opentelemetry.io/contrib$ go mod graph | grep ' github.com/ledisdb'
github.com/astaxie/beego@v1.12.3 github.com/ledisdb/ledisdb@v0.0.0-20200510135210-d35789ec47e6

~/src/go.opentelemetry.io/contrib$ go mod graph | grep ' github.com/astaxie/beego@'
go.opentelemetry.io/contrib/instrumentation/github.com/astaxie/beego/otelbeego github.com/astaxie/beego@v1.12.3
go.opentelemetry.io/contrib/instrumentation/github.com/astaxie/beego/otelbeego/example github.com/astaxie/beego@v1.12.3
go.opentelemetry.io/contrib/instrumentation/github.com/astaxie/beego/otelbeego/test github.com/astaxie/beego@v1.12.3
go.opentelemetry.io/contrib/instrumentation/github.com/astaxie/beego/otelbeego@v0.41.1 github.com/astaxie/beego@v1.12.3

go mod tidy in the module go.opentelemetry.io/contrib/instrumentation/github.com/astaxie/beego/otelbeego does not preserve any requirement for package github.com/ugorji/go/codec because that module doesn't use it. (There is no ambiguous import there because the package isn't imported in that module at all.)

go mod tidy in the module go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgen also doesn't preserve a requirement on github.com/ugorji/go v1.2.9, because lazy module loading always resolves the ambiguity in favor of the module explicitly listed in the go.mod file. (The package is imported, but lazy module loading makes it unambiguous.)

So the root of the bug here seems to be that lazy module loading does not take effect in workspace mode, and the interaction between lazy module loading, workspace mode, and go mod tidy is such that you can't fix the problem by adding a requirement in any one module.

(CC @matloob)

bcmills commented 1 year ago

I'll see what I can do about supporting lazy module loading in workspace mode, but in the interim there is a workaround. When we load the module graph for a workspace, we also apply the exclude directives from each of the modules in the workspace.

So you can run go mod edit -exclude=github.com/ugorji/go@v0.0.0-20171122102828-84cb69a8af83 in one or more of the modules in the workspace, and that will prune out the problematic dependency for the whole workspace. (I suggest the otelbeego modules because they are the ones that introduce the node into the module graph in the first place, but you could also put it in otelgin because that's the module that really needs to have it pruned out.)

~/src/go.opentelemetry.io/contrib$ git diff
diff --git i/instrumentation/github.com/astaxie/beego/otelbeego/example/go.mod w/instrumentation/github.com/astaxie/beego/otelbeego/example/go.mod
index bb0b98e2..b13cdfa2 100644
--- i/instrumentation/github.com/astaxie/beego/otelbeego/example/go.mod
+++ w/instrumentation/github.com/astaxie/beego/otelbeego/example/go.mod
@@ -40,3 +40,5 @@ require (
        google.golang.org/protobuf v1.30.0 // indirect
        gopkg.in/yaml.v2 v2.4.0 // indirect
 )
+
+exclude github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83
diff --git i/instrumentation/github.com/astaxie/beego/otelbeego/go.mod w/instrumentation/github.com/astaxie/beego/otelbeego/go.mod
index b53b2900..a88c751a 100644
--- i/instrumentation/github.com/astaxie/beego/otelbeego/go.mod
+++ w/instrumentation/github.com/astaxie/beego/otelbeego/go.mod
@@ -38,3 +38,5 @@ require (
        gopkg.in/yaml.v2 v2.4.0 // indirect
        gopkg.in/yaml.v3 v3.0.1 // indirect
 )
+
+exclude github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83
diff --git i/instrumentation/github.com/astaxie/beego/otelbeego/test/go.mod w/instrumentation/github.com/astaxie/beego/otelbeego/test/go.mod
index 5204c090..46adf681 100644
--- i/instrumentation/github.com/astaxie/beego/otelbeego/test/go.mod
+++ w/instrumentation/github.com/astaxie/beego/otelbeego/test/go.mod
@@ -45,3 +45,5 @@ replace (
        go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp => ../../../../../net/http/otelhttp
        go.opentelemetry.io/contrib/propagators/b3 => ../../../../../../propagators/b3
 )
+
+exclude github.com/ugorji/go v0.0.0-20171122102828-84cb69a8af83

~/src/go.opentelemetry.io/contrib$ go test -c -o /dev/null go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/...
?       go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/example    [no test files]
pellared commented 1 year ago

@bcmills Thanks a lot for quick feedback, great explanation and even workaround ❤️