Skip to content

Commit

Permalink
cmd/go: ignore retracted versions when converting revisions to versions
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jay Conrod committed Oct 9, 2020
1 parent d4a5797 commit 712cba3
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 11 deletions.
5 changes: 2 additions & 3 deletions src/cmd/go/internal/modfetch/codehost/codehost.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
7 changes: 5 additions & 2 deletions src/cmd/go/internal/modfetch/codehost/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modfetch/codehost/vcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
78 changes: 73 additions & 5 deletions src/cmd/go/internal/modfetch/coderepo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
62 changes: 62 additions & 0 deletions src/cmd/go/testdata/script/mod_retract_pseudo_base.txt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 712cba3

Please sign in to comment.