Skip to content

Commit

Permalink
cmd/go: avoid upgrading to +incompatible versions if the latest compa…
Browse files Browse the repository at this point in the history 
…tible one has a go.mod file

Previously we would always “upgrade” to the semantically-highest
version, even if a newer compatible version exists.

That made certain classes of mistakes irreversible: in general we
expect users to address bad releases by releasing a new (higher)
version, but if the bad release was an unintended +incompatible
version, then no release that includes a go.mod file can ever have a
higher version, and the bad release will be treated as “latest”
forever.

Instead, when considering a +incompatible version we now consult the
latest compatible (v0 or v1) release first. If the compatible release
contains a go.mod file, we ignore the +incompatible releases unless
they are expicitly requested (by version, commit ID, or branch name).

Fixes #34165
Updates #34189

Change-Id: I7301eb963bbb91b21d3b96a577644221ed988ab7
Reviewed-on: https://go-review.googlesource.com/c/go/+/204440
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
  • Loading branch information
Bryan C. Mills committed Nov 6, 2019
1 parent 649f341 commit de70de6
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 30 deletions.
34 changes: 27 additions & 7 deletions doc/go1.14.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ <h2 id="tools">Tools</h2>

<h3 id="go-command">Go command</h3>

<h4 id="vendor">Vendoring</h4>
<!-- golang.org/issue/33848 -->

<p>
When the main module contains a top-level <code>vendor</code> directory and
its <code>go.mod</code> file specifies <code>go</code> <code>1.14</code> or
Expand All @@ -106,20 +108,15 @@ <h3 id="go-command">Go command</h3>
<code>-mod=vendor</code> is set.
</p>

<h4 id="go-flags">Flags</h4>

<p><!-- golang.org/issue/32502, golang.org/issue/30345 -->
The <code>go</code> <code>get</code> command no longer accepts
the <code>-mod</code> flag. Previously, the flag's setting either
<a href="https://golang.org/issue/30345">was ignored</a> or
<a href="https://golang.org/issue/32502">caused the build to fail</a>.
</p>

<p><!-- golang.org/issue/30748 -->
The <code>go</code> command now includes snippets of plain-text error messages
from module proxies and other HTTP servers.
An error message will only be shown if it is valid UTF-8 and consists of only
graphic characters and spaces.
</p>

<p><!-- golang.org/issue/31481 -->
<code>-modcacherw</code> is a new flag that instructs the <code>go</code>
command to leave newly-created directories in the module cache at their
Expand All @@ -141,10 +138,33 @@ <h3 id="go-command">Go command</h3>
trimming the ".mod" extension and appending ".sum".
</p>

<h4 id="incompatible-versions"><code>+incompatible</code> versions</h4>
<!-- golang.org/issue/34165 -->

<p>
If the latest version of a module contains a <code>go.mod</code> file,
<code>go</code> <code>get</code> will no longer upgrade to an
<a href="/cmd/go/#hdr-Module_compatibility_and_semantic_versioning">incompatible</a>
major version of that module unless such a version is requested explicitly
or is already required.
<code>go</code> <code>list</code> also omits incompatible major versions
for such a module when fetching directly from version control, but may
include them if reported by a proxy.
</p>

<h4 id="module-downloading">Module downloading</h4>

<p><!-- golang.org/issue/26092 -->
The <code>go</code> command now supports Subversion repositories in module mode.
</p>

<p><!-- golang.org/issue/30748 -->
The <code>go</code> command now includes snippets of plain-text error messages
from module proxies and other HTTP servers.
An error message will only be shown if it is valid UTF-8 and consists of only
graphic characters and spaces.
</p>

<h2 id="runtime">Runtime</h2>

<p>
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modfetch/repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type Repo interface {

// A Rev describes a single revision in a module repository.
type RevInfo struct {
Version string // version string
Version string // suggested version string for this revision
Time time.Time // commit time

// These fields are used for Stat of arbitrary rev,
Expand Down
119 changes: 97 additions & 22 deletions src/cmd/go/internal/modload/query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"os"
pathpkg "path"
"path/filepath"
"strings"
"sync"

Expand Down Expand Up @@ -90,10 +91,20 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
badVersion := func(v string) (*modfetch.RevInfo, error) {
return nil, fmt.Errorf("invalid semantic version %q in range %q", v, query)
}
var ok func(module.Version) bool
var prefix string
var preferOlder bool
var mayUseLatest bool
matchesMajor := func(v string) bool {
_, pathMajor, ok := module.SplitPathVersion(path)
if !ok {
return false
}
return module.CheckPathMajor(v, pathMajor) == nil
}
var (
ok func(module.Version) bool
prefix string
preferOlder bool
mayUseLatest bool
preferIncompatible bool = strings.HasSuffix(current, "+incompatible")
)
switch {
case query == "latest":
ok = allowed
Expand Down Expand Up @@ -126,6 +137,9 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
ok = func(m module.Version) bool {
return semver.Compare(m.Version, v) <= 0 && allowed(m)
}
if !matchesMajor(v) {
preferIncompatible = true
}

case strings.HasPrefix(query, "<"):
v := query[len("<"):]
Expand All @@ -135,6 +149,9 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
ok = func(m module.Version) bool {
return semver.Compare(m.Version, v) < 0 && allowed(m)
}
if !matchesMajor(v) {
preferIncompatible = true
}

case strings.HasPrefix(query, ">="):
v := query[len(">="):]
Expand All @@ -145,6 +162,9 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
return semver.Compare(m.Version, v) >= 0 && allowed(m)
}
preferOlder = true
if !matchesMajor(v) {
preferIncompatible = true
}

case strings.HasPrefix(query, ">"):
v := query[len(">"):]
Expand All @@ -159,12 +179,18 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
return semver.Compare(m.Version, v) > 0 && allowed(m)
}
preferOlder = true
if !matchesMajor(v) {
preferIncompatible = true
}

case semver.IsValid(query) && isSemverPrefix(query):
ok = func(m module.Version) bool {
return matchSemverPrefix(query, m.Version) && allowed(m)
}
prefix = query + "."
if !matchesMajor(query) {
preferIncompatible = true
}

default:
// Direct lookup of semantic version or commit identifier.
Expand Down Expand Up @@ -217,6 +243,10 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
if err != nil {
return nil, err
}
releases, prereleases, err := filterVersions(path, versions, ok, preferIncompatible)
if err != nil {
return nil, err
}

lookup := func(v string) (*modfetch.RevInfo, error) {
rev, err := repo.Stat(v)
Expand All @@ -237,28 +267,18 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
}

if preferOlder {
for _, v := range versions {
if semver.Prerelease(v) == "" && ok(module.Version{Path: path, Version: v}) {
return lookup(v)
}
if len(releases) > 0 {
return lookup(releases[0])
}
for _, v := range versions {
if semver.Prerelease(v) != "" && ok(module.Version{Path: path, Version: v}) {
return lookup(v)
}
if len(prereleases) > 0 {
return lookup(prereleases[0])
}
} else {
for i := len(versions) - 1; i >= 0; i-- {
v := versions[i]
if semver.Prerelease(v) == "" && ok(module.Version{Path: path, Version: v}) {
return lookup(v)
}
if len(releases) > 0 {
return lookup(releases[len(releases)-1])
}
for i := len(versions) - 1; i >= 0; i-- {
v := versions[i]
if semver.Prerelease(v) != "" && ok(module.Version{Path: path, Version: v}) {
return lookup(v)
}
if len(prereleases) > 0 {
return lookup(prereleases[len(prereleases)-1])
}
}

Expand Down Expand Up @@ -302,6 +322,52 @@ func matchSemverPrefix(p, v string) bool {
return len(v) > len(p) && v[len(p)] == '.' && v[:len(p)] == p && semver.Prerelease(v) == ""
}

// filterVersions classifies versions into releases and pre-releases, filtering
// out:
// 1. versions that do not satisfy the 'ok' predicate, and
// 2. "+incompatible" versions, if a compatible one satisfies the predicate
// and the incompatible version is not preferred.
func filterVersions(path string, versions []string, ok func(module.Version) bool, preferIncompatible bool) (releases, prereleases []string, err error) {
var lastCompatible string
for _, v := range versions {
if !ok(module.Version{Path: path, Version: v}) {
continue
}

if !preferIncompatible {
if !strings.HasSuffix(v, "+incompatible") {
lastCompatible = v
} else if lastCompatible != "" {
// If the latest compatible version is allowed and has a go.mod file,
// ignore any version with a higher (+incompatible) major version. (See
// https://golang.org/issue/34165.) Note that we even prefer a
// compatible pre-release over an incompatible release.

ok, err := versionHasGoMod(module.Version{Path: path, Version: lastCompatible})
if err != nil {
return nil, nil, err
}
if ok {
break
}

// No acceptable compatible release has a go.mod file, so the versioning
// for the module might not be module-aware, and we should respect
// legacy major-version tags.
preferIncompatible = true
}
}

if semver.Prerelease(v) != "" {
prereleases = append(prereleases, v)
} else {
releases = append(releases, v)
}
}

return releases, prereleases, nil
}

type QueryResult struct {
Mod module.Version
Rev *modfetch.RevInfo
Expand Down Expand Up @@ -590,3 +656,12 @@ func ModuleHasRootPackage(m module.Version) (bool, error) {
_, ok := dirInModule(m.Path, m.Path, root, isLocal)
return ok, nil
}

func versionHasGoMod(m module.Version) (bool, error) {
root, _, err := fetch(m)
if err != nil {
return false, err
}
fi, err := os.Stat(filepath.Join(root, "go.mod"))
return err == nil && !fi.IsDir(), nil
}
64 changes: 64 additions & 0 deletions src/cmd/go/testdata/script/mod_prefer_compatible.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Regression test for golang.org/issue/34189 and golang.org/issue/34165:
# @latest, @upgrade, and @patch should prefer compatible versions over
# +incompatible ones, even if offered by a proxy.

[!net] skip

env GO111MODULE=on
env GOPROXY=
env GOSUMDB=

# github.com/russross/blackfriday v2.0.0+incompatible exists,
# and should be resolved if we ask for v2.0 explicitly.

go list -m github.com/russross/[email protected]
stdout '^github.com/russross/blackfriday v2\.0\.0\+incompatible$'

# blackfriday v1.5.2 has a go.mod file, so v1.5.2 should be preferred over
# v2.0.0+incompatible when resolving latest, upgrade, and patch.

go list -m github.com/russross/blackfriday@latest
stdout '^github.com/russross/blackfriday v1\.'

go list -m github.com/russross/blackfriday@upgrade
stdout '^github.com/russross/blackfriday v1\.'

go list -m github.com/russross/blackfriday@patch
stdout '^github.com/russross/blackfriday v1\.'

# If we're fetching directly from version control, ignored +incompatible
# versions should also be omitted by 'go list'.

# (Note that they may still be included in results from a proxy: in proxy mode,
# we would need to fetch the whole zipfile for the latest compatible version in
# order to determine whether it contains a go.mod file, and part of the point of
# the proxy is to avoid fetching unnecessary data.)

env GOPROXY=direct

go list -versions -m github.com/russross/blackfriday github.com/russross/blackfriday
stdout '^github.com/russross/blackfriday v1\.5\.1 v1\.5\.2' # and possibly others
! stdout ' v2\.'

# However, if the latest compatible version does not include a go.mod file,
# +incompatible versions should still be listed, as they may still reflect the
# intent of the module author.

go list -versions -m github.com/rsc/legacytest
stdout '^github.com/rsc/legacytest v1\.0\.0 v1\.1\.0-pre v1\.2\.0 v2\.0\.0\+incompatible'

# If we're fetching directly from version control, asking for a commit hash
# corresponding to a +incompatible version should continue to produce the
# +incompatible version tagged for that commit, even if it is no longer listed.

go list -m github.com/russross/blackfriday@cadec560ec52
stdout '^github.com/russross/blackfriday v2\.0\.0\+incompatible$'

# Similarly, requesting an untagged commit should continue to produce a +incompatible
# pseudo-version.

go list -m github.com/rsc/legacytest@7303f7796364
stdout '^github.com/rsc/legacytest v2\.0\.1-0\.20180717164253-7303f7796364\+incompatible$'

-- go.mod --
module github.com/golang.org/issue/34165

0 comments on commit de70de6

Please sign in to comment.