internal/mod/mvs: adapt to CUE semantics

Note: experimental feature.

The mvs tests rely on using module.Version in a way that
doesn't conform to the new requirements for that type.
Given that mvs doesn't actually care that it's specifically
dealing with module.Version per se, we make it generic
across any version-like value.
This allows us to avoid rewriting all the (fairly extensive) tests
and seems a reasonably natural fit for the generic nature of
the MVS algorithm itself.

For #2330.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: Ib1b16c3267ac98fde30347f118f0eb6b2764a473
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1168705
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: rogpeppe

internal/mod/module/module.go

@@ -103,7 +103,7 @@ func MustNewVersion(path string, vers string) Version {
 }
 
 // NewVersion forms a Version from the given path and version.
-// The version must be canonical or empty.
+// The version must be canonical, empty or "none".
 // If the path doesn't have a major version suffix, one will be added
 // if the version isn't empty; if the version is empty, it's an error.
 func NewVersion(path string, vers string) (Version, error) {

internal/mod/module/versions.go

@@ -0,0 +1,45 @@
+package module
+
+import (
+	"golang.org/x/mod/semver"
+)
+
+// Versions implements mvs.Versions[Version].
+type Versions struct{}
+
+// New implements mvs.Versions[Version].Version.
+func (Versions) Version(v Version) string {
+	return v.Version()
+}
+
+// New implements mvs.Versions[Version].Path.
+func (Versions) Path(v Version) string {
+	return v.Path()
+}
+
+// New implements mvs.Versions[Version].New.
+func (Versions) New(p, v string) (Version, error) {
+	return NewVersion(p, v)
+}
+
+// Max implements mvs.Reqs.Max.
+//
+// It is consistent with semver.Compare except that 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.
+//
+// See [mvs.Reqs] for more detail.
+func (Versions) Max(v1, v2 string) string {
+	if v1 == "none" || v2 == "" {
+		return v2
+	}
+	if v2 == "none" || v1 == "" {
+		return v1
+	}
+	if semver.Compare(v1, v2) > 0 {
+		return v1
+	}
+	return v2
+}

internal/mod/module/versions_test.go

@@ -0,0 +1,8 @@
+package module_test
+
+import (
+	"cuelang.org/go/internal/mod/module"
+	"cuelang.org/go/internal/mod/mvs"
+)
+
+var _ mvs.Versions[module.Version] = module.Versions{}

internal/mod/mvs/errors.go

@@ -1,5 +1,3 @@
-//go:build ignore
-
 // Copyright 2020 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
@@ -9,20 +7,19 @@ package mvs
 import (
 	"fmt"
 	"strings"
-
-	"golang.org/x/mod/module"
 )
 
 // BuildListError decorates an error that occurred gathering requirements
 // while constructing a build list. BuildListError prints the chain
 // of requirements to the module where the error occurred.
-type BuildListError struct {
+type BuildListError[V comparable] struct {
 	Err   error
-	stack []buildListErrorElem
+	stack []buildListErrorElem[V]
+	vs    Versions[V]
 }
 
-type buildListErrorElem struct {
-	m module.Version
+type buildListErrorElem[V comparable] struct {
+	m V
 
 	// nextReason is the reason this module depends on the next module in the
 	// stack. Typically either "requires", or "updating to".
@@ -37,69 +34,61 @@ type buildListErrorElem struct {
 // explicit upgrade or downgrade (as opposed to an existing requirement in a
 // go.mod file). A nil isVersionChange function indicates that none of the path
 // steps are due to explicit version changes.
-func NewBuildListError(err error, path []module.Version, isVersionChange func(from, to module.Version) bool) *BuildListError {
-	stack := make([]buildListErrorElem, 0, len(path))
+func NewBuildListError[V comparable](err error, path []V, vs Versions[V], isVersionChange func(from, to V) bool) *BuildListError[V] {
+	stack := make([]buildListErrorElem[V], 0, len(path))
 	for len(path) > 1 {
 		reason := "requires"
 		if isVersionChange != nil && isVersionChange(path[0], path[1]) {
 			reason = "updating to"
 		}
-		stack = append(stack, buildListErrorElem{
+		stack = append(stack, buildListErrorElem[V]{
 			m:          path[0],
 			nextReason: reason,
 		})
 		path = path[1:]
 	}
-	stack = append(stack, buildListErrorElem{m: path[0]})
+	stack = append(stack, buildListErrorElem[V]{m: path[0]})
 
-	return &BuildListError{
+	return &BuildListError[V]{
 		Err:   err,
 		stack: stack,
+		vs:    vs,
 	}
 }
 
 // Module returns the module where the error occurred. If the module stack
 // is empty, this returns a zero value.
-func (e *BuildListError) Module() module.Version {
+func (e *BuildListError[V]) Module() V {
 	if len(e.stack) == 0 {
-		return module.Version{}
+		return *new(V)
 	}
 	return e.stack[len(e.stack)-1].m
 }
 
-func (e *BuildListError) Error() string {
+func (e *BuildListError[V]) Error() string {
 	b := &strings.Builder{}
 	stack := e.stack
 
 	// Don't print modules at the beginning of the chain without a
 	// version. These always seem to be the main module or a
 	// synthetic module ("target@").
-	for len(stack) > 0 && stack[0].m.Version == "" {
+	for len(stack) > 0 && e.vs.Version(stack[0].m) == "" {
 		stack = stack[1:]
 	}
 
 	if len(stack) == 0 {
 		b.WriteString(e.Err.Error())
 	} else {
 		for _, elem := range stack[:len(stack)-1] {
-			fmt.Fprintf(b, "%s %s\n\t", elem.m, elem.nextReason)
+			fmt.Fprintf(b, "%v %s\n\t", elem.m, elem.nextReason)
 		}
-		// Ensure that the final module path and version are included as part of the
-		// error message.
 		m := stack[len(stack)-1].m
-		if mErr, ok := e.Err.(*module.ModuleError); ok {
-			actual := module.Version{Path: mErr.Path, Version: mErr.Version}
-			if v, ok := mErr.Err.(*module.InvalidVersionError); ok {
-				actual.Version = v.Version
-			}
-			if actual == m {
-				fmt.Fprintf(b, "%v", e.Err)
-			} else {
-				fmt.Fprintf(b, "%s (replaced by %s): %v", m, actual, mErr.Err)
-			}
-		} else {
-			fmt.Fprintf(b, "%v", module.VersionError(m, e.Err))
-		}
+		fmt.Fprintf(b, "%v: %v", m, e.Err)
+		// TODO the original mvs code was careful to ensure that the final module path
+		// and version were included as part of the error message, but it did that
+		// by checking for mod/module-specific error types, but we don't want this
+		// package to depend on module. We could potentially do it by making those
+		// errors implement interface types defined in this package.
 	}
 	return b.String()
 }