Add option to specify go version from go.mod or go.work #21
Conversation
|
Thanks. I just discovered that feature of setup-go recently and was thinking about adding this. I'll review it later today or in the morning. |
# Conflicts: # .github/workflows/integration.yml # action.yml
There was a problem hiding this comment.
The code looks good. I like that you used perl for the regex. Since it's a dependency anyway, we might as well use it for what it is good for 😄
I requested a couple of changes. Let me know if you have any questions or want help implementing them.
I'm not 100% sure if my suggestion of >=1.N works with semver-select. I'll check on that and report back if there are issues.
src/lib
Outdated
| return | ||
| fi | ||
|
|
||
| perl -nle 'print $1 if /^go (\d\.\d+)/' < "$go_file_path" |
There was a problem hiding this comment.
I think it is valid to have space before "go" and more than one space between "go" and the version constraint.
The doc also gives 1.21rc1 as an example value, so we should capture to the end of the token.
It also says that 1.16 is assumed when there is no directive. So either select_go_version_from_file should return "1.16" when there is no match or src/run should set it to "1.16" when select_go_version_from_file returns nothing.
There was a problem hiding this comment.
The new regexp should cater for that, and made 1.16 the default in case of no match
src/run
Outdated
| # shellcheck disable=2153 # false positive about GO_VERSION being a misspelling of go_version | ||
| constraint="$GO_VERSION" | ||
| if [ "$constraint" = "tip" ] || [ "$constraint" = "gotip" ]; then | ||
| constraint='*' | ||
| INSTALL_GO_TIP=1 | ||
| fi | ||
|
|
||
| if [ -z "$constraint" ]; then | ||
| constraint="$(select_go_version_from_file "$GITHUB_WORKSPACE/$GO_VERSION_FILE")" |
There was a problem hiding this comment.
I'm not sure that using the output of select_go_version_from_file as the constraint is the right thing to do here. The go.mod doc says the version is the minimum version to use. So it seems like when the directive is 1.18 the constraint should be >=1.18 and 1.21rc1 should be >=1.21rc1.
I think sticking a >= at the beginning of the constraint is all we need to do here. It may not work quite right in the time between go1.22rc1 and go1.22, but this PR doesn't need to address that.
Treat go version from file as minimum required
Co-authored-by: WillAbides <233500+WillAbides@users.noreply.github.com>
|
Nice, thanks :) |
Something I've been missing from actions/setup-go.
It makes sure at least one of the inputs
go-versionorgo-version-fileis set, with the former taking precedence.