From 45a6687b13ddb3011713302e81643d3ec355ae89 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 2 Apr 2024 15:04:52 +0100 Subject: [PATCH] Revert "Go: Deal with incorrect toolchain versions" --- .../cli/go-autobuilder/go-autobuilder.go | 2 +- go/extractor/diagnostics/diagnostics.go | 17 +---- go/extractor/project/project.go | 38 +++-------- go/extractor/toolchain/toolchain.go | 65 +------------------ go/extractor/toolchain/toolchain_test.go | 6 -- .../diagnostics.expected | 31 --------- .../invalid-toolchain-version/src/go.mod | 3 - .../invalid-toolchain-version/src/main.go | 5 -- .../invalid-toolchain-version/test.py | 19 ------ .../go/go-version-bump/diagnostics.expected | 31 --------- .../go/go-version-bump/src/go.mod | 3 - .../go/go-version-bump/src/main.go | 3 - .../all-platforms/go/go-version-bump/test.py | 18 ----- .../diagnostics.expected | 2 +- 14 files changed, 12 insertions(+), 231 deletions(-) delete mode 100644 go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/diagnostics.expected delete mode 100644 go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/go.mod delete mode 100644 go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/main.go delete mode 100644 go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/test.py delete mode 100644 go/ql/integration-tests/all-platforms/go/go-version-bump/diagnostics.expected delete mode 100644 go/ql/integration-tests/all-platforms/go/go-version-bump/src/go.mod delete mode 100644 go/ql/integration-tests/all-platforms/go/go-version-bump/src/main.go delete mode 100644 go/ql/integration-tests/all-platforms/go/go-version-bump/test.py diff --git a/go/extractor/cli/go-autobuilder/go-autobuilder.go b/go/extractor/cli/go-autobuilder/go-autobuilder.go index 08f8477cac7..b2e2a78666f 100644 --- a/go/extractor/cli/go-autobuilder/go-autobuilder.go +++ b/go/extractor/cli/go-autobuilder/go-autobuilder.go @@ -417,7 +417,7 @@ func installDependencies(workspace project.GoWorkspace) { } else { if workspace.Modules == nil { project.InitGoModForLegacyProject(workspace.BaseDir) - workspace.Modules = project.LoadGoModules(true, []string{filepath.Join(workspace.BaseDir, "go.mod")}) + workspace.Modules = project.LoadGoModules([]string{filepath.Join(workspace.BaseDir, "go.mod")}) } // get dependencies for all modules diff --git a/go/extractor/diagnostics/diagnostics.go b/go/extractor/diagnostics/diagnostics.go index 385c611aac8..c02af8afb92 100644 --- a/go/extractor/diagnostics/diagnostics.go +++ b/go/extractor/diagnostics/diagnostics.go @@ -497,7 +497,7 @@ func EmitNewerSystemGoRequired(requiredVersion string) { func EmitExtractionFailedForProjects(path []string) { emitDiagnostic( "go/autobuilder/extraction-failed-for-project", - "Unable to extract some Go projects", + fmt.Sprintf("Unable to extract %d Go projects", len(path)), fmt.Sprintf( "The following %d Go project%s could not be extracted successfully:\n\n`%s`\n", len(path), @@ -508,18 +508,3 @@ func EmitExtractionFailedForProjects(path []string) { noLocation, ) } - -func EmitInvalidToolchainVersion(goModPath string, version string) { - emitDiagnostic( - "go/autobuilder/invalid-go-toolchain-version", - "Invalid Go toolchain version", - strings.Join([]string{ - "As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).", - fmt.Sprintf("`%s` in `%s` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.", version, goModPath), - }, - "\n\n"), - severityWarning, - fullVisibility, - &locationStruct{File: goModPath}, - ) -} diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 92134733856..22c0f856c79 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -176,13 +176,10 @@ func findGoModFiles(root string) []string { return util.FindAllFilesWithName(root, "go.mod", "vendor") } -// A regular expression for the Go toolchain version syntax. -var toolchainVersionRe *regexp.Regexp = regexp.MustCompile(`(?m)^([0-9]+\.[0-9]+\.[0-9]+)$`) - // Given a list of `go.mod` file paths, try to parse them all. The resulting array of `GoModule` objects // will be the same length as the input array and the objects will contain at least the `go.mod` path. // If parsing the corresponding file is successful, then the parsed contents will also be available. -func LoadGoModules(emitDiagnostics bool, goModFilePaths []string) []*GoModule { +func LoadGoModules(goModFilePaths []string) []*GoModule { results := make([]*GoModule, len(goModFilePaths)) for i, goModFilePath := range goModFilePaths { @@ -204,25 +201,6 @@ func LoadGoModules(emitDiagnostics bool, goModFilePaths []string) []*GoModule { } results[i].Module = modFile - - // If this `go.mod` file specifies a Go language version, that version is `1.21` or greater, and - // there is no `toolchain` directive, check that it is a valid Go toolchain version. Otherwise, - // `go` commands which try to download the right version of the Go toolchain will fail. We detect - // this situation and emit a diagnostic. - if modFile.Toolchain == nil && modFile.Go != nil && - !toolchainVersionRe.Match([]byte(modFile.Go.Version)) && semver.Compare("v"+modFile.Go.Version, "v1.21.0") >= 0 { - diagnostics.EmitInvalidToolchainVersion(goModFilePath, modFile.Go.Version) - - modPath := filepath.Dir(goModFilePath) - - log.Printf( - "`%s` is not a valid toolchain version, trying to install it explicitly using the canonical representation in `%s`.", - modFile.Go.Version, - modPath, - ) - - toolchain.InstallVersion(modPath, modFile.Go.Version) - } } return results @@ -231,7 +209,7 @@ func LoadGoModules(emitDiagnostics bool, goModFilePaths []string) []*GoModule { // Given a path to a `go.work` file, this function attempts to parse the `go.work` file. If unsuccessful, // we attempt to discover `go.mod` files within subdirectories of the directory containing the `go.work` // file ourselves. -func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { +func discoverWorkspace(workFilePath string) GoWorkspace { log.Printf("Loading %s...\n", workFilePath) baseDir := filepath.Dir(workFilePath) workFileSrc, err := os.ReadFile(workFilePath) @@ -245,7 +223,7 @@ func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { return GoWorkspace{ BaseDir: baseDir, - Modules: LoadGoModules(emitDiagnostics, goModFilePaths), + Modules: LoadGoModules(goModFilePaths), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, baseDir), } @@ -262,7 +240,7 @@ func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { return GoWorkspace{ BaseDir: baseDir, - Modules: LoadGoModules(emitDiagnostics, goModFilePaths), + Modules: LoadGoModules(goModFilePaths), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, baseDir), } @@ -285,7 +263,7 @@ func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { return GoWorkspace{ BaseDir: baseDir, WorkspaceFile: workFile, - Modules: LoadGoModules(emitDiagnostics, goModFilePaths), + Modules: LoadGoModules(goModFilePaths), DepMode: GoGetWithModules, ModMode: ModReadonly, // Workspaces only support "readonly" } @@ -308,7 +286,7 @@ func discoverWorkspaces(emitDiagnostics bool) []GoWorkspace { for i, goModFile := range goModFiles { results[i] = GoWorkspace{ BaseDir: filepath.Dir(goModFile), - Modules: LoadGoModules(emitDiagnostics, []string{goModFile}), + Modules: LoadGoModules([]string{goModFile}), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, filepath.Dir(goModFile)), } @@ -325,7 +303,7 @@ func discoverWorkspaces(emitDiagnostics bool) []GoWorkspace { results := make([]GoWorkspace, len(goWorkFiles)) for i, workFilePath := range goWorkFiles { - results[i] = discoverWorkspace(emitDiagnostics, workFilePath) + results[i] = discoverWorkspace(workFilePath) } // Add all stray `go.mod` files (i.e. those not referenced by `go.work` files) @@ -357,7 +335,7 @@ func discoverWorkspaces(emitDiagnostics bool) []GoWorkspace { log.Printf("Module %s is not referenced by any go.work file; adding it separately.\n", goModFile) results = append(results, GoWorkspace{ BaseDir: filepath.Dir(goModFile), - Modules: LoadGoModules(emitDiagnostics, []string{goModFile}), + Modules: LoadGoModules([]string{goModFile}), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, filepath.Dir(goModFile)), }) diff --git a/go/extractor/toolchain/toolchain.go b/go/extractor/toolchain/toolchain.go index 40ab402e11d..104894c5975 100644 --- a/go/extractor/toolchain/toolchain.go +++ b/go/extractor/toolchain/toolchain.go @@ -18,15 +18,7 @@ func IsInstalled() bool { return err == nil } -// The default Go version that is available on a system and a set of all versions -// that we know are installed on the system. var goVersion = "" -var goVersions = map[string]struct{}{} - -// Adds an entry to the set of installed Go versions for the normalised `version` number. -func addGoVersion(version string) { - goVersions[semver.Canonical("v"+version)] = struct{}{} -} // Returns the current Go version as returned by 'go version', e.g. go1.14.4 func GetEnvGoVersion() string { @@ -35,7 +27,7 @@ func GetEnvGoVersion() string { // download the version of Go specified in there. That may either fail or result in us just // being told what's already in 'go.mod'. Setting 'GOTOOLCHAIN' to 'local' will force it // to use the local Go toolchain instead. - cmd := Version() + cmd := exec.Command("go", "version") cmd.Env = append(os.Environ(), "GOTOOLCHAIN=local") out, err := cmd.CombinedOutput() @@ -44,59 +36,10 @@ func GetEnvGoVersion() string { } goVersion = parseGoVersion(string(out)) - addGoVersion(goVersion[2:]) } return goVersion } -// Determines whether, to our knowledge, `version` is available on the current system. -func HasGoVersion(version string) bool { - _, found := goVersions[semver.Canonical("v"+version)] - return found -} - -// Attempts to install the Go toolchain `version`. -func InstallVersion(workingDir string, version string) bool { - // No need to install it if we know that it is already installed. - if HasGoVersion(version) { - return true - } - - // Construct a command to invoke `go version` with `GOTOOLCHAIN=go1.N.0` to give - // Go a valid toolchain version to download the toolchain we need; subsequent commands - // should then work even with an invalid version that's still in `go.mod` - toolchainArg := "GOTOOLCHAIN=go" + semver.Canonical("v" + version)[1:] - versionCmd := Version() - versionCmd.Dir = workingDir - versionCmd.Env = append(os.Environ(), toolchainArg) - versionCmd.Stdout = os.Stdout - versionCmd.Stderr = os.Stderr - - log.Printf( - "Trying to install Go %s using its canonical representation in `%s`.", - version, - workingDir, - ) - - // Run the command. If something goes wrong, report it to the log and signal failure - // to the caller. - if versionErr := versionCmd.Run(); versionErr != nil { - log.Printf( - "Failed to invoke `%s go version` in %s: %s\n", - toolchainArg, - versionCmd.Dir, - versionErr.Error(), - ) - - return false - } - - // Add the version to the set of versions that we know are installed and signal - // success to the caller. - addGoVersion(version) - return true -} - // Returns the current Go version in semver format, e.g. v1.14.4 func GetEnvGoSemVer() string { goVersion := GetEnvGoVersion() @@ -164,9 +107,3 @@ func VendorModule(path string) *exec.Cmd { modVendor.Dir = path return modVendor } - -// Constructs a command to run `go version`. -func Version() *exec.Cmd { - version := exec.Command("go", "version") - return version -} diff --git a/go/extractor/toolchain/toolchain_test.go b/go/extractor/toolchain/toolchain_test.go index e0f70a283e6..66d16f97964 100644 --- a/go/extractor/toolchain/toolchain_test.go +++ b/go/extractor/toolchain/toolchain_test.go @@ -14,9 +14,3 @@ func TestParseGoVersion(t *testing.T) { } } } - -func TestHasGoVersion(t *testing.T) { - if HasGoVersion("1.21") { - t.Error("Expected HasGoVersion(\"1.21\") to be false, but got true") - } -} diff --git a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/diagnostics.expected b/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/diagnostics.expected deleted file mode 100644 index 2e5d732e53e..00000000000 --- a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/diagnostics.expected +++ /dev/null @@ -1,31 +0,0 @@ -{ - "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 - } -} diff --git a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/go.mod b/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/go.mod deleted file mode 100644 index bee3365b899..00000000000 --- a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/go.mod +++ /dev/null @@ -1,3 +0,0 @@ -go 1.21 - -module example diff --git a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/main.go b/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/main.go deleted file mode 100644 index 79058077776..00000000000 --- a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/src/main.go +++ /dev/null @@ -1,5 +0,0 @@ -package main - -func main() { - -} diff --git a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/test.py b/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/test.py deleted file mode 100644 index 2824c219d37..00000000000 --- a/go/ql/integration-tests/all-platforms/go/diagnostics/invalid-toolchain-version/test.py +++ /dev/null @@ -1,19 +0,0 @@ -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"]) diff --git a/go/ql/integration-tests/all-platforms/go/go-version-bump/diagnostics.expected b/go/ql/integration-tests/all-platforms/go/go-version-bump/diagnostics.expected deleted file mode 100644 index 2e5d732e53e..00000000000 --- a/go/ql/integration-tests/all-platforms/go/go-version-bump/diagnostics.expected +++ /dev/null @@ -1,31 +0,0 @@ -{ - "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 - } -} diff --git a/go/ql/integration-tests/all-platforms/go/go-version-bump/src/go.mod b/go/ql/integration-tests/all-platforms/go/go-version-bump/src/go.mod deleted file mode 100644 index 18793f7e295..00000000000 --- a/go/ql/integration-tests/all-platforms/go/go-version-bump/src/go.mod +++ /dev/null @@ -1,3 +0,0 @@ -go 1.21 - -module test diff --git a/go/ql/integration-tests/all-platforms/go/go-version-bump/src/main.go b/go/ql/integration-tests/all-platforms/go/go-version-bump/src/main.go deleted file mode 100644 index 38dd16da61a..00000000000 --- a/go/ql/integration-tests/all-platforms/go/go-version-bump/src/main.go +++ /dev/null @@ -1,3 +0,0 @@ -package main - -func main() {} diff --git a/go/ql/integration-tests/all-platforms/go/go-version-bump/test.py b/go/ql/integration-tests/all-platforms/go/go-version-bump/test.py deleted file mode 100644 index 43c7d1b38e8..00000000000 --- a/go/ql/integration-tests/all-platforms/go/go-version-bump/test.py +++ /dev/null @@ -1,18 +0,0 @@ -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"]) diff --git a/go/ql/integration-tests/all-platforms/go/two-go-mods-one-failure/diagnostics.expected b/go/ql/integration-tests/all-platforms/go/two-go-mods-one-failure/diagnostics.expected index c37938d5c7c..02805a60c99 100644 --- a/go/ql/integration-tests/all-platforms/go/two-go-mods-one-failure/diagnostics.expected +++ b/go/ql/integration-tests/all-platforms/go/two-go-mods-one-failure/diagnostics.expected @@ -18,7 +18,7 @@ "source": { "extractorName": "go", "id": "go/autobuilder/extraction-failed-for-project", - "name": "Unable to extract some Go projects" + "name": "Unable to extract 1 Go projects" }, "visibility": { "cliSummaryTable": true,