]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go/internal/modfetch: simplify handling of weird version tags
authorBryan C. Mills <bcmills@google.com>
Thu, 24 Feb 2022 21:12:34 +0000 (16:12 -0500)
committerGopher Robot <gobot@golang.org>
Tue, 10 May 2022 18:25:37 +0000 (18:25 +0000)
This fixes an obscure bug in 'go list -versions' if the repo contains
a tag with an explicit "+incompatible" suffix. However, I've never
seen such a repo in the wild; mostly it's an attempt to wrap my brain
around the code and simplify things a bit for the future.

Updates #51324
Updates #51312

Change-Id: I1b078b5db36470cf61aaa85b5244c99b5ee2c842
Reviewed-on: https://go-review.googlesource.com/c/go/+/387917
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
src/cmd/go/internal/modfetch/codehost/codehost.go
src/cmd/go/internal/modfetch/codehost/git.go
src/cmd/go/internal/modfetch/coderepo.go
src/cmd/go/internal/modfetch/coderepo_test.go
src/cmd/go/testdata/script/mod_list_odd_tags.txt [new file with mode: 0644]

index 31dc811752b83e5fc24d5c41464435cfba3d1962..e08a84b32c67212477b28254f2e7d11960345e17 100644 (file)
@@ -65,7 +65,7 @@ type Repo interface {
 
        // RecentTag returns the most recent tag on rev or one of its predecessors
        // with the given prefix. allowed may be used to filter out unwanted versions.
-       RecentTag(rev, prefix string, allowed func(string) bool) (tag string, err error)
+       RecentTag(rev, prefix string, allowed func(tag string) bool) (tag string, err error)
 
        // DescendsFrom reports whether rev or any of its ancestors has the given tag.
        //
index 9c8fd42833cc7e96122987a1836a073d9c8ccaec..853d43bc5b2ea4bdfbaeabac96d8f3cae64a6644 100644 (file)
@@ -523,7 +523,7 @@ func (r *gitRepo) ReadFile(rev, file string, maxSize int64) ([]byte, error) {
        return out, nil
 }
 
-func (r *gitRepo) RecentTag(rev, prefix string, allowed func(string) bool) (tag string, err error) {
+func (r *gitRepo) RecentTag(rev, prefix string, allowed func(tag string) bool) (tag string, err error) {
        info, err := r.Stat(rev)
        if err != nil {
                return "", err
@@ -553,15 +553,11 @@ func (r *gitRepo) RecentTag(rev, prefix string, allowed func(string) bool) (tag
                        if !strings.HasPrefix(line, prefix) {
                                continue
                        }
-
-                       semtag := line[len(prefix):]
-                       // Consider only tags that are valid and complete (not just major.minor prefixes).
-                       // NOTE: Do not replace the call to semver.Compare with semver.Max.
-                       // We want to return the actual tag, not a canonicalized version of it,
-                       // and semver.Max currently canonicalizes (see golang.org/issue/32700).
-                       if c := semver.Canonical(semtag); c == "" || !strings.HasPrefix(semtag, c) || !allowed(semtag) {
+                       if !allowed(line) {
                                continue
                        }
+
+                       semtag := line[len(prefix):]
                        if semver.Compare(semtag, highest) > 0 {
                                highest = semtag
                        }
index dfaf16def61f278a487239110237cbefe1bebb72..ff1cea1d94caf91fe3c72d6637265ec4557bc086 100644 (file)
@@ -159,7 +159,18 @@ func (r *codeRepo) Versions(prefix string) ([]string, error) {
                if r.codeDir != "" {
                        v = v[len(r.codeDir)+1:]
                }
-               if v == "" || v != module.CanonicalVersion(v) || module.IsPseudoVersion(v) {
+               if v == "" || v != semver.Canonical(v) {
+                       // Ignore non-canonical tags: Stat rewrites those to canonical
+                       // pseudo-versions. Note that we compare against semver.Canonical here
+                       // instead of module.CanonicalVersion: revToRev strips "+incompatible"
+                       // suffixes before looking up tags, so a tag like "v2.0.0+incompatible"
+                       // would not resolve at all. (The Go version string "v2.0.0+incompatible"
+                       // refers to the "v2.0.0" version tag, which we handle below.)
+                       continue
+               }
+               if module.IsPseudoVersion(v) {
+                       // Ignore tags that look like pseudo-versions: Stat rewrites those
+                       // unambiguously to the underlying commit, and tagToVersion drops them.
                        continue
                }
 
@@ -540,23 +551,21 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e
        // major version and +incompatible constraints. Use that version as the
        // pseudo-version base so that the pseudo-version sorts higher. Ignore
        // retracted versions.
-       allowedMajor := func(major string) func(v string) bool {
-               return func(v string) bool {
-                       return ((major == "" && canUseIncompatible(v)) || semver.Major(v) == major) && !isRetracted(v)
+       tagAllowed := func(tag string) bool {
+               v, _ := tagToVersion(tag)
+               if v == "" {
+                       return false
                }
+               if !module.MatchPathMajor(v, r.pathMajor) && !canUseIncompatible(v) {
+                       return false
+               }
+               return !isRetracted(v)
        }
        if pseudoBase == "" {
-               var tag string
-               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.
-                       tag, _ = r.code.RecentTag(info.Name, tagPrefix, allowedMajor("v1"))
-                       if tag == "" {
-                               tag, _ = r.code.RecentTag(info.Name, tagPrefix, allowedMajor("v0"))
-                       }
+               tag, _ := r.code.RecentTag(info.Name, tagPrefix, tagAllowed)
+               if tag != "" {
+                       pseudoBase, _ = tagToVersion(tag)
                }
-               pseudoBase, _ = tagToVersion(tag)
        }
 
        return checkCanonical(module.PseudoVersion(r.pseudoMajor, pseudoBase, info.Time, info.Short))
@@ -920,6 +929,10 @@ func (r *codeRepo) retractedVersions() (func(string) bool, error) {
 
        for i, v := range versions {
                if strings.HasSuffix(v, "+incompatible") {
+                       // We're looking for the latest release tag that may list retractions in a
+                       // go.mod file. +incompatible versions necessarily do not, and they start
+                       // at major version 2 — which is higher than any version that could
+                       // validly contain a go.mod file.
                        versions = versions[:i]
                        break
                }
index bb9268adb87c4cf1862056b30fd4f4659cacc1d2..8d0eb00544ad5c8c6f2bae696b6d28369ae6163b 100644 (file)
@@ -506,6 +506,69 @@ var codeRepoTests = []codeRepoTest{
                short:   "80beb17a1603",
                time:    time.Date(2022, 2, 22, 20, 55, 7, 0, time.UTC),
        },
+
+       // A version tag with explicit build metadata is valid but not canonical.
+       // It should resolve to a pseudo-version based on the same tag.
+       {
+               vcs:     "git",
+               path:    "vcs-test.golang.org/git/odd-tags.git",
+               rev:     "v0.1.0+build-metadata",
+               version: "v0.1.1-0.20220223184835-9d863d525bbf",
+               name:    "9d863d525bbfcc8eda09364738c4032393711a56",
+               short:   "9d863d525bbf",
+               time:    time.Date(2022, 2, 23, 18, 48, 35, 0, time.UTC),
+       },
+       {
+               vcs:     "git",
+               path:    "vcs-test.golang.org/git/odd-tags.git",
+               rev:     "9d863d525bbf",
+               version: "v0.1.1-0.20220223184835-9d863d525bbf",
+               name:    "9d863d525bbfcc8eda09364738c4032393711a56",
+               short:   "9d863d525bbf",
+               time:    time.Date(2022, 2, 23, 18, 48, 35, 0, time.UTC),
+       },
+       {
+               vcs:     "git",
+               path:    "vcs-test.golang.org/git/odd-tags.git",
+               rev:     "latest",
+               version: "v0.1.1-0.20220223184835-9d863d525bbf",
+               name:    "9d863d525bbfcc8eda09364738c4032393711a56",
+               short:   "9d863d525bbf",
+               time:    time.Date(2022, 2, 23, 18, 48, 35, 0, time.UTC),
+       },
+
+       // A version tag with an erroneous "+incompatible" suffix should resolve using
+       // only the prefix before the "+incompatible" suffix, not the "+incompatible"
+       // tag itself. (Otherwise, we would potentially have two different commits
+       // both named "v2.0.0+incompatible".) However, the tag is still valid semver
+       // and can still be used as the base for an unambiguous pseudo-version.
+       {
+               vcs:  "git",
+               path: "vcs-test.golang.org/git/odd-tags.git",
+               rev:  "v2.0.0+incompatible",
+               err:  `unknown revision v2.0.0`,
+       },
+       {
+               vcs:     "git",
+               path:    "vcs-test.golang.org/git/odd-tags.git",
+               rev:     "12d19af20458",
+               version: "v2.0.1-0.20220223184802-12d19af20458+incompatible",
+               name:    "12d19af204585b0db3d2a876ceddf5b9323f5a4a",
+               short:   "12d19af20458",
+               time:    time.Date(2022, 2, 23, 18, 48, 2, 0, time.UTC),
+       },
+
+       // Similarly, a pseudo-version must resolve to the named commit, even if a tag
+       // matching that pseudo-version is present on a *different* commit.
+       {
+               vcs:     "git",
+               path:    "vcs-test.golang.org/git/odd-tags.git",
+               rev:     "v3.0.0-20220223184802-12d19af20458",
+               version: "v3.0.0-20220223184802-12d19af20458+incompatible",
+               name:    "12d19af204585b0db3d2a876ceddf5b9323f5a4a",
+               short:   "12d19af20458",
+               time:    time.Date(2022, 2, 23, 18, 48, 2, 0, time.UTC),
+       },
 }
 
 func TestCodeRepo(t *testing.T) {
@@ -730,6 +793,11 @@ var codeRepoVersionsTests = []struct {
                path:     "gopkg.in/natefinch/lumberjack.v2",
                versions: []string{"v2.0.0"},
        },
+       {
+               vcs:      "git",
+               path:     "vcs-test.golang.org/git/odd-tags.git",
+               versions: nil,
+       },
 }
 
 func TestCodeRepoVersions(t *testing.T) {
diff --git a/src/cmd/go/testdata/script/mod_list_odd_tags.txt b/src/cmd/go/testdata/script/mod_list_odd_tags.txt
new file mode 100644 (file)
index 0000000..c1f40cd
--- /dev/null
@@ -0,0 +1,13 @@
+[short] skip
+[!exec:git] skip
+[!net] skip
+
+env GOPROXY=direct
+
+go list -m vcs-test.golang.org/git/odd-tags.git@latest
+stdout -count=1 '^.'
+stdout '^vcs-test.golang.org/git/odd-tags.git v0.1.1-0.20220223184835-9d863d525bbf$'
+
+go list -m -versions vcs-test.golang.org/git/odd-tags.git
+stdout -count=1 '^.'
+stdout '^vcs-test.golang.org/git/odd-tags.git$'  # No versions listed — the odd tags are filtered out.