From 02e55058585de904ace02ebbb936dcf44c879c6d Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 23 Feb 2022 11:55:08 -0500 Subject: [PATCH] [release-branch.go1.16] cmd/go: avoid +incompatible major versions if a go.mod file exists in a subdirectory for that version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous versions of the 'go' command would reject a pseudo-version passed to 'go get' if that pseudo-version had a mismatched major version and lacked a "+incompatible" suffix. However, they would erroneously accept a version *with* a "+incompatible" suffix even if the repo contained a vN/go.mod file for the same major version, and would generate a "+incompatible" pseudo-version or version if the user requested a tag, branch, or commit hash. This change uniformly rejects "vN.…" without "+incompatible", and also avoids resolving to "vN.…+incompatible", when vN/go.mod exists. To maintain compatibility with existing go.mod files, it still accepts "vN.…+incompatible" if the version is requested explicitly as such and the repo root lacks a go.mod file. Fixes #51331 Updates #51324 Updates #36438 Change-Id: I2b16150c73fc2abe4d0a1cd34cb1600635db7139 Reviewed-on: https://go-review.googlesource.com/c/go/+/387675 Trust: Bryan Mills Reviewed-by: Michael Matloob (cherry picked from commit 5a9fc946b42cc987db41eabcfcbaffd2fb310d94) Reviewed-on: https://go-review.googlesource.com/c/go/+/387923 Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov --- src/cmd/go/internal/modfetch/coderepo.go | 53 ++++++++++++++----- src/cmd/go/internal/modfetch/coderepo_test.go | 48 +++++++++++++++++ 2 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/cmd/go/internal/modfetch/coderepo.go b/src/cmd/go/internal/modfetch/coderepo.go index db7496faf6515..ef0acfe965af4 100644 --- a/src/cmd/go/internal/modfetch/coderepo.go +++ b/src/cmd/go/internal/modfetch/coderepo.go @@ -305,17 +305,46 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e // // (If the version is +incompatible, then the go.mod file must not exist: // +incompatible is not an ongoing opt-out from semantic import versioning.) - var canUseIncompatible func() bool - canUseIncompatible = func() bool { - var ok bool - if r.codeDir == "" && r.pathMajor == "" { + incompatibleOk := map[string]bool{} + canUseIncompatible := func(v string) bool { + if r.codeDir != "" || r.pathMajor != "" { + // A non-empty codeDir indicates a module within a subdirectory, + // which necessarily has a go.mod file indicating the module boundary. + // A non-empty pathMajor indicates a module path with a major-version + // suffix, which must match. + return false + } + + ok, seen := incompatibleOk[""] + if !seen { _, errGoMod := r.code.ReadFile(info.Name, "go.mod", codehost.MaxGoMod) - if errGoMod != nil { - ok = true + ok = (errGoMod != nil) + incompatibleOk[""] = ok + } + if !ok { + // A go.mod file exists at the repo root. + return false + } + + // Per https://go.dev/issue/51324, previous versions of the 'go' command + // didn't always check for go.mod files in subdirectories, so if the user + // requests a +incompatible version explicitly, we should continue to allow + // it. Otherwise, if vN/go.mod exists, expect that release tags for that + // major version are intended for the vN module. + if v != "" && !strings.HasSuffix(statVers, "+incompatible") { + major := semver.Major(v) + ok, seen = incompatibleOk[major] + if !seen { + _, errGoModSub := r.code.ReadFile(info.Name, path.Join(major, "go.mod"), codehost.MaxGoMod) + ok = (errGoModSub != nil) + incompatibleOk[major] = ok + } + if !ok { + return false } } - canUseIncompatible = func() bool { return ok } - return ok + + return true } // checkCanonical verifies that the canonical version v is compatible with the @@ -367,7 +396,7 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e base := strings.TrimSuffix(v, "+incompatible") var errIncompatible error if !module.MatchPathMajor(base, r.pathMajor) { - if canUseIncompatible() { + if canUseIncompatible(base) { v = base + "+incompatible" } else { if r.pathMajor != "" { @@ -495,7 +524,7 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e // Save the highest non-retracted canonical tag for the revision. // If we don't find a better match, we'll use it as the canonical version. if tagIsCanonical && semver.Compare(highestCanonical, v) < 0 && !isRetracted(v) { - if module.MatchPathMajor(v, r.pathMajor) || canUseIncompatible() { + if module.MatchPathMajor(v, r.pathMajor) || canUseIncompatible(v) { highestCanonical = v } } @@ -513,12 +542,12 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e // retracted versions. allowedMajor := func(major string) func(v string) bool { return func(v string) bool { - return (major == "" || semver.Major(v) == major) && !isRetracted(v) + return ((major == "" && canUseIncompatible(v)) || semver.Major(v) == major) && !isRetracted(v) } } if pseudoBase == "" { var tag string - if r.pseudoMajor != "" || canUseIncompatible() { + if r.pseudoMajor != "" || canUseIncompatible("") { tag, _ = r.code.RecentTag(info.Name, tagPrefix, allowedMajor(r.pseudoMajor)) } else { // Allow either v1 or v0, but not incompatible higher versions. diff --git a/src/cmd/go/internal/modfetch/coderepo_test.go b/src/cmd/go/internal/modfetch/coderepo_test.go index d98ea87da2c3d..bb9268adb87c4 100644 --- a/src/cmd/go/internal/modfetch/coderepo_test.go +++ b/src/cmd/go/internal/modfetch/coderepo_test.go @@ -458,6 +458,54 @@ var codeRepoTests = []codeRepoTest{ rev: "v3.0.0-devel", err: `resolves to version v0.1.1-0.20220203155313-d59622f6e4d7 (v3.0.0-devel is not a tag)`, }, + + // If v2/go.mod exists, then we should prefer to match the "v2" + // pseudo-versions to the nested module, and resolve the module in the parent + // directory to only compatible versions. + // + // However (https://go.dev/issue/51324), previous versions of the 'go' command + // didn't always do so, so if the user explicitly requests a +incompatible + // version (as would be present in an existing go.mod file), we should + // continue to allow it. + { + vcs: "git", + path: "vcs-test.golang.org/git/v2sub.git", + rev: "80beb17a1603", + version: "v0.0.0-20220222205507-80beb17a1603", + name: "80beb17a16036f17a5aedd1bb5bd6d407b3c6dc5", + short: "80beb17a1603", + time: time.Date(2022, 2, 22, 20, 55, 7, 0, time.UTC), + }, + { + vcs: "git", + path: "vcs-test.golang.org/git/v2sub.git", + rev: "v2.0.0", + err: `module contains a go.mod file, so module path must match major version ("vcs-test.golang.org/git/v2sub.git/v2")`, + }, + { + vcs: "git", + path: "vcs-test.golang.org/git/v2sub.git", + rev: "v2.0.1-0.20220222205507-80beb17a1603", + err: `module contains a go.mod file, so module path must match major version ("vcs-test.golang.org/git/v2sub.git/v2")`, + }, + { + vcs: "git", + path: "vcs-test.golang.org/git/v2sub.git", + rev: "v2.0.0+incompatible", + version: "v2.0.0+incompatible", + name: "5fcd3eaeeb391d399f562fd45a50dac9fc34ae8b", + short: "5fcd3eaeeb39", + time: time.Date(2022, 2, 22, 20, 53, 33, 0, time.UTC), + }, + { + vcs: "git", + path: "vcs-test.golang.org/git/v2sub.git", + rev: "v2.0.1-0.20220222205507-80beb17a1603+incompatible", + version: "v2.0.1-0.20220222205507-80beb17a1603+incompatible", + name: "80beb17a16036f17a5aedd1bb5bd6d407b3c6dc5", + short: "80beb17a1603", + time: time.Date(2022, 2, 22, 20, 55, 7, 0, time.UTC), + }, } func TestCodeRepo(t *testing.T) {