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

Add 'go-version' Output #85

Merged
merged 2 commits into from Apr 8, 2022

Conversation

mcdonnnj
Copy link
Contributor

This provides a go-version output from this Action to provide the version that was installed. This is useful if you specify only a partial version such as 1.15, but need the full version installed such as 1.15.5 for later use in a workflow. This is similar to the same functionality in actions/setup-python -
https://github.com/actions/setup-python/blob/41b7212b1668f5de9d65e9c82aa777e6bbedb3a8/action.yml#L14-L16

Example

build.yml:

jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - id: setup-go
        uses: action/setup-go@v2.1.4
        with:
          go-version: '1.15'
      - name: Lookup go cache directory
        run: echo "GOCACHE_DIR=$(go env GOCACHE)" >> $GITHUB_ENV
      - uses: actions/cache@v2
        with:
          path: |
            ${{ env.GOCACHE_DIR }}
          key: "test-${{ runner.os }}-go${{ steps.setup-go.outputs.go-version }}"

@mcdonnnj
Copy link
Contributor Author

mcdonnnj commented Feb 7, 2021

Just a friendly bump/ping.

@mvdan
Copy link

mvdan commented Apr 7, 2022

Friendly bump @brcrista @magnetikonline :)

action.yml Outdated
@@ -10,6 +10,9 @@ inputs:
token:
description: Used to pull node distributions from go-versions. Since there's a default, this is typically not supplied by the user.
default: ${{ github.token }}
outputs:
go-version:
description: 'The installed Go version. Useful when given a version range as input.'
runs:
using: 'node12'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcdonnnj you should do a git rebase main - some of this is out of date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, with no attention on this pull request it fell off my radar.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens to the best of us @mcdonnnj - just figured it might shake out any merge issues for you. 👍

src/main.ts Outdated
// based on go/src/cmd/go/internal/version/version.go:
// fmt.Printf("go version %s %s/%s\n", runtime.Version(), runtime.GOOS, runtime.GOARCH)
// expecting go<version> for runtime.Version()
let goVersionOutput = [...goVersion.split(' ')[2]].slice(2).join('');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcdonnnj is this reliable cross-platform? Probably?

Also this is a simpler way?

Suggested change
let goVersionOutput = [...goVersion.split(' ')[2]].slice(2).join('');
let goVersionOutput = goVersion.split(' ')[2].slice(2);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably avoid the use of goVersionOutput entirely too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@magnetikonline Sorry I had to knock the cobwebs out because it's been a long while since I looked at this pull request. I'm not sure why I felt I needed to expand it to an array and join() but yes your suggestion works fine. I'd also be happy to remove the variable entirely if that is preferred.

I just double-checked that this worked cross-platform in this Action run with jobs using ubuntu-latest, macos-latest, and windows-latest.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to @magnetikonline's suggestion, but we can also remove the magic number '2':

Suggested change
let goVersionOutput = [...goVersion.split(' ')[2]].slice(2).join('');
let goVersionOutput = goVersion.split(' ')[2].slice('go'.length);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also be great if you could factor out this version-parsing logic to its own function and add a quick test for it. I think this is enough:

const goVersionOutput = "go version go1.16.6 darwin/amd64";
expect(parseGoVersion(goVersionOutput)).toBe("1.16.6")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@magnetikonline @brcrista Please see aa36c7f for your consideration. Please let me know if you'd like me to make any changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry was out for the weekend. @brcrista nice solution to the magic number 2 👍

@magnetikonline
Copy link
Contributor

@mvdan I left some comments - but I don't have merge rights (I'm not a GitHub staffer!).

Over to @brcrista 👍 😄

This provides the semver version of Go that has been installed. This is useful
if only a major or minor version has been provided as the input go-version
value.
@mcdonnnj mcdonnnj force-pushed the improvement/add_go-version_output branch from 6adf460 to 2930331 Compare April 7, 2022 03:37
@mcdonnnj mcdonnnj requested a review from a team April 7, 2022 03:37
@brcrista
Copy link
Contributor

brcrista commented Apr 8, 2022

@mcdonnnj looks like CI is failing. Could you run npm run format-check and fix any issues?

https://github.com/actions/setup-go/runs/5885728533?check_suite_focus=true#step:5:10

Simplify how the version is extracted and add a simple test at the same
time.

Co-authored-by: Peter Mescalchin <peter@magnetikonline.com>
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>
@mcdonnnj mcdonnnj force-pushed the improvement/add_go-version_output branch from aa36c7f to b2f8b6e Compare April 8, 2022 15:31
@mcdonnnj
Copy link
Contributor Author

mcdonnnj commented Apr 8, 2022

@mcdonnnj looks like CI is failing. Could you run npm run format-check and fix any issues?

https://github.com/actions/setup-go/runs/5885728533?check_suite_focus=true#step:5:10

@brcrista I've force-pushed with the corrected file.

Copy link
Contributor

@brcrista brcrista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@brcrista brcrista merged commit 4a4352b into actions:main Apr 8, 2022
@mcdonnnj mcdonnnj deleted the improvement/add_go-version_output branch April 8, 2022 16:24
@magnetikonline
Copy link
Contributor

Good result all round, worthy addition 👍

panticmilos pushed a commit to panticmilos/setup-go that referenced this pull request Aug 4, 2022
* Add go-version to action outputs

This provides the semver version of Go that has been installed. This is useful
if only a major or minor version has been provided as the input go-version
value.

* Convert version extraction to a function

Simplify how the version is extracted and add a simple test at the same
time.

Co-authored-by: Peter Mescalchin <peter@magnetikonline.com>
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>

Co-authored-by: Peter Mescalchin <peter@magnetikonline.com>
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>
n33pm pushed a commit to n33pm/setup-go that referenced this pull request Oct 17, 2022
* Add go-version to action outputs

This provides the semver version of Go that has been installed. This is useful
if only a major or minor version has been provided as the input go-version
value.

* Convert version extraction to a function

Simplify how the version is extracted and add a simple test at the same
time.

Co-authored-by: Peter Mescalchin <peter@magnetikonline.com>
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>

Co-authored-by: Peter Mescalchin <peter@magnetikonline.com>
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>
adilhusain-s pushed a commit to adilhusain-s/setup-go that referenced this pull request Feb 6, 2023
* Add go-version to action outputs

This provides the semver version of Go that has been installed. This is useful
if only a major or minor version has been provided as the input go-version
value.

* Convert version extraction to a function

Simplify how the version is extracted and add a simple test at the same
time.

Co-authored-by: Peter Mescalchin <peter@magnetikonline.com>
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>

Co-authored-by: Peter Mescalchin <peter@magnetikonline.com>
Co-authored-by: Brian Cristante <33549821+brcrista@users.noreply.github.com>
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.

None yet

4 participants