x/exp/cmd/gorelease: err when main module requests lower version than cycle
When there is a cycle like,
main@v1.0.0 -> foo@v1.0.0
foo@v1.0.0 -> main@v1.5.0
And the user suggests main@v1.1.1 (or anything <= 1.5.0), report an error, since
MVS will never choose that version.
Fixes golang/go#37567
Change-Id: I95628c49e0949116ec8019364f5c760dcc7c80a3
Reviewed-on: https://go-review.googlesource.com/c/exp/+/273288
Run-TryBot: Jean de Klerk <deklerk@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Trust: Jean de Klerk <deklerk@google.com>
diff --git a/cmd/gorelease/gorelease.go b/cmd/gorelease/gorelease.go
index 8152903..10860bd 100644
--- a/cmd/gorelease/gorelease.go
+++ b/cmd/gorelease/gorelease.go
@@ -242,14 +242,15 @@
}
type moduleInfo struct {
- modRoot string // module root directory
- repoRoot string // repository root directory (may be "")
- modPath string // module path in go.mod
- version string // resolved version or "none"
- versionQuery string // a query like "latest" or "dev-branch", if specified
- versionInferred bool // true if the version was unspecified and inferred
- modPathMajor string // major version suffix like "/v3" or ".v2"
- tagPrefix string // prefix for version tags if module not in repo root
+ modRoot string // module root directory
+ repoRoot string // repository root directory (may be "")
+ modPath string // module path in go.mod
+ version string // resolved version or "none"
+ versionQuery string // a query like "latest" or "dev-branch", if specified
+ versionInferred bool // true if the version was unspecified and inferred
+ highestTransitiveVersion string // version of the highest transitive self-dependency (cycle)
+ modPathMajor string // major version suffix like "/v3" or ".v2"
+ tagPrefix string // prefix for version tags if module not in repo root
goModPath string // file path to go.mod
goModData []byte // content of go.mod
@@ -391,6 +392,19 @@
m.diagnostics = append(m.diagnostics, prepareDiagnostics...)
m.diagnostics = append(m.diagnostics, loadDiagnostics...)
+ highestVersion, err := findSelectedVersion(tmpLoadDir, m.modPath)
+ if err != nil {
+ return moduleInfo{}, err
+ }
+
+ if highestVersion != "" {
+ // A version of the module is included in the transitive dependencies.
+ // Add it to the moduleInfo so that the release report stage can use it
+ // in verifying the version or suggestion a new version, depending on
+ // whether the user provided a version already.
+ m.highestTransitiveVersion = highestVersion
+ }
+
return m, nil
}
@@ -1202,3 +1216,18 @@
}
return pairs
}
+
+// findSelectedVersion returns the highest version of the given modPath at
+// modDir, if a module cycle exists. modDir should be a writable directory
+// containing the go.mod for modPath.
+//
+// If no module cycle exists, it returns empty string.
+func findSelectedVersion(modDir, modPath string) (latestVersion string, err error) {
+ cmd := exec.Command("go", "list", "-m", "-f", "{{.Version}}", "--", modPath)
+ cmd.Dir = modDir
+ out, err := cmd.Output()
+ if err != nil {
+ return "", cleanCmdError(err)
+ }
+ return strings.TrimSpace(string(out)), nil
+}
diff --git a/cmd/gorelease/report.go b/cmd/gorelease/report.go
index ee95d13..aede072 100644
--- a/cmd/gorelease/report.go
+++ b/cmd/gorelease/report.go
@@ -191,6 +191,11 @@
over the base version (%s).`, r.base.version)
return
}
+
+ if r.release.highestTransitiveVersion != "" && semver.Compare(r.release.highestTransitiveVersion, r.release.version) > 0 {
+ setNotValid(`Module indirectly depends on a higher version of itself (%s).
+ `, r.release.highestTransitiveVersion)
+ }
}
// suggestReleaseVersion suggests a new version consistent with observed
@@ -219,8 +224,14 @@
var major, minor, patch, pre string
if r.base.version != "none" {
+ minVersion := r.base.version
+ if r.release.highestTransitiveVersion != "" && semver.Compare(r.release.highestTransitiveVersion, minVersion) > 0 {
+ setNotValid("Module indirectly depends on a higher version of itself (%s) than the base version (%s).", r.release.highestTransitiveVersion, r.base.version)
+ return
+ }
+
var err error
- major, minor, patch, pre, _, err = parseVersion(r.base.version)
+ major, minor, patch, pre, _, err = parseVersion(minVersion)
if err != nil {
panic(fmt.Sprintf("could not parse base version: %v", err))
}
@@ -253,6 +264,7 @@
patch = incDecimal(patch)
}
setVersion(fmt.Sprintf("v%s.%s.%s", major, minor, patch))
+ return
}
// canVerifyReleaseVersion returns true if we can safely suggest a new version
diff --git a/cmd/gorelease/testdata/README.md b/cmd/gorelease/testdata/README.md
index 28e171b..9be53a3 100644
--- a/cmd/gorelease/testdata/README.md
+++ b/cmd/gorelease/testdata/README.md
@@ -33,7 +33,7 @@
* `mod`: sets the module path. Must be specified together with `version`. Copies
the content of a module out of the test proxy into a temporary directory
where `gorelease` is run.
-* `version`: specified together with `mod`. sets the version to retreive from
+* `version`: specified together with `mod`, it sets the version to retrieve from
the test proxy.
* `base`: the value of the `-base` flag passed to `gorelease`.
* `release`: the value of the `-version` flag passed to `gorelease`.
diff --git a/cmd/gorelease/testdata/cycle/README.md b/cmd/gorelease/testdata/cycle/README.md
new file mode 100644
index 0000000..3596c8b
--- /dev/null
+++ b/cmd/gorelease/testdata/cycle/README.md
@@ -0,0 +1 @@
+This directory is for tests related to module cycles.
\ No newline at end of file
diff --git a/cmd/gorelease/testdata/cycle/cycle_suggest.test b/cmd/gorelease/testdata/cycle/cycle_suggest.test
new file mode 100644
index 0000000..3e8252f
--- /dev/null
+++ b/cmd/gorelease/testdata/cycle/cycle_suggest.test
@@ -0,0 +1,7 @@
+mod=example.com/cycle
+base=v1.0.0
+version=v1.0.0
+success=false
+-- want --
+Cannot suggest a release version.
+Module indirectly depends on a higher version of itself (v1.5.0) than the base version (v1.0.0).
\ No newline at end of file
diff --git a/cmd/gorelease/testdata/cycle/cycle_verify.test b/cmd/gorelease/testdata/cycle/cycle_verify.test
new file mode 100644
index 0000000..64e1fdf
--- /dev/null
+++ b/cmd/gorelease/testdata/cycle/cycle_verify.test
@@ -0,0 +1,8 @@
+mod=example.com/cycle
+base=v1.0.0
+version=v1.0.0
+release=v1.0.1
+success=false
+-- want --
+v1.0.1 is not a valid semantic version for this release.
+Module indirectly depends on a higher version of itself (v1.5.0).
\ No newline at end of file
diff --git a/cmd/gorelease/testdata/mod/example.com_cycle_v1.0.0.txt b/cmd/gorelease/testdata/mod/example.com_cycle_v1.0.0.txt
new file mode 100644
index 0000000..7200d20
--- /dev/null
+++ b/cmd/gorelease/testdata/mod/example.com_cycle_v1.0.0.txt
@@ -0,0 +1,15 @@
+-- go.sum --
+example.com/cycle v1.5.0 h1:j2Bju3xKUT09utc7WwS5sXwrOSVUr5a7vOzOyB4ivac=
+example.com/cycle v1.5.0/go.mod h1://AqZbyNHeLOKZB3J/UPPXaBvk3nCqvqVRbPkffDx60=
+example.com/cycledep v1.0.0/go.mod h1:Gc4hO1S1BMZaxOcGHwCRmdVcQP8+jAu/PyEgLdGe0xU=
+-- go.mod --
+module example.com/cycle
+
+go 1.12
+
+require example.com/cycledep v1.0.0
+require example.com/cycle v1.5.0
+-- cycle/main.go --
+package main
+
+import _ "example.com/cycledep"
\ No newline at end of file
diff --git a/cmd/gorelease/testdata/mod/example.com_cycle_v1.5.0.txt b/cmd/gorelease/testdata/mod/example.com_cycle_v1.5.0.txt
new file mode 100644
index 0000000..be01d7f
--- /dev/null
+++ b/cmd/gorelease/testdata/mod/example.com_cycle_v1.5.0.txt
@@ -0,0 +1,6 @@
+-- go.mod --
+module example.com/cycle
+
+go 1.12
+-- cycle/a.go --
+package a
\ No newline at end of file
diff --git a/cmd/gorelease/testdata/mod/example.com_cycledep_v1.0.0.txt b/cmd/gorelease/testdata/mod/example.com_cycledep_v1.0.0.txt
new file mode 100644
index 0000000..6ea4610
--- /dev/null
+++ b/cmd/gorelease/testdata/mod/example.com_cycledep_v1.0.0.txt
@@ -0,0 +1,10 @@
+-- go.mod --
+module example.com/cycledep
+
+go 1.12
+
+require example.com/cycle v1.5.0
+-- cycledep/a.go --
+package a
+
+import _ "example.com/cycle"
