cmd/gorelease: support comparing replacement modules
Downloaded modules (currently, those fetched according to the -base
flag) may now have module paths in go.mod different than the module
path used to fetch them with 'go mod download'. This lets gorelease
compare modules that have been copied to another location (e.g., a
soft fork). The module path in go.mod is used when loading packages.
Fixes golang/go#39666
Change-Id: I33bbdab3fe5c4374f749fb965e9cc7339a1f6a8f
Reviewed-on: https://go-review.googlesource.com/c/exp/+/277116
Trust: Jay Conrod <jayconrod@google.com>
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jean de Klerk <deklerk@google.com>
diff --git a/cmd/gorelease/gorelease.go b/cmd/gorelease/gorelease.go
index 83ed99c..005984c 100644
--- a/cmd/gorelease/gorelease.go
+++ b/cmd/gorelease/gorelease.go
@@ -243,7 +243,7 @@
type moduleInfo struct {
modRoot string // module root directory
repoRoot string // repository root directory (may be "")
- modPath string // module path
+ 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
@@ -393,7 +393,9 @@
// loadDownloadedModule downloads a module and loads information about it and
// its packages from the module cache.
//
-// modPath is the module's path.
+// modPath is the module path used to fetch the module. The module's path in
+// go.mod (m.modPath) may be different, for example in a soft fork intended as
+// a replacement.
//
// version is the version to load. It may be "none" (indicating nothing should
// be loaded), "" (the highest available version below max should be used), a
@@ -403,9 +405,6 @@
// 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}
@@ -414,10 +413,10 @@
}
var ok bool
- _, m.modPathMajor, ok = module.SplitPathVersion(m.modPath)
+ _, m.modPathMajor, ok = module.SplitPathVersion(modPath)
if !ok {
// we just validated the path above.
- panic(fmt.Sprintf("could not find version suffix in module path %q", m.modPath))
+ panic(fmt.Sprintf("could not find version suffix in module path %q", modPath))
}
if version == "none" {
@@ -457,12 +456,27 @@
m.version = version
}
- // Load packages.
+ // Download the module into the cache and load the mod file.
+ // Note that goModPath is $GOMODCACHE/cache/download/$modPath/@v/$version.mod,
+ // which is not inside modRoot. This is what the go command uses. Even if
+ // the module didn't have a go.mod file, one will be synthesized there.
v := module.Version{Path: modPath, Version: m.version}
- if m.modRoot, err = downloadModule(v); err != nil {
+ if m.modRoot, m.goModPath, err = downloadModule(v); err != nil {
return moduleInfo{}, err
}
- tmpLoadDir, tmpGoModData, tmpGoSumData, err := prepareLoadDir(nil, modPath, m.modRoot, m.version, true)
+ if m.goModData, err = ioutil.ReadFile(m.goModPath); err != nil {
+ return moduleInfo{}, err
+ }
+ if m.goModFile, err = modfile.ParseLax(m.goModPath, m.goModData, nil); err != nil {
+ return moduleInfo{}, err
+ }
+ if m.goModFile.Module == nil {
+ return moduleInfo{}, fmt.Errorf("%s: missing module directive", m.goModPath)
+ }
+ m.modPath = m.goModFile.Module.Mod.Path
+
+ // Load packages.
+ tmpLoadDir, tmpGoModData, tmpGoSumData, err := prepareLoadDir(nil, m.modPath, m.modRoot, m.version, true)
if err != nil {
return moduleInfo{}, err
}
@@ -471,23 +485,10 @@
err = fmt.Errorf("removing temporary load directory: %v", err)
}
}()
- if m.pkgs, _, err = loadPackages(modPath, m.modRoot, tmpLoadDir, tmpGoModData, tmpGoSumData); err != nil {
+ if m.pkgs, _, err = loadPackages(m.modPath, m.modRoot, tmpLoadDir, tmpGoModData, tmpGoSumData); err != nil {
return moduleInfo{}, err
}
- // Attempt to load the mod file, if it exists.
- m.goModPath = filepath.Join(m.modRoot, "go.mod")
- if m.goModData, err = ioutil.ReadFile(m.goModPath); err != nil && !os.IsNotExist(err) {
- return moduleInfo{}, fmt.Errorf("reading go.mod: %v", err)
- }
- if err == nil {
- m.goModFile, err = modfile.ParseLax(m.goModPath, m.goModData, nil)
- if err != nil {
- return moduleInfo{}, err
- }
- }
- // The modfile might not exist, leading to err != nil. That's OK - continue.
-
return m, nil
}
@@ -578,10 +579,12 @@
}
}
- if release.version != "" {
- r.validateVersion()
- } else if r.similarModPaths() {
- r.suggestVersion()
+ if r.canVerifyReleaseVersion() {
+ if release.version == "" {
+ r.suggestReleaseVersion()
+ } else {
+ r.validateReleaseVersion()
+ }
}
return r, nil
@@ -825,7 +828,7 @@
// downloadModule downloads a specific version of a module to the
// module cache using 'go mod download'.
-func downloadModule(m module.Version) (modRoot string, err error) {
+func downloadModule(m module.Version) (modRoot, goModPath string, err error) {
defer func() {
if err != nil {
err = &downloadError{m: m, err: cleanCmdError(err)}
@@ -840,7 +843,7 @@
// If it didn't read go.mod in this case, we wouldn't need a temp directory.
tmpDir, err := ioutil.TempDir("", "gorelease-download")
if err != nil {
- return "", err
+ return "", "", err
}
defer os.Remove(tmpDir)
cmd := exec.Command("go", "mod", "download", "-json", "--", m.Path+"@"+m.Version)
@@ -850,26 +853,26 @@
if err != nil {
var ok bool
if xerr, ok = err.(*exec.ExitError); !ok {
- return "", err
+ return "", "", err
}
}
// If 'go mod download' exited unsuccessfully but printed well-formed JSON
// with an error, return that error.
- parsed := struct{ Dir, Error string }{}
+ parsed := struct{ Dir, GoMod, Error string }{}
if jsonErr := json.Unmarshal(out, &parsed); jsonErr != nil {
if xerr != nil {
- return "", cleanCmdError(xerr)
+ return "", "", cleanCmdError(xerr)
}
- return "", jsonErr
+ return "", "", jsonErr
}
if parsed.Error != "" {
- return "", errors.New(parsed.Error)
+ return "", "", errors.New(parsed.Error)
}
if xerr != nil {
- return "", cleanCmdError(xerr)
+ return "", "", cleanCmdError(xerr)
}
- return parsed.Dir, nil
+ return parsed.Dir, parsed.GoMod, nil
}
// prepareLoadDir creates a temporary directory and a go.mod file that requires
diff --git a/cmd/gorelease/report.go b/cmd/gorelease/report.go
index fdf6d3e..ee95d13 100644
--- a/cmd/gorelease/report.go
+++ b/cmd/gorelease/report.go
@@ -87,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 if r.release.version != "" && r.similarModPaths() {
+ } else if r.release.version != "" && r.canVerifyReleaseVersion() {
if r.release.tagPrefix == "" {
fmt.Fprintf(buf, "%s is a valid semantic version for this release.\n", r.release.version)
@@ -131,10 +131,10 @@
}
}
-// validateVersion checks whether r.release.version is valid.
+// validateReleaseVersion checks whether r.release.version is valid.
// If r.release.version is not valid, an error is returned explaining why.
// r.release.version must be set.
-func (r *report) validateVersion() {
+func (r *report) validateReleaseVersion() {
if r.release.version == "" {
panic("validateVersion called without version")
}
@@ -193,8 +193,9 @@
}
}
-// suggestVersion suggests a new version consistent with observed changes.
-func (r *report) suggestVersion() {
+// suggestReleaseVersion suggests a new version consistent with observed
+// changes.
+func (r *report) suggestReleaseVersion() {
setNotValid := func(format string, args ...interface{}) {
r.versionInvalid = &versionMessage{
message: "Cannot suggest a release version.",
@@ -254,9 +255,13 @@
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 {
+// canVerifyReleaseVersion returns true if we can safely suggest a new version
+// or if we can verify the version passed in with -version is safe to tag.
+func (r *report) canVerifyReleaseVersion() bool {
+ // For now, return true if the base and release module paths are the same,
+ // ignoring the major version suffix.
+ // TODO(#37562, #39192, #39666, #40267): there are many more situations when
+ // we can't verify a new version.
basePath := strings.TrimSuffix(r.base.modPath, r.base.modPathMajor)
releasePath := strings.TrimSuffix(r.release.modPath, r.release.modPathMajor)
return basePath == releasePath
diff --git a/cmd/gorelease/testdata/basic/v1_fork_base_verify.test b/cmd/gorelease/testdata/basic/v1_fork_base_verify.test
new file mode 100644
index 0000000..8840112
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v1_fork_base_verify.test
@@ -0,0 +1,17 @@
+# Compare a fork (with module path example.com/basic, downloaded from
+# example.com/basicfork) with a local module (with module path
+# example.com/basic).
+mod=example.com/basic
+version=v1.1.2
+base=example.com/basicfork@v1.1.2
+release=v1.1.2
+-- want --
+example.com/basicfork/a
+-----------------------
+Incompatible changes:
+- A3: removed
+
+example.com/basicfork/b
+-----------------------
+Incompatible changes:
+- package removed
