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

Jump to bottom

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

Open
pellared opened this issue May 11, 2023 · 4 comments
Open

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

pellared opened this issue May 11, 2023 · 4 comments
Labels
GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@pellared
Copy link

pellared commented May 11, 2023 •
edited

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: open-telemetry/opentelemetry-go-contrib#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.
@pellared pellared mentioned this issue May 11, 2023
Closed
@bcmills
Copy link
Contributor

bcmills commented May 11, 2023 •
edited

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
Copy link
Contributor

bcmills commented May 11, 2023 •
edited

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
Copy link
Contributor

bcmills commented May 11, 2023 •
edited

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]

@bcmills bcmills changed the title go workspaces: ambiguous import cmd/go: implement lazy module loading in workspace mode to avoid ambiguous import errors May 11, 2023
@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go modules labels May 11, 2023
@bcmills bcmills added this to the Backlog milestone May 11, 2023
@pellared
Copy link
Author

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

@pellared pellared mentioned this issue May 16, 2023
Merged
@pellared pellared mentioned this issue Jun 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@pellared @bcmills