Skip to content

Commit

Permalink
cmd/go: adjust heuristics for skipping +incompatible versions
Browse files Browse the repository at this point in the history
We know of at least one module (github.com/stripe/stripe-go) that has
a run of +incompatible versions, followed by a run of versions with
go.mod files, followed by another run of +incompatible versions.

We want the heuristics for showing +incompatible versions to reflect
the authors' current intent, and it seems clear that the current
intent of the authors of that module is for users of the unversioned
import path to still be on +incompatible versions.

To respect that intent, we need to keep checking for +incompatible
versions even after we have seen a lower major version with an
explicit go.mod file.

However, we still don't want to download every single version of the
module to check it. A given major version should have a consistent,
canonical import path, so the path (as inferred by the presence or
absence of a go.mod file) should be the same for every release across
that major version.

To avoid unnecessary overhead — and to allow module authors to correct
accidental changes to a major version's import path — we check only
the most recent release of each major version. If a release
accidentally changes the import path in either direction (by deleting
or adding a go.mod file), it can be corrected by issuing a single
subsequent release of that major version to restore the correct path.

I manually verified that, with this change,
github.com/stripe/stripe-go@latest reverts to v68.7.0+incompatible
as it was in Go 1.13.
The other regression tests for #34165 continue to pass.

Updates #34165

Change-Id: I5daff3cd2123f94c7c49519babf4eecd509f169e
Reviewed-on: https://go-review.googlesource.com/c/go/+/212317
Reviewed-by: Jay Conrod <jayconrod@google.com>
  • Loading branch information
Bryan C. Mills committed Jan 8, 2020
1 parent 2248fc6 commit 817afe8
Showing 1 changed file with 30 additions and 37 deletions.
67 changes: 30 additions & 37 deletions src/cmd/go/internal/modfetch/coderepo.go
Expand Up @@ -191,22 +191,6 @@ func (r *codeRepo) appendIncompatibleVersions(list, incompatible []string) ([]st
return list, nil
}

// We assume that if the latest release of any major version has a go.mod
// file, all subsequent major versions will also have go.mod files (and thus
// be ineligible for use as +incompatible versions).
// If we're wrong about a major version, users will still be able to 'go get'
// specific higher versions explicitly — they just won't affect 'latest' or
// appear in 'go list'.
//
// Conversely, we assume that if the latest release of any major version lacks
// a go.mod file, all versions also lack go.mod files. If we're wrong, we may
// include a +incompatible version that isn't really valid, but most
// operations won't try to use that version anyway.
//
// These optimizations bring
// 'go list -versions -m github.com/openshift/origin' down from 1m58s to 0m37s.
// That's still not great, but a substantial improvement.

versionHasGoMod := func(v string) (bool, error) {
_, err := r.code.ReadFile(v, "go.mod", codehost.MaxGoMod)
if err == nil {
Expand Down Expand Up @@ -241,32 +225,41 @@ func (r *codeRepo) appendIncompatibleVersions(list, incompatible []string) ([]st
}
}

var lastMajor string
var (
lastMajor string
lastMajorHasGoMod bool
)
for i, v := range incompatible {
major := semver.Major(v)
if major == lastMajor {
list = append(list, v+"+incompatible")
continue
}

rem := incompatible[i:]
j := sort.Search(len(rem), func(j int) bool {
return semver.Major(rem[j]) != major
})
latestAtMajor := rem[j-1]

ok, err := versionHasGoMod(latestAtMajor)
if err != nil {
return nil, err
}
if ok {
// This major version has a go.mod file, so it is not allowed as
// +incompatible. Subsequent major versions are likely to also have
// go.mod files, so stop here.
break
if major != lastMajor {
rem := incompatible[i:]
j := sort.Search(len(rem), func(j int) bool {
return semver.Major(rem[j]) != major
})
latestAtMajor := rem[j-1]

var err error
lastMajor = major
lastMajorHasGoMod, err = versionHasGoMod(latestAtMajor)
if err != nil {
return nil, err
}
}

lastMajor = major
if lastMajorHasGoMod {
// The latest release of this major version has a go.mod file, so it is
// not allowed as +incompatible. It would be confusing to include some
// minor versions of this major version as +incompatible but require
// semantic import versioning for others, so drop all +incompatible
// versions for this major version.
//
// If we're wrong about a minor version in the middle, users will still be
// able to 'go get' specific tags for that version explicitly — they just
// won't appear in 'go list' or as the results for queries with inequality
// bounds.
continue
}
list = append(list, v+"+incompatible")
}

Expand Down

0 comments on commit 817afe8

Please sign in to comment.