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 to specify Go version #75

Closed
x1ddos opened this issue Aug 23, 2020 · 13 comments · Fixed by #144
Closed

Allow to specify Go version #75

x1ddos opened this issue Aug 23, 2020 · 13 comments · Fixed by #144

Comments

@x1ddos
Copy link

x1ddos commented Aug 23, 2020

The linter github action doesn't allow specifying a Go version. It overwrtes it to 1 in https://github.com/golangci/golangci-lint-action/blob/1d3f25b/src/install.ts#L60.

It would be very useful if the linter picked the Go version specified in go.mod or at least had an action parameter. I made a working PoC in https://github.com/x1ddos/golangci-lint-action/commit/6f8897f but it's for the action version 1.2.2.

We have a project which can't upgrade to the latest go1.15 and had to remove the action in digitalbitbox/bitbox-wallet-app#982.

@theckman
Copy link

theckman commented Aug 23, 2020

One thing to note, is that the version in the go.mod file is intended to indicate minimum supported version not desired version. Picking the version specified from that would deviate from the intent of the Modules system, so I'd be reluctant to take that route.

Based on the intent, a module with go 1.14 in the go.mod file should work on 1.15.

@x1ddos
Copy link
Author

x1ddos commented Aug 23, 2020

Yeah, good point about minimal supported version. So, maybe just allowing to specify go-version parameter would work best in the end.

Based on the intent, a module with go 1.14 in the go.mod file should work on 1.15.

It should but sometimes doesn't in practice. Here's some output from running the linter on https://github.com/digitalbitbox/bitbox-wallet-app.

$ go version
go version go1.14.7 linux/amd64

$ golangci-lint --version
golangci-lint has version v1.27.0 built from (unknown, mod sum: "h1:VYLx63qb+XJsHdZ27PMS2w5JZacN0XG8ffUwe7yQomo=") on (unknown)

$ go clean -cache
$ golangci-lint run
$ echo $?
0
$ go version
go version go1.15 linux/amd64

$ golangci-lint --version
golangci-lint has version v1.27.0 built from (unknown, mod sum: "h1:VYLx63qb+XJsHdZ27PMS2w5JZacN0XG8ffUwe7yQomo=") on (unknown)

$ go clean -cache
$ golangci-lint run
level=warning msg="[runner] Can't run linter goanalysis_metalinter: goprintffuncname: failed prerequisites: [inspect@github.com/digitalbitbox/bitbox-wallet-app/frontends/qt/server: analysis skipped: errors in package: [src/github.com/digitalbitbox/bitbox-wallet-app/frontends/qt/server/server.go:40:8: could not import C (cgo preprocessing failed)]]"
level=warning msg="[runner] Can't run linter unused: buildir: analysis skipped: errors in package: [src/github.com/digitalbitbox/bitbox-wallet-app/frontends/qt/server/server.go:40:8: could not import C (cgo preprocessing failed)]"
level=error msg="Running error: buildir: analysis skipped: errors in package: [src/github.com/digitalbitbox/bitbox-wallet-app/frontends/qt/server/server.go:40:8: could not import C (cgo preprocessing failed)]"
$ go version
go version go1.14.7 linux/amd64

$ golangci-lint --version
golangci-lint has version v1.30.0 built from (unknown, mod sum: "h1:UhdK5WbO0GBd7W+k2lOD7BEJH4Wsa7zKfw8m3/aEJGQ=") on (unknown)

$ go clean -cache
$ golangci-lint run
WARN [linters context] Panic: exhaustive: package "btc" (isInitialPkg: false, needAnalyzeSou[7/1076]
e): interface conversion: ast.Expr is *ast.SelectorExpr, not *ast.Ident: goroutine 17548 [running]:
runtime/debug.Stack(0x14f69cf, 0x3c, 0xc00266b290)
        /usr/local/go/src/runtime/debug/stack.go:24 +0x9d
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1(0xc001b63560)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:508 +0x1b5
panic(0x13157e0, 0xc012a2c810)
        /usr/local/go/src/runtime/panic.go:969 +0x166
github.com/nishanths/exhaustive.missingCasesTextEdit(0xc00064c0c0, 0xc007c51000, 0xc009a8a600, 0xc01
6872810, 0xc0047f69b0, 0xc012a2c750, 0x0, 0x0, 0x0, 0x0, ...)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:312 +0x873
github.com/nishanths/exhaustive.computeFix(0xc00f278000, 0xc00064c0c0, 0xc007c51000, 0xc016872810, 0
xc0047f69b0, 0x0, 0xc012a2c750, 0x0, 0x0, 0x0, ...)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:174 +0xd8
github.com/nishanths/exhaustive.reportSwitch(0xc00f278000, 0xc016872810, 0xc00c304400, 0xc0047f69b0,
 0xc012a2c750, 0x0, 0xc007c51000)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:146 +0x53d
github.com/nishanths/exhaustive.checkSwitchStatements.func1(0x1683e20, 0xc016872810, 0x1, 0xc004032c
00, 0x4, 0x20, 0x1)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:117 +0x474
golang.org/x/tools/go/ast/inspector.(*Inspector).WithStack(0xc00b2b5d40, 0xc002746c20, 0x1, 0x1, 0xc
00266bc30)
        pkg/mod/golang.org/x/tools@v0.0.0-20200724022722-7017fd6b1305/go/ast/inspector
/inspector.go:126 +0x13f
github.com/nishanths/exhaustive.checkSwitchStatements(0xc00f278000, 0xc00b2b5d40, 0xc002746cb8)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:24 +0xd6
github.com/nishanths/exhaustive.run(0xc00f278000, 0x51354fe14, 0x1f1a600, 0xc00fb9aa70, 0x2)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/exh
austive.go:170 +0x111
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc001b63560)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:590 +0xa25
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:512 +0x2a
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc0012905a0, 0x1477177, 0xa
, 0xc0031f6770)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/timeutils/stopwatch.go:1
11 +0x50
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc001b63560)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:511 +0x91
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc00b855
ef0, 0xc001b63560)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:1059 +0x61
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:1054 +0x306
$ go version
go version go1.15 linux/amd64

$ golangci-lint --version
golangci-lint has version v1.30.0 built from (unknown, mod sum: "h1:UhdK5WbO0GBd7W+k2lOD7BEJH4Wsa7zKfw8m3/aEJGQ=") on (unknown)

$ go clean -cache
$ golangci-lint run
WARN [linters context] Panic: exhaustive: package "btc" (isInitialPkg: false, needAnalyzeSource: tru
e): interface conversion: ast.Expr is *ast.SelectorExpr, not *ast.Ident: goroutine 18546 [running]:
runtime/debug.Stack(0x14f69cf, 0x3c, 0xc001d67290)
        /usr/local/go/src/runtime/debug/stack.go:24 +0x9d
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1(0xc002eef680)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:508 +0x1b5
panic(0x13157e0, 0xc003f023f0)
        /usr/local/go/src/runtime/panic.go:969 +0x166
github.com/nishanths/exhaustive.missingCasesTextEdit(0xc0013840c0, 0xc006b45380, 0xc004410400, 0xc00
1cdbda0, 0xc00185eaa0, 0xc003f02330, 0x0, 0x0, 0x0, 0x0, ...)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:312 +0x873
github.com/nishanths/exhaustive.computeFix(0xc006418780, 0xc0013840c0, 0xc006b45380, 0xc001cdbda0, 0
xc00185eaa0, 0x0, 0xc003f02330, 0x0, 0x0, 0x0, ...)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:174 +0xd8
github.com/nishanths/exhaustive.reportSwitch(0xc006418780, 0xc001cdbda0, 0xc004402300, 0xc00185eaa0,
 0xc003f02330, 0x0, 0xc006b45380)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:146 +0x53d
github.com/nishanths/exhaustive.checkSwitchStatements.func1(0x1683e20, 0xc001cdbda0, 0x1, 0xc0027ad2
00, 0x4, 0x20, 0x1)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:117 +0x474
golang.org/x/tools/go/ast/inspector.(*Inspector).WithStack(0xc0060cc160, 0xc003852c20, 0x1, 0x1, 0xc
001d67c30)
        pkg/mod/golang.org/x/tools@v0.0.0-20200724022722-7017fd6b1305/go/ast/inspector
/inspector.go:126 +0x13f
github.com/nishanths/exhaustive.checkSwitchStatements(0xc006418780, 0xc0060cc160, 0xc003852cb8)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/swi
tch.go:24 +0xd6
github.com/nishanths/exhaustive.run(0xc006418780, 0x3786740b7, 0x1f1a600, 0xc009bb7360, 0x2)
        pkg/mod/github.com/nishanths/exhaustive@v0.0.0-20200708172631-8866003e3856/exh
austive.go:170 +0x111
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc002eef680)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:590 +0xa25
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:512 +0x2a
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc001729c70, 0x1477177, 0xa
, 0xc0019c8f70)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/timeutils/stopwatch.go:1
11 +0x50
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc002eef680)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/run
ner.go:511 +0x91
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc0061ca
b50, 0xc002eef680)
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/runner.go:1059 +0x61
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
        pkg/mod/github.com/golangci/golangci-lint@v1.30.0/pkg/golinters/goanalysis/runner.go:1054 +0x306
WARN [runner] Can't run linter unused: buildir: analysis skipped: errors in package: [src/github.com/digitalbitbox/bitbox-wallet-app/frontends/qt/server/server.go:40:8: could not import C (cgo preprocessing failed)]
ERRO Running error: buildir: analysis skipped: errors in package: [src/github.com/digitalbitbox/bitbox-wallet-app/frontends/qt/server/server.go:40:8: could not import C (cgo preprocessing failed)]

Even if it did work, the project we're working on can't officially support go1.15 due to some dependencies like gomobile and gopherjs.

@x1ddos x1ddos changed the title Honor Go version specified in go.mod Allow to specify Go version Aug 23, 2020
@SVilgelm
Copy link
Member

IMO the golangci-lint should not call the setup-go, it should be a requirement, or ti should check if the go is installed, the do nothing

@x1ddos
Copy link
Author

x1ddos commented Aug 24, 2020

IMO the golangci-lint should not call the setup-go, it should be a requirement, or ti should check if the go is installed, the do nothing

Yeah, that would be another option although more distruptive to all existing users of the action who already rely on setup-go.

@camh-
Copy link

camh- commented Dec 28, 2020

Perhaps check if Go is already set up and dont run setup-go in that case?

My problem right now is that I am using Go 1.16beta1 - I have setup-go configured in my workflow to install that, but golangci-lint replaces it with 1.15.6 (current latest stable). And now lint is failing because the embed package does not exist in Go 1.15.6.

@SVilgelm
Copy link
Member

I think the action should not install the go if it is already installed.
I preparad a draft pull request: #144
@camh- Could you please test it on your project with go1.16? Just replace v2 with fix-75 in action:

uses: golangci/golangci-lint-action@fix-75

@SVilgelm
Copy link
Member

so, there is problem if I use golangci-lint-action then GitHub provides an image with pre installed go 1.14.x, so the go is always installed, I cannot reproduce the situation when go is not installed and the action will install it.

@SVilgelm
Copy link
Member

so we need to decide Should we install go at all?

@camh-
Copy link

camh- commented Dec 29, 2020

@SVilgelm Feedback from your first comment, even if it turns out not to be a proper change.

I added this to my workflow:

+    - name: Install golangci-lint
+      uses: golangci/golangci-lint-action@fix-75
+      with:
+        version: v1.33.2
+        args: --version
+    - run: echo '/home/runner/golangci-lint-1.33.2-linux-amd64' >> $GITHUB_PATH

I run it with --version and add to the PATH because I run golangci-lint from my Makefile, so I really only want the action to install lint and not run it. It would be nice if the action would add it to the path, because that follow-up action is a bit hacky, assuming the architecture and duplicating the version string.

But that all worked properly.

Since detecting if Go is installed is not viable, how about another parameter - installGo which defaults to true (current behaviour), so we can just manually switch it off. Additional parameters are a bit messy though, so I understand any hesitation to add more.

@SVilgelm
Copy link
Member

@camh- I updated the PR with skip-go-installation parameter:

      - name: golangci-lint
        uses: golangci/golangci-lint-action@fix-75
        with:
          skip-go-installation: true

I also added the golangci-lint binary to the PATH: https://github.com/sv-tools/bumptag/pull/64/checks?check_run_id=1620125930

@camh-
Copy link

camh- commented Dec 29, 2020

@SVilgelm I can confirm that this latest version works too. I've removed my step that adds to the path, and I've added skip-go-installation: true to my lint step.

👍

@cameronelliott
Copy link

Any chance you guys could do a release with this in it????
Really appreciate the action, thank you.
@SVilgelm

@camh-
Copy link

camh- commented Jan 29, 2021

Any chance you guys could do a release with this in it????

Until they do, you can use @master instead of @v2 in your action. That's what I'm doing at the moment: https://github.com/foxygoat/foxtrot/blob/86646cb58e7ebe90daf354e5005143b71a260961/.github/workflows/cicd.yaml#L26

ricky-rav added a commit to ricky-rav/ovn-kubernetes that referenced this issue Aug 9, 2022
... as it might install a later version by desing golangci/golangci-lint-action#75

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
ricky-rav added a commit to ricky-rav/ovn-kubernetes that referenced this issue Aug 9, 2022
... since golangci-lint might install a later version by design: golangci/golangci-lint-action#75

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
girishmg pushed a commit to ovn-org/ovn-kubernetes that referenced this issue Aug 9, 2022
... since golangci-lint might install a later version by design: golangci/golangci-lint-action#75

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
liornoy added a commit to liornoy/metallb-operator that referenced this issue Jan 18, 2023
The golangci-lint-action@v2 installed the latest golang version,
running it on the 0.12v branch that is pinned to the old go v1.16.

This commit calls for setup-go@v2 for v1.16,
and then calls golangci-lint with: skip-go-installation: true.
as recommended here: golangci/golangci-lint-action#75
liornoy added a commit to liornoy/metallb-operator that referenced this issue Feb 12, 2023
The metallb-operator is using go v1.16, however, the lint lane
in the CI is downloading the latest go.

This change fixes the version to v1.16 by first downloading go v1.16
using actions/setup-go@v2, and then calling golangci-lint with the
skip-go-installation: true.

This solution was recommended here: golangci/golangci-lint-action#75
liornoy added a commit to liornoy/metallb-operator that referenced this issue Feb 12, 2023
This metallb-operator v0.12 is using go v1.16, however, the lint lane
in the CI is installing the latest go.

This change fixes the version to v1.16 by first downloading go v1.16
using actions/setup-go@v2, and then calling golangci-lint with the
skip-go-installation: true.

This solution was recommended here: golangci/golangci-lint-action#75
liornoy added a commit to liornoy/metallb-operator that referenced this issue Feb 12, 2023
This metallb-operator v0.12 is using go v1.16, however, the lint lane
in the CI is installing the latest go.

This change fixes the version to v1.16 by first downloading go v1.16
using actions/setup-go@v2, and then calling golangci-lint with the
skip-go-installation: true.

This solution was recommended here: golangci/golangci-lint-action#75
liornoy added a commit to liornoy/metallb-operator that referenced this issue Feb 13, 2023
This metallb-operator v0.12 is using go v1.16, however, the lint lane
in the CI is installing the latest go.

This change fixes the version to v1.16 by first downloading go v1.16
using actions/setup-go@v2, and then calling golangci-lint with the
skip-go-installation: true.

This solution was recommended here: golangci/golangci-lint-action#75
fedepaol pushed a commit to metallb/metallb-operator that referenced this issue Apr 11, 2023
This metallb-operator v0.12 is using go v1.16, however, the lint lane
in the CI is installing the latest go.

This change fixes the version to v1.16 by first downloading go v1.16
using actions/setup-go@v2, and then calling golangci-lint with the
skip-go-installation: true.

This solution was recommended here: golangci/golangci-lint-action#75
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.

5 participants