cmd/go/internal/modload: don't report path errors when loading retractions

When we load module retractions from the latest version of a module,
it's possible that the latest version has a different module path than
the version we're loading. This can happen when a module is renamed or
a go.mod file is added for the first time.

We should not report an error in this case. Retractions should still
apply to old aliases of a module.

Fixes #41350

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

1 files changed, 138 insertions, 116 deletions

diff --git a/src/cmd/go/internal/modload/modfile.go b/src/cmd/go/internal/modload/modfile.go
index 18dd293ac9..6457a7d968 100644
--- a/src/cmd/go/internal/modload/modfile.go
+++ b/src/cmd/go/internal/modload/modfile.go
@@ -122,12 +122,21 @@ func checkRetractions(ctx context.Context, m module.Version) error {
 
 		// Load go.mod for that version.
 		// If the version is replaced, we'll load retractions from the replacement.
+		//
 		// If there's an error loading the go.mod, we'll return it here.
 		// These errors should generally be ignored by callers of checkRetractions,
 		// since they happen frequently when we're offline. These errors are not
 		// equivalent to ErrDisallowed, so they may be distinguished from
 		// retraction errors.
-		summary, err := goModSummary(module.Version{Path: path, Version: rev.Version})
+		//
+		// We load the raw file here: the go.mod file may have a different module
+		// path that we expect if the module or its repository was renamed.
+		// We still want to apply retractions to other aliases of the module.
+		rm := module.Version{Path: path, Version: rev.Version}
+		if repl := Replacement(rm); repl.Path != "" {
+			rm = repl
+		}
+		summary, err := rawGoModSummary(rm)
 		if err != nil {
 			return &entry{err: err}
 		}
@@ -378,87 +387,90 @@ func goModSummary(m module.Version) (*modFileSummary, error) {
 		panic("internal error: goModSummary called on the Target module")
 	}
 
-	type cached struct {
-		summary *modFileSummary
-		err     error
-	}
-	c := goModSummaryCache.Do(m, func() interface{} {
-		if cfg.BuildMod == "vendor" {
-			summary := &modFileSummary{
-				module: module.Version{Path: m.Path},
-			}
-			if vendorVersion[m.Path] != m.Version {
-				// This module is not vendored, so packages cannot be loaded from it and
-				// it cannot be relevant to the build.
-				return cached{summary, nil}
-			}
+	if cfg.BuildMod == "vendor" {
+		summary := &modFileSummary{
+			module: module.Version{Path: m.Path},
+		}
+		if vendorVersion[m.Path] != m.Version {
+			// This module is not vendored, so packages cannot be loaded from it and
+			// it cannot be relevant to the build.
+			return summary, nil
+		}
 
-			// For every module other than the target,
-			// return the full list of modules from modules.txt.
-			readVendorList()
+		// For every module other than the target,
+		// return the full list of modules from modules.txt.
+		readVendorList()
 
-			// TODO(#36876): Load the "go" version from vendor/modules.txt and store it
-			// in rawGoVersion with the appropriate key.
+		// TODO(#36876): Load the "go" version from vendor/modules.txt and store it
+		// in rawGoVersion with the appropriate key.
 
-			// We don't know what versions the vendored module actually relies on,
-			// so assume that it requires everything.
-			summary.require = vendorList
-			return cached{summary, nil}
-		}
+		// We don't know what versions the vendored module actually relies on,
+		// so assume that it requires everything.
+		summary.require = vendorList
+		return summary, nil
+	}
 
-		actual := Replacement(m)
-		if actual.Path == "" {
-			actual = m
-		}
-		summary, err := rawGoModSummary(actual)
-		if err != nil {
-			return cached{nil, err}
-		}
+	actual := Replacement(m)
+	if actual.Path == "" {
+		actual = m
+	}
+	summary, err := rawGoModSummary(actual)
+	if err != nil {
+		return nil, err
+	}
 
-		if actual.Version == "" {
-			// The actual module is a filesystem-local replacement, for which we have
-			// unfortunately not enforced any sort of invariants about module lines or
-			// matching module paths. Anything goes.
-			//
-			// TODO(bcmills): Remove this special-case, update tests, and add a
-			// release note.
-		} else {
-			if summary.module.Path == "" {
-				return cached{nil, module.VersionError(actual, errors.New("parsing go.mod: missing module line"))}
-			}
+	if actual.Version == "" {
+		// The actual module is a filesystem-local replacement, for which we have
+		// unfortunately not enforced any sort of invariants about module lines or
+		// matching module paths. Anything goes.
+		//
+		// TODO(bcmills): Remove this special-case, update tests, and add a
+		// release note.
+	} else {
+		if summary.module.Path == "" {
+			return nil, module.VersionError(actual, errors.New("parsing go.mod: missing module line"))
+		}
 
-			// In theory we should only allow mpath to be unequal to m.Path here if the
-			// version that we fetched lacks an explicit go.mod file: if the go.mod file
-			// is explicit, then it should match exactly (to ensure that imports of other
-			// packages within the module are interpreted correctly). Unfortunately, we
-			// can't determine that information from the module proxy protocol: we'll have
-			// to leave that validation for when we load actual packages from within the
-			// module.
-			if mpath := summary.module.Path; mpath != m.Path && mpath != actual.Path {
-				return cached{nil, module.VersionError(actual, fmt.Errorf(`parsing go.mod:
+		// In theory we should only allow mpath to be unequal to m.Path here if the
+		// version that we fetched lacks an explicit go.mod file: if the go.mod file
+		// is explicit, then it should match exactly (to ensure that imports of other
+		// packages within the module are interpreted correctly). Unfortunately, we
+		// can't determine that information from the module proxy protocol: we'll have
+		// to leave that validation for when we load actual packages from within the
+		// module.
+		if mpath := summary.module.Path; mpath != m.Path && mpath != actual.Path {
+			return nil, module.VersionError(actual, fmt.Errorf(`parsing go.mod:
 	module declares its path as: %s
-	        but was required as: %s`, mpath, m.Path))}
-			}
+	        but was required as: %s`, mpath, m.Path))
 		}
+	}
 
-		if index != nil && len(index.exclude) > 0 {
-			// Drop any requirements on excluded versions.
-			nonExcluded := summary.require[:0]
+	if index != nil && len(index.exclude) > 0 {
+		// Drop any requirements on excluded versions.
+		// Don't modify the cached summary though, since we might need the raw
+		// summary separately.
+		haveExcludedReqs := false
+		for _, r := range summary.require {
+			if index.exclude[r] {
+				haveExcludedReqs = true
+				break
+			}
+		}
+		if haveExcludedReqs {
+			s := new(modFileSummary)
+			*s = *summary
+			s.require = make([]module.Version, 0, len(summary.require))
 			for _, r := range summary.require {
 				if !index.exclude[r] {
-					nonExcluded = append(nonExcluded, r)
+					s.require = append(s.require, r)
 				}
 			}
-			summary.require = nonExcluded
+			summary = s
 		}
-		return cached{summary, nil}
-	}).(cached)
-
-	return c.summary, c.err
+	}
+	return summary, nil
 }
 
-var goModSummaryCache par.Cache // module.Version → goModSummary result
-
 // rawGoModSummary returns a new summary of the go.mod file for module m,
 // ignoring all replacements that may apply to m and excludes that may apply to
 // its dependencies.
@@ -469,62 +481,72 @@ func rawGoModSummary(m module.Version) (*modFileSummary, error) {
 		panic("internal error: rawGoModSummary called on the Target module")
 	}
 
-	summary := new(modFileSummary)
-	var f *modfile.File
-	if m.Version == "" {
-		// m is a replacement module with only a file path.
-		dir := m.Path
-		if !filepath.IsAbs(dir) {
-			dir = filepath.Join(ModRoot(), dir)
-		}
-		gomod := filepath.Join(dir, "go.mod")
+	type cached struct {
+		summary *modFileSummary
+		err     error
+	}
+	c := rawGoModSummaryCache.Do(m, func() interface{} {
+		summary := new(modFileSummary)
+		var f *modfile.File
+		if m.Version == "" {
+			// m is a replacement module with only a file path.
+			dir := m.Path
+			if !filepath.IsAbs(dir) {
+				dir = filepath.Join(ModRoot(), dir)
+			}
+			gomod := filepath.Join(dir, "go.mod")
 
-		data, err := lockedfile.Read(gomod)
-		if err != nil {
-			return nil, module.VersionError(m, fmt.Errorf("reading %s: %v", base.ShortPath(gomod), err))
-		}
-		f, err = modfile.ParseLax(gomod, data, nil)
-		if err != nil {
-			return nil, module.VersionError(m, fmt.Errorf("parsing %s: %v", base.ShortPath(gomod), err))
-		}
-	} else {
-		if !semver.IsValid(m.Version) {
-			// Disallow the broader queries supported by fetch.Lookup.
-			base.Fatalf("go: internal error: %s@%s: unexpected invalid semantic version", m.Path, m.Version)
+			data, err := lockedfile.Read(gomod)
+			if err != nil {
+				return cached{nil, module.VersionError(m, fmt.Errorf("reading %s: %v", base.ShortPath(gomod), err))}
+			}
+			f, err = modfile.ParseLax(gomod, data, nil)
+			if err != nil {
+				return cached{nil, module.VersionError(m, fmt.Errorf("parsing %s: %v", base.ShortPath(gomod), err))}
+			}
+		} else {
+			if !semver.IsValid(m.Version) {
+				// Disallow the broader queries supported by fetch.Lookup.
+				base.Fatalf("go: internal error: %s@%s: unexpected invalid semantic version", m.Path, m.Version)
+			}
+
+			data, err := modfetch.GoMod(m.Path, m.Version)
+			if err != nil {
+				return cached{nil, err}
+			}
+			f, err = modfile.ParseLax("go.mod", data, nil)
+			if err != nil {
+				return cached{nil, module.VersionError(m, fmt.Errorf("parsing go.mod: %v", err))}
+			}
 		}
 
-		data, err := modfetch.GoMod(m.Path, m.Version)
-		if err != nil {
-			return nil, err
+		if f.Module != nil {
+			summary.module = f.Module.Mod
 		}
-		f, err = modfile.ParseLax("go.mod", data, nil)
-		if err != nil {
-			return nil, module.VersionError(m, fmt.Errorf("parsing go.mod: %v", err))
+		if f.Go != nil && f.Go.Version != "" {
+			rawGoVersion.LoadOrStore(m, f.Go.Version)
+			summary.goVersionV = "v" + f.Go.Version
 		}
-	}
-
-	if f.Module != nil {
-		summary.module = f.Module.Mod
-	}
-	if f.Go != nil && f.Go.Version != "" {
-		rawGoVersion.LoadOrStore(m, f.Go.Version)
-		summary.goVersionV = "v" + f.Go.Version
-	}
-	if len(f.Require) > 0 {
-		summary.require = make([]module.Version, 0, len(f.Require))
-		for _, req := range f.Require {
-			summary.require = append(summary.require, req.Mod)
+		if len(f.Require) > 0 {
+			summary.require = make([]module.Version, 0, len(f.Require))
+			for _, req := range f.Require {
+				summary.require = append(summary.require, req.Mod)
+			}
 		}
-	}
-	if len(f.Retract) > 0 {
-		summary.retract = make([]retraction, 0, len(f.Retract))
-		for _, ret := range f.Retract {
-			summary.retract = append(summary.retract, retraction{
-				VersionInterval: ret.VersionInterval,
-				Rationale:       ret.Rationale,
-			})
+		if len(f.Retract) > 0 {
+			summary.retract = make([]retraction, 0, len(f.Retract))
+			for _, ret := range f.Retract {
+				summary.retract = append(summary.retract, retraction{
+					VersionInterval: ret.VersionInterval,
+					Rationale:       ret.Rationale,
+				})
+			}
 		}
-	}
 
-	return summary, nil
+		return cached{summary, nil}
+	}).(cached)
+
+	return c.summary, c.err
 }
+
+var rawGoModSummaryCache par.Cache // module.Version → rawGoModSummary result