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

Set GOTOOLCHAIN=local if go-version[-file] is set and GOTOOLCHAIN is not already present #491

Open
Frederick888 opened this issue Jul 18, 2024 · 4 comments
Labels
feature request New feature or request to improve the current logic

Comments

@Frederick888
Copy link

Frederick888 commented Jul 18, 2024

Description:
When a user sets go-version[-file] explicitly, they'd often expect the version (range) to be enforced.

So I think we should set GOTOOLCHAIN=local to disable automatic toolchain switching in this case.

This is similar to #420 but with a bit more details.

Justification:
We maintain a few libraries where we want to make sure they are compatible with the last two Go major version releases (1.X and 1.X-1 to be clear, i.e. minor versions in SemVer).

We don't dictate which Go version contributors use, especially patch versions, but we have set up a GHA matrix like:

jobs:
  test:
    name: Test
    runs-on: ubuntu-latest
    strategy:
      fail-fast: false
      matrix:
        go_version:
          - 1.21.x
          - 1.22.x
    steps:
      - uses: actions/setup-go@v5
        with:
          go-version: ${{ matrix.go_version }}

Now for example, someone who uses Go 1.22.x may accidentally put a go 1.22.5 line in go.mod, and both jobs in the testing matrix will still pass. If it isn't caught in review, due to the changes since Go 1.21, AFAIU the library will require its consumers to upgrade to Go 1.22.5+, which is apparently unexpected.

So I think when a user specifies a Go version, we should set GOTOOLCHAIN=local to disable this new Go behaviour. If they do want it to happen, they can set an Action or job level GOTOOLCHAIN=auto (or don't set go-version[-file] if they don't care about it at all).

Are you willing to submit a PR?

Yes.

@Frederick888 Frederick888 added feature request New feature or request to improve the current logic needs triage labels Jul 18, 2024
@priya-kinthali
Copy link

Hello @Frederick888 👋
Thank you for this feature request. We will investigate it and get back to you as soon as we have some feedback.

@codyoss
Copy link

codyoss commented Aug 14, 2024

Setting GOTOOLCHAIN=local is the choice made in the official golang container images: docker-library/golang#472

That issue mentions CI for one of the reasons why they wanted to make this change. I personally believe this is likely the behaviour most would expect when declaring a specific go version to set up in a CI environment as well. This would be a large behaviour change though over the default of auto and should perhaps be made as a major version change if this request is accepted.

@matthewhughes934
Copy link

there's some similar discussion in #457 (see also #460)

@myitcv
Copy link

myitcv commented Sep 3, 2024

@Frederick888 - completely agree with your logic here. The setting of a specific version of Go expresses intent: the default behaviour of GOTOOLCHAIN=auto subverts that intent, by silently switching to other toolchain versions.

We are, for now, setting GOTOOLCHAIN=local in all our CI workflows.

haines added a commit to haines/cerbos-sdk-go that referenced this issue Nov 1, 2024
haines added a commit to cerbos/cerbos-sdk-go that referenced this issue Nov 4, 2024
- Skip UDS tests if not running on Linux
  docker/for-mac#483
- Test against correct Go versions
  actions/setup-go#491

Signed-off-by: Andrew Haines <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

No branches or pull requests

5 participants