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

x/tools/gopls: evaluation of build tags used in imported packages must be optional #68549

Open
Zyl9393 opened this issue Jul 22, 2024 · 4 comments
Labels
gopls/metadata Issues related to metadata loading in gopls gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@Zyl9393
Copy link

Zyl9393 commented Jul 22, 2024

Go version

go1.22.3 windows/amd64
gopls v0.16.1

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Zyl\AppData\Local\go-build
set GOENV=C:\Users\Zyl\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=D:\Projects\Code\Go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=D:\Projects\Code\Go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.3
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=D:\Projects\MyProject\src\client\go.mod
set GOWORK=D:\Projects\MyProject\src\go.work
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\Zyl\AppData\Local\Temp\go-build383807685=/tmp/go-build -gno-record-gcc-switches

What did you do?

While dealing with endianness, I introduced build tags to a package which has its own go.mod. After the change, one file would contain

//go:build 386 || amd64 || amd64p32 || alpha || arm || arm64 || loong64 || mipsle || mips64le || mips64p32le || nios2 || ppc64le || riscv || riscv64 || sh || wasm

and another would contain

//go:build armbe || arm64be || m68k || mips || mips64 || mips64p32 || ppc || ppc64 || s390 || s390x || shbe || sparc || sparc64

As a consequence, all code importing this package is now evaluated for GOOS=aix with GOARCH=ppc64, causing a large amount of problems to be marked, making me lose of track of actual problems.

Additionally, type redeclarations are detected in spite of the build tags being disjoint. (?)

To fix it, I specified "go.buildTags": "amd64" in .vscode/settings.json.

What did you see happen?

Compilation errors for architectures other than amd64 are still being reported.

What did you expect to see?

That gopls would no longer report compilation errors for architectures other than amd64.

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jul 22, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jul 22, 2024
@findleyr
Copy link
Contributor

Additionally, type redeclarations are detected in spite of the build tags being disjoint. (?)

This should not happen. Perhaps for the particular set of tags selected, there really are redeclarations?

And sorry for the poor experience here. This should be improved.

Generally, I think we should consider (1) restricting diagnostics for "dynamic" build tags only to open packages, and (2) implementing #65757 to specify a fixed set of builds (which would implicitly disable dynamic build selection.

@Zyl9393
Copy link
Author

Zyl9393 commented Jul 23, 2024

Perhaps for the particular set of tags selected, there really are redeclarations?

No. I took direct inspiration from encoding/binary/native_endian_big.go and encoding/binary/native_endian_little.go, which have the same build tags respectively. There are only two declarations of the offending type; one in each file:
image
After removing all the build tags in both files and all code in one of the two files, saving, putting it all back, and saving again, this particular issue was gone. Does gopls manage some sort of on-disk cache? Restarting it did not fix the issue, but this comment/uncomment maneuver did.

only to open packages

I should probably clarify that the package causing the issue is indeed opened within the troubled workspace. I have plans to move it elsewhere following more progress on it; I reckon the order of doing these things should not be dictated by assistive tooling.

Can you help me understand why setting go.buildTags would be expected to be ineffective in this scenario?

@adonovan adonovan added the gopls/metadata Issues related to metadata loading in gopls label Jul 25, 2024
@hyangah hyangah modified the milestones: Unreleased, gopls/backlog Aug 4, 2024
@hyangah
Copy link
Contributor

hyangah commented Aug 4, 2024

Does gopls manage some sort of on-disk cache? Restarting it did not fix the issue, but this comment/uncomment maneuver did.

Yes, gopls maintains on-disk cache.

why setting go.buildTags would be expected to be ineffective in this scenario?

I can guess two reasons:

  1. go.buildTags is passed to go command's -tags flag value. Even though platform-specific source selection is controlled with the //go:build statement like other tags, os and arch are special and expected to be controlled with GOOS and GOARCH env vars. Passing os/arch values to -tags may look working but that's a coincidence. It can eventually cause issues. See go/build: BuildTags should reject tags with the same name as a known os or arch #45488.
  2. Since v0.15.0, gopls tries to determine GOOS/GOARCH heuristically to process open source files. See https://github.com/golang/tools/releases/tag/gopls%2Fv0.15.0 Specifying os/arch using go.buildTags can potentially confuse gopls.

@findleyr Why/how gopls happened to pick up GOOS=aix and GOARCH=ppc64, given those build tags?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/metadata Issues related to metadata loading in gopls gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants