Consider $GO environment variable make precheck checks
#20750
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tklauser
added
release-note/misc
For backports it's usually convenient to be able to set the $GO environment variable to the version used in the particular stable branch, e.g. on v1.10 we'd set GO=go1.16.15. However, that setting is currently not considered in contrib/scripts/check-fmt.sh. If there were changes in how gofmt formats code between Go versions and the system's Go version is newer than the $GO setting, using that gofmt binary might lead to check-fmt failure. Signed-off-by: Tobias Klauser <tobias@cilium.io>
For backports it's usually convenient to be able to set the $GO
environment variable to the version used in the particular stable
branch, e.g. on v1.10 we'd set GO=go1.16.15. However, that setting is
currently not considered in tools/customvet. This can lead to unexpected
failures such as the followin when running `GO=go1.16.15 make precheck`
on v1.10 branch:
contrib/scripts/custom-vet-check.sh
main: internal error: package "sync" without types was imported from "github.com/cilium/cilium/pkg/cleanup"
exit status 1
Signed-off-by: Tobias Klauser <tobias@cilium.io>
tklauser
force-pushed
the
pr/tklauser/check-fmt-go-version
branch
from
3585c8b
to
bc314ee
Compare
tklauser
changed the title
$GO in check-fmt.sh$GO environment variable make precheck checks
joestringer
reviewed
Aug 2, 2022
| @@ -3,13 +3,15 @@ | |||
| set -e | |||
| set -o pipefail | |||
|
|
|||
| _goroot=$(${GO:-go} env GOROOT) | |||
There was a problem hiding this comment.
This is pretty elegant.. hmm.
So far I got away without GOROOT, personally I find it's a bit opinionated and a pain to deal with.. maybe we could set the default for _goroot to "" iff $GO is not set..?
There was a problem hiding this comment.
FWIW, the Go toolchain will always return a non-empty value for "go env GOROOT", even if $GOROOT isn't set in the env. It'll derive it relative to the go binary invoked, unless it's overriden by some other means.
There was a problem hiding this comment.
🤯 I totally thought I ran this and got errors out, but now that I try it today it's fine. -ENO☕ I guess!
ldelossa
approved these changes
Aug 2, 2022
Me neither :) that's the beauty about "go env". |
joestringer
approved these changes
Aug 3, 2022
maintainer-s-little-helper
bot
moved this from Needs backport from master
to Backport pending to v1.10
in 1.11.8
maintainer-s-little-helper
bot
moved this from Needs backport from master
to Backport pending to v1.10
in 1.10.14
maintainer-s-little-helper
bot
moved this from Needs backport from master
to Backport pending to v1.12
in 1.12.1
tklauser
added
backport-done/1.11
tklauser
moved this from Backport pending to v1.10
to Backport done to v1.10
in 1.10.14
This was referenced Aug 11, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For backports it's usually convenient to be able to set the
$GOenvironment variable to the version used in the particular stable
branch, e.g. on v1.10 we'd set
GO=go1.16.15. However, that setting iscurrently not considered in all check scripts invoked by
make precheck.See individual commit messages for details.