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

x/tools/gopls: build constraints not respected for new files #37101

Closed
myitcv opened this issue Feb 7, 2020 · 4 comments
Closed

x/tools/gopls: build constraints not respected for new files #37101

myitcv opened this issue Feb 7, 2020 · 4 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
vscode-go: gopls by default

Comments

@myitcv
Copy link
Member

myitcv commented Feb 7, 2020

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

$ go version
go version devel +b7689f5aa3 Fri Jan 31 06:02:00 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200205190317-6e8b36d2c76b
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.8-0.20200205190317-6e8b36d2c76b

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/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/.vim/plugged/govim/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-build540934445=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Starting with an empty module, create (and save) the following files in sequence:

-- main.go --
package main

import (
	"fmt"
)

func main() {
	fmt.Println(hello())
}

-- hello_linux.go --
package main

func hello() string {
	return "linux"
}

-- hello_darwin.go --
package main

func hello() string {
	return "darwin"
}

What did you expect to see?

No diagnostic errors.

What did you see instead?

PublishDiagnostics callback: &protocol.PublishDiagnosticsParams{
    URI:         "file:///home/myitcv/gostuff/src/github.com/myitcv/playground/hello_darwin.go",
    Version:     57,
    Diagnostics: {
        {
            Range: protocol.Range{
                Start: protocol.Position{Line:2, Character:5},
                End:   protocol.Position{Line:2, Character:10},
            },
            Severity:           1,
            Code:               nil,
            Source:             "compiler",
            Message:            "hello redeclared in this block",
            Tags:               nil,
            RelatedInformation: nil,
        },
    },
}
PublishDiagnostics callback: &protocol.PublishDiagnosticsParams{
    URI:         "file:///home/myitcv/gostuff/src/github.com/myitcv/playground/hello_linux.go",
    Version:     65,
    Diagnostics: {
        {
            Range: protocol.Range{
                Start: protocol.Position{Line:2, Character:5},
                End:   protocol.Position{Line:2, Character:10},
            },
            Severity:           1,
            Code:               nil,
            Source:             "compiler",
            Message:            "other declaration of hello",
            Tags:               nil,
            RelatedInformation: nil,
        },
    },
}

If you close and restart the editor (or indeed gopls) the diagnostics go away.

cc @stamblerre

FYI @leitzler
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Feb 7, 2020
@gopherbot gopherbot added this to the Unreleased milestone Feb 7, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Feb 7, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.4.0 Feb 7, 2020
@stamblerre stamblerre modified the milestones: gopls/v0.4.0, gopls/v0.5.0 Apr 2, 2020
@myitcv myitcv mentioned this issue Apr 29, 2020
Closed
@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v1.0.0 Jul 22, 2020
@heschi
Copy link
Contributor

heschi commented Sep 2, 2020

Overlay processing ignores build tags. I'll look at it.

https://github.com/golang/tools/blob/36b1a880d5d1df625063b05d248a3ae77e083487/go/packages/golist_overlay.go#L172-L173

@heschi heschi self-assigned this Sep 2, 2020
@heschi
Copy link
Contributor

heschi commented Sep 2, 2020

Fixing this would involve copying a couple hundred lines of (go/build.Context).MatchFile somewhere, since it doesn't accept file content, only a file name. Then we need to know which tags are active, which involves either some gnarly argument parsing or a call to go list. @stamblerre has expressed concerns about the performance cost of doing that on each packages.Load call. To fix that, we'd need to either put most of the logic into gopls rather than go/packages, or pass data into go/packages so that it doesn't have to re-fetch it every time. go/packages already does a go env call to read stuff that gopls already knows, so that could be a net win.

@heschi heschi removed their assignment Sep 2, 2020
@stamblerre
Copy link
Contributor

Now that native overlays will be added in 1.16 (#41598), I don't think it makes sense to fix this issue in go/packages' overlay processing. Let's move this issue out of the v1.0 milestone and check back in after 1.16 is released.

@stamblerre stamblerre reopened this Sep 25, 2020
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Sep 25, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default Nov 13, 2020
@stamblerre
Copy link
Contributor

I've just confirmed that this is fixed with Go 1.16. Closing.

vscode-go: gopls by default automation moved this from Needs Triage to Done Nov 13, 2020
@stamblerre stamblerre removed this from the gopls/vscode-go milestone Nov 13, 2020
@golang golang locked and limited conversation to collaborators Nov 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
No open projects
vscode-go: gopls by default
  
Done
Milestone
No milestone
Development

No branches or pull requests
4 participants
@myitcv @stamblerre @gopherbot @heschi