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

More specific handling/detection of Go toolchain versions #457

Open
matthewhughes934 opened this issue Feb 14, 2024 · 10 comments
Open

More specific handling/detection of Go toolchain versions #457

matthewhughes934 opened this issue Feb 14, 2024 · 10 comments
Labels
feature request New feature or request to improve the current logic

Comments

@matthewhughes934
Copy link

matthewhughes934 commented Feb 14, 2024

Description:

The goal is to just have an approach to manage toolchain installs, the following is a suggestion and open to discussion/changes:

  • If the user provides a specific version, ensure we use exactly that version (possibly requires setting GOTOOLCHAIN=local in the environment, see docs)
  • If the user provides a go-version-file, and:
    • The file has a toolchain directive: then use that since the docs say

      The toolchain line declares a suggested toolchain to use with the module or workspace

      This is in comparison with the go version, which is the minimum version of Go required to install the module (the assumption being: we're working within the module, not install it)

    • The file has no toolchain directive: then use the version from the go directive (current behaviour)

Justification:

At the moment this action does not consider toolchain versions. This leads to:

  • Some duplicate work, the action will install a version of Go, a toolchain will be detected, the toolchain will be detected and then another version of Go installed
  • Unexpected behaviour: if you specify this action runs with some Go version (e.g. 1.21.0) but your go.mod contains a toolchain directive for a newer version (e.g. 1.22.0) then, without any other configuration/environment setup, any go commands will be run using go 1.22.0
  • Tar errors on cache restore after toolchain installation #424 is related to not managing toolchain installs

You can see this in this example repo, which is just a repo with a go.mod whose go directive is 1.21.0 and whose toolchain directive is 1.22.0 and some actions that display the version of Go under some conditions, of interest is when we specify the action with version 1.21.0: we ask for 1.21.0 but a toolchain install will result in 1.22.0 being installed and run

Are you willing to submit a PR?

Yep, but there should be some discussion around which approach to take before implementing anything

EDIT: I noticed that the official Go docker images also set GOTOOLCHAIN=local https://github.com/docker-library/golang/blob/dae3405a325073e8ad7c8c378ebdf2540d8565c4/Dockerfile-linux.template#L162C3-L163C1 with some discussion on the linked issue there: docker-library/golang#472

@matthewhughes934 matthewhughes934 added feature request New feature or request to improve the current logic needs triage labels Feb 14, 2024
@HarithaVattikuti
Copy link
Contributor

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

@matthewhughes934
Copy link
Author

matthewhughes934 commented Mar 3, 2024

I have a PoC for this: #460 (it also addresses #424 as a side-effect)

Another option is to add some more configuration, when using go-version-file add a new option like go-version-directive that can be one of "go", "toolchain", defaulting this to "go" I think would be a non-breaking change

vincenthsh added a commit to vincenthsh/fogg that referenced this issue Apr 24, 2024
switch off of `go-version-file` in the Github Actions, because it doesn't work great with the new `go mod tidy` format that go 1.22 does. See:

* [Improve toolchain handling actions/setup-go#460](actions/setup-go#460)
* [More specific handling/detection of Go toolchain versions actions/setup-go#457](actions/setup-go#457)
vincenthsh added a commit to vincenthsh/fogg that referenced this issue Apr 24, 2024
switch off of `go-version-file` in the Github Actions, because it doesn't work great with the new `go mod tidy` format that go 1.22 does. See:

* [Improve toolchain handling actions/setup-go#460](actions/setup-go#460)
* [More specific handling/detection of Go toolchain versions actions/setup-go#457](actions/setup-go#457)
vincenthsh added a commit to vincenthsh/fogg that referenced this issue Apr 24, 2024
switch off of `go-version-file` in the Github Actions, because it doesn't work great with the new `go mod tidy` format that go 1.22 does. See:

* [Improve toolchain handling actions/setup-go#460](actions/setup-go#460)
* [More specific handling/detection of Go toolchain versions actions/setup-go#457](actions/setup-go#457)
@StefMa
Copy link

StefMa commented Jun 4, 2024

Any updates on this @HarithaVattikuti ?

@jalaziz
Copy link

jalaziz commented Jul 6, 2024

Would love to see this getting more attention, especially since there's an associate PR. The go-version-file support should be consider broken without support for the toolchain directive since it will not do what any reasonable Go developer would expect it to.

@alicebob
Copy link

I got some tests which switched to 1.23.0.

As a workaround, setting the env: key worked for me to keep the tests running 1.22.6:

      - name: Setup Go
        uses: actions/setup-go@v5
        with:
          go-version: "1.22"
        id: go
        env:
          GOTOOLCHAIN: local
Run actions/setup-go@v5
Setup go version spec 1.22
Found in cache @ /opt/hostedtoolcache/go/1.22.6/x64
Added go to the path
Successfully set up Go version 1.22
/opt/hostedtoolcache/go/1.22.6/x64/bin/go env GOMODCACHE
/opt/hostedtoolcache/go/1.22.6/x64/bin/go env GOCACHE
/home/runner/go/pkg/mod
/home/runner/.cache/go-build
Cache is not found
go version go1.22.6 linux/amd64

@trim21
Copy link

trim21 commented Aug 26, 2024

env:
GOTOOLCHAIN: local

Is it enough to only add env to setup-go steps?

Will go download new toolchain in following steps?

@alicebob
Copy link

alicebob commented Aug 26, 2024

Argl, the next step did just that:

go: downloading go1.23.0 (linux/amd64)

Okay, guess it needs to be everywhere.

Edit: that works. My tests now fail nicely when there are some vendored libs which need 1.23.

@trim21
Copy link

trim21 commented Aug 26, 2024

Argl, the next step did just that:

go: downloading go1.23.0 (linux/amd64)

Okay, guess it needs to be everywhere.

github actions supports setting env for whole job or jobs in same workflow

@alicebob
Copy link

@trim21 is right, this works for now as a workaround:

env:
  GOTOOLCHAIN: local

in .github/workflows/youraction.yml

(Now back to what I should be doing, instead of fighting with build tools.)

phm07 added a commit to hetznercloud/cli that referenced this issue Sep 18, 2024
This PR makes the CI pipeline use Go 1.23 for building and testing. Only
setting the `toolchain` in `go.mod` is not enough, because the [setup-go
action does not support the `toolchain` directive
yet](actions/setup-go#457), we have to set the
Go version to 1.23 in all setup-go CI steps manually.
We could also set the `go` version in `go.mod` to 1.23 since we do not
have any dependents, but that would be misusing the `go` directive,
since it is only intended to specify the minimum supported language
version of the project.
@int128
Copy link

int128 commented Nov 16, 2024

I hope this action respects toolchain directive of go.mod.

Here is a workaround to install the toolchain version of Go.

jobs:
  test:
    steps:
      - uses: actions/checkout@v4
      - id: toolchain
        run: echo "version=$(sed -ne '/^toolchain /s/^toolchain go//p' go.mod)" >> "$GITHUB_OUTPUT"
      - uses: actions/setup-go@v5
        with:
          go-version: ${{ steps.toolchain.outputs.version }}

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

7 participants