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: consider static configuration for multiple builds #65757

Open
findleyr opened this issue Feb 16, 2024 · 2 comments
Open

x/tools/gopls: consider static configuration for multiple builds #65757

findleyr opened this issue Feb 16, 2024 · 2 comments
Labels
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

@findleyr
Copy link
Member

findleyr commented Feb 16, 2024

This is a follow up to #29202. In that issue, we added dynamic support for tracking multiple GOOS/GOARCH combinations, based on the set of open files. (e.g. if you open foo_windows.go, gopls will create a new View with GOOS=windows).

In some cases, it may be desirable for users to explicitly configure these builds, for example if they know a priori that they are targeting several ports, or sets of build tags. We should consider supporting this, for example via configuration like so:

"builds": [{
  "env": ["GOOS=windows", "GOWORK=off"],
  "flags": ["-tags", "example"]
}, {
  // another build...
}]
@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 Feb 16, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 16, 2024
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.16.0 Feb 16, 2024
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Mar 13, 2024
@bersace

This comment has been minimized.

@firelizzard18
Copy link
Contributor

@findleyr @hyangah I think this is an opportunity to bring vscode-go and gopls's notions of build configuration closer. I would like to see one of two scenarios:

  1. gopls and vscode-go use identical configuration for builds (moving forward). As in, vscode-go has a setting e.g. "go.builds": [...] that it uses for its own logic as well as passing it verbatim to gopls; or
  2. vscode-go passes configuration to gopls which then passes build configuration info back to vscode-go.

Of those two, I think option (1) is more viable because option (2) would mean vscode-go is somewhat crippled if gopls is not present or has an issue. Alternatively, we could do a hybrid: vscode-go reads the build configuration and acts on it; but if gopls is present then vscode-go passes the build configuration to it and gopls returns an 'expanded' build configuration. I think this could give us the best of both worlds. For example, if no build configuration is specified then vscode-go will do its best to infer it, but if gopls is present then it will provide a detailed build configuration including tool paths, etc.

I'd also like to talk about the current state of build configuration and how we can improve on it. As discussed on golang/vscode-go#3670, there are some issues with go.buildFlags, go.buildTags, go.testFlags, and friends. The key issue is build tags, due to how those settings are handled and how that can affect how tests are detected and executed, and adding go.benchFlags would make it extra complicated (golang/vscode-go#1904). I propose:

  • Build tags have their own configuration option in a build configuration, sibling to "env", "flags", etc.
  • Specifying build tags in "flags"/"testFlags"/etc is explicitly unsupported and will either be ignored or lead to undefined behavior.
  • go.testTags is no longer supported?
  • Instead of buildFlags []string; testFlags []string where (if specified) testFlags completely replaces buildFlags when testing, both flag sets are strongly typed so flags can be specifically overridden
  • type BuildFlags and type TestFlags include extra []string for anything we don't have a strongly typed field for, but we explicitly state that using extra may lead to the issues mentioned above.

I suggest something along these lines for build configuration:

build_config.go
type BuildConfiguration struct {
	Env   map[string]string
	Tags  []string
	Flags BuildFlags

	TestEnv    map[string]string
	TestFlags  TestFlags
	BenchFlags TestFlags
}

type commonFlags struct {
	// Flags that are valid for `go build` and `go test`

	Extra     []string // Additional flags, may cause issues e.g. -tags
	Ldflags   []string
	Gcflags   []string
	Cover     bool
	Covermode string
	Coverpkg  []string
}

type BuildFlags struct {
	commonFlags

	// Flags that are only valid for `go build`
}

type TestFlags struct {
	commonFlags

	// Flags that are only valid for `go test`
	Count int
	CPU   int
	Short bool
}
  • That's certainly not all the flags we should have, just enough to demonstrate the concept.
  • There are some flags that I think should be omitted. Specifically some flags can interfere with the function of the test explorer, such as -v and -json.
  • vscode-go should probably have some notion of the "default build configuration" whether "default" is just the first one or there's a field that marks which one is the default or something, to facilitate backwards compatibility with go.buildFlags/etc. vscode-go will merge the old config options into the default build configuration. Alternatives are: an implicit "legacy" build config generated from the old options, or completely ignoring them when a build configuration is specified; but I think both of those are worse.
  • vscode-go has go.testEnvFile - maybe we want to include something like that (probably also for the regular environment).
  • If we move forward with a (1) + (2) hybrid then I think the build configuration should have additional options such as the go binary path so gopls can pass those back to vscode-go.

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

4 participants