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

chore: pin go directive to 1.23.0 #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dnwe
Copy link

@dnwe dnwe commented Feb 25, 2025

PR #85 bumped the Go directive to 1.23.6 rather than 1.23.0, which requires corresponding Go directive bumps in all consumers. Instead use both a go and toolchain directive as follows:

  • relax go directive to 1.23.0 to match the N-1 and N support statement and permit run/build with go1.23.0 and newer (Go treats this as mandatory)
  • use the toolchain directive to recommend the latest 1.23.x (Go treats this as advisory)

- relax go directive to 1.23.0 to match the N-1 and N support statement
  and permit run/build with go1.23.0 and newer (Go treats this as
  mandatory)
- use the toolchain directive to recommend the latest 1.23.x (Go treats
  this as advisory)

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
@dnwe dnwe requested a review from a team as a code owner February 25, 2025 09:40
@hslatman
Copy link
Member

hslatman commented Feb 25, 2025

We've had mixed results using the toolchain directive, and we've been following the latest patch releases of the N-1 recently. Do you have concrete examples of dependents to which this applies, outside of the Smallstep projects?

Note that this is a fork of the zmap/zcrypto project, and we haven't kept up with most, if not all, of the changes that are in the upstream.

@hslatman hslatman self-assigned this Feb 25, 2025
@dnwe
Copy link
Author

dnwe commented Feb 25, 2025

We've had mixed results using the toolchain directive, and we've been following the latest patch releases of the N-1 recently

Not sure what you mean by mixed results?

From a CI perspective, in your reusable GitHub workflows you're asking setup-go to use "stable" (N) and (optionally) "oldstable" (N-1) which currently would be the latest Go 1.23.x and Go 1.24.x releases known by actions/setup-go. After installing those Go will look at the toolchain directive and if that is newer that what is installed, it will attempt to fetch a newer toolchain if one exists (setup-go uses the default GOTOOLCHAIN=auto so it will prefer 'local' if that is newer than the toolchain directive). You can add an env: block with GOTOOLCHAIN=local if you want to always use whatever setup-go has fetched and ignore the toolchain directive entirely.

Local developers using the default GOTOOLCHAIN=auto will have the same behaviour of newer toolchains being silently fetched as needed. If a local developer has set GOTOOLCHAIN=local then it just needs to be some version of Go 1.23.x to allow compilation to succeed

@marten-seemann
Copy link

Local developers using the default GOTOOLCHAIN=auto will have the same behaviour of newer toolchains being silently fetched as needed.

That seems like an anti-feature to me, as does the entire go toolchain directive. To be honest, I don't understand why it was added in the first place, and I've been fighting against go tooling polluting my go.mod file with it ever since. I don't want things to happen "silently". If I'm compiling with Go 1.x.y, I want Go 1.x.y to be used, nothing else.

@hslatman
Copy link
Member

We've had mixed results using the toolchain directive, and we've been following the latest patch releases of the N-1 recently

Not sure what you mean by mixed results?

You could say mixed results literally, as well as figuratively, i.e. which Go version is used where exactly, and to what extent things broke 😅

From a CI perspective, in your reusable GitHub workflows you're asking setup-go to use "stable" (N) and (optionally) "oldstable" (N-1) which currently would be the latest Go 1.23.x and Go 1.24.x releases known by actions/setup-go. After installing those Go will look at the toolchain directive and if that is newer that what is installed, it will attempt to fetch a newer toolchain if one exists (setup-go uses the default GOTOOLCHAIN=auto so it will prefer 'local' if that is newer than the toolchain directive). You can add an env: block with GOTOOLCHAIN=local if you want to always use whatever setup-go has fetched and ignore the toolchain directive entirely.

I'm aware of GOTOOLCHAIN=local, and that probably should be the default, at least in our CI. We've been hit by a Go version bump in CI before, which broke functionality for a subset of users of our platform. Could we've prevented that? Most likely. But we're quite busy building things, and are now erring on the safe side.

@dnwe
Copy link
Author

dnwe commented Feb 25, 2025

Local developers using the default GOTOOLCHAIN=auto will have the same behaviour of newer toolchains being silently fetched as needed.

That seems like an anti-feature to me, as does the entire go toolchain directive. To be honest, I don't understand why it was added in the first place, and I've been fighting against go tooling polluting my go.mod file with it ever since. I don't want things to happen "silently". If I'm compiling with Go 1.x.y, I want Go 1.x.y to be used, nothing else.

@marten-seemann I don't disagree, I also think it was an anti-feature when they changed the go directive to both require/include the minor version (i.g. go1.22.0 rather than go1.22), to additionally be a mandatory requirement rather than a warning, and also to require your application's go directive to be the maximum value found in the go.mod of any of your dependencies.

In Go 1.22 then even made it so you couldn't use go 1.22 in go.mod and were required to include a minor version as go 1.22.0. Then they changed their mind again in go 1.23 in this comment on this well-cited GH issue on the confusion around the go directive changes:

image

golang/go#62278 (comment)

However, that's really saying that go itself will just silently assume 1.23.0 for module requirements and toolchain rather than the more useful 1.23.x (as-in any 1.23 version)

Unfortunately from a GitHub Actions perspective, setup-go doesn't currently know or use go toolchain versions, it's just manually parsing the go.mod for 'go' directives itself and hasn't really kept up with the 1.22 and 1.23 changes here. There's an issue tracking that over here

@dnwe
Copy link
Author

dnwe commented Feb 25, 2025

@hslatman

I'm aware of GOTOOLCHAIN=local, and that probably should be the default, at least in our CI. We've been hit by a Go version bump in CI before, which broke functionality for a subset of users of our platform. Could we've prevented that? Most likely. But we're quite busy building things, and are now erring on the safe side.

As mentioned above though, your workflow is currently just setting "stable" and "oldstable" aliases in actions/setup-go anyway, so you're already getting silently rather than explicitly bumped regardless of any go directive or toolchain directive right? It would be fine to pin to an explicit build version in your CI and bump that either manually or via renovate/dependabot), but that's not happening today regardless

@marten-seemann
Copy link

Ok, so here's my understanding (I haven't researched this in depth, so this might be wrong): If I put go 1.23 in my go.mod, then all that's enforced is that the compiler is Go 1.23.x (or newer). It will just use whatever is installed.

If I put go 1.23.y in my go.mod it will enforce x >= y. I can only see very few situations where I'd want this (basically only if go1.23.y-1 prevented the library from working at all, so you'd practically rarely use this, if ever.

Isn't that exactly what we want?

@dnwe
Copy link
Author

dnwe commented Feb 25, 2025

Unfortunately go1.23.6 mod tidy still seems to just immediately replace that with:

-go 1.23
+go 1.23.0
+
+toolchain go1.23.6

@dnwe
Copy link
Author

dnwe commented Feb 25, 2025

But yes, if you're willing to accept the .0 you can omit the toolchain

As-in:

go 1.23.0

@marten-seemann
Copy link

But yes, if you're willing to accept the .0 you can omit the toolchain

As-in:

go 1.23.0

What's the meaning of this? Any Go 1.23.x version?

@dnwe
Copy link
Author

dnwe commented Feb 25, 2025

Yes, it means "language version 1.23.0 and toolchain version 1.23.0"

So you'd get behaviour like this:

  • GOTOOLCHAIN=local and go on $PATH that is older than 1.23.0 it would refuse to compile saying "your toolchain is too old"
  • GOTOOLCHAIN=auto and go on $PATH that is older than 1.23.0 it would fetch 1.23.0 and compile with that
  • GOTOOLCHAIN=local and go on $PATH that is equal to or newer than 1.23.0 it would use whatever version is installed locally
  • GOTOOLCHAIN=auto and go on $PATH equal to or newer than 1.23.0 it would use whatever version is installed locally

Also note that in all cases above if you have go 1.24.0 installed locally as go on your $PATH then you'll be compiling with that locally rather than go 1.23.x

@marten-seemann
Copy link

Looks like we're not only ones unhappy with the toolchain directive: golang/go#65847.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants