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

all, x/build/cmd/relui: automate go directive maintenance in golang.org/x repositories #69095

Closed
dmitshur opened this issue Aug 27, 2024 · 48 comments
Assignees
Labels
Builders x/build issues (builders, bots, dashboards) Proposal Proposal-Accepted
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Aug 27, 2024

Abstract

The value of the go directive in golang.org/x repositories is automatically maintained to be at least 1.(N-1).0, where Go 1.N is the most recent major Go release, and Go 1.(N-1) is the previous major Go release.

Background

In the beginning, there was the GOPATH mode and versions of dependencies of golang.org/x repositories weren't explicitly tracked. Go 1.11 introduced the module mode, and over time it became the default mode. All golang.org/x repositories had an initial go.mod file checked in, and that file was maintained manually. This meant that a bug fix or a new feature in one golang.org/x repository didn't benefit another golang.org/x repository until someone chose to manually update that dependency. It also meant that eventual updates sometimes jumped many versions at once to catch up. This was resolved in 2022, when an automated monthly relui workflow began to create tags and update golang.org/x dependencies across all golang.org/x repositories (issue #48523).

At this point there are upwards of 35 golang.org/x repositories. Owners of each repository update the "go" directive manually, ad-hoc, so golang.org/x repositories may receive different levels of "go" directive maintenance. For example, owners of the golang.org/x/mod module wished to use the new-to-Go-1.22 go/version package as soon as Go 1.23 came out, and so its "go" directive was recently updated to "1.22.0". On the other hand, golang.org/x/image hasn't been updated in a while, and its "go" directive is currently still at "1.18", which itself was an upgrade from "1.12" in CL 526895 as part of bringing all golang.org/x repos to use at minimum Go 1.18 language version (issue #60268).

Leaving go directive maintenance to be done entirely manually creates the possibility of some repositories staying on an older Go language version longer. When there's enough of a need to finally upgrade it to a recent Go language version, this requires a change across multiple major Go releases at once, which can be harder to review. Having continuous, smaller incremental upgrades requires creating many CLs for all of golang.org/x repositories every 6 months, which is toilsome if always done manually.

Design

Design document at go.dev/design/69095-x-repo-continuous-go.

CC @golang/release.

@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) Proposal labels Aug 27, 2024
@gopherbot gopherbot added this to the Proposal milestone Aug 27, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608955 mentions this issue: design/69095-x-repo-continuous-go.md: add design

gopherbot pushed a commit to golang/proposal that referenced this issue Aug 27, 2024
For golang/go#69095.

Change-Id: I6aa58154739456432b60ea332d324c5b7b048612
Reviewed-on: https://go-review.googlesource.com/c/proposal/+/608955
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
@adonovan adonovan moved this to Incoming in Proposals Aug 28, 2024
@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@rsc
Copy link
Contributor

rsc commented Aug 29, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals Aug 29, 2024
@findleyr
Copy link
Member

findleyr commented Aug 30, 2024

I think we should perhaps emphasize that once the go.mod go directive is updated to Go version 1.N-1.X, is our policy is going to be that package maintainers in that module should feel free to also break the build at older Go versions. This is implied by the reference to using the go/version package, but should be highlighted.

For example, right now x/telemetry has 1.20 in its go.mod file, but still builds with 1.19 (and this "feature" will still be relied upon by v0.16.2 of gopls, which is the final version of gopls supporting building with 1.19, before #65917 takes effect).

With that said, I think this proposal is good for Go overall. Not only will it reduce the toil of go directive maintenance, but it will also help eliminate significant complexity in these repositories related to build support for older Go versions. Additionally, with the help of all the work that has been done to make it easier to use a recent Go toolchain, I think this policy may indirectly help nudge the ecosystem away from using unsupported versions of Go.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/610716 mentions this issue: go.mod: for consistency with other repos, use 1.22.0 in go.mod

gopherbot pushed a commit to golang/tools that referenced this issue Sep 4, 2024
For golang/go#69095

Change-Id: I181e405753866699e70632e5057bb3c7e39e7dfb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/610716
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Robert Findley <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
@rsc
Copy link
Contributor

rsc commented Sep 11, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is #69095 (comment).

@adonovan
Copy link
Member

If/when this proposal is accepted, we should add links to the statement of the new policy from the Go 1 compatibility promise and from the Go release policy, to ensure it is discoverable. (And perhaps from the README or go.mod file in the root of each x/ repo.)

@ldemailly
Copy link

ldemailly commented Sep 17, 2024

For libraries, I think not aggressively forcing upgrades on dependencies is a good thing, and so is staying compatible for more than 2 versions, when 1.23 is just released for instance, probably 1.20 should still be the most recent "forced" upgrade

So imo go.mod go line should just be the language features, not compiler bug fixes etc... that's handled by toolchain and build in final product and govulncheck.

An example of disruptive forced upgrade is x/tools 0.25 which suddenly jumped form go 1.19 to go 1.22(.0) for seemingly no "need"

Specifically I think a lot of even the newer libraries really only need "go 1.18", ie generics and should stay that way.

That is not to say that people shouldn't be using a recent, supported, fully patched go version (eg 1.22.7 at time of this writing), yet libraries should indicate language issue (ie that they are or are not yet using a new iterator etc) not push a cascade of updates downstream

Edit: also while here I can add my pet peeve: the go line should be language version (ie 1.23) not specific patch level (ie 1.23.0) - until 1.21 it was like that and should imo have stayed that way... semver is perfect for toolchain and specific releases but imo doesn't belong in the go line, that .0 just muddies everything (and looks off to see 1.22.0 when 1.22.7 is out)

@adonovan
Copy link
Member

An example of disruptive forced upgrade is x/tools 0.25 which suddenly jumped form go 1.19 to go 1.22(.0) for seemingly no "need"

It might appear that there was no need because x/tools was working fine with go1.19 only a few weeks ago. But the fact that it was continuing to build with go1.19, when the only supported Go toolchains at that time were go1.21 and go1.22 (and pre-go1.23), was imposing a significant burden on those of us who work in x/tools and those who work on the toolchain (since they had to be careful not to break x/tools). It has been a significant drag on the whole team for a number of years, and was in fact part of the genesis of the forward compatibility system.

@gopherbot gopherbot moved this from Likely Accept to Accepted in Proposals Sep 18, 2024
@gopherbot
Copy link
Contributor

gopherbot commented Sep 18, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— aclements for the proposal review group

The proposal is #69095 (comment).

@gopherbot gopherbot changed the title proposal: all, x/build/cmd/relui: automate go directive maintenance in golang.org/x repositories all, x/build/cmd/relui: automate go directive maintenance in golang.org/x repositories Sep 18, 2024
@gopherbot gopherbot modified the milestones: Proposal, Backlog Sep 18, 2024
@dmitshur dmitshur self-assigned this Sep 18, 2024
gopherbot pushed a commit to golang/build that referenced this issue Feb 21, 2025
I originally planned to use the existing CloudBuildClient.RunScript API
for this task, similarly to how it's used for monthly x/ repo tagging.
One key limitation of the RunScript API is that it requires the caller
to pass the exact files that will be copied out to Google Cloud Storage.
That works fine if go.mod and go.sum files were the only ones to change,
but 'go fix ./...' may update arbitrary other files too.

It's not too hard to expand RunScript to be able to copy directories
recursively, but that means it'd be copying the entire x/ repo content
even if only a few files changed. It's possible to try to detect which
files changed, and copy only those, or to create a .patch file.

However, we're already relying on the Cloud Build to clone the repo and
run commands to generate files, so it can work well for the Cloud Build
invocation to also take on the responsibility of mailing the auto-submit
CL, avoiding the need to copy anything to temporary storage only so that
relui can do the mailing itself. Also reuse git-generate as a building
block, pinning a known version for stability.

The result is an API that's very easy to use, and has the benefit of
including the script that describes the change in the commit message.

For golang/go#69095.

Change-Id: I78bb69567044e334fea693bf32a02de86696e7c9
Reviewed-on: https://go-review.googlesource.com/c/build/+/651336
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Feb 21, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/651737 mentions this issue: internal/task: migrate x509 UpdateBundle to GenerateAutoSubmitChange API

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/651736 mentions this issue: internal/task: detect "no CL needed" in GenerateAutoSubmitChange

@thaJeztah
Copy link
Contributor

I noticed these automated updates; does this mean Go effectively abandoned MVS, which I always interpreted as "modules should specify the minimum version required" (and only update if needed). For example, golang/text@518d9c0 updated go.mod with no changes in code. All golang.org/x/ modules also seem to be following CalVer pretending-to-be SemVer, with minimum versions being updated across them (often without actual changes in what's consumed).

thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 25, 2025
Go maintainers started to unconditionally update the minimum go version
for golang.org/x/ dependencies to go1.23, which means that we'll no longer
be able to support any version below that when updating those dependencies;

> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.

This updates our minimum version to go1.23, as we won't be able to maintain
compatibility with older versions because of the above.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 25, 2025
No code-changes, but updates the minimum go version to go1.23:

> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.

full diff: golang/crypto@v0.33.0...v0.34.0

Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah added a commit to thaJeztah/cli that referenced this issue Feb 25, 2025
Go maintainers started to unconditionally update the minimum go version
for golang.org/x/ dependencies to go1.23, which means that we'll no longer
be able to support any version below that when updating those dependencies;

> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.

This updates our minimum version to go1.23, as we won't be able to maintain
compatibility with older versions because of the above.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah added a commit to thaJeztah/cli that referenced this issue Feb 25, 2025
No code-changes, but updates the minimum go version to go1.23:

> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.

full diff: golang/crypto@v0.31.0...v0.34.0

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@findleyr
Copy link
Member

For example, golang/text@518d9c0 updated go.mod with no changes in code

In many of these x/repos we will start using new language features and standard library APIs after the go directive is upgraded. This certainly happened in x/tools, and I expect that it will happen elsewhere. I wouldn't expect the automation that updates the go.mod to also modify the code.

@thaJeztah
Copy link
Contributor

But the update was not required at the time the bot updated; to a further extent; isn't that where //go:build goXXX build-tags were designed for?

I understand if there are specific cases where using a new Go feature is the only option, but in many cases it may not be a strict requirement.

Also in many cases updating the inter-module versions (between golang.org/x packages) was done for no reason, other than doing a monthly release and update (in some cases a release with no changes). On multiple occasions this lead to a large number of code changes to pull in to get a security update.

And in quite some of those cases, I saw a custom branch being used for Go itself; if such updates are (from the Go maintainers perspective) "best practice" (latest is always greatest!) then, such branches wouldn't be needed for Go itself?

gopherbot pushed a commit to golang/build that referenced this issue Feb 25, 2025
The API of GenerateAutoSubmitChange aspirationally promised that:

// If the requested contents match the state of the repository,
// no change is created and the returned change ID will be empty.

That edge case typically doesn't come up during the task of maintaining
the go directive, so it was hard to test. Now that the happy path where
there are generated files works well, it's easier to add handling for
the "nothing changed" case. In particular, the UpdateBundle task of
BundleNSSRootsTask (which is migrated to the new API in the next CL)
makes for a convenient way to test this out both for the real Cloud
Build client and the fake.

For golang/go#69095.

Change-Id: I9a4271f2dd7b463433403190d5588d39b8416751
Reviewed-on: https://go-review.googlesource.com/c/build/+/651736
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 25, 2025
The GenerateAutoSubmitChange API is hopefully slightly simpler, but
mostly I'm using this small task as a convenient way to test out both
the real Cloud Build implementation and the fake at a more fine-grained
level than the TestRelease/major test.

As an upside, there are now tests for this task, and the new API won't
create empty CLs just to abandon them when there's nothing to update.
No more of these:

https://go-review.googlesource.com/q/repo:crypto+author:[email protected]+%22x509roots/fallback:+update+bundle%22+is:abandoned

For golang/go#69095.
For golang/go#57792.

Change-Id: I17e78ff0f26b0d5417e65b24a97e10cf7fb74349
Reviewed-on: https://go-review.googlesource.com/c/build/+/651737
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
erikdubbelboer pushed a commit to valyala/fasthttp that referenced this issue Feb 26, 2025
Some of our dependences require the supported versions of Go.

For example github.com/golang/crypto now requires 1.23 or higher.
See: golang/crypto@89ff08d

For more information on the new policy of the Go team see: golang/go#69095
erikdubbelboer pushed a commit to valyala/fasthttp that referenced this issue Feb 26, 2025
Some of our dependences require the supported versions of Go.

For example github.com/golang/crypto now requires 1.23 or higher.
See: golang/crypto@89ff08d

For more information on the new policy of the Go team see: golang/go#69095
erikdubbelboer pushed a commit to valyala/fasthttp that referenced this issue Feb 26, 2025
Some of our dependences require the supported versions of Go.

For example github.com/golang/crypto now requires 1.23 or higher.
See: golang/crypto@89ff08d

For more information on the new policy of the Go team see: golang/go#69095
erikdubbelboer added a commit to valyala/fasthttp that referenced this issue Feb 26, 2025
Some of our dependences require the supported versions of Go.

For example github.com/golang/crypto now requires 1.23 or higher.
See: golang/crypto@89ff08d

For more information on the new policy of the Go team see: golang/go#69095

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
erikdubbelboer added a commit to valyala/fasthttp that referenced this issue Feb 26, 2025
Some of our dependences require the supported versions of Go.

For example github.com/golang/crypto now requires 1.23 or higher.
See: golang/crypto@89ff08d

For more information on the new policy of the Go team see: golang/go#69095

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@ldemailly
Copy link

Seconding ^

To beat on dead horse that left the station (ah!) - this is very unfortunate mess getting worse since 1.21 and the abandon of the go line as language version and minimum - imo there shouldn't be "just update of go.mod" but instead, it should come with the first PR that actually requires the new version

@silverwind
Copy link

FWIW, Some of these changes like in x/crypto changed the go directive from without .0 suffix to with it, which causes issues in consumers where go.mod is rewritten with toolchain directives, see #65847 (comment).

The only workaround to avoid the toolchain directive I've found is to upgrade the go directive to 1.24, but this seems like useless churn as my package is compatible with go >= 1.23 actually.

thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 28, 2025
Go maintainers started to unconditionally update the minimum go version
for golang.org/x/ dependencies to go1.23, which means that we'll no longer
be able to support any version below that when updating those dependencies;

> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.

This updates our minimum version to go1.23, as we won't be able to maintain
compatibility with older versions because of the above.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah added a commit to thaJeztah/docker that referenced this issue Feb 28, 2025
No code-changes, but updates the minimum go version to go1.23:

> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.

full diff: golang/crypto@v0.33.0...v0.34.0

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) Proposal Proposal-Accepted
Projects
Archived in project
Status: Accepted
Development

No branches or pull requests

9 participants