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: once Go 1.23 is released, require Go 1.21+ (forward compatibility) to build gopls #65917

Closed
5 tasks done
findleyr opened this issue Feb 24, 2024 · 28 comments
Closed
5 tasks done
Assignees
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
Contributor

findleyr commented Feb 24, 2024

As described in some detail in #50825, Go version compatibility has long been a pain point for gopls. At the time that issue was written, we hadn't deprecated support for building gopls at any Go versions. Since then, we've successfully dropped support for Go 1.12-1.17, and very soon will drop support for 1.18. These deprecations have for the most part gone smoothly, and we haven't heard about users getting stranded as a result.

Nevertheless, gopls's nominal support for building with the past 4 Go versions continues to cause friction, as outlined in #50825. With Go 1.21's forward compatibility, we look forward to the day when gopls' minimum supported Go version is 1.21, so that we can rely on forward compatibility to build gopls with the latest Go toolchain.

Per the support window of 4 trailing Go versions, that day would occur upon the release of Go 1.24. This issue proposes to move that day up one release cycle, to the release of Go 1.23 this August.

The rationale is as follows:

  • The original support window of 4 trailing Go versions was related to the limited availability of recent Go versions in various operating system distributions and cloud providers. Since then the situation has improved significantly, and most release pipelines stay pretty up to date.
  • Better support for language versions in the Go toolchain (go list, go/types, etc.) has made it easier to use the latest version of Go in your development environment while targeting deployment with older Go versions.
  • Upon the release of Go 1.22, our survey results from VS Code indicated that 91% of our users were on Go 1.20, Go 1.21, or Go 1.22. Translating this to the release of Go 1.23, this suggests that 91% of users would be on Go 1.21 or later. That's a very large majority.
  • VS Code support for managing Go and gopls installations has improved significantly (and we can do more before 1.23 if necessary).
  • We have plans for standard library contributions in 1.23 that we want all gopls users to benefit from.

This last point is crucial: one of our major goals for 2024 is improved refactoring, and a foundation of that will be improvements to the AST, parser, and type checker (for example fixing #20744). Additionally, with type parameters on aliases (#46477) likely to land in Go 1.23, we are doing a lot of work (in #64581) to handle the new representation of Aliases added to go/types in Go 1.22. It would be a lot of additional effort (and arguably infeasible) to land these improvements with Go 1.23 while simultaneously continuing to support gopls built with older Go versions.

So, in summary, I think requiring Go 1.21+ to build gopls (via downloading the 1.23 toolchain) will save us a lot of resources and allow us to deliver a more reliable and robust product, and there seems to be little downside to dropping support for Go 1.20 at the time 1.23 is released.

EDIT: here are the actions to close this bug, assuming there are no objections:

  • update the version support documentation at https://github.com/golang/tools/blob/master/gopls/README.md
  • update the version compatibility table in VS Code; gopls@v0.16.0 will be the last gopls version to support both Go 1.19 and Go 1.20.
  • start warning users who build gopls@v0.16.0 with Go 1.19 or Go 1.20
  • Once go1.23rc1 is released, update gopls/go.mod with go 1.23rc1, and officially drop compatibility for older Go versions.
  • make a decision about what to do for CI coverage. It seems OK to leave one legacy LUCI builders, since they will exercise Go command integration with older Go versions

CC @golang/tools-team

@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 24, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 24, 2024
@pjweinb
Copy link

pjweinb commented Feb 24, 2024 via email

@mvdan
Copy link
Member

mvdan commented Feb 25, 2024

I fully agree with this. I still hear some users talking about their wish for gopls to be faster or more robust, but I have yet to hear about any pain relating to older Go versions.

Perhaps the first release to require Go 1.21 or later could point users to the last gopls version supporting older Go versions, which could be useful as a temporary fallback.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.16.0 Feb 26, 2024
@findleyr
Copy link
Contributor Author

I fully agree with this. I still hear some users talking about their wish for gopls to be faster or more robust, but I have yet to hear about any pain relating to older Go versions.

@mvdan thanks for that evidence. I think we may hear about some pain related to this, if we really are requiring 9% of users to take action if they want to upgrade, but I think that inconvenience is well offset by the improved robustness and team efficiency resulting from needing to support building with only the latest Go version.

Perhaps the first release to require Go 1.21 or later could point users to the last gopls version supporting older Go versions, which could be useful as a temporary fallback.

We'll definitely do that. We also have a practice of warning folks interactively from the (N-1)st gopls version when their Go version will be dropped by the Nth gopls version.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/578039 mentions this issue: gopls: use static hooks rather than optional hooks

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/588763 mentions this issue: gopls: warn about Go 1.19 and Go 1.20

gopherbot pushed a commit to golang/tools that referenced this issue Jun 3, 2024
Update the support table to warn when users install gopls with Go 1.19
or 1.20, or have these older Go versions in their PATH.
Clarify current and future support in the README.

Fixes golang/go#50825
Updates golang/go#65917

Change-Id: I99de1a7717a8cf99cae1a561ced63e9724dfff66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/588763
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
@findleyr
Copy link
Contributor Author

findleyr commented Jun 3, 2024

v0.16.0 tasks are now complete on this issue. Moving to the v0.17.0 milestone to finish.

@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Jun 3, 2024
@findleyr
Copy link
Contributor Author

With respect to CI coverage: we decided to continue maintaining legacy LUCI builders for the last 3 go versions (not 4, as before). This narrows our support window slightly for Go command integration, but data indicate only a small fraction of users are on go version N-3. This also means that once 1.23 is released, we can rely on toolchain upgrades to build gopls (thanks to https://crrev.com/c/5589446), and get integration testing with older Go versions for ~free: since gopls integration tests run on a temp directory, they will integrate with the local toolchain go command. We can simply drop the 1.20 builder.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/593682 mentions this issue: x/tools: update go.mod to 1.22 and gopls to go1.23

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/593683 mentions this issue: x/tools: assume go1.22 and simplify

@dylan-bourque
Copy link

dylan-bourque commented Jun 21, 2024

For an alternative point of view, some development orgs (like mine) are not able to immediately move to new releases of Go right away due to external constraints (regulatory/compliance restrictions, "conservative" leadership, etc.), including explicitly setting GOTOOLCHAIN=local to (mostly) disable automatic toolchain updates.

We do have some engineers who adopt new versions immediately, sometimes even during RC stages, to do verification testing against our codebase, but for the majority of our engineers it will be some time before they are using Go 1.23. We are probably about evenly distributed between 1.21.x and 1.22.x today. There will also be a delay for those running 1.21.x today moving to 1.22.x once 1.23 is released.

With this change, most of our engineers will be unable to go install golang.org/x/tools/gopls@latest (which is what VS Code does internally to update gopls IIRC) because they will be running an "old" version of Go (1.22.x or 1.21.x) even though that version is fully supported by Go itself. This will continue to be the case until/unless everyone updates to 1.23.x.

While I understand the engineering trade-offs related to supporting prior versions of Go, a change that requires all users update to the latest release of Go in order to use gopls seems like an undesirable user experience.

@findleyr
Copy link
Contributor Author

findleyr commented Jun 21, 2024

Thank you for sharing this, @dylan-bourque (and for the discussion in Slack). CC @rsc as I think he would be interested in this perspective.

A point of confusion for me is that when installing gopls@latest you are intentionally getting the latest version of gopls and its dependencies (which includes some limited third party dependencies, such as staticcheck). I don't see why this is fundamentally different from updating the Go toolchain. I'm not being rhetorical: is there something I'm missing that's different? Is the concern that the toolchain is distributed as binary? I understand that organizations may have concerns about upgrading Go in their production workloads, but those concerns don't seem to apply to developer tools. Put differently, what would the policy be if we distributed gopls as a binary? Would you only use older versions of gopls? If so, you can still use older versions of gopls with older Go versions.

We expected that the largest barrier to building with the most recent version of Go was the pain of managing multiple go versions, and that this barrier was mostly eliminated with toolchain upgrades.

To repeat some of what we discussed in Slack: we are not choosing to use the latest version of Go out of a principle that we should only support the latest version. We are choosing this path because we think it leads to the best outcome for our users. While some support burden is alleviated by not having to support goN-3 and goN-2 anymore, the largest win for us as gopls maintainers comes when we don't have to support goN-1, because that means we only ever have to support one version of go/parser and go/types. We really felt this pain recently when adding support for the new gotypesalias=1 behavior.

Furthermore, only supporting building with goN means that if we want to make improvements in the standard library, we take advantage of those improvements in 2-6 months (depending on where we are in the Go dev cycle), rather than 8-12 months if supporting the last two versions. This fundamentally changes the way we approach some problems.

There are also other second order benefits to only supporting the latest Go version. For example, some of the value gopls brings to the Go team is simply being a large and complex project, written in Go, to inform our understanding of the language. If we're stuck targeting an old Go version, we lose a lot of the value of our experience. I personally observed this with generics: while I was very involved with the generics implementation in go/types, it wasn't until within the last year that I could actually use generics in my day to day work on gopls.

So, in summary, while I wouldn't be fundamentally opposed to only advancing gopls's go version in its go.mod file to goN-1, we'd need evidence that the benefit to users of this more conservative approach offsets the benefits I described above.

@mvdan
Copy link
Member

mvdan commented Jun 25, 2024

With this change, most of our engineers will be unable to go install golang.org/x/tools/gopls@latest (which is what VS Code does internally to update gopls IIRC) because they will be running an "old" version of Go (1.22.x or 1.21.x) even though that version is fully supported by Go itself. This will continue to be the case until/unless everyone updates to 1.23.x.

It seems to me like IDEs like VS Code should be adjusted to no longer blindly install gopls@latest if the user is on an older Go major version (e.g. Go 1.21.x) and isn't using automatic toolchain upgrades (either because they're not yet available to that older version, or because the user set GOTOOLCHAIN=local).

@findleyr In such a scenario, what should the IDE do to figure out what is the newest gopls version it can safely install without forcing the user to upgrade their Go version? The IDE should show a warning in any case, but I suspect that allowing the user to install a slightly older gopls version is better than refusing to continue. From reading https://github.com/golang/tools/releases I can see that gopls v0.16.x is the highest that a user stuck with Go 1.21.x as a build version can install, but there's nowhere to programmatically obtain that data.

Perhaps you could publish a JSON file in a well-known place, to map the latest minor version of gopls that each Go build version can install without toolchain upgrades, like:

{
    "go1.22": "v0.16",
    "go1.21": "v0.16",
    "go1.20": "v0.16",
    "go1.19": "v0.16",
    "go1.18": "v0.15",
    [...]
}

@findleyr
Copy link
Contributor Author

@mvdan you're right, we've been copying the version compatibility table to VS Code, but it would be better if it were programmatically available somewhere.

For example, we could publish a tiny submodule consisting of just a JSON table, and perhaps a small binary that outputs the version to install (considering GOTOOLCHAIN), or installs gopls, or both. CC @hyangah: Hana, I know VS Code has essentially this in its upgrade logic (which is written in Go). Should we move that to a common location so that it can be used by other editors?

@hyangah
Copy link
Contributor

hyangah commented Jun 26, 2024

The gopls wants to depend on the latest version of go standard library. IMO that's not different from gopls choosing to upgrade its other 3rd party dependencies, except that the only way to use the latest go standard library is to use the latest toolchain.

With this change, most of our engineers will be unable to go install golang.org/x/tools/gopls@latest (which is what VS Code does internally to update gopls IIRC) because they will be running an "old" version of Go (1.22.x or 1.21.x) even though that version is fully supported by Go itself. This will continue to be the case until/unless everyone updates to 1.23.x.

It seems to me like IDEs like VS Code should be adjusted to no longer blindly install gopls@latest if the user is on an older Go major version (e.g. Go 1.21.x) and isn't using automatic toolchain upgrades (either because they're not yet available to that older version, or because the user set GOTOOLCHAIN=local).

Using another helper binary to pick up the buildable gopls version is definitely an option to consider. But, why is it ok to run the (arbitrary) helper binary, but it is not the official go binary (which are signed, and went through a rigorous release process)? We can also embed the version list in the extension; the extension already does it for other 3rd party tools like gofumpt, staticcheck, golangci-lint.

But, note that there is a risk of the old gopls version no longer works with the latest editor extensions. We only actively test against the latest gopls versions.

For example, VS Code Go now delegates many features and functionalities to the gopls and plans to move more to gopls. Test discovery, unittest generation, struct tags modifications are some of the tasks in our plan. They will stop working if gopls is old.

I think setting GOTOOLCHAIN=auto for gopls or other tools installation is the best for the VS Code Go extension.

We will improve the VS Code Go extension to provide a useful feedback message when the installation cannot complete.

Can we discuss the case where using GOTOOLCHAIN=auto go install gopls@latest is undesirable? That will help us formulate the feedback message. Some cases and resolution I have in my mind:

  • Working in an air-gapped environment: go install won't work anyway. Provide the manual installation instruction.
  • Working with the old version of go that is out of the supported version range -> downgrade the vscode go extension and install the compatible version of gopls.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/605295 mentions this issue: main.star: drop release-branch.go1.21

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/605000 mentions this issue: main.star: drop TOOLS_GO_BRANCHES

gopherbot pushed a commit to golang/build that referenced this issue Aug 14, 2024
The Go 1.23.0 release is out, so 1.22 is the oldest supported release.¹
The previous infrastructure (cmd/coordinator) does this automatically,²
whereas here I authored this CL manually. We'll explore automating it
later on³, possibly with relui generating the CL.

This also prepares to drop the special cased extra branches for x/tools
per plan in go.dev/issue/65917, but does so in the follow up CL so that
it can be submitted slightly later, separately.

¹ https://go.dev/doc/devel/release#policy
² It too used to be manual, until we automated it in go.dev/issue/34097
  in 2019.
³ I said the same thing 6 months ago in CL 562395. That said, we need
  to do a bit more one-off changes here since x/tools is dropping its
  extended Go testing.

For golang/go#65917.

Change-Id: Id5228b484995ded1094aaeb6b747f2f507ed241d
Reviewed-on: https://go-review.googlesource.com/c/build/+/605295
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/610395 mentions this issue: main.star: make 1.19 and 1.20 TOOLS_GO_BRANCHES trybots optional

gopherbot pushed a commit to golang/build that referenced this issue Sep 3, 2024
We still want to be able to trigger them in the very immediate future,
but they don't need to be included by default and block submission.

For golang/go#65917.

Change-Id: I2d8d0602c631bd7335488c4c473081812fc07dde
Reviewed-on: https://go-review.googlesource.com/c/build/+/610395
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/610435 mentions this issue: all: with 1.23 out, update Go directive to Go 1.22

gopherbot pushed a commit to golang/tools that referenced this issue Sep 4, 2024
Now that 1.23 is out, and 1.19 and 1.20 trybots have been updated to be
optional in CL 610395, we can update the go.mod files to use Go 1.22.6.
This will unblock tagging x-repos, since there won't be conflicts with
x/mod.

A subsequent CL will update the gopls go.mod file to 1.23.1, once that
minor version is released (shortly).

For golang/go#65917

Change-Id: I663918f7be5a3e340703ae82e13b93e9e0f2877d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/610435
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/610936 mentions this issue: gopls/go.mod: update the go directive to 1.23.1

gopherbot pushed a commit to golang/build that referenced this issue Sep 6, 2024
The Go 1.23.0 release is out. This drops many of the special cased
extra branches for x/tools per plan in go.dev/issue/65917, keeping
only go1.21 as a special case for the needs of gopls v0.17.0.

For golang/go#65917.

Change-Id: I2af34126099b58f6445a5198f2b06c654df0ae6d
Reviewed-on: https://go-review.googlesource.com/c/build/+/605000
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/611837 mentions this issue: x/tools/gopls: delete code obsoleted by go1.23

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/612037 mentions this issue: gopls: use Func.Signature everywhere

gopherbot pushed a commit to golang/tools that referenced this issue Sep 9, 2024
The util/slices package has been deleted; but util/maps
has been renamed to moremaps since it still has some
useful things.

Note: the standard maps.Clone may return nil.

Updates golang/go#65917

Change-Id: Ide8cbb9aa13d80a35cdab258912a6e18a7db97c6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/611837
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Sep 9, 2024
Updates golang/go#65917

Change-Id: I20bec8b0a1778f0d2d81f729d12ba966799c7805
Reviewed-on: https://go-review.googlesource.com/c/tools/+/612037
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/612055 mentions this issue: internal/settings: simplify linking now that we only build with 1.23

gopherbot pushed a commit to golang/tools that referenced this issue Sep 9, 2024
Since gopls now only builds with the latest version of Go, we no longer
need special linking to align with the compatibility windows of gufumpt
or staticcheck.

Updates golang/go#65917

Change-Id: I7d5ebe6807b34ed8d44e726c7a6585d4c7c7e696
Reviewed-on: https://go-review.googlesource.com/c/tools/+/612055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
@ldemailly
Copy link

Curious why you jumped directly from 1.19 in 0.24.0 to 1.22 in 0.25.0 ?

This basically cascade force 1.22 to every downstream which for a library is not desirable

(ps I'm asking that about txtar and not gopls but they happen to share the go.mod)

@findleyr
Copy link
Contributor Author

@ldemailly that is really a question about #69095. x/tools/gopls is a nested submodule inside of x/tools. This issue is related to the change in x/tools only to the extent that we did not want to block the change in x/tools.

@ldemailly
Copy link

Thanks for the reply, I came here because that's what the CL that changed go.mod pointed to :) will go to #69095 to argue about only changing go line if the code actually needs it

@findleyr
Copy link
Contributor Author

Indeed, should have mentioned #69095 in that commit. Thanks.

@adonovan
Copy link
Member

I will go to #69095 to argue about only changing go line if the code actually needs it

To be frank, your chances of success are slim. The purpose of the policy is to unburden us from caring about (building, testing, debugging) older versions of the toolchain; upgrading the go.mod file only if the code actually needs it would defeat that.

gopherbot pushed a commit to golang/tools that referenced this issue Sep 20, 2024
This CL does not include simplifications to gopls.

Updates golang/go#65917
Updates golang/go#69095

Change-Id: I2b54992681e2c671324e22668d8401962a1d2363
Reviewed-on: https://go-review.googlesource.com/c/tools/+/593683
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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

8 participants