cmd/gorelease: support comparison of modules with different paths
For golang/go#39666
Change-Id: I7e69ef14a3f6b3996ceea2a70ec4ce1e33f912c6
Reviewed-on: https://go-review.googlesource.com/c/exp/+/238839
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/cmd/gorelease/gorelease.go b/cmd/gorelease/gorelease.go
index e024710..83ed99c 100644
--- a/cmd/gorelease/gorelease.go
+++ b/cmd/gorelease/gorelease.go
@@ -64,8 +64,11 @@
// "latest", or "none". If the version is "none", gorelease will not compare the
// current version against any previous version; it will only validate the
// current version. This is useful for checking the first release of a new major
-// version. If -base is not specified, gorelease will attempt to infer a base
-// version from the -version flag and available released versions.
+// version. The version may be preceded by a different module path and an '@',
+// like -base=example.com/mod/v2@v2.5.2. This is useful to compare against
+// an earlier major version or a fork. If -base is not specified, gorelease will
+// attempt to infer a base version from the -version flag and available released
+// versions.
//
// -version=version: The proposed version to be released. If specified,
// gorelease will confirm whether this version is consistent with changes made
@@ -160,8 +163,8 @@
fs := flag.NewFlagSet("gorelease", flag.ContinueOnError)
fs.Usage = func() {}
fs.SetOutput(ioutil.Discard)
- var baseVersion, releaseVersion string
- fs.StringVar(&baseVersion, "base", "", "previous version to compare against")
+ var baseOpt, releaseVersion string
+ fs.StringVar(&baseOpt, "base", "", "previous version to compare against")
fs.StringVar(&releaseVersion, "version", "", "proposed version to be released")
if err := fs.Parse(args); err != nil {
return false, &usageError{err: err}
@@ -170,6 +173,7 @@
if len(fs.Args()) > 0 {
return false, usageErrorf("no arguments allowed")
}
+
if releaseVersion != "" {
if semver.Build(releaseVersion) != "" {
return false, usageErrorf("release version %q is not a canonical semantic version: build metadata is not supported", releaseVersion)
@@ -178,12 +182,26 @@
return false, usageErrorf("release version %q is not a canonical semantic version", releaseVersion)
}
}
- if baseVersion != "" && semver.Canonical(baseVersion) == baseVersion && releaseVersion != "" {
- if cmp := semver.Compare(baseVersion, releaseVersion); cmp == 0 {
- return false, usageErrorf("-base and -version must be different")
- } else if cmp > 0 {
- return false, usageErrorf("base version (%q) must be lower than release version (%q)", baseVersion, releaseVersion)
+
+ var baseModPath, baseVersion string
+ if at := strings.Index(baseOpt, "@"); at >= 0 {
+ baseModPath = baseOpt[:at]
+ baseVersion = baseOpt[at+1:]
+ } else if dot, slash := strings.Index(baseOpt, "."), strings.Index(baseOpt, "/"); dot >= 0 && slash >= 0 && dot < slash {
+ baseModPath = baseOpt
+ } else {
+ baseVersion = baseOpt
+ }
+ if baseModPath == "" {
+ if baseVersion != "" && semver.Canonical(baseVersion) == baseVersion && releaseVersion != "" {
+ if cmp := semver.Compare(baseOpt, releaseVersion); cmp == 0 {
+ return false, usageErrorf("-base and -version must be different")
+ } else if cmp > 0 {
+ return false, usageErrorf("base version (%q) must be lower than release version (%q)", baseVersion, releaseVersion)
+ }
}
+ } else if baseModPath != "" && baseVersion == "none" {
+ return false, usageErrorf(`base version (%q) cannot have version "none" with explicit module path`, baseOpt)
}
// Find the local module and repository root directories.
@@ -201,8 +219,12 @@
// Find the base version if there is one, download it, and load packages from
// the module cache.
- baseModPath := release.modPath // TODO(golang.org/issue/39666): allow different module path
- base, err := loadDownloadedModule(baseModPath, baseVersion, releaseVersion)
+ var max string
+ if baseModPath == "" {
+ baseModPath = release.modPath
+ max = releaseVersion
+ }
+ base, err := loadDownloadedModule(baseModPath, baseVersion, max)
if err != nil {
return false, err
}
@@ -381,6 +403,9 @@
// to max will not be considered. Typically, loadDownloadedModule is used to
// load the base version, and max is the release version.
func loadDownloadedModule(modPath, version, max string) (m moduleInfo, err error) {
+ // TODO(#39666): support downloaded modules that are "soft forks", where the
+ // module path in go.mod is different from modPath.
+
// Check the module path and version.
// If the version is a query, resolve it to a canonical version.
m = moduleInfo{modPath: modPath}
@@ -474,18 +499,12 @@
// version control tag to use (with an appropriate prefix, for modules not
// in the repository root directory).
func makeReleaseReport(base, release moduleInfo) (report, error) {
- if base.modPath != release.modPath {
- // TODO(golang.org/issue/39666): allow base and release path to be different.
- panic(fmt.Sprintf("base module path %q is different than release module path %q", base.modPath, release.modPath))
- }
- modPath := release.modPath
-
// Compare each pair of packages.
// Ignore internal packages.
// If we don't have a base version to compare against,
// just check the new packages for errors.
shouldCompare := base.version != "none"
- isInternal := func(pkgPath string) bool {
+ isInternal := func(modPath, pkgPath string) bool {
if !hasPathPrefix(pkgPath, modPath) {
panic(fmt.Sprintf("package %s not in module %s", pkgPath, modPath))
}
@@ -501,17 +520,17 @@
base: base,
release: release,
}
- for _, pair := range zipPackages(base.pkgs, release.pkgs) {
+ for _, pair := range zipPackages(base.modPath, base.pkgs, release.modPath, release.pkgs) {
basePkg, releasePkg := pair.base, pair.release
switch {
case releasePkg == nil:
// Package removed
- if !isInternal(basePkg.PkgPath) || len(basePkg.Errors) > 0 {
+ if internal := isInternal(base.modPath, basePkg.PkgPath); !internal || len(basePkg.Errors) > 0 {
pr := packageReport{
path: basePkg.PkgPath,
baseErrors: basePkg.Errors,
}
- if !isInternal(basePkg.PkgPath) {
+ if !internal {
pr.Report = apidiff.Report{
Changes: []apidiff.Change{{
Message: "package removed",
@@ -524,12 +543,12 @@
case basePkg == nil:
// Package added
- if !isInternal(releasePkg.PkgPath) && shouldCompare || len(releasePkg.Errors) > 0 {
+ if internal := isInternal(release.modPath, releasePkg.PkgPath); !internal && shouldCompare || len(releasePkg.Errors) > 0 {
pr := packageReport{
path: releasePkg.PkgPath,
releaseErrors: releasePkg.Errors,
}
- if !isInternal(releasePkg.PkgPath) && shouldCompare {
+ if !internal && shouldCompare {
// If we aren't comparing against a base version, don't say
// "package added". Only report packages with errors.
pr.Report = apidiff.Report{
@@ -544,7 +563,10 @@
default:
// Matched packages
- if !isInternal(basePkg.PkgPath) && basePkg.Name != "main" && releasePkg.Name != "main" {
+ // Both packages are internal or neither; we only consider path components
+ // after the module path.
+ internal := isInternal(release.modPath, releasePkg.PkgPath)
+ if !internal && basePkg.Name != "main" && releasePkg.Name != "main" {
pr := packageReport{
path: basePkg.PkgPath,
baseErrors: basePkg.Errors,
@@ -558,7 +580,7 @@
if release.version != "" {
r.validateVersion()
- } else {
+ } else if r.similarModPaths() {
r.suggestVersion()
}
@@ -1056,24 +1078,27 @@
// If a package is in one list but not the other (because it was added or
// removed between releases), a pair will be returned with a nil
// base or release field.
-func zipPackages(basePkgs, releasePkgs []*packages.Package) []packagePair {
+func zipPackages(baseModPath string, basePkgs []*packages.Package, releaseModPath string, releasePkgs []*packages.Package) []packagePair {
baseIndex, releaseIndex := 0, 0
var pairs []packagePair
for baseIndex < len(basePkgs) || releaseIndex < len(releasePkgs) {
var basePkg, releasePkg *packages.Package
+ var baseSuffix, releaseSuffix string
if baseIndex < len(basePkgs) {
basePkg = basePkgs[baseIndex]
+ baseSuffix = trimPathPrefix(basePkg.PkgPath, baseModPath)
}
if releaseIndex < len(releasePkgs) {
releasePkg = releasePkgs[releaseIndex]
+ releaseSuffix = trimPathPrefix(releasePkg.PkgPath, releaseModPath)
}
var pair packagePair
- if basePkg != nil && (releasePkg == nil || basePkg.PkgPath < releasePkg.PkgPath) {
+ if basePkg != nil && (releasePkg == nil || baseSuffix < releaseSuffix) {
// Package removed
pair = packagePair{basePkg, nil}
baseIndex++
- } else if releasePkg != nil && (basePkg == nil || releasePkg.PkgPath < basePkg.PkgPath) {
+ } else if releasePkg != nil && (basePkg == nil || releaseSuffix < baseSuffix) {
// Package added
pair = packagePair{nil, releasePkg}
releaseIndex++
diff --git a/cmd/gorelease/report.go b/cmd/gorelease/report.go
index cea58a4..fdf6d3e 100644
--- a/cmd/gorelease/report.go
+++ b/cmd/gorelease/report.go
@@ -65,10 +65,14 @@
}
}
+ baseVersion := r.base.version
+ if r.base.modPath != r.release.modPath {
+ baseVersion = r.base.modPath + "@" + baseVersion
+ }
if r.base.versionInferred {
- fmt.Fprintf(buf, "Inferred base version: %s\n", r.base.version)
+ fmt.Fprintf(buf, "Inferred base version: %s\n", baseVersion)
} else if r.base.versionQuery != "" {
- fmt.Fprintf(buf, "Base version: %s (%s)\n", r.base.version, r.base.versionQuery)
+ fmt.Fprintf(buf, "Base version: %s (%s)\n", baseVersion, r.base.versionQuery)
}
if len(r.release.diagnostics) > 0 {
@@ -83,7 +87,7 @@
} else {
fmt.Fprintf(buf, "Suggested version: %[1]s (with tag %[2]s%[1]s)\n", r.release.version, r.release.tagPrefix)
}
- } else {
+ } else if r.release.version != "" && r.similarModPaths() {
if r.release.tagPrefix == "" {
fmt.Fprintf(buf, "%s is a valid semantic version for this release.\n", r.release.version)
@@ -175,7 +179,7 @@
}
// Check that compatible / incompatible changes are consistent.
- if semver.Major(r.base.version) == "v0" {
+ if semver.Major(r.base.version) == "v0" || r.base.modPath != r.release.modPath {
return
}
if r.haveIncompatibleChanges {
@@ -202,6 +206,11 @@
r.release.versionInferred = true
}
+ if r.base.modPath != r.release.modPath {
+ setNotValid("Base module path is different from release.")
+ return
+ }
+
if r.haveReleaseErrors || r.haveBaseErrors {
setNotValid("Errors were found.")
return
@@ -245,6 +254,14 @@
setVersion(fmt.Sprintf("v%s.%s.%s", major, minor, patch))
}
+// similarModPaths returns true if r.base and r.release are versions of the same
+// module or different major versions of the same module.
+func (r *report) similarModPaths() bool {
+ basePath := strings.TrimSuffix(r.base.modPath, r.base.modPathMajor)
+ releasePath := strings.TrimSuffix(r.release.modPath, r.release.modPathMajor)
+ return basePath == releasePath
+}
+
// requirementsChanged reports whether requirements have changed from base to
// version.
//
diff --git a/cmd/gorelease/testdata/basic/v1_fork_base_modpath_version_verify.test b/cmd/gorelease/testdata/basic/v1_fork_base_modpath_version_verify.test
new file mode 100644
index 0000000..eaa57be
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v1_fork_base_modpath_version_verify.test
@@ -0,0 +1,11 @@
+mod=example.com/basicfork
+base=example.com/basic@v1.1.1
+version=v1.1.2
+release=v1.1.2
+-- want --
+example.com/basic/a
+-------------------
+Incompatible changes:
+- A2: removed
+Compatible changes:
+- A3: added
diff --git a/cmd/gorelease/testdata/basic/v1_v2_base_modpath_query_verify.test b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_query_verify.test
new file mode 100644
index 0000000..1d56841
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_query_verify.test
@@ -0,0 +1,17 @@
+mod=example.com/basic/v2
+base=example.com/basic@>=v1.1.0
+version=v2.0.1
+release=v2.0.1
+-- want --
+example.com/basic/a
+-------------------
+Incompatible changes:
+- A2: removed
+
+example.com/basic/b
+-------------------
+Incompatible changes:
+- package removed
+
+Base version: example.com/basic@v1.1.0 (>=v1.1.0)
+v2.0.1 is a valid semantic version for this release.
diff --git a/cmd/gorelease/testdata/basic/v1_v2_base_modpath_suggest.test b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_suggest.test
new file mode 100644
index 0000000..5f916a9
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_suggest.test
@@ -0,0 +1,18 @@
+mod=example.com/basic/v2
+base=example.com/basic
+version=v2.1.0
+success=false
+-- want --
+example.com/basic/a
+-------------------
+Compatible changes:
+- A2: added
+
+example.com/basic/v2/b
+----------------------
+Compatible changes:
+- package added
+
+Inferred base version: example.com/basic@v1.1.2
+Cannot suggest a release version.
+Base module path is different from release.
diff --git a/cmd/gorelease/testdata/basic/v1_v2_base_modpath_verify.test b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_verify.test
new file mode 100644
index 0000000..6533f3a
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_verify.test
@@ -0,0 +1,17 @@
+mod=example.com/basic/v2
+base=example.com/basic
+version=v2.1.0
+release=v2.1.0
+-- want --
+example.com/basic/a
+-------------------
+Compatible changes:
+- A2: added
+
+example.com/basic/v2/b
+----------------------
+Compatible changes:
+- package added
+
+Inferred base version: example.com/basic@v1.1.2
+v2.1.0 is a valid semantic version for this release.
diff --git a/cmd/gorelease/testdata/basic/v1_v2_base_modpath_version_verify.test b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_version_verify.test
new file mode 100644
index 0000000..22ed2f4
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v1_v2_base_modpath_version_verify.test
@@ -0,0 +1,16 @@
+mod=example.com/basic/v2
+base=example.com/basic@v1.1.0
+version=v2.0.1
+release=v2.0.1
+-- want --
+example.com/basic/a
+-------------------
+Incompatible changes:
+- A2: removed
+
+example.com/basic/b
+-------------------
+Incompatible changes:
+- package removed
+
+v2.0.1 is a valid semantic version for this release.
diff --git a/cmd/gorelease/testdata/errors/base_modpath_none.test b/cmd/gorelease/testdata/errors/base_modpath_none.test
new file mode 100644
index 0000000..bb438d9
--- /dev/null
+++ b/cmd/gorelease/testdata/errors/base_modpath_none.test
@@ -0,0 +1,8 @@
+mod=example.com/basic/v2
+base=example.com/basic@none
+error=true
+
+-- want --
+usage: gorelease [-base=version] [-version=version]
+base version ("example.com/basic@none") cannot have version "none" with explicit module path
+For more information, run go doc golang.org/x/exp/cmd/gorelease
diff --git a/cmd/gorelease/testdata/internalcompat/README.txt b/cmd/gorelease/testdata/internalcompat/README.txt
new file mode 100644
index 0000000..e3de7e5
--- /dev/null
+++ b/cmd/gorelease/testdata/internalcompat/README.txt
@@ -0,0 +1,20 @@
+Modules example.com/internalcompat/{a,b} are copies. One could be a fork
+of the other. An external package p exposes a type from a package q
+within the same module.
+
+gorelease should not report differences between these packages. The types
+are distinct, but they correspond (in apidiff terminology), which is the
+important property when considering differences between modules.
+
+There are three use cases to consider:
+
+1. One module substitutes for the other via a `replace` directive.
+ Only the replacement module is used, and the package paths are effectively
+ identical, so the types are not distinct.
+2. One module subsititutes for the other by rewriting `import` statements
+ globally. All references to the original type become references to the
+ new type, so there is no conflict.
+3. One module substitutes for the other by rewriting some `import` statements
+ but not others (for example, those within a specific consumer package).
+ In this case, the types are distinct, and even if there are no changes,
+ the types are not compatible.
diff --git a/cmd/gorelease/testdata/internalcompat/internalcompat.test b/cmd/gorelease/testdata/internalcompat/internalcompat.test
new file mode 100644
index 0000000..8bc32fa
--- /dev/null
+++ b/cmd/gorelease/testdata/internalcompat/internalcompat.test
@@ -0,0 +1,6 @@
+mod=example.com/internalcompat/b
+version=v1.0.0
+release=v1.0.0
+base=example.com/internalcompat/a@v1.0.0
+
+-- want --
diff --git a/cmd/gorelease/testdata/mod/example.com_basicfork_v1.1.2.txt b/cmd/gorelease/testdata/mod/example.com_basicfork_v1.1.2.txt
new file mode 100644
index 0000000..63cb887
--- /dev/null
+++ b/cmd/gorelease/testdata/mod/example.com_basicfork_v1.1.2.txt
@@ -0,0 +1,13 @@
+-- go.mod --
+module example.com/basicfork
+
+go 1.12
+-- a/a.go --
+package a
+
+func A() int { return 1 }
+func A3() int { return 4 }
+-- b/b.go --
+package b
+
+func B() int { return 3 }
diff --git a/cmd/gorelease/testdata/mod/example.com_internalcompat_a_v1.0.0.txt b/cmd/gorelease/testdata/mod/example.com_internalcompat_a_v1.0.0.txt
new file mode 100644
index 0000000..5f15418
--- /dev/null
+++ b/cmd/gorelease/testdata/mod/example.com_internalcompat_a_v1.0.0.txt
@@ -0,0 +1,16 @@
+-- go.mod --
+module example.com/internalcompat/a
+
+go 1.15
+
+-- p/p.go --
+package p
+
+import "example.com/internalcompat/a/q"
+
+var V q.Q
+
+-- q/q.go --
+package q
+
+type Q struct{}
diff --git a/cmd/gorelease/testdata/mod/example.com_internalcompat_b_v1.0.0.txt b/cmd/gorelease/testdata/mod/example.com_internalcompat_b_v1.0.0.txt
new file mode 100644
index 0000000..318db36
--- /dev/null
+++ b/cmd/gorelease/testdata/mod/example.com_internalcompat_b_v1.0.0.txt
@@ -0,0 +1,16 @@
+-- go.mod --
+module example.com/internalcompat/b
+
+go 1.15
+
+-- p/p.go --
+package p
+
+import "example.com/internalcompat/b/q"
+
+var V q.Q
+
+-- q/q.go --
+package q
+
+type Q struct{}
