From 5c76382762cfc34b7a7678668460f127fec4a35b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 13 Mar 2020 16:46:51 -0400 Subject: [PATCH] cmd/go/internal/modload: add a "v" prefix to the indexed go version This allows semver-based comparisons of the version without additional allocations. Also comment on the reason for the loops that iterate over modFile instead. (I was reading the vendor code in order to add the lazy-loading version check, and this section was a bit unclear to me.) For #36460 Change-Id: I11559d81ffb4eba0e4e10e6fa3c01990b11f9180 Reviewed-on: https://go-review.googlesource.com/c/go/+/240622 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod Reviewed-by: Michael Matloob --- src/cmd/go/internal/modload/init.go | 6 +++--- src/cmd/go/internal/modload/modfile.go | 14 ++++++++------ src/cmd/go/internal/modload/vendor.go | 4 +++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index 93027c44c4..71c7b158b8 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -483,15 +483,15 @@ func setDefaultBuildMod() { if fi, err := os.Stat(filepath.Join(modRoot, "vendor")); err == nil && fi.IsDir() { modGo := "unspecified" - if index.goVersion != "" { - if semver.Compare("v"+index.goVersion, "v1.14") >= 0 { + if index.goVersionV != "" { + if semver.Compare(index.goVersionV, "v1.14") >= 0 { // The Go version is at least 1.14, and a vendor directory exists. // Set -mod=vendor by default. cfg.BuildMod = "vendor" cfg.BuildModReason = "Go version in go.mod is at least 1.14 and vendor directory exists." return } else { - modGo = index.goVersion + modGo = index.goVersionV[1:] } } diff --git a/src/cmd/go/internal/modload/modfile.go b/src/cmd/go/internal/modload/modfile.go index 9f4ec5a49f..9a166cae54 100644 --- a/src/cmd/go/internal/modload/modfile.go +++ b/src/cmd/go/internal/modload/modfile.go @@ -20,7 +20,7 @@ type modFileIndex struct { data []byte dataNeedsFix bool // true if fixVersion applied a change while parsing data module module.Version - goVersion string + goVersionV string // GoVersion with "v" prefix require map[module.Version]requireMeta replace map[module.Version]module.Version exclude map[module.Version]bool @@ -66,9 +66,11 @@ func indexModFile(data []byte, modFile *modfile.File, needsFix bool) *modFileInd i.module = modFile.Module.Mod } - i.goVersion = "" + i.goVersionV = "" if modFile.Go != nil { - i.goVersion = modFile.Go.Version + // We're going to use the semver package to compare Go versions, so go ahead + // and add the "v" prefix it expects once instead of every time. + i.goVersionV = "v" + modFile.Go.Version } i.require = make(map[module.Version]requireMeta, len(modFile.Require)) @@ -114,11 +116,11 @@ func (i *modFileIndex) modFileIsDirty(modFile *modfile.File) bool { } if modFile.Go == nil { - if i.goVersion != "" { + if i.goVersionV != "" { return true } - } else if modFile.Go.Version != i.goVersion { - if i.goVersion == "" && cfg.BuildMod == "readonly" { + } else if "v"+modFile.Go.Version != i.goVersionV { + if i.goVersionV == "" && cfg.BuildMod == "readonly" { // go.mod files did not always require a 'go' version, so do not error out // if one is missing — we may be inside an older module in the module // cache, and should bias toward providing useful behavior. diff --git a/src/cmd/go/internal/modload/vendor.go b/src/cmd/go/internal/modload/vendor.go index 71f68efbcc..9f34b829fc 100644 --- a/src/cmd/go/internal/modload/vendor.go +++ b/src/cmd/go/internal/modload/vendor.go @@ -133,7 +133,7 @@ func checkVendorConsistency() { readVendorList() pre114 := false - if modFile.Go == nil || semver.Compare("v"+modFile.Go.Version, "v1.14") < 0 { + if semver.Compare(index.goVersionV, "v1.14") < 0 { // Go versions before 1.14 did not include enough information in // vendor/modules.txt to check for consistency. // If we know that we're on an earlier version, relax the consistency check. @@ -150,6 +150,8 @@ func checkVendorConsistency() { } } + // Iterate over the Require directives in their original (not indexed) order + // so that the errors match the original file. for _, r := range modFile.Require { if !vendorMeta[r.Mod].Explicit { if pre114 { -- 2.50.0