[release-branch.go1.17] cmd/go: avoid +incompatible major versions if a go.mod file...

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 #51332
Updates #51324
Updates #36438

Change-Id: I2b16150c73fc2abe4d0a1cd34cb1600635db7139
Reviewed-on: https://go-review.googlesource.com/c/go/+/387675
Trust: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
(cherry picked from commit 5a9fc946b42cc987db41eabcfcbaffd2fb310d94)
Reviewed-on: https://go-review.googlesource.com/c/go/+/387922
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/cmd/go/internal/modfetch/coderepo.go b/src/cmd/go/internal/modfetch/coderepo.go
index 0ee707025d044d4f87ae6fd0ca1b25787fb8d467..cd04f95a7fecbc51420a06b667517d3939680dd9 100644 (file)
--- 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 d98ea87da2c3dfe36a7448eee0ca3cc087784a75..bb9268adb87c4cf1862056b30fd4f4659cacc1d2 100644 (file)
--- 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) {