cmd/go/internal/modload: document mvsReqs.Max

The version "" denotes the main module, which has no version. The
mvs.Reqs interface documentation hints this is allowed, but it's not
obvious from the implementation in modload.mvsReqs.Max.

Also, replace a related TODO with a comment in mvs.Downgrade.

Fixes #39042

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

diff --git a/src/cmd/go/internal/modload/mvs.go b/src/cmd/go/internal/modload/mvs.go
index a4bdf3ee0089551f81c5e9032da88dc1a19db8eb..5dd009d31dfa6368f9498baf8f186c0e69fd4b9a 100644 (file)
--- a/src/cmd/go/internal/modload/mvs.go
+++ b/src/cmd/go/internal/modload/mvs.go
@@ -157,6 +157,12 @@ func (r *mvsReqs) required(mod module.Version) ([]module.Version, error) {
        return r.modFileToList(f), nil
 }
 
+// Max returns the maximum of v1 and v2 according to semver.Compare.
+//
+// As a special case, the version "" is considered higher than all other
+// versions. The main module (also known as the target) has no version and must
+// be chosen over other versions of the same module in the module dependency
+// graph.
 func (*mvsReqs) Max(v1, v2 string) string {
        if v1 != "" && semver.Compare(v1, v2) == -1 {
                return v2

diff --git a/src/cmd/go/internal/mvs/mvs.go b/src/cmd/go/internal/mvs/mvs.go
index dd3b3ccb865ecfedb49e909c15462853e8db8b15..1f8eaa1f601cea2a4f506c6d98a1e92da81f387b 100644 (file)
--- a/src/cmd/go/internal/mvs/mvs.go
+++ b/src/cmd/go/internal/mvs/mvs.go
@@ -115,7 +115,21 @@ func (e *BuildListError) Error() string {
 }
 
 // BuildList returns the build list for the target module.
-// The first element is the target itself, with the remainder of the list sorted by path.
+//
+// target is the root vertex of a module requirement graph. For cmd/go, this is
+// typically the main module, but note that this algorithm is not intended to
+// be Go-specific: module paths and versions are treated as opaque values.
+//
+// reqs describes the module requirement graph and provides an opaque method
+// for comparing versions.
+//
+// BuildList traverses the graph and returns a list containing the highest
+// version for each visited module. The first element of the returned list is
+// target itself; reqs.Max requires target.Version to compare higher than all
+// other versions, so no other version can be selected. The remaining elements
+// of the list are sorted by path.
+//
+// See https://research.swtch.com/vgo-mvs for details.
 func BuildList(target module.Version, reqs Reqs) ([]module.Version, error) {
        return buildList(target, reqs, nil)
 }
@@ -220,10 +234,9 @@ func buildList(target module.Version, reqs Reqs, upgrade func(module.Version) (m
        // The final list is the minimum version of each module found in the graph.
 
        if v := min[target.Path]; v != target.Version {
-               // TODO(jayconrod): there is a special case in modload.mvsReqs.Max
-               // that prevents us from selecting a newer version of a module
-               // when the module has no version. This may only be the case for target.
-               // Should we always panic when target has a version?
+               // target.Version will be "" for modload, the main client of MVS.
+               // "" denotes the main module, which has no version. However, MVS treats
+               // version strings as opaque, so "" is not a special value here.
                // See golang.org/issue/31491, golang.org/issue/29773.
                panic(fmt.Sprintf("mistake: chose version %q instead of target %+v", v, target)) // TODO: Don't panic.
        }