]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: ignore retracted versions when converting revisions to versions
authorJay Conrod <jayconrod@google.com>
Fri, 9 Oct 2020 19:16:13 +0000 (15:16 -0400)
committerJay Conrod <jayconrod@google.com>
Fri, 9 Oct 2020 21:47:07 +0000 (21:47 +0000)
When a module author retracts a version, the go command should act as
if it doesn't exist unless it's specifically requested.

When converting a revision to a version, we should ignore tags for
retracted versions. For example, if the tag v1.0.0 is retracted, and
branch B points to the same revision, we should convert B to a
pseudo-version, not v1.0.0. Similarly, if B points to a commit after
v1.0.0, we should not use v1.0.0 as the base; we can use an earlier
non-retracted tag or no base.

Fixes #41700

Change-Id: Ia596b05b0780e5acfe6616a04e94d24bd342fbae
Reviewed-on: https://go-review.googlesource.com/c/go/+/261079
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jay Conrod <jayconrod@google.com>

src/cmd/go/internal/modfetch/codehost/codehost.go
src/cmd/go/internal/modfetch/codehost/git.go
src/cmd/go/internal/modfetch/codehost/vcs.go
src/cmd/go/internal/modfetch/coderepo.go
src/cmd/go/testdata/script/mod_retract_pseudo_base.txt [new file with mode: 0644]

index d85eddf767be100f3a4d7da79140466464d88fe8..df4cfdab1af6566131c47bd2d36538d75f4e1cbc 100644 (file)
@@ -79,9 +79,8 @@ type Repo interface {
        ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, err error)
 
        // RecentTag returns the most recent tag on rev or one of its predecessors
-       // with the given prefix and major version.
-       // An empty major string matches any major version.
-       RecentTag(rev, prefix, major string) (tag string, err error)
+       // 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)
 
        // DescendsFrom reports whether rev or any of its ancestors has the given tag.
        //
index 31921324a7d10e3fdddc32ec5fcaacf379490d7f..5a35829c98c046d220414755f3d435b3bf73ee3a 100644 (file)
@@ -644,7 +644,7 @@ func (r *gitRepo) readFileRevs(tags []string, file string, fileMap map[string]*F
        return missing, nil
 }
 
-func (r *gitRepo) RecentTag(rev, prefix, major string) (tag string, err error) {
+func (r *gitRepo) RecentTag(rev, prefix string, allowed func(string) bool) (tag string, err error) {
        info, err := r.Stat(rev)
        if err != nil {
                return "", err
@@ -680,7 +680,10 @@ func (r *gitRepo) RecentTag(rev, prefix, major string) (tag string, err error) {
                        // 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) && (major == "" || semver.Major(c) == major) && semver.Compare(semtag, highest) > 0 {
+                       if c := semver.Canonical(semtag); c == "" || !strings.HasPrefix(semtag, c) || !allowed(semtag) {
+                               continue
+                       }
+                       if semver.Compare(semtag, highest) > 0 {
                                highest = semtag
                        }
                }
index 7284557f4ba3377b36ed87ccf8315cf6a4544ae3..6278cb21e13aced372387e56d50ae4a957f11b17 100644 (file)
@@ -395,7 +395,7 @@ func (r *vcsRepo) ReadFileRevs(revs []string, file string, maxSize int64) (map[s
        return nil, vcsErrorf("ReadFileRevs not implemented")
 }
 
-func (r *vcsRepo) RecentTag(rev, prefix, major string) (tag string, err error) {
+func (r *vcsRepo) RecentTag(rev, prefix string, allowed func(string) bool) (tag string, err error) {
        // We don't technically need to lock here since we're returning an error
        // uncondititonally, but doing so anyway will help to avoid baking in
        // lock-inversion bugs.
index d043903336c0514de45ed23fedf09c19ee2c10e3..d99a31d36051ced712a476ad181253ab3f07d251 100644 (file)
@@ -419,9 +419,14 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e
                tagPrefix = r.codeDir + "/"
        }
 
+       isRetracted, err := r.retractedVersions()
+       if err != nil {
+               isRetracted = func(string) bool { return false }
+       }
+
        // tagToVersion returns the version obtained by trimming tagPrefix from tag.
-       // If the tag is invalid or a pseudo-version, tagToVersion returns an empty
-       // version.
+       // If the tag is invalid, retracted, or a pseudo-version, tagToVersion returns
+       // an empty version.
        tagToVersion := func(tag string) (v string, tagIsCanonical bool) {
                if !strings.HasPrefix(tag, tagPrefix) {
                        return "", false
@@ -436,6 +441,9 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e
                if v == "" || !strings.HasPrefix(trimmed, v) {
                        return "", false // Invalid or incomplete version (just vX or vX.Y).
                }
+               if isRetracted(v) {
+                       return "", false
+               }
                if v == trimmed {
                        tagIsCanonical = true
                }
@@ -500,15 +508,24 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e
                return checkGoMod()
        }
 
+       // Find the highest tagged version in the revision's history, subject to
+       // 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 == "" || semver.Major(v) == major) && !isRetracted(v)
+               }
+       }
        if pseudoBase == "" {
                var tag string
                if r.pseudoMajor != "" || canUseIncompatible() {
-                       tag, _ = r.code.RecentTag(info.Name, tagPrefix, r.pseudoMajor)
+                       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, "v1")
+                       tag, _ = r.code.RecentTag(info.Name, tagPrefix, allowedMajor("v1"))
                        if tag == "" {
-                               tag, _ = r.code.RecentTag(info.Name, tagPrefix, "v0")
+                               tag, _ = r.code.RecentTag(info.Name, tagPrefix, allowedMajor("v0"))
                        }
                }
                pseudoBase, _ = tagToVersion(tag) // empty if the tag is invalid
@@ -869,6 +886,57 @@ func (r *codeRepo) modPrefix(rev string) string {
        return r.modPath + "@" + rev
 }
 
+func (r *codeRepo) retractedVersions() (func(string) bool, error) {
+       versions, err := r.Versions("")
+       if err != nil {
+               return nil, err
+       }
+
+       for i, v := range versions {
+               if strings.HasSuffix(v, "+incompatible") {
+                       versions = versions[:i]
+                       break
+               }
+       }
+       if len(versions) == 0 {
+               return func(string) bool { return false }, nil
+       }
+
+       var highest string
+       for i := len(versions) - 1; i >= 0; i-- {
+               v := versions[i]
+               if semver.Prerelease(v) == "" {
+                       highest = v
+                       break
+               }
+       }
+       if highest == "" {
+               highest = versions[len(versions)-1]
+       }
+
+       data, err := r.GoMod(highest)
+       if err != nil {
+               return nil, err
+       }
+       f, err := modfile.ParseLax("go.mod", data, nil)
+       if err != nil {
+               return nil, err
+       }
+       retractions := make([]modfile.VersionInterval, len(f.Retract))
+       for _, r := range f.Retract {
+               retractions = append(retractions, r.VersionInterval)
+       }
+
+       return func(v string) bool {
+               for _, r := range retractions {
+                       if semver.Compare(r.Low, v) <= 0 && semver.Compare(v, r.High) <= 0 {
+                               return true
+                       }
+               }
+               return false
+       }, nil
+}
+
 func (r *codeRepo) Zip(dst io.Writer, version string) error {
        if version != module.CanonicalVersion(version) {
                return fmt.Errorf("version %s is not canonical", version)
diff --git a/src/cmd/go/testdata/script/mod_retract_pseudo_base.txt b/src/cmd/go/testdata/script/mod_retract_pseudo_base.txt
new file mode 100644 (file)
index 0000000..93609f3
--- /dev/null
@@ -0,0 +1,62 @@
+# When converting a commit to a pseudo-version, don't use a retracted version
+# as the base.
+# Verifies golang.org/issue/41700.
+
+[!net] skip
+[!exec:git] skip
+env GOPROXY=direct
+env GOSUMDB=off
+go mod init m
+
+# Control: check that v1.0.0 is the only version and is retracted.
+go list -m -versions vcs-test.golang.org/git/retract-pseudo.git
+stdout '^vcs-test.golang.org/git/retract-pseudo.git$'
+go list -m -versions -retracted vcs-test.golang.org/git/retract-pseudo.git
+stdout '^vcs-test.golang.org/git/retract-pseudo.git v1.0.0$'
+
+# 713affd19d7b is a commit after v1.0.0. Don't use v1.0.0 as the base.
+go list -m vcs-test.golang.org/git/retract-pseudo.git@713affd19d7b
+stdout '^vcs-test.golang.org/git/retract-pseudo.git v0.0.0-20201009173747-713affd19d7b$'
+
+# 64c061ed4371 is the commit v1.0.0 refers to. Don't convert to v1.0.0.
+go list -m vcs-test.golang.org/git/retract-pseudo.git@64c061ed4371
+stdout '^vcs-test.golang.org/git/retract-pseudo.git v0.0.0-20201009173747-64c061ed4371'
+
+# A retracted version is a valid base. Retraction should not validate existing
+# pseudo-versions, nor should it turn invalid pseudo-versions valid.
+go get -d vcs-test.golang.org/git/retract-pseudo.git@v1.0.1-0.20201009173747-713affd19d7b
+go list -m vcs-test.golang.org/git/retract-pseudo.git
+stdout '^vcs-test.golang.org/git/retract-pseudo.git v1.0.1-0.20201009173747-713affd19d7b$'
+
+! go get -d vcs-test.golang.org/git/retract-pseudo.git@v1.0.1-0.20201009173747-64c061ed4371
+stderr '^go get vcs-test.golang.org/git/retract-pseudo.git@v1.0.1-0.20201009173747-64c061ed4371: vcs-test.golang.org/git/retract-pseudo.git@v1.0.1-0.20201009173747-64c061ed4371: invalid pseudo-version: tag \(v1.0.0\) found on revision 64c061ed4371 is already canonical, so should not be replaced with a pseudo-version derived from that tag$'
+
+-- retract-pseudo.sh --
+#!/bin/bash
+
+# This is not part of the test.
+# Run this to generate and update the repository on vcs-test.golang.org.
+
+set -euo pipefail
+
+rm -rf retract-pseudo
+mkdir retract-pseudo
+cd retract-pseudo
+git init
+
+# Create the module.
+# Retract v1.0.0 and tag v1.0.0 at the same commit.
+# The module has no unretracted release versions.
+go mod init vcs-test.golang.org/git/retract-pseudo.git
+go mod edit -retract v1.0.0
+echo 'package p' >p.go
+git add -A
+git commit -m 'create module retract-pseudo'
+git tag v1.0.0
+
+# Commit a trivial change so the default branch does not point to v1.0.0.
+git mv p.go q.go
+git commit -m 'trivial change'
+
+zip -r ../retract-pseudo.zip .
+gsutil cp ../retract-pseudo.zip gs://vcs-test/git/retract-pseudo.zip