Description
Welcome
- Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.Yes, I've searched similar issues on GitHub and didn't find any.Yes, I've read the typecheck section of the FAQ (https://golangci-lint.run/usage/faq/#why-do-you-have-typecheck-errors).Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.). (https://golangci-lint.run/usage/linters/)
Description of the problem
We are running golangci-lint using a Golang make/rake -like tool Mage.
There, we have defined a mage target that runs linting as well as other tasks one-by-one in serial order. We encountered that golangci-lint exits too early and the rest of the tasks following that were not executed.
Further examples are provided in the repo URL in the reproductible code section, but in short, when defining mage targets such as:
func Lint(...) {...} // define golangci-lint execution
func VulnCheck(...) {...} // define vuln/scan package execution
func CheckAll(ctx context.Context) {
mg.SerialCtxDeps(ctx, Lint, VulnCheck)
}running mage checkall skips the latter.
Now, when running golangci-lint with debugger it seems, that:
- an early exit using
os.exitwas run inpkg/commands/root.go:persistentPostRunfunction. - when the executor is initialized with
NewExecutor()in here thepersistentPostRunfunction is configured asPersistentPostRunEhandler for Cobra
So when linting is run in serial with other commands, golangci-lint exits the task pipeline too early with os.exit even when successful, and rest of the tasks are not run. Small (but not sure if relevant) detail is that in this context, golangci-lint is being used as a package library to not require users to install the CLI tool on the host, but using it as an utility library instead. Expected behavior is, that the serial command execution pipeline is exited early only when errors are encountered in the tasks in the pipeline, or finishes all the tasks with successful exit code.
Version of golangci-lint
v1.54.2
Configuration
run:
timeout: 15mGo environment
$ go version && go env
go version go1.21.3 darwin/arm64
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/foo/Library/Caches/go-build'
GOENV='/Users/foo/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/foo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/foo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.21.3/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.21.3/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.3'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/foo/Code/golangci-mage-sample/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/d7/ngb4dbgs5_10mdxzcxdnvms00000gn/T/go-build1531559415=/tmp/go-build -gno-record-gcc-switches -fno-common'Verbose output of running
❯ mage checkallverboselint # please see the example code for sample how this is executed in verbose mode
INFO [config_reader] Config search paths: [./ /Users/tomi/Code/golangci-mage-sample /Users/tomi/Code /Users/tomi /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 6 linters: [errcheck gosimple govet ineffassign staticcheck unused]
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|files|name|exports_file|imports|types_sizes) took 152.774375ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 936.709µs
INFO [linters_context/goanalysis] analyzers took 2.813149455s with top 10 stages: fact_purity: 758.80058ms, buildir: 650.577291ms, typedness: 481.034205ms, nilness: 296.102543ms, ctrlflow: 244.157582ms, fact_deprecated: 227.482294ms, printf: 76.501167ms, SA5012: 53.968748ms, inspect: 21.671835ms, SA4014: 440.917µs
INFO [runner] processing took 1.418µs with stages: max_same_issues: 333ns, skip_dirs: 167ns, severity-rules: 125ns, cgo: 125ns, nolint: 125ns, max_from_linter: 84ns, source_code: 42ns, identifier_marker: 42ns, skip_files: 42ns, filename_unadjuster: 42ns, path_prettifier: 42ns, path_shortener: 42ns, autogenerated_exclude: 42ns, diff: 42ns, exclude: 41ns, fixer: 41ns, path_prefixer: 41ns, max_per_file_from_linter: 0s, uniq_by_line: 0s, exclude-rules: 0s, sort_results: 0s
INFO [runner] linters took 1.06527s with stages: goanalysis_metalinter: 1.065250959s
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 14 samples, avg is 177.3MB, max is 252.6MB
INFO Execution took 1.22676475sA minimal reproducible example or link to a public repository
Sample repo with reproduction guide: https://github.com/kraashen/golangci-lint-mage-issue-sample
Validation
- Yes, I've included all information above (version, config, etc.).
Metadata
Metadata
Assignees
Type
Projects
Milestone
Relationships
Participants
Activity
boring-cyborg commentedon Oct 18, 2023
Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.
[-]Persistent post run exits early when run is successful[/-][+]Persistent post run exits early when run is successful in Magefile targets[/+]ldez commentedon Oct 24, 2023
Hello,
golangci-lint is not designed to be used as a library and we discourage that.
This means, for example, that the exposed elements of golangci-lint can be changed at any time, and those changes will not imply a specific version change.
Golangci-lint is designed to be a CLI tool only.
I understand your problem but you are using golangci-lint in a way that we highly discourage.
My first thought is to not accept changes that encourage the use of golangci-lint as a library.
heppu commentedon Oct 24, 2023
Regardless of use case I would argue that having multiple exit points for application is bad practice and makes the control flow harder to follow.
ldez commentedon Oct 24, 2023
This is a purely theoretical point of view, and I can agree with that, but a rule should be evaluated inside a context, not as a dogma.
We need to control the exit code and cobra doesn't provide a way to control it.
So we have no choice.
I can use this argument in an opposite way: currently, we control all (except some edge cases) exit codes in one place.
heppu commentedon Oct 24, 2023
Sure, I was just merely suggesting that this might still be worth looking into regardless the circumtances where user encountered this issue.
ldez commentedon Oct 24, 2023
My message was oriented as an answer to the user but I evaluated the whole context.
If the change was something useful outside of the user's context from my perspective, I would have added a sentence along these lines and maybe merged the related PR.
I didn't close the issue, to get more feedback, because it was possible I missed something.
Thank you for interacting and trying to find a parallel way.
Your argument led me to think that my first thought was right 😄
heppu commentedon Oct 24, 2023
Well, I'm glad to help :D