From 648b28c1d22863fe9803ece4f2b9d7d86b4c419e Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:14:38 +0100 Subject: [PATCH 01/16] Go: Add `ToolchainVersionToSemVer` with tests --- go/extractor/toolchain/toolchain.go | 21 +++++++++++++-------- go/extractor/toolchain/toolchain_test.go | 13 +++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/go/extractor/toolchain/toolchain.go b/go/extractor/toolchain/toolchain.go index 01befa4384ea..effc398b8bbf 100644 --- a/go/extractor/toolchain/toolchain.go +++ b/go/extractor/toolchain/toolchain.go @@ -104,23 +104,28 @@ func InstallVersion(workingDir string, version string) bool { return true } -// Returns the current Go version in semver format, e.g. v1.14.4 -func GetEnvGoSemVer() string { - goVersion := GetEnvGoVersion() - if !strings.HasPrefix(goVersion, "go") { - log.Fatalf("Expected 'go version' output of the form 'go1.2.3'; got '%s'", goVersion) +// Converts a Go toolchain version of the form "go1.2.3" to a semantic version. +func ToolchainVersionToSemVer(toolchainVersion string) string { + if !strings.HasPrefix(toolchainVersion, "go") { + log.Fatalf("Expected Go toolchain version of the form 'go1.2.3'; got '%s'", toolchainVersion) } + // Go versions don't follow the SemVer format, but the only exception we normally care about // is release candidates; so this is a horrible hack to convert e.g. `go1.22rc1` into `go1.22-rc1` // which is compatible with the SemVer specification - rcIndex := strings.Index(goVersion, "rc") + rcIndex := strings.Index(toolchainVersion, "rc") if rcIndex != -1 { - return semver.Canonical("v"+goVersion[2:rcIndex]) + "-" + goVersion[rcIndex:] + return semver.Canonical("v"+toolchainVersion[2:rcIndex]) + "-" + toolchainVersion[rcIndex:] } else { - return semver.Canonical("v" + goVersion[2:]) + return semver.Canonical("v" + toolchainVersion[2:]) } } +// Returns the current Go version in semver format, e.g. v1.14.4 +func GetEnvGoSemVer() string { + return ToolchainVersionToSemVer(GetEnvGoVersion()) +} + // The 'go version' command may output warnings on separate lines before // the actual version string is printed. This function parses the output // to retrieve just the version string. diff --git a/go/extractor/toolchain/toolchain_test.go b/go/extractor/toolchain/toolchain_test.go index e0f70a283e67..d43a69da7f82 100644 --- a/go/extractor/toolchain/toolchain_test.go +++ b/go/extractor/toolchain/toolchain_test.go @@ -20,3 +20,16 @@ func TestHasGoVersion(t *testing.T) { t.Error("Expected HasGoVersion(\"1.21\") to be false, but got true") } } + +func testToolchainVersionToSemVer(t *testing.T, toolchainVersion string, expectedSemVer string) { + result := ToolchainVersionToSemVer(toolchainVersion) + if result != expectedSemVer { + t.Errorf("Expected ToolchainVersionToSemVer(\"%s\") to be %s, but got %s.", toolchainVersion, expectedSemVer, result) + } +} + +func TestToolchainVersionToSemVer(t *testing.T) { + testToolchainVersionToSemVer(t, "go1.20", "v1.20.0") + testToolchainVersionToSemVer(t, "go1.20.1", "v1.20.1") + testToolchainVersionToSemVer(t, "go1.20rc1", "v1.20.0-rc1") +} From 628064118fb55ce6c3bd63d51317ee1b68f33e9e Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:21:26 +0100 Subject: [PATCH 02/16] Go: Use `Toolchain` directives in `go.work` files, if available --- go/extractor/project/project.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 13f6f455ba1d..310abd65d1bf 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -60,8 +60,17 @@ type GoVersionInfo struct { // 1. The Go version specified in the `go.work` file, if any. // 2. The greatest Go version specified in any `go.mod` file, if any. func (workspace *GoWorkspace) RequiredGoVersion() GoVersionInfo { - if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil { - // If we have parsed a `go.work` file, return the version number from it. + // If we have parsed a `go.work` file, we prioritise versions from it over those in individual `go.mod` + // files. We are interested in toolchain versions, so if there is an explicit toolchain declaration in + // a `go.work` file, we use that. Otherwise, we fall back to the language version in the `go.work` file + // and use that as toolchain version. If we didn't parse a `go.work` file, then we try to find the + // greatest version contained in `go.mod` files. + if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Toolchain != nil { + return GoVersionInfo{ + Version: toolchain.ToolchainVersionToSemVer(workspace.WorkspaceFile.Toolchain.Name), + Found: true, + } + } else if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil { return GoVersionInfo{Version: workspace.WorkspaceFile.Go.Version, Found: true} } else if workspace.Modules != nil && len(workspace.Modules) > 0 { // Otherwise, if we have `go.work` files, find the greatest Go version in those. From 4e701a12db40f9a692986ef4321c77cea982c81f Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:27:16 +0100 Subject: [PATCH 03/16] Go: Refactor `go.mod` version retrieval into its own method --- go/extractor/project/project.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 310abd65d1bf..00f921745f8a 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -37,6 +37,18 @@ type GoModule struct { Module *modfile.File // The parsed contents of the `go.mod` file } +// Tries to find the Go toolchain version required for this module. +func (module *GoModule) RequiredGoVersion() GoVersionInfo { + if module.Module != nil && module.Module.Go != nil { + return GoVersionInfo{ + Version: module.Module.Go.Version, + Found: true, + } + } else { + return tryReadGoDirective(module.Path) + } +} + // Represents information about a Go project workspace: this may either be a folder containing // a `go.work` file or a collection of `go.mod` files. type GoWorkspace struct { @@ -76,16 +88,10 @@ func (workspace *GoWorkspace) RequiredGoVersion() GoVersionInfo { // Otherwise, if we have `go.work` files, find the greatest Go version in those. var greatestVersion string = "" for _, module := range workspace.Modules { - if module.Module != nil && module.Module.Go != nil { - // If we have parsed the file, retrieve the version number we have already obtained. - if greatestVersion == "" || semver.Compare("v"+module.Module.Go.Version, "v"+greatestVersion) > 0 { - greatestVersion = module.Module.Go.Version - } - } else { - modVersion := tryReadGoDirective(module.Path) - if modVersion.Found && (greatestVersion == "" || semver.Compare("v"+modVersion.Version, "v"+greatestVersion) > 0) { - greatestVersion = modVersion.Version - } + moduleVersionInfo := module.RequiredGoVersion() + + if greatestVersion == "" || semver.Compare("v"+moduleVersionInfo.Version, "v"+greatestVersion) > 0 { + greatestVersion = moduleVersionInfo.Version } } From 4666ed99578c4f03325cbcdbdcc8e05b5356f81a Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:39:15 +0100 Subject: [PATCH 04/16] Go: Add constructors for `GoVersionInfo` --- go/extractor/project/project.go | 40 ++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 00f921745f8a..eb383b9d9fd4 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -40,10 +40,7 @@ type GoModule struct { // Tries to find the Go toolchain version required for this module. func (module *GoModule) RequiredGoVersion() GoVersionInfo { if module.Module != nil && module.Module.Go != nil { - return GoVersionInfo{ - Version: module.Module.Go.Version, - Found: true, - } + return VersionFound(module.Module.Go.Version) } else { return tryReadGoDirective(module.Path) } @@ -68,6 +65,22 @@ type GoVersionInfo struct { Found bool } +// Represents a `GoVersionInfo` indicating that no version was found. +var VersionNotFound GoVersionInfo = GoVersionInfo{"", false} + +// Constructs a `GoVersionInfo` for a version we found. +func VersionFound(version string) GoVersionInfo { + // Add the "v" required by `semver` if there isn't one yet. + if !strings.HasPrefix(version, "v") { + version = "v" + version + } + + return GoVersionInfo{ + Version: version, + Found: true, + } +} + // Determines the version of Go that is required by this workspace. This is, in order of preference: // 1. The Go version specified in the `go.work` file, if any. // 2. The greatest Go version specified in any `go.mod` file, if any. @@ -78,36 +91,33 @@ func (workspace *GoWorkspace) RequiredGoVersion() GoVersionInfo { // and use that as toolchain version. If we didn't parse a `go.work` file, then we try to find the // greatest version contained in `go.mod` files. if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Toolchain != nil { - return GoVersionInfo{ - Version: toolchain.ToolchainVersionToSemVer(workspace.WorkspaceFile.Toolchain.Name), - Found: true, - } + return VersionFound(toolchain.ToolchainVersionToSemVer(workspace.WorkspaceFile.Toolchain.Name)) } else if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil { - return GoVersionInfo{Version: workspace.WorkspaceFile.Go.Version, Found: true} + return VersionFound(workspace.WorkspaceFile.Go.Version) } else if workspace.Modules != nil && len(workspace.Modules) > 0 { // Otherwise, if we have `go.work` files, find the greatest Go version in those. var greatestVersion string = "" for _, module := range workspace.Modules { moduleVersionInfo := module.RequiredGoVersion() - if greatestVersion == "" || semver.Compare("v"+moduleVersionInfo.Version, "v"+greatestVersion) > 0 { + if greatestVersion == "" || semver.Compare(moduleVersionInfo.Version, greatestVersion) > 0 { greatestVersion = moduleVersionInfo.Version } } // If we have found some version, return it. if greatestVersion != "" { - return GoVersionInfo{Version: greatestVersion, Found: true} + return VersionFound(greatestVersion) } } - return GoVersionInfo{Version: "", Found: false} + return VersionNotFound } // Finds the greatest Go version required by any of the given `workspaces`. // Returns a `GoVersionInfo` value with `Found: false` if no version information is available. func RequiredGoVersion(workspaces *[]GoWorkspace) GoVersionInfo { - greatestGoVersion := GoVersionInfo{Version: "", Found: false} + greatestGoVersion := VersionNotFound for _, workspace := range *workspaces { goVersionInfo := workspace.RequiredGoVersion() if goVersionInfo.Found && (!greatestGoVersion.Found || semver.Compare("v"+goVersionInfo.Version, "v"+greatestGoVersion.Version) > 0) { @@ -598,9 +608,9 @@ func tryReadGoDirective(path string) GoVersionInfo { matches := versionRe.FindSubmatch(goMod) if matches != nil { if len(matches) > 1 { - return GoVersionInfo{string(matches[1]), true} + return VersionFound(string(matches[1])) } } } - return GoVersionInfo{"", false} + return VersionNotFound } From c9d1aa6354e7e9f8e1a7c83c78e13e36841224e7 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:40:41 +0100 Subject: [PATCH 05/16] Go: Use `Toolchain` directives in `go.mod` files, if available --- go/extractor/project/project.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index eb383b9d9fd4..f282834672f8 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -39,6 +39,9 @@ type GoModule struct { // Tries to find the Go toolchain version required for this module. func (module *GoModule) RequiredGoVersion() GoVersionInfo { + if module.Module != nil && module.Module.Toolchain != nil { + return VersionFound(toolchain.ToolchainVersionToSemVer(module.Module.Toolchain.Name)) + } if module.Module != nil && module.Module.Go != nil { return VersionFound(module.Module.Go.Version) } else { From 01597ec23b6b10c427af480735ba2babbe84925a Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:56:07 +0100 Subject: [PATCH 06/16] Go: Add `GoVersionToSemVer` --- go/extractor/toolchain/toolchain.go | 23 ++++++++++++++--------- go/extractor/toolchain/toolchain_test.go | 13 +++++++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/go/extractor/toolchain/toolchain.go b/go/extractor/toolchain/toolchain.go index effc398b8bbf..1d9a3da081e3 100644 --- a/go/extractor/toolchain/toolchain.go +++ b/go/extractor/toolchain/toolchain.go @@ -104,21 +104,26 @@ func InstallVersion(workingDir string, version string) bool { return true } +// Converts a Go version to a semantic version. +func GoVersionToSemVer(goVersion string) string { + // Go versions don't follow the SemVer format, but the only exception we normally care about + // is release candidates; so this is a horrible hack to convert e.g. `1.22rc1` into `1.22-rc1` + // which is compatible with the SemVer specification + rcIndex := strings.Index(goVersion, "rc") + if rcIndex != -1 { + return semver.Canonical("v"+goVersion[:rcIndex]) + "-" + goVersion[rcIndex:] + } else { + return semver.Canonical("v" + goVersion) + } +} + // Converts a Go toolchain version of the form "go1.2.3" to a semantic version. func ToolchainVersionToSemVer(toolchainVersion string) string { if !strings.HasPrefix(toolchainVersion, "go") { log.Fatalf("Expected Go toolchain version of the form 'go1.2.3'; got '%s'", toolchainVersion) } - // Go versions don't follow the SemVer format, but the only exception we normally care about - // is release candidates; so this is a horrible hack to convert e.g. `go1.22rc1` into `go1.22-rc1` - // which is compatible with the SemVer specification - rcIndex := strings.Index(toolchainVersion, "rc") - if rcIndex != -1 { - return semver.Canonical("v"+toolchainVersion[2:rcIndex]) + "-" + toolchainVersion[rcIndex:] - } else { - return semver.Canonical("v" + toolchainVersion[2:]) - } + return GoVersionToSemVer(toolchainVersion[2:]) } // Returns the current Go version in semver format, e.g. v1.14.4 diff --git a/go/extractor/toolchain/toolchain_test.go b/go/extractor/toolchain/toolchain_test.go index d43a69da7f82..8f60aa9a665f 100644 --- a/go/extractor/toolchain/toolchain_test.go +++ b/go/extractor/toolchain/toolchain_test.go @@ -21,6 +21,19 @@ func TestHasGoVersion(t *testing.T) { } } +func testGoVersionToSemVer(t *testing.T, goVersion string, expectedSemVer string) { + result := GoVersionToSemVer(goVersion) + if result != expectedSemVer { + t.Errorf("Expected GoVersionToSemVer(\"%s\") to be %s, but got %s.", goVersion, expectedSemVer, result) + } +} + +func TestGoVersionToSemVer(t *testing.T) { + testGoVersionToSemVer(t, "1.20", "v1.20.0") + testGoVersionToSemVer(t, "1.20.1", "v1.20.1") + testGoVersionToSemVer(t, "1.20rc1", "v1.20.0-rc1") +} + func testToolchainVersionToSemVer(t *testing.T, toolchainVersion string, expectedSemVer string) { result := ToolchainVersionToSemVer(toolchainVersion) if result != expectedSemVer { From 69bf334a3328027391c9da057db286b83ffb538d Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:58:15 +0100 Subject: [PATCH 07/16] Go: Use `GoVersionToSemVer` --- go/extractor/project/project.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index f282834672f8..98ad3b8ffeef 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -43,7 +43,7 @@ func (module *GoModule) RequiredGoVersion() GoVersionInfo { return VersionFound(toolchain.ToolchainVersionToSemVer(module.Module.Toolchain.Name)) } if module.Module != nil && module.Module.Go != nil { - return VersionFound(module.Module.Go.Version) + return VersionFound(toolchain.GoVersionToSemVer(module.Module.Go.Version)) } else { return tryReadGoDirective(module.Path) } @@ -96,7 +96,7 @@ func (workspace *GoWorkspace) RequiredGoVersion() GoVersionInfo { if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Toolchain != nil { return VersionFound(toolchain.ToolchainVersionToSemVer(workspace.WorkspaceFile.Toolchain.Name)) } else if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil { - return VersionFound(workspace.WorkspaceFile.Go.Version) + return VersionFound(toolchain.GoVersionToSemVer(workspace.WorkspaceFile.Go.Version)) } else if workspace.Modules != nil && len(workspace.Modules) > 0 { // Otherwise, if we have `go.work` files, find the greatest Go version in those. var greatestVersion string = "" @@ -611,7 +611,7 @@ func tryReadGoDirective(path string) GoVersionInfo { matches := versionRe.FindSubmatch(goMod) if matches != nil { if len(matches) > 1 { - return VersionFound(string(matches[1])) + return VersionFound(toolchain.GoVersionToSemVer(string(matches[1]))) } } } From 0174a16d5adcfb5b2a290681d6fb52ae931ab9de Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 13:05:12 +0100 Subject: [PATCH 08/16] Go: Add tests for `RequiredGoVersion` --- go/extractor/project/project_test.go | 81 ++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/go/extractor/project/project_test.go b/go/extractor/project/project_test.go index b7485960b5fd..62bf88c6cccc 100644 --- a/go/extractor/project/project_test.go +++ b/go/extractor/project/project_test.go @@ -28,14 +28,18 @@ func TestStartsWithAnyOf(t *testing.T) { testStartsWithAnyOf(t, filepath.Join("foo", "bar"), filepath.Join("foo", "baz"), false) } -func testHasInvalidToolchainVersion(t *testing.T, contents string) bool { - modFile, err := modfile.Parse("test.go", []byte(contents), nil) +func parseModFile(t *testing.T, contents string) *modfile.File { + modFile, err := modfile.Parse("go.mod", []byte(contents), nil) if err != nil { t.Errorf("Unable to parse %s: %s.\n", contents, err.Error()) } - return hasInvalidToolchainVersion(modFile) + return modFile +} + +func testHasInvalidToolchainVersion(t *testing.T, contents string) bool { + return hasInvalidToolchainVersion(parseModFile(t, contents)) } func TestHasInvalidToolchainVersion(t *testing.T) { @@ -62,3 +66,74 @@ func TestHasInvalidToolchainVersion(t *testing.T) { } } } + +func parseWorkFile(t *testing.T, contents string) *modfile.WorkFile { + workFile, err := modfile.ParseWork("go.work", []byte(contents), nil) + + if err != nil { + t.Errorf("Unable to parse %s: %s.\n", contents, err.Error()) + } + + return workFile +} + +func TestRequiredGoVersion(t *testing.T) { + type ModVersionPair struct { + FileContents string + ExpectedVersion string + } + + modules := []ModVersionPair{ + {"go 1.20", "v1.20.0"}, + {"go 1.21.2", "v1.21.2"}, + {"go 1.21rc1", "v1.21.0-rc1"}, + {"go 1.21rc1\ntoolchain go1.22.0", "v1.22.0"}, + {"go 1.21rc1\ntoolchain go1.22rc1", "v1.22.0-rc1"}, + } + + for _, testData := range modules { + // `go.mod` and `go.work` files have mostly the same format + modFile := parseModFile(t, testData.FileContents) + workFile := parseWorkFile(t, testData.FileContents) + mod := GoModule{ + Path: "test", // irrelevant + Module: modFile, + } + work := GoWorkspace{ + WorkspaceFile: workFile, + } + + result := mod.RequiredGoVersion() + if !result.Found { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got nothing:\n%s", + testData.ExpectedVersion, + testData.FileContents, + ) + } else if result.Version != testData.ExpectedVersion { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got %s:\n%s", + testData.ExpectedVersion, + result.Version, + testData.FileContents, + ) + } + + result = work.RequiredGoVersion() + if !result.Found { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got nothing:\n%s", + testData.ExpectedVersion, + testData.FileContents, + ) + } else if result.Version != testData.ExpectedVersion { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got %s:\n%s", + testData.ExpectedVersion, + result.Version, + testData.FileContents, + ) + } + } + +} From c6809c46f594999d9210e0db2e61174a673bd2c7 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 14:27:05 +0100 Subject: [PATCH 09/16] Go: Improve documentation for `GoVersionToSemVer` and `ToolchainVersionToSemVer` --- go/extractor/toolchain/toolchain.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/go/extractor/toolchain/toolchain.go b/go/extractor/toolchain/toolchain.go index 1d9a3da081e3..03f42b5a851f 100644 --- a/go/extractor/toolchain/toolchain.go +++ b/go/extractor/toolchain/toolchain.go @@ -104,7 +104,8 @@ func InstallVersion(workingDir string, version string) bool { return true } -// Converts a Go version to a semantic version. +// Converts a Go version to a semantic version. For example, "1.20" becomes "v1.20" and +// "1.22rc1" becomes "v1.22.0-rc1". func GoVersionToSemVer(goVersion string) string { // Go versions don't follow the SemVer format, but the only exception we normally care about // is release candidates; so this is a horrible hack to convert e.g. `1.22rc1` into `1.22-rc1` @@ -117,7 +118,8 @@ func GoVersionToSemVer(goVersion string) string { } } -// Converts a Go toolchain version of the form "go1.2.3" to a semantic version. +// Converts a Go toolchain version to a semantic version. For example, "go1.2.3" becomes "v1.2.3" +// and "go1.20rc1" becomes "v1.20.0-rc1". func ToolchainVersionToSemVer(toolchainVersion string) string { if !strings.HasPrefix(toolchainVersion, "go") { log.Fatalf("Expected Go toolchain version of the form 'go1.2.3'; got '%s'", toolchainVersion) From 42ee9afc698fd5d49bb22675a1aa5e5d93cad82d Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 14:29:40 +0100 Subject: [PATCH 10/16] Go: Improve documentation for `GoVersionInfo` --- go/extractor/project/project.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 98ad3b8ffeef..ceea637f87bd 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -60,11 +60,15 @@ type GoWorkspace struct { Extracted bool // A value indicating whether this workspace was extracted successfully } -// Represents a nullable version string. +// Represents a nullable version string. Use `VersionNotFound` and `VersionFound` +// instead of constructing values of this type directly. type GoVersionInfo struct { - // The version string, if any + // The semantic version string, such as "v1.20.0-rc1", if any. + // This is a valid semantic version if `Found` is `true` or the empty string if not. Version string - // A value indicating whether a version string was found + // A value indicating whether a version string was found. + // If this value is `true`, then `Version` is a valid semantic version. + // IF this value is `false`, then `Version` is the empty string. Found bool } From 8a9fd8c6194454281755f8078749451db8c06ec4 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 14:42:06 +0100 Subject: [PATCH 11/16] Go: Add `FormatSemVer` function --- go/extractor/project/project.go | 7 +------ go/extractor/util/util.go | 9 +++++++++ go/extractor/util/util_test.go | 21 +++++++++++++++++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index ceea637f87bd..f1789b17cfc9 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -77,13 +77,8 @@ var VersionNotFound GoVersionInfo = GoVersionInfo{"", false} // Constructs a `GoVersionInfo` for a version we found. func VersionFound(version string) GoVersionInfo { - // Add the "v" required by `semver` if there isn't one yet. - if !strings.HasPrefix(version, "v") { - version = "v" + version - } - return GoVersionInfo{ - Version: version, + Version: util.FormatSemVer(version), Found: true, } } diff --git a/go/extractor/util/util.go b/go/extractor/util/util.go index 2ae6a2b0cd2b..3eecaa5da56b 100644 --- a/go/extractor/util/util.go +++ b/go/extractor/util/util.go @@ -409,3 +409,12 @@ func getImportPathFromRepoURL(repourl string) string { path = regexp.MustCompile(`^/+|\.git$`).ReplaceAllString(path, "") return host + "/" + path } + +// Prepends "v" to `version`, if not already there. +func FormatSemVer(version string) string { + if !strings.HasPrefix(version, "v") { + version = "v" + version + } + + return version +} diff --git a/go/extractor/util/util_test.go b/go/extractor/util/util_test.go index 45d32bda3e1b..27c65b81a56a 100644 --- a/go/extractor/util/util_test.go +++ b/go/extractor/util/util_test.go @@ -20,3 +20,24 @@ func TestGetImportPathFromRepoURL(t *testing.T) { } } } + +func TestFormatSemVer(t *testing.T) { + type TestPair struct { + Input string + Expected string + } + + tests := []TestPair{ + {"1", "v1"}, + {"v1", "v1"}, + {"1.2.3", "v1.2.3"}, + {"v1.2.3", "v1.2.3"}, + } + + for _, pair := range tests { + actual := FormatSemVer(pair.Input) + if actual != pair.Expected { + t.Errorf("Expected FormatSemVer(\"%s\") to be \"%s\", but got \"%s\".", pair.Input, pair.Expected, actual) + } + } +} From dfa23197c73685ddc032c31f10fb92479b5e6288 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 14:51:48 +0100 Subject: [PATCH 12/16] Go: Add `UnformatSemVer` function --- go/extractor/util/util.go | 9 +++++++++ go/extractor/util/util_test.go | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/go/extractor/util/util.go b/go/extractor/util/util.go index 3eecaa5da56b..4975527fd0c8 100644 --- a/go/extractor/util/util.go +++ b/go/extractor/util/util.go @@ -418,3 +418,12 @@ func FormatSemVer(version string) string { return version } + +// Removes "v" from `semver`, if present. +func UnformatSemVer(semver string) string { + if strings.HasPrefix(semver, "v") { + return semver[1:] + } + + return semver +} diff --git a/go/extractor/util/util_test.go b/go/extractor/util/util_test.go index 27c65b81a56a..c00dc91a7637 100644 --- a/go/extractor/util/util_test.go +++ b/go/extractor/util/util_test.go @@ -40,4 +40,11 @@ func TestFormatSemVer(t *testing.T) { t.Errorf("Expected FormatSemVer(\"%s\") to be \"%s\", but got \"%s\".", pair.Input, pair.Expected, actual) } } + + for _, pair := range tests { + actual := UnformatSemVer(pair.Input) + if actual != pair.Expected[1:] { + t.Errorf("Expected UnformatSemVer(\"%s\") to be \"%s\", but got \"%s\".", pair.Input, pair.Expected, actual) + } + } } From a8d87ea4ea5f7eb27f56c65d9360faa4ae0205f1 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 14:58:54 +0100 Subject: [PATCH 13/16] Go: Make `getVersionToInstall` resilient to different version strings --- go/extractor/autobuilder/BUILD.bazel | 1 + go/extractor/autobuilder/build-environment.go | 29 ++++++++++--------- .../autobuilder/build-environment_test.go | 18 +++++++++++- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/go/extractor/autobuilder/BUILD.bazel b/go/extractor/autobuilder/BUILD.bazel index e40dc3a03215..702d34e63ecd 100644 --- a/go/extractor/autobuilder/BUILD.bazel +++ b/go/extractor/autobuilder/BUILD.bazel @@ -23,4 +23,5 @@ go_test( name = "autobuilder_test", srcs = ["build-environment_test.go"], embed = [":autobuilder"], + deps = ["//go/extractor/util"], ) diff --git a/go/extractor/autobuilder/build-environment.go b/go/extractor/autobuilder/build-environment.go index 3f03ff922af7..b0041d895be2 100644 --- a/go/extractor/autobuilder/build-environment.go +++ b/go/extractor/autobuilder/build-environment.go @@ -8,6 +8,7 @@ import ( "github.com/github/codeql-go/extractor/diagnostics" "github.com/github/codeql-go/extractor/project" "github.com/github/codeql-go/extractor/toolchain" + "github.com/github/codeql-go/extractor/util" "golang.org/x/mod/semver" ) @@ -30,13 +31,13 @@ func (v versionInfo) String() string { // Check if `version` is lower than `minGoVersion`. Note that for this comparison we ignore the // patch part of the version, so 1.20.1 and 1.20 are considered equal. func belowSupportedRange(version string) bool { - return semver.Compare(semver.MajorMinor("v"+version), "v"+minGoVersion) < 0 + return semver.Compare(semver.MajorMinor(util.FormatSemVer(version)), util.FormatSemVer(minGoVersion)) < 0 } // Check if `version` is higher than `maxGoVersion`. Note that for this comparison we ignore the // patch part of the version, so 1.20.1 and 1.20 are considered equal. func aboveSupportedRange(version string) bool { - return semver.Compare(semver.MajorMinor("v"+version), "v"+maxGoVersion) > 0 + return semver.Compare(semver.MajorMinor(util.FormatSemVer(version)), util.FormatSemVer(maxGoVersion)) > 0 } // Check if `version` is lower than `minGoVersion` or higher than `maxGoVersion`. Note that for @@ -113,7 +114,7 @@ func getVersionWhenGoModVersionTooHigh(v versionInfo) (msg, version string) { "). Requesting the maximum supported version of Go (" + maxGoVersion + ")." version = maxGoVersion diagnostics.EmitGoModVersionTooHighAndEnvVersionTooLow(msg) - } else if semver.Compare("v"+maxGoVersion, "v"+v.goEnvVersion) > 0 { + } else if semver.Compare(util.FormatSemVer(maxGoVersion), util.FormatSemVer(v.goEnvVersion)) > 0 { // The version in the `go.mod` file is above the supported range. The version of Go that // is installed is supported and below the maximum supported version. We install the // maximum supported version as a best effort. @@ -195,7 +196,7 @@ func getVersionWhenGoModVersionSupported(v versionInfo) (msg, version string) { v.goModVersion + ")." version = v.goModVersion diagnostics.EmitGoModVersionSupportedAndGoEnvUnsupported(msg) - } else if semver.Compare("v"+v.goModVersion, "v"+v.goEnvVersion) > 0 { + } else if semver.Compare(util.FormatSemVer(v.goModVersion), util.FormatSemVer(v.goEnvVersion)) > 0 { // The version of Go that is installed is supported. The version in the `go.mod` file is // supported and is higher than the version that is installed. We install the version from // the `go.mod` file. @@ -233,18 +234,18 @@ func getVersionWhenGoModVersionSupported(v versionInfo) (msg, version string) { // +-----------------------+-----------------------+-----------------------+-----------------------------------------------------+------------------------------------------------+ func getVersionToInstall(v versionInfo) (msg, version string) { if !v.goModVersionFound { - return getVersionWhenGoModVersionNotFound(v) - } - - if aboveSupportedRange(v.goModVersion) { - return getVersionWhenGoModVersionTooHigh(v) - } - - if belowSupportedRange(v.goModVersion) { - return getVersionWhenGoModVersionTooLow(v) + msg, version = getVersionWhenGoModVersionNotFound(v) + } else if aboveSupportedRange(v.goModVersion) { + msg, version = getVersionWhenGoModVersionTooHigh(v) + } else if belowSupportedRange(v.goModVersion) { + msg, version = getVersionWhenGoModVersionTooLow(v) + } else { + msg, version = getVersionWhenGoModVersionSupported(v) } - return getVersionWhenGoModVersionSupported(v) + // Make sure that we return a normal version string, not one starting with "v" + version = util.UnformatSemVer(version) + return } // Output some JSON to stdout specifying the version of Go to install, unless `version` is the diff --git a/go/extractor/autobuilder/build-environment_test.go b/go/extractor/autobuilder/build-environment_test.go index 994381b0b4c7..c045b131a167 100644 --- a/go/extractor/autobuilder/build-environment_test.go +++ b/go/extractor/autobuilder/build-environment_test.go @@ -1,6 +1,10 @@ package autobuilder -import "testing" +import ( + "testing" + + "github.com/github/codeql-go/extractor/util" +) func TestGetVersionToInstall(t *testing.T) { tests := map[versionInfo]string{ @@ -44,5 +48,17 @@ func TestGetVersionToInstall(t *testing.T) { if actual != expected { t.Errorf("Expected getVersionToInstall(\"%s\") to be \"%s\", but got \"%s\".", input, expected, actual) } + + if input.goEnvVersionFound { + input.goEnvVersion = util.FormatSemVer(input.goEnvVersion) + } + if input.goEnvVersionFound { + input.goModVersion = util.FormatSemVer(input.goModVersion) + } + + _, actual = getVersionToInstall(input) + if actual != expected { + t.Errorf("Expected getVersionToInstall(\"%s\") to be \"%s\", but got \"%s\".", input, expected, actual) + } } } From 97b95337b64aa25cc749c959bc33b3eda2e002dd Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 15:06:20 +0100 Subject: [PATCH 14/16] Go: Make toolchain functions resilient to different version formats --- go/extractor/toolchain/BUILD.bazel | 1 + go/extractor/toolchain/toolchain.go | 10 +++--- go/extractor/toolchain/toolchain_test.go | 42 ++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/go/extractor/toolchain/BUILD.bazel b/go/extractor/toolchain/BUILD.bazel index fde8d327e9e2..f3c0fc3d217c 100644 --- a/go/extractor/toolchain/BUILD.bazel +++ b/go/extractor/toolchain/BUILD.bazel @@ -17,4 +17,5 @@ go_test( name = "toolchain_test", srcs = ["toolchain_test.go"], embed = [":toolchain"], + deps = ["//go/extractor/util"], ) diff --git a/go/extractor/toolchain/toolchain.go b/go/extractor/toolchain/toolchain.go index 03f42b5a851f..4f45c36f461f 100644 --- a/go/extractor/toolchain/toolchain.go +++ b/go/extractor/toolchain/toolchain.go @@ -25,7 +25,7 @@ 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{}{} + goVersions[semver.Canonical(util.FormatSemVer(version))] = struct{}{} } // Returns the current Go version as returned by 'go version', e.g. go1.14.4 @@ -58,7 +58,7 @@ func GetEnvGoVersion() string { // Determines whether, to our knowledge, `version` is available on the current system. func HasGoVersion(version string) bool { - _, found := goVersions[semver.Canonical("v"+version)] + _, found := goVersions[semver.Canonical(util.FormatSemVer(version))] return found } @@ -72,7 +72,7 @@ func InstallVersion(workingDir string, version string) bool { // 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:] + toolchainArg := "GOTOOLCHAIN=go" + semver.Canonical(util.FormatSemVer(version))[1:] versionCmd := Version() versionCmd.Dir = workingDir versionCmd.Env = append(os.Environ(), toolchainArg) @@ -112,9 +112,9 @@ func GoVersionToSemVer(goVersion string) string { // which is compatible with the SemVer specification rcIndex := strings.Index(goVersion, "rc") if rcIndex != -1 { - return semver.Canonical("v"+goVersion[:rcIndex]) + "-" + goVersion[rcIndex:] + return semver.Canonical(util.FormatSemVer(goVersion[:rcIndex])) + "-" + goVersion[rcIndex:] } else { - return semver.Canonical("v" + goVersion) + return semver.Canonical(util.FormatSemVer(goVersion)) } } diff --git a/go/extractor/toolchain/toolchain_test.go b/go/extractor/toolchain/toolchain_test.go index 8f60aa9a665f..574370117825 100644 --- a/go/extractor/toolchain/toolchain_test.go +++ b/go/extractor/toolchain/toolchain_test.go @@ -1,6 +1,10 @@ package toolchain -import "testing" +import ( + "testing" + + "github.com/github/codeql-go/extractor/util" +) func TestParseGoVersion(t *testing.T) { tests := map[string]string{ @@ -16,9 +20,41 @@ 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") + versions := []string{"1.21", "v1.22", "1.22.3", "v1.21rc4"} + + // All versions should be unknown. + for _, version := range versions { + if HasGoVersion(version) { + t.Errorf("Expected HasGoVersion(\"%s\") to be false, but got true", version) + } + + if HasGoVersion(util.FormatSemVer(version)) { + t.Errorf("Expected HasGoVersion(\"%s\") to be false, but got true", util.FormatSemVer(version)) + } + + if HasGoVersion(util.UnformatSemVer(version)) { + t.Errorf("Expected HasGoVersion(\"%s\") to be false, but got true", util.UnformatSemVer(version)) + } + + // Add the version in preparation for the next part of the test. + addGoVersion(version) } + + // Now we should have all of the versions. + for _, version := range versions { + if !HasGoVersion(version) { + t.Errorf("Expected HasGoVersion(\"%s\") to be true, but got false", version) + } + + if !HasGoVersion(util.FormatSemVer(version)) { + t.Errorf("Expected HasGoVersion(\"%s\") to be true, but got false", util.FormatSemVer(version)) + } + + if !HasGoVersion(util.UnformatSemVer(version)) { + t.Errorf("Expected HasGoVersion(\"%s\") to be true, but got false", util.UnformatSemVer(version)) + } + } + } func testGoVersionToSemVer(t *testing.T, goVersion string, expectedSemVer string) { From f64e617da29564a499ce24697f79bb70b9e5866a Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 15:42:50 +0100 Subject: [PATCH 15/16] Go: Make `RequiredGoVersion` resilient to different version formats and add tests --- go/extractor/project/project.go | 2 +- go/extractor/project/project_test.go | 111 ++++++++++++++++++--------- 2 files changed, 76 insertions(+), 37 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index f1789b17cfc9..852c78ea108f 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -122,7 +122,7 @@ func RequiredGoVersion(workspaces *[]GoWorkspace) GoVersionInfo { greatestGoVersion := VersionNotFound for _, workspace := range *workspaces { goVersionInfo := workspace.RequiredGoVersion() - if goVersionInfo.Found && (!greatestGoVersion.Found || semver.Compare("v"+goVersionInfo.Version, "v"+greatestGoVersion.Version) > 0) { + if goVersionInfo.Found && (!greatestGoVersion.Found || semver.Compare(util.FormatSemVer(goVersionInfo.Version), util.FormatSemVer(greatestGoVersion.Version)) > 0) { greatestGoVersion = goVersionInfo } } diff --git a/go/extractor/project/project_test.go b/go/extractor/project/project_test.go index 62bf88c6cccc..72f1dcde0686 100644 --- a/go/extractor/project/project_test.go +++ b/go/extractor/project/project_test.go @@ -77,13 +77,34 @@ func parseWorkFile(t *testing.T, contents string) *modfile.WorkFile { return workFile } -func TestRequiredGoVersion(t *testing.T) { - type ModVersionPair struct { - FileContents string - ExpectedVersion string +type FileVersionPair struct { + FileContents string + ExpectedVersion string +} + +func checkRequiredGoVersionResult(t *testing.T, fun string, file string, testData FileVersionPair, result GoVersionInfo) { + if !result.Found { + t.Errorf( + "Expected %s to return %s for the below `%s` file, but got nothing:\n%s", + fun, + testData.ExpectedVersion, + file, + testData.FileContents, + ) + } else if result.Version != testData.ExpectedVersion { + t.Errorf( + "Expected %s to return %s for the below `%s` file, but got %s:\n%s", + fun, + testData.ExpectedVersion, + file, + result.Version, + testData.FileContents, + ) } +} - modules := []ModVersionPair{ +func TestRequiredGoVersion(t *testing.T) { + testFiles := []FileVersionPair{ {"go 1.20", "v1.20.0"}, {"go 1.21.2", "v1.21.2"}, {"go 1.21rc1", "v1.21.0-rc1"}, @@ -91,49 +112,67 @@ func TestRequiredGoVersion(t *testing.T) { {"go 1.21rc1\ntoolchain go1.22rc1", "v1.22.0-rc1"}, } - for _, testData := range modules { + var modules []*GoModule = []*GoModule{} + + for _, testData := range testFiles { // `go.mod` and `go.work` files have mostly the same format modFile := parseModFile(t, testData.FileContents) workFile := parseWorkFile(t, testData.FileContents) - mod := GoModule{ + mod := &GoModule{ Path: "test", // irrelevant Module: modFile, } - work := GoWorkspace{ + work := &GoWorkspace{ WorkspaceFile: workFile, } result := mod.RequiredGoVersion() - if !result.Found { - t.Errorf( - "Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got nothing:\n%s", - testData.ExpectedVersion, - testData.FileContents, - ) - } else if result.Version != testData.ExpectedVersion { - t.Errorf( - "Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got %s:\n%s", - testData.ExpectedVersion, - result.Version, - testData.FileContents, - ) - } + checkRequiredGoVersionResult(t, "mod.RequiredGoVersion()", "go.mod", testData, result) result = work.RequiredGoVersion() - if !result.Found { - t.Errorf( - "Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got nothing:\n%s", - testData.ExpectedVersion, - testData.FileContents, - ) - } else if result.Version != testData.ExpectedVersion { - t.Errorf( - "Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got %s:\n%s", - testData.ExpectedVersion, - result.Version, - testData.FileContents, - ) - } + checkRequiredGoVersionResult(t, "work.RequiredGoVersion()", "go.work", testData, result) + + modules = append(modules, mod) + } + + // Create a test workspace with all the modules in one workspace. + workspace := GoWorkspace{ + Modules: modules, + } + workspaceVer := "v1.22.0" + + result := RequiredGoVersion(&[]GoWorkspace{workspace}) + if !result.Found { + t.Errorf( + "Expected RequiredGoVersion to return %s, but got nothing.", + workspaceVer, + ) + } else if result.Version != workspaceVer { + t.Errorf( + "Expected RequiredGoVersion to return %s, but got %s.", + workspaceVer, + result.Version, + ) } + // Create test workspaces for each module. + workspaces := []GoWorkspace{} + + for _, mod := range modules { + workspaces = append(workspaces, GoWorkspace{Modules: []*GoModule{mod}}) + } + + result = RequiredGoVersion(&workspaces) + if !result.Found { + t.Errorf( + "Expected RequiredGoVersion to return %s, but got nothing.", + workspaceVer, + ) + } else if result.Version != workspaceVer { + t.Errorf( + "Expected RequiredGoVersion to return %s, but got %s.", + workspaceVer, + result.Version, + ) + } } From 12bc9f84bd4aabc91b170232e1748956871e928b Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 15:46:11 +0100 Subject: [PATCH 16/16] Go: Remove extra "v"s in Autobuilder --- go/extractor/cli/go-autobuilder/go-autobuilder.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/extractor/cli/go-autobuilder/go-autobuilder.go b/go/extractor/cli/go-autobuilder/go-autobuilder.go index 2e9731c989b8..1185ce61fa62 100644 --- a/go/extractor/cli/go-autobuilder/go-autobuilder.go +++ b/go/extractor/cli/go-autobuilder/go-autobuilder.go @@ -537,12 +537,12 @@ func installDependenciesAndBuild() { // This diagnostic is not required if the system Go version is 1.21 or greater, since the // Go tooling should install required Go versions as needed. - if semver.Compare(toolchain.GetEnvGoSemVer(), "v1.21.0") < 0 && greatestGoVersion.Found && semver.Compare("v"+greatestGoVersion.Version, toolchain.GetEnvGoSemVer()) > 0 { - diagnostics.EmitNewerGoVersionNeeded(toolchain.GetEnvGoSemVer(), "v"+greatestGoVersion.Version) + if semver.Compare(toolchain.GetEnvGoSemVer(), "v1.21.0") < 0 && greatestGoVersion.Found && semver.Compare(greatestGoVersion.Version, toolchain.GetEnvGoSemVer()) > 0 { + diagnostics.EmitNewerGoVersionNeeded(toolchain.GetEnvGoSemVer(), greatestGoVersion.Version) if val, _ := os.LookupEnv("GITHUB_ACTIONS"); val == "true" { log.Printf( "A go.mod file requires version %s of Go, but version %s is installed. Consider adding an actions/setup-go step to your workflow.\n", - "v"+greatestGoVersion.Version, + greatestGoVersion.Version, toolchain.GetEnvGoSemVer()) } }