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: improve handling for build tags #29202

Open
stamblerre opened this issue Dec 12, 2018 · 61 comments
Open

x/tools/gopls: improve handling for build tags #29202

stamblerre opened this issue Dec 12, 2018 · 61 comments
Labels
FeatureRequest 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.
Milestone

Comments

@stamblerre
Copy link
Contributor

stamblerre commented Dec 12, 2018

We need to handle build tags correctly in the LSP. If a user is on Linux, they should still see completions, diagnostics, etc. for files tagged with any other OS.

@gopherbot gopherbot added this to the Unreleased milestone Dec 12, 2018
@stamblerre stamblerre self-assigned this Dec 19, 2018
@stamblerre stamblerre added gopls Issues related to the Go language server, gopls. and removed gopls Issues related to the Go language server, gopls. labels Mar 12, 2019
@glerchundi
Copy link

Bitten by this also, lost an hour trying to figure out why after migrating to gopls basic functionalities didn't work...no import-organize, no format, no nothing. Afterwards I found that it's was working properly for everything except for files with build tags as mentioned in #31286.

@myitcv
Copy link
Member

myitcv commented Apr 28, 2019

I'm wondering whether this is the right issue to become the umbrella issue for our long-running discussion about context?

@sjdweb
Copy link

sjdweb commented May 1, 2019

This is killing me with integration tests that have build tags. Any workarounds known?

@christiankiely
Copy link

@sjdweb current workaround I'm using is to set the GOFLAGS environment variable. This is the configuration I'm using in VS Code:

"gopls.env": {
        "GOFLAGS": "-tags=test"
    },

@myitcv
Copy link
Member

myitcv commented May 2, 2019

This was brought up on Tuesday's golang-tools call.

@christiankiely - the "editor level" GOFLAGS approach your describe works in some cases, but not all. Because setting the flag in that way sets the flag for all modules and packages, i.e. for the gopls instance used by the editor.

In discussions we've had thus far it seems for a complete solution we need to be able to define the sets of valid build flags at various levels:

  • Editor level (i.e. throughout editor)
  • Module level
  • Package (tree) level
  • File level

We also need:

  • the ability to specify, from within the editor, which build flags are in operation, because for a given file/package/module there might be multiple valid options
  • the ability to then pass this choice on a per request basis to gopls

The conclusion on Tuesday's call was broadly that there are other higher priority issues right now; add to that, a solution that doesn't involve LSP spec changes isn't exactly jumping out at us.

cc @ianthehat @dominikh

@breml
Copy link
Contributor

breml commented May 11, 2019

Bitten by this as well. I use build tags to distinguish between normal builds and debug builds. Now VSCode reports logError redeclared in this block error and the file is constantly marked as having errors.

@gracenoah
Copy link

Another populate use case: when using wire, there are two copies of the initializers:

  1. The template in a file with a build tag that never gets compiled
  2. The real one generated by wire with no build tags

gopls highlights them as duplicate definitions because it ignores build tags.

Is there a short term workaround in this simpler use case? Some way to make gopls ignore files with the wireinject build tag?

@stamblerre
Copy link
Contributor Author

Thank you for sharing this info. We realize that this is an issue that affects a lot of people, so we're trying to come up with a viable solution. It's a complicated problem, but to start, I think we can try to disable diagnostics on files that would be excluded given your GOOS/GOARCH.

@evanphx
Copy link
Contributor

evanphx commented May 25, 2019

As a data point, I'm experiencing this today in any package that picks up os/user because it uses build tags to decide how to wire in OS user lookup. In my case, it's that analysis fails because cgoLookupHost isn't defined as files aren't being evaluated.

I understand that you'd like for gopls to allow for reading and analysis of files intended for a different GOOS that the one that the editor is on, but given how packages are structured, you'd having to perform analysis on the matrix of build tags. That seems.... time consuming at best. Plumbing through a default tag set seems like an easy way to at least get folks moving again.

I'm happy to look into how to do that if need be! Please just let me know.

@evanphx
Copy link
Contributor

evanphx commented May 25, 2019

Looks like it's also firing from net looking like:

/usr/local/Cellar/go/1.12/libexec/src/net/lookup_unix.go:81:24: undeclared name: cgoLookupHost

@zchee
Copy link
Contributor

zchee commented May 26, 2019

@evanphx I also found this issue.
The workaround is launch gopls use CGO_ENABLED=0 or something. Note that might be vscode can't that workaround AFAIK. Also not solved if set CGO_ENABLED=0 to env. Because it is late to place.
#29202 (comment)

Now I develop the client side for IDE written in Go, it's implemented set some env to launch gopls binary using set to exec.Command.Env. I did solve the use it.
Anyway, We should including CGO_ENABLED=0 to gopls send Build info.

@evanphx
Copy link
Contributor

evanphx commented May 26, 2019

@zchee Ah! That makes a lot of sense, I'll start it with CGO_ENABLED=0 and see how it works.

@zchee
Copy link
Contributor

zchee commented May 27, 2019

@evanphx
Also, I was send CL (but I think still not user friendly) for support build tags.
https://go-review.googlesource.com/c/tools/+/178782
It solved if your code needs any build tags such as purego, static.

@zchee
Copy link
Contributor

zchee commented May 27, 2019

@evanphx
FYI, ref for your issue root cause:
#31705

@zchee
Copy link
Contributor

zchee commented Jun 4, 2019

@stamblerre
Can we close this issue by below CL?

https://go-review.googlesource.com/c/tools/+/178782

@stamblerre
Copy link
Contributor Author

I think we will still need special handling for build tags so that users don't need special configurations to get gopls features in files tagged for different build systems.

@calebstewart
Copy link

This and #49657 were the only relevant results for my internet searches, so, for historical reasons, my solution for emacs is to use setenv at the global scope and modify GOFLAGS. This isn't perfect but has worked for me:

(setenv "GOFLAGS" "-tags=linux,windows")

Is there a plan to make something like this a permanent in gopls? Is there some problem with just specifying multiple platform tags by default? It hasn't seemed to cause any problems for me specifically. It doesn't make much sense to me that gopls would care about the platform, and simply failing for platform-tagged files breaks the entire workflow of developing cross-platform applications. This is almost certainly an undocumented bug in gopls, unless I missed some documentation some where (which I'd be happy if someone shared 😄).

@hyangah
Copy link
Contributor

hyangah commented Nov 23, 2021

@calebstewart That's insteresting. gopls heavily depends on go list and other go commands to parse go code, and by your setting -tags=linux,windows, the flag is passed to all the go commands gopls uses. If you see gopls and so go commands work with -tags=linux,windows, that's by accident, not intentional.

And, more puzzling thing is that AFAIK platform (os/arch) are special and go commands select them based on GOOS/GOARCH environment variables, not based on the -tags flag in most cases. So, I am not sure what's working and what's not working with specifying OSes using -tags.

@hsiaosiyuan0
Copy link

hsiaosiyuan0 commented Nov 25, 2021

for vscode users, update your PROJECT_ROOT/.vscode/settings.json like below may resolve this problem:

{
  "go.toolsEnvVars": {
    "GOOS": "js",
    "GOARCH": "wasm"
  }
}

@calebstewart
Copy link

@hyangah interesting, it could have been some configuration specific to this project (it's a work project, and I'm not the one who configured it), then, but that seemed to be the best approximation of the suggestions I was seeing online. Admittedly, I'm not exactly sure why it worked for me either, but all I can say is that it did. 👀

@ahmafi
Copy link

ahmafi commented Dec 7, 2021

I have a similar problem, I have separate files (config_windows.go, config_linux.go, config_darwin.go) for setting platform-specific configs, and by default, I don't get any code completion on Windows or darwin codes (I'm using VSCode on Linux) and it shows the following message:

No packages found for open file /home/amir/projects/Memor/memor/pkg/config/config_darwin.go: <nil>.
If this file contains build tags, try adding "-tags=<build tag>" to your gopls "buildFlags" configuration (see (https://github.com/golang/tools/blob/master/gopls/doc/settings.md#buildflags-string).
Otherwise, see the troubleshooting guidelines for help investigating (https://github.com/golang/tools/blob/master/gopls/doc/troubleshooting.md).go list

after adding the following tags I'll get a duplicate declaration error on these files because I'm declaring the same things in these 3 files.

"gopls": {
  "build.buildFlags": ["-tags=linux,windows,darwin"],
}

@Michael-F-Ellis
Copy link

Michael-F-Ellis commented Jan 12, 2022

@ahmafi I can reproduce the same error messages from vscode/gopls with a trivially simple project using containing 3 files:

common.go

package main

import "fmt"

func common() {
	fmt.Println("This is common")
}

xmain.go

//go:build xmain

package main

import "fmt"

func main() {
	fmt.Println("This is xmain")
	common()
}

ymain.go

//go:build ymain

package main

import "fmt"

func main() {
	fmt.Println("This is ymain")
	common()
}

With no gopls build flags defined, I get the "no packages found" message. If I add the following to settings.json, I get the duplicate declaration error.

"gopls": {
		"build.buildFlags": [
			"xmain",
			"ymain",
		],
	},

There are no errors reported when I run go vet on the project and I'm able to build and run either variant, e.g. go build -tags xmain without errors.

@stamblerre This is a major limitation for one of my current projects. I'd love to see a solution or a viable workaround.

@findleyr
Copy link
Contributor

@Michael-F-Ellis you should be able to work on xmain OR ymain by setting just one of those two build tags, the same as you do when you run go build -tags xmain. As described in #49104 (comment), go vet and go build have the same limitations as gopls: only one set of build tags can be active at a given time.

The critical missing feature for gopls is the ability to edit using multiple sets of build tags simultaneously.

@syncjuncture

This comment was marked as off-topic.

frobware added a commit to openshift/cluster-ingress-operator that referenced this issue Apr 22, 2022
This is largely a quality of life/tooling improvement. I've removed
the build e2e tags in the test/e2e directory and explicitly made the
existing `make test` rule only run in the ./pkg directory; it
previously ran from the top-of-the-tree (which is totally reasonable).

The two make targets `test` and `e2e` continue to test what they
previously did. The motivation for this change was not having to futz
with my gopls configuration when editing files in test/e2e.

There are other ways to skin this cat that also involve no build tags
at all. This satisfied my current needs.

Ideally gopls gains build tag support:

  golang/go#29202
frobware added a commit to frobware/cluster-ingress-operator that referenced this issue May 12, 2022
This is largely a quality of life/tooling improvement. I've removed
the build e2e tags in the test/e2e directory and explicitly made the
existing `make test` rule only run in the ./pkg directory; it
previously ran from the top-of-the-tree (which is totally reasonable).

The two make targets `test` and `e2e` continue to test what they
previously did. The motivation for this change was not having to futz
with my gopls configuration when editing files in test/e2e.

There are other ways to skin this cat that also involve no build tags
at all. This satisfied my current needs.

Ideally gopls gains build tag support:

  golang/go#29202
@alexgille
Copy link

Hello,

Is this topic still in progress or should we not expect any improvement regarding this limitation?

Thanks,
Best regards

@findleyr
Copy link
Contributor

findleyr commented Aug 4, 2022

Hi all, this issue deserves an update (and sorry @alexgille for missing your inquiry above). Let me summarize the current state of things:

We know that support for multiple sets of build tags is one of the most desired features for gopls. Unfortunately, build tags pose a fundamental problem that we can't fix without changing many other things about gopls. Because several of gopls' algorithms rely on having an active graph of go/types.Packages in memory, and go/types relies on the canonical identity of its objects, gopls can't simply process multiple sets of build tags without maintaining two copies of the package graph (because ~everything will at some point in its transitive imports depend on a package for which there are multiple definitions based on build tags). The package graph drives most of gopls' memory usage, so maintaining N copies of the package graph corresponds to approximately an N-fold increase in memory (and often CPU). Given that memory usage is a huge pain point for our users, a solution that depends on throwing memory at the problem will probably not be viable for many users.

Even if that solution is acceptable for some users with small workspaces, it poses a significant burden on the code in an area where we have historically had a lot of bugs: package metadata tracking and package invalidation. I am currently trying to refactor that area of gopls and fix all the bugs. In gopls@v0.9.0 the representation of metadata was refactored, which contributed a piece of the performance improvements observed in that release. In gopls@v0.9.2, I'm trying to fix all the outstanding bugs we know about related to metadata. So here also, we are hesitant to throw more complexity at the problem.

Longer term, an even higher priority for us is to make gopls scale better and load faster. In order to achieve this, we will inevitably need to break our dependence on the package graph. The first step toward this is rewriting key algorithms to be based on data structures that can be incrementally updated at the package level. Once we do this, the problem of handling multiple sets of build tags becomes tractable.

So resolving this issue can't realistically happen soon, but is part of our longer terms goals for gopls.

In the meantime, I think we can still improve the status-quo. For example:

@Torwalt
Copy link

Torwalt commented Aug 31, 2022

For nvim-lspconfig users (with nvim-lsp-installer) following settings worked for me:

lspconfig.gopls.setup {
    settings = {
        gopls = {
            env = {
                GOFLAGS = "-tags=windows,linux,unittest"
            }
        }
    }
}

with files looking like

thing_test.go:

//go:build unittest

package thing_test

and thing.go

package thing

all lsp features work.

@eric-burel
Copy link

eric-burel commented Jan 12, 2023

Cross-posting from golang/vscode-go#1799:

Is there an alternative without relying on setting env variable in VS code settings?
VS code doesn't support nesting settings so it's annoying when you mix go server code and go wasm code or more broadly different architectures.

Maybe a top-level comment in the file? This seems to have been mentioned at the begging of this thread: #29202 (comment), #29202 (comment)

(there are many messages, sorry if it has already been mentioned/answered)

@inliquid
Copy link

@eric-burel as a workaround you can open projects in different VS Code instances. WASM client code can be placed somewhere in project dir and can have it's own workspace settings in .vscode/settings.json. I'm using it this way for many years already.

@404cn
Copy link

404cn commented Sep 19, 2023

For emacs and eglot users:

(setq-default eglot-workspace-configuration
              '((:gopls
                 (:ui.completion.usePlaceholders . t)
                 (:ui.diagnostic.staticcheck . t)
                 (:build.buildFlags . ["-tags" "sometag"]))))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest 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
None yet
Development

No branches or pull requests