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 an option to set the Go version #1143

Closed
ldez opened this issue May 20, 2024 · 4 comments · Fixed by #1144 or #1147
Closed

Add an option to set the Go version #1143

ldez opened this issue May 20, 2024 · 4 comments · Fixed by #1144 or #1147

Comments

@ldez
Copy link
Contributor

ldez commented May 20, 2024

Summary

Hello,

I know that my request is really specific to golangci-lint, and if you don't want to do that I will understand.

The new way for gosec to detect the Go version has a huge impact on golangci-lint performances.

The best solution for us is to have an option to set the Go version to bypass the usage of go list.

Steps to reproduce the behavior

golangci/golangci-lint#4735

gosec version

v2.20.0

Go version (output of 'go version')

$ go version
go version go1.22.3 linux/amd64

Operating system / Environment

Not dependent on the OS

@ccojocar
Copy link
Member

ccojocar commented May 21, 2024

@ldez The new approach is using the go list to parse the go version first from the local go module file, if this is not found will fall back to runtime version. Is go list introducing performance issues?

I am thinking to keep the current behaviour as default and introduce an environment variable to bypass the go list parsing something like GOSECNOLOCALVERSION. Would this work for you?

@ldez
Copy link
Contributor Author

ldez commented May 21, 2024

Is go list introducing performance issues?

Yes because golangci-lint loads the rules for each package, so a go list is done a lot of times (nb of rules that require the Go version)*(nb of packages).

I am thinking to keep the current behaviour as default and introduce an environment variable to bypass the go list parsing something like GOSECNOLOCALVERSION. Would this work for you?

An env var can work.

Note: currently, only 2 rules (G601 and G113) use the GoVersion function but the more you will use it and more you will also be impacted by this performance problem.

@ccojocar
Copy link
Member

@ldez I added an environment variable which should disabled the parsing of Go version from module file present in the project. Please have a look at the README.

@ldez
Copy link
Contributor Author

ldez commented May 22, 2024

@ccojocar I was expected to be able to set the go version through the env var because the runtime version will not be right.

// GoVersion returns parsed version of Go mod version and fallback to runtime version if not found.
func GoVersion() (int, int, int) {
	if env, ok := os.LookupEnv("GOSECGOVERSION"); ok {
		return parseGoVersion(strings.TrimPrefix(env, "go"))
	}

	goVersion, err := goModVersion()
	if err != nil {
		return parseGoVersion(strings.TrimPrefix(runtime.Version(), "go"))
	}

	return parseGoVersion(goVersion)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants