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

Allow setting Go version from go.mod file #46

Closed
bewuethr opened this issue Oct 4, 2023 · 5 comments · Fixed by #47
Closed

Allow setting Go version from go.mod file #46

bewuethr opened this issue Oct 4, 2023 · 5 comments · Fixed by #47
Assignees

Comments

@bewuethr
Copy link
Contributor

bewuethr commented Oct 4, 2023

setup-go supports setting the Go version from the go.mod file; for Go 1.20 and older, it'll use the latest cached patch version on the same minor version, and for Go 1.21+, it'll use the exact version from go.mod, which now also includes the patch version.

Would it be possible to expose the go-version-file input in gh-extension-precompile as well? I previously extracted the Go version in a separate run step with go mod edit -json | jq -r '.Go', but that breaks if go.mod uses 1.21.0 or later, since the default Go on Actions runners, 1.20.8, considers the go.mod incorrectly formatted. I now use awk, which is less robust, but it would be nice to just leverage the existing setup-go functionality.

Happy to create a PR for this if there's interest, potentially including the check-latest flag from setup-go as well.

@samcoe
Copy link
Contributor

samcoe commented Oct 5, 2023

@bewuethr Exposing a go-version-file input that goes directly to setup-go action makes sense to me, we would be open to a PR for the feature. Let's hold off on check-latest input though as the team would like more time to think on if we want to support that.

@andyfeller
Copy link
Contributor

Having done this for another GitHub Action, I thought I'd share the solution we ended up with:

inputs:
  go-version:
    description: The version of go to use.
    required: false
  go-version-file:
    description: Path to the go.mod or go.work file to determine version of go to use
    required: false
runs:
  using: composite
  steps:
    - name: Setup Go
      uses: actions/setup-go@v4
      with:
        # The default go version is managed here because actions/setup-go favors go-version over go-version-file,
        # requiring us to only pass it if no other inputs are provided.
        #
        # Otherwise, we pass along the values given, letting the user catch the warning notice in the logs
        # and picking either go-version or go-version-file.
        go-version: ${{(inputs.go-version-file == '' && inputs.go-version == '') && '1.20' || inputs.go-version}}
        go-version-file: ${{inputs.go-version-file}}

In keeping with actions/setup-go, the go-version input will be favored over go-version-file if both are provided. If neither are provided, then the 1.20 within the expression will be used.

@bewuethr
Copy link
Contributor Author

In keeping with actions/setup-go, the go-version input will be favored over go-version-file if both are provided. If neither are provided, then the 1.20 within the expression will be used.

Couldn't you use 1.20 as the default for inputs.go-version, and then pass through both values directly, no additional expressions required? Something like

inputs:
  go-version:
    description: The version of go to use.
    required: false
    default: '1.20'
  go-version-file:
    description: Path to the go.mod or go.work file to determine version of go to use
    required: false
runs:
  using: composite
  steps:
    - name: Setup Go
      uses: actions/setup-go@v4
      with:
        go-version: ${{ inputs.go-version }}
        go-version-file: ${{ inputs.go-version-file }}

@andyfeller
Copy link
Contributor

andyfeller commented Oct 26, 2023

So the problem is that go-version always takes precedence from actions/setup-go. So if you pass in go.mod for go-version-file, it won't pick it up because the input default value will always be passed in. Thus, we have to do some silly conditional hackery like this to only pass 1 value in when one is set.

@andyfeller
Copy link
Contributor

created #47 for this

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 a pull request may close this issue.

3 participants