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

Discussion: get rid of setup-go dep, due to outdated code and inability to update any dependencies #365

Closed
SVilgelm opened this issue Nov 1, 2021 · 14 comments

Comments

@SVilgelm
Copy link
Member

SVilgelm commented Nov 1, 2021

Hello, I would like to ask everyone in @golangci/team and @golangci/core-team about you opinion in removing the setup-go as direct dependency.

Now we are stuck in upgrading any dependencies, fixing any vulns, due to inability to build the setup-go package.

So if we remove it then we will need to explicitly install go, like

    steps:
      - uses: actions/checkout@v2
      - uses: actions/setup-go@v2  # <-- here
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v2

of course it will require to bump v3.

@SVilgelm SVilgelm pinned this issue Nov 1, 2021
@ldez
Copy link
Member

ldez commented Nov 1, 2021

Hello,

The problem is that Go is required to run golangci-lint.

Then we have to provide something to inform the users.

@SVilgelm
Copy link
Member Author

SVilgelm commented Nov 1, 2021

The problem is that Go is required to run golangci-lint.

sure, we can check if go is not installed, then just fail the step with an error message

@magnetikonline
Copy link

magnetikonline commented Nov 1, 2021

I'm explicitly installing Golang in lint workflows anyway, so this works for me.

To avoid excessive boilerplate, I've authored a composite action (simple yaml based action) - which can now include other actions in their steps as of a few weeks ago (previously only shell steps were possible as steps).

Using a composite action I can execute both setup-go and then golangci-lint actions together and reference this as a one liner from other workflows.

Maybe we can offer up a simple composite action under this organisation, offering users this alternative use path - and keeping setup-go out of any deps? Seems like the best of both worlds?

@SVilgelm
Copy link
Member Author

SVilgelm commented Nov 1, 2021

I'm explicitly installing Golang in lint workflows anyway, so this works for me.

I'm doing it as well

composite action

I think this is a good idea

@magnetikonline
Copy link

Just to circle back @SVilgelm - this is how I'm handing it in a small composite action. Something I think could be offered up in the https://github.com/golangci org.

https://github.com/flipgroup/action-golang-with-lint

Of course a more complete version could pass over all allowed lint action arguments. Note I'm having to set skip-go-installation: true - it's not critical, but stops the action doing unrequired work/saves some runtime.

@meling
Copy link

meling commented Jan 19, 2022

Not sure if my problem is related to this issue, but I got this error:

 Error: ParseFS not declared by package template (typecheck)

Presumably this is because ubuntu-latest has Go version lower than 1.16. Do I have to install Go 1.16 or later using actions/setup-go@v2, as this issue is seemingly suggesting to drop?

Ergonomically, it would be great if golangci-lint would pick a Go version at least equal to or higher than the one specified in go.mod. IMO, there is no reason to run golangci-lint using a lower version than the one specified in go.mod. I'm not sure what is the right approach for picking the Go versions to use though without the actions/setup-go@v2??

Here is the yml:

jobs:
  golangci:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v2
        with:
          version: latest
          skip-go-installation: true
          skip-pkg-cache: true
          skip-build-cache: true
          args: --disable errcheck --timeout 5m
          only-new-issues: true

@butuzov
Copy link
Member

butuzov commented Jan 19, 2022

@golangci-lint-action will install the latest available for setup-go action go's version. (you can check the list here, at least if it still within major version 1, but you have set skip-go-installation to true, so it uses a preinstalled version.

@meling
Copy link

meling commented Jan 21, 2022

@golangci-lint-action will install the latest available for setup-go action go's version. (you can check the list here, at least if it still within major version 1, but you have set skip-go-installation to true, so it uses a preinstalled version.

When you say preinstalled version, is that from ubuntu-latest and something that golangci-lint-action cannot influence? I can certainly remove the skip-go-installation, but it would be nice if golangci-lint-action could use an existing go installation, that is at least as new as the the one specified in go.mod. This would presumably speed up execution. For my use case, I'm not sure it makes sense to run golangci-lint-action with multiple Go versions.

meling added a commit to quickfeed/quickfeed that referenced this issue Jan 26, 2022
The go installation available to golangci-lint when running via
github actions is Go v1.15 or maybe even lower, causing problems
using newer API calls. That is, golangci-lint's Go version should
be higher than v1.15.

See this issue for details:
golangci/golangci-lint-action#365
@StevenACoffman
Copy link
Contributor

I would like to proceed with getting rid of the setup-go dep as it has been blocking any other improvements for a while now.

@SVilgelm
Copy link
Member Author

sure, go ahead

@SVilgelm
Copy link
Member Author

the code is merged, so I will work on updating some other dependencies and the will bump v3 of the action

@StevenACoffman
Copy link
Contributor

@SVilgelm can you add this note to the compatibility section of the README.md:

+* `v3.0.0+` requires explicit setup-go installation step prior to using this action

@SVilgelm
Copy link
Member Author

@StevenACoffman here it is: https://github.com/golangci/golangci-lint-action/pull/404/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R12

@SVilgelm
Copy link
Member Author

ok, closing this issue, because v3 is released :)
https://github.com/golangci/golangci-lint-action/releases/tag/v3.0.0

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

No branches or pull requests

6 participants