-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Go: Deal with incorrect toolchain versions #15979
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
3649af3
Go: Add test for `go/autobuilder/invalid-go-toolchain-version` diagno…
mbg be027e2
Go: Emit diagnostic for invalid toolchain versions
mbg 96a6dd7
Go: Move `go version` command construction into its own function
mbg 0d527b2
Go: Keep track of all installed toolchains that we know of
mbg c74d634
Go: Run `go` with a valid toolchain version if we have found an inval…
mbg 86bf4fb
Go: Make diagnostic names static
mbg ab255d7
Go: Fix `semver`-related logic bugs
mbg 6ea9982
Go: Add unit test to sanity check `HasGoVersion`
mbg 6b1d1d4
Go: Add integration test for incorrect version format logic
mbg 45b41bb
Go: Mirror stdout/stderr output in `InstallVersion`
mbg 977ac71
Update toolchain_test.go
mbg f6c22d4
Update toolchain_test.go
mbg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
31 changes: 31 additions & 0 deletions
31
...gration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/diagnostics.expected
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
{ | ||
"location": { | ||
"file": "go.mod" | ||
}, | ||
"markdownMessage": "As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).\n\n`1.21` in `go.mod` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.", | ||
"severity": "warning", | ||
"source": { | ||
"extractorName": "go", | ||
"id": "go/autobuilder/invalid-go-toolchain-version", | ||
"name": "Invalid Go toolchain version" | ||
}, | ||
"visibility": { | ||
"cliSummaryTable": true, | ||
"statusPage": true, | ||
"telemetry": true | ||
} | ||
} | ||
{ | ||
"markdownMessage": "A single `go.mod` file was found.\n\n`go.mod`", | ||
"severity": "note", | ||
"source": { | ||
"extractorName": "go", | ||
"id": "go/autobuilder/single-root-go-mod-found", | ||
"name": "A single `go.mod` file was found in the root" | ||
}, | ||
"visibility": { | ||
"cliSummaryTable": false, | ||
"statusPage": false, | ||
"telemetry": true | ||
} | ||
} |
3 changes: 3 additions & 0 deletions
3
go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/go.mod
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
go 1.21 | ||
|
||
module example |
5 changes: 5 additions & 0 deletions
5
go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/main.go
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package main | ||
|
||
func main() { | ||
|
||
} |
19 changes: 19 additions & 0 deletions
19
go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/test.py
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import os | ||
import subprocess | ||
|
||
from create_database_utils import * | ||
from diagnostics_test_utils import * | ||
|
||
# Set up a GOPATH relative to this test's root directory; | ||
# we set os.environ instead of using extra_env because we | ||
# need it to be set for the call to "go clean -modcache" later | ||
goPath = os.path.join(os.path.abspath(os.getcwd()), ".go") | ||
os.environ['GOPATH'] = goPath | ||
os.environ['LGTM_INDEX_IMPORT_PATH'] = "test" | ||
run_codeql_database_create([], lang="go", source="src") | ||
|
||
check_diagnostics() | ||
|
||
# Clean up the temporary GOPATH to prevent Bazel failures next | ||
# time the tests are run; see https://github.com/golang/go/issues/27161 | ||
subprocess.call(["go", "clean", "-modcache"]) |
31 changes: 31 additions & 0 deletions
31
go/ql/integration-tests/all-platforms/go/go-version-bump/diagnostics.expected
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
{ | ||
"location": { | ||
"file": "go.mod" | ||
}, | ||
"markdownMessage": "As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).\n\n`1.21` in `go.mod` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.", | ||
"severity": "warning", | ||
"source": { | ||
"extractorName": "go", | ||
"id": "go/autobuilder/invalid-go-toolchain-version", | ||
"name": "Invalid Go toolchain version" | ||
}, | ||
"visibility": { | ||
"cliSummaryTable": true, | ||
"statusPage": true, | ||
"telemetry": true | ||
} | ||
} | ||
{ | ||
"markdownMessage": "A single `go.mod` file was found.\n\n`go.mod`", | ||
"severity": "note", | ||
"source": { | ||
"extractorName": "go", | ||
"id": "go/autobuilder/single-root-go-mod-found", | ||
"name": "A single `go.mod` file was found in the root" | ||
}, | ||
"visibility": { | ||
"cliSummaryTable": false, | ||
"statusPage": false, | ||
"telemetry": true | ||
} | ||
} |
3 changes: 3 additions & 0 deletions
3
go/ql/integration-tests/all-platforms/go/go-version-bump/src/go.mod
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
go 1.21 | ||
|
||
module test |
3 changes: 3 additions & 0 deletions
3
go/ql/integration-tests/all-platforms/go/go-version-bump/src/main.go
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
package main | ||
|
||
func main() {} |
18 changes: 18 additions & 0 deletions
18
go/ql/integration-tests/all-platforms/go/go-version-bump/test.py
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import os | ||
import subprocess | ||
|
||
from create_database_utils import * | ||
from diagnostics_test_utils import * | ||
|
||
# Set up a GOPATH relative to this test's root directory; | ||
# we set os.environ instead of using extra_env because we | ||
# need it to be set for the call to "go clean -modcache" later | ||
goPath = os.path.join(os.path.abspath(os.getcwd()), ".go") | ||
os.environ['GOPATH'] = goPath | ||
run_codeql_database_create([], lang="go", source="src") | ||
|
||
check_diagnostics() | ||
|
||
# Clean up the temporary GOPATH to prevent Bazel failures next | ||
# time the tests are run; see https://github.com/golang/go/issues/27161 | ||
subprocess.call(["go", "clean", "-modcache"]) |
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider marking this as "installed" regardless to avoid repeatedly attempting to install the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about this. It seems like a rare scenario where we would repeatedly attempt to install the same version and that fails for whatever reason.
If I had implemented this set specifically for the new check, then okay, but since the implementation is more general, it might lead to weird results down the line if we want to utilise the set of installed versions for something else and find that in some cases those aren't actually installed.