cmd/go/internal/modfetch: add detail to "missing go.mod" errors
Explain what's wrong better than "missing or invalid go.mod".
While we're here, improve the signal-to-noise ratio too.
Before this CL, using the scenario from golang/go#24522:
$ vgo get github.com/rsc/blackfriday/v2
go: github.com/rsc/blackfriday/v2 v2.0.1: missing or invalid go.mod
go get github.com/rsc/blackfriday/v2: missing or invalid go.mod
$ vgo get github.com/rsc/blackfriday/v2@v2.0.1-bad
go: github.com/rsc/blackfriday/v2 v2.0.1-bad: missing or invalid go.mod
go get github.com/rsc/blackfriday/v2@v2.0.1-bad: missing or invalid go.mod
After this CL:
$ vgo get github.com/rsc/blackfriday/v2
go: github.com/rsc/blackfriday/v2@v2.0.0: missing github.com/rsc/blackfriday/go.mod and .../v2/go.mod at revision v2.0.0
go: error loading module requirements
$ vgo get github.com/rsc/blackfriday/v2@v2.0.1-bad
go: github.com/rsc/blackfriday/v2@v2.0.1-bad: go.mod has non-.../v2 module path "github.com/rsc/blackfriday" (and .../v2/go.mod does not exist) at revision v2.0.1-bad
go: error loading module requirements
Fixes golang/go#24522.
Change-Id: I1471729cdde8345130d771f26b1501638b7452d2
Reviewed-on: https://go-review.googlesource.com/122399
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/vendor/cmd/go/internal/modfetch/coderepo.go b/vendor/cmd/go/internal/modfetch/coderepo.go
index daaea9f..33be117 100644
--- a/vendor/cmd/go/internal/modfetch/coderepo.go
+++ b/vendor/cmd/go/internal/modfetch/coderepo.go
@@ -12,7 +12,6 @@
"io/ioutil"
"os"
"path"
- "path/filepath"
"regexp"
"strings"
"time"
@@ -224,60 +223,100 @@
if err != nil {
return "", "", nil, err
}
- if r.pathMajor == "" || strings.HasPrefix(r.pathMajor, ".") {
- if r.codeDir == "" {
- return rev, "", nil, nil
- }
- file1 := path.Join(r.codeDir, "go.mod")
- gomod1, err1 := r.code.ReadFile(rev, file1, codehost.MaxGoMod)
- if err1 != nil {
- if os.IsNotExist(err1) {
- return "", "", nil, errors.New("missing go.mod")
- }
- return "", "", nil, fmt.Errorf("reading go.mod: %v", err1)
- }
- return rev, r.codeDir, gomod1, nil
- }
- // Suppose pathMajor is "/v2".
- // Either go.mod should claim v2 and v2/go.mod should not exist,
- // or v2/go.mod should exist and claim v2. Not both.
- // Note that we don't check the full path, just the major suffix,
- // because of replacement modules. This might be a fork of
- // the real module, found at a different path, usable only in
- // a replace directive.
+ // Load info about go.mod but delay consideration
+ // (except I/O error) until we rule out v2/go.mod.
file1 := path.Join(r.codeDir, "go.mod")
- file2 := path.Join(r.codeDir, r.pathMajor[1:], "go.mod")
gomod1, err1 := r.code.ReadFile(rev, file1, codehost.MaxGoMod)
- gomod2, err2 := r.code.ReadFile(rev, file2, codehost.MaxGoMod)
-
if err1 != nil && !os.IsNotExist(err1) {
- return "", "", nil, fmt.Errorf("reading %s: %v", file1, err1)
+ return "", "", nil, fmt.Errorf("reading %s/%s at revision %s: %v", r.pathPrefix, file1, rev, err1)
}
- if err2 != nil && !os.IsNotExist(err2) {
- return "", "", nil, fmt.Errorf("reading %s: %v", file2, err2)
+ mpath1 := modfile.ModulePath(gomod1)
+ found1 := err1 == nil && isMajor(mpath1, r.pathMajor)
+
+ var file2 string
+ if r.pathMajor != "" && !strings.HasPrefix(r.pathMajor, ".") {
+ // Suppose pathMajor is "/v2".
+ // Either go.mod should claim v2 and v2/go.mod should not exist,
+ // or v2/go.mod should exist and claim v2. Not both.
+ // Note that we don't check the full path, just the major suffix,
+ // because of replacement modules. This might be a fork of
+ // the real module, found at a different path, usable only in
+ // a replace directive.
+ dir2 := path.Join(r.codeDir, r.pathMajor[1:])
+ file2 = path.Join(dir2, "go.mod")
+ gomod2, err2 := r.code.ReadFile(rev, file2, codehost.MaxGoMod)
+ if err2 != nil && !os.IsNotExist(err2) {
+ return "", "", nil, fmt.Errorf("reading %s/%s at revision %s: %v", r.pathPrefix, file2, rev, err2)
+ }
+ mpath2 := modfile.ModulePath(gomod2)
+ found2 := err2 == nil && isMajor(mpath2, r.pathMajor)
+
+ if found1 && found2 {
+ return "", "", nil, fmt.Errorf("%s/%s and ...%s/go.mod both have ...%s module paths at revision %s", r.pathPrefix, file1, r.pathMajor, r.pathMajor, rev)
+ }
+ if found2 {
+ return rev, dir2, gomod2, nil
+ }
+ if err2 == nil {
+ if mpath2 == "" {
+ return "", "", nil, fmt.Errorf("%s/%s is missing module path at revision %s", r.pathPrefix, file2, rev)
+ }
+ return "", "", nil, fmt.Errorf("%s/%s has non-...%s module path %q at revision %s", r.pathPrefix, file2, r.pathMajor, mpath2, rev)
+ }
}
- found1 := err1 == nil && isMajor(gomod1, r.pathMajor)
- found2 := err2 == nil && isMajor(gomod2, r.pathMajor)
-
- if err2 == nil && !found2 {
- return "", "", nil, fmt.Errorf("%s has non-...%s module path", file2, r.pathMajor)
- }
- if found1 && found2 {
- return "", "", nil, fmt.Errorf("both %s and %s claim ...%s module", file1, file2, r.pathMajor)
- }
- if found2 {
- return rev, filepath.Join(r.codeDir, r.pathMajor), gomod2, nil
- }
+ // Not v2/go.mod, so it's either go.mod or nothing. Which is it?
if found1 {
+ // Explicit go.mod with matching module path OK.
return rev, r.codeDir, gomod1, nil
}
- return "", "", nil, fmt.Errorf("missing or invalid go.mod")
+ if err1 == nil {
+ // Explicit go.mod with non-matching module path disallowed.
+ suffix := ""
+ if file2 != "" {
+ suffix = fmt.Sprintf(" (and ...%s/go.mod does not exist)", r.pathMajor)
+ }
+ if mpath1 == "" {
+ return "", "", nil, fmt.Errorf("%s is missing module path%s at revision %s", file1, suffix, rev)
+ }
+ if r.pathMajor != "" { // ".v1", ".v2" for gopkg.in
+ return "", "", nil, fmt.Errorf("%s has non-...%s module path %q%s at revision %s", file1, r.pathMajor, mpath1, suffix, rev)
+ }
+ return "", "", nil, fmt.Errorf("%s has post-%s module path %q%s at revision %s", file1, semver.Major(version), mpath1, suffix, rev)
+ }
+
+ if r.codeDir == "" && (r.pathMajor == "" || strings.HasPrefix(r.pathMajor, ".")) {
+ // Implicit go.mod at root of repo OK for v0/v1 and for gopkg.in.
+ return rev, "", nil, nil
+ }
+
+ // Implicit go.mod below root of repo or at v2+ disallowed.
+ // Be clear about possibility of using either location for v2+.
+ if file2 != "" {
+ return "", "", nil, fmt.Errorf("missing %s/go.mod and ...%s/go.mod at revision %s", r.pathPrefix, r.pathMajor, rev)
+ }
+ return "", "", nil, fmt.Errorf("missing %s/go.mod at revision %s", r.pathPrefix, rev)
}
-func isMajor(gomod []byte, pathMajor string) bool {
- return strings.HasSuffix(modfile.ModulePath(gomod), pathMajor)
+func isMajor(mpath, pathMajor string) bool {
+ if mpath == "" {
+ return false
+ }
+ if pathMajor == "" {
+ // mpath must NOT have version suffix.
+ i := len(mpath)
+ for i > 0 && '0' <= mpath[i-1] && mpath[i-1] <= '9' {
+ i--
+ }
+ if i < len(mpath) && i >= 2 && mpath[i-1] == 'v' && mpath[i-2] == '/' {
+ // Found valid suffix.
+ return false
+ }
+ return true
+ }
+ // Otherwise pathMajor is ".v1", ".v2" (gopkg.in), or "/v2", "/v3" etc.
+ return strings.HasSuffix(mpath, pathMajor)
}
func (r *codeRepo) GoMod(version string) (data []byte, err error) {
diff --git a/vendor/cmd/go/internal/modfetch/coderepo_test.go b/vendor/cmd/go/internal/modfetch/coderepo_test.go
index fcf849b..ac88471 100644
--- a/vendor/cmd/go/internal/modfetch/coderepo_test.go
+++ b/vendor/cmd/go/internal/modfetch/coderepo_test.go
@@ -91,7 +91,7 @@
name: "80d85c5d4d17598a0e9055e7c175a32b415d6128",
short: "80d85c5d4d17",
time: time.Date(2018, 2, 19, 23, 10, 6, 0, time.UTC),
- ziperr: "missing or invalid go.mod",
+ ziperr: "missing github.com/rsc/vgotest1/go.mod and .../v2/go.mod at revision v2.0.0",
},
{
path: "github.com/rsc/vgotest1",
@@ -126,8 +126,8 @@
name: "80d85c5d4d17598a0e9055e7c175a32b415d6128",
short: "80d85c5d4d17",
time: time.Date(2018, 2, 19, 23, 10, 6, 0, time.UTC),
- gomoderr: "missing or invalid go.mod",
- ziperr: "missing or invalid go.mod",
+ gomoderr: "missing github.com/rsc/vgotest1/go.mod and .../v2/go.mod at revision v2.0.0",
+ ziperr: "missing github.com/rsc/vgotest1/go.mod and .../v2/go.mod at revision v2.0.0",
},
{
path: "github.com/rsc/vgotest1/v54321",
@@ -136,7 +136,7 @@
name: "80d85c5d4d17598a0e9055e7c175a32b415d6128",
short: "80d85c5d4d17",
time: time.Date(2018, 2, 19, 23, 10, 6, 0, time.UTC),
- ziperr: "missing or invalid go.mod",
+ ziperr: "missing github.com/rsc/vgotest1/go.mod and .../v54321/go.mod at revision 80d85c5d4d17",
},
{
path: "github.com/rsc/vgotest1/submod",
@@ -193,7 +193,7 @@
name: "f18795870fb14388a21ef3ebc1d75911c8694f31",
short: "f18795870fb1",
time: time.Date(2018, 2, 19, 23, 16, 4, 0, time.UTC),
- gomoderr: "v2/go.mod has non-.../v2 module path",
+ gomoderr: "github.com/rsc/vgotest1/v2/go.mod has non-.../v2 module path \"github.com/rsc/vgotest\" at revision v2.0.3",
},
{
path: "github.com/rsc/vgotest1/v2",
@@ -202,7 +202,7 @@
name: "1f863feb76bc7029b78b21c5375644838962f88d",
short: "1f863feb76bc",
time: time.Date(2018, 2, 20, 0, 3, 38, 0, time.UTC),
- gomoderr: "both go.mod and v2/go.mod claim .../v2 module",
+ gomoderr: "github.com/rsc/vgotest1/go.mod and .../v2/go.mod both have .../v2 module paths at revision v2.0.4",
},
{
path: "github.com/rsc/vgotest1/v2",
@@ -262,7 +262,7 @@
// Because it's a package, Stat should fail entirely.
path: "github.com/rsc/quote/buggy",
rev: "c4d4236f",
- err: "missing go.mod",
+ err: "missing github.com/rsc/quote/buggy/go.mod at revision c4d4236f9242",
},
{
path: "gopkg.in/yaml.v2",
@@ -441,6 +441,8 @@
tt.name = remap(tt.name, m)
tt.short = remap(tt.short, m)
tt.rev = remap(tt.rev, m)
+ tt.gomoderr = remap(tt.gomoderr, m)
+ tt.ziperr = remap(tt.ziperr, m)
t.Run(strings.Replace(tt.path, "/", "_", -1)+"/"+tt.rev, f)
tt = old
}
@@ -449,6 +451,7 @@
}
var hgmap = map[string]string{
+ "github.com/rsc/vgotest1/": "vcs-test.golang.org/hg/vgotest1.hg/",
"f18795870fb14388a21ef3ebc1d75911c8694f31": "a9ad6d1d14eb544f459f446210c7eb3b009807c6",
"ea65f87c8f52c15ea68f3bdd9925ef17e20d91e9": "f1fc0f22021b638d073d31c752847e7bf385def7",
"b769f2de407a4db81af9c5de0a06016d60d2ea09": "92c7eb888b4fac17f1c6bd2e1060a1b881a3b832",
@@ -469,8 +472,11 @@
}
}
}
- if i := strings.LastIndex(name, "-"); i >= 0 { // remap tail of pseudo-version
- return name[:i+1] + remap(name[i+1:], m)
+ for k, v := range m {
+ name = strings.Replace(name, k, v, -1)
+ if codehost.AllHex(k) {
+ name = strings.Replace(name, k[:12], v[:12], -1)
+ }
}
return name
}
@@ -598,7 +604,7 @@
},
{
path: "github.com/rsc/vgotest1/subdir",
- err: "missing go.mod",
+ err: "missing github.com/rsc/vgotest1/subdir/go.mod at revision a08abb797a67",
},
{
path: "swtch.com/testmod",
diff --git a/vendor/cmd/go/internal/modload/load.go b/vendor/cmd/go/internal/modload/load.go
index 8c1fac2..fe3f64d 100644
--- a/vendor/cmd/go/internal/modload/load.go
+++ b/vendor/cmd/go/internal/modload/load.go
@@ -98,7 +98,9 @@
paths = append(paths, "all") // will expand after load completes
case search.IsMetaPackage(pkg): // std, cmd
- fmt.Fprintf(os.Stderr, "go: warning: %q matches no packages when using modules\n", pkg)
+ list := search.AllPackages(pkg)
+ roots = append(roots, list...)
+ paths = append(paths, list...)
case strings.Contains(pkg, "..."):
// TODO: Don't we need to reevaluate this one last time once the build list stops changing?
@@ -830,11 +832,13 @@
gomod := filepath.Join(dir, "go.mod")
data, err := ioutil.ReadFile(gomod)
if err != nil {
- return nil, err
+ base.Errorf("go: parsing %s: %v", base.ShortPath(gomod), err)
+ return nil, ErrRequire
}
f, err := modfile.Parse(gomod, data, nil)
if err != nil {
- return nil, err
+ base.Errorf("go: parsing %s: %v", base.ShortPath(gomod), err)
+ return nil, ErrRequire
}
var list []module.Version
for _, r := range f.Require {
@@ -851,40 +855,41 @@
if !semver.IsValid(mod.Version) {
// Disallow the broader queries supported by fetch.Lookup.
- panic(fmt.Errorf("invalid semantic version %q for %s", mod.Version, mod.Path))
- // TODO: return nil, fmt.Errorf("invalid semantic version %q", mod.Version)
+ base.Fatalf("go: internal error: %s@%s: unexpected invalid semantic version", mod.Path, mod.Version)
}
data, err := modfetch.GoMod(mod.Path, mod.Version)
if err != nil {
- base.Errorf("go: %s %s: %v\n", mod.Path, mod.Version, err)
- return nil, err
+ base.Errorf("go: %s@%s: %v\n", mod.Path, mod.Version, err)
+ return nil, ErrRequire
}
f, err := modfile.Parse("go.mod", data, nil)
if err != nil {
- return nil, fmt.Errorf("parsing downloaded go.mod: %v", err)
+ base.Errorf("go: %s@%s: parsing go.mod: %v", mod.Path, mod.Version, err)
+ return nil, ErrRequire
}
if f.Module == nil {
- return nil, fmt.Errorf("%v@%v go.mod: missing module line", mod.Path, mod.Version)
+ base.Errorf("go: %s@%s: parsing go.mod: missing module line", mod.Path, mod.Version)
+ return nil, ErrRequire
}
if mpath := f.Module.Mod.Path; mpath != origPath && mpath != mod.Path {
- return nil, fmt.Errorf("downloaded %q and got module %q", mod.Path, mpath)
+ base.Errorf("go: %s@%s: parsing go.mod: unexpected module path %q", mod.Path, mod.Version, mpath)
+ return nil, ErrRequire
}
var list []module.Version
for _, req := range f.Require {
list = append(list, req.Mod)
}
- if false {
- fmt.Fprintf(os.Stderr, "REQLIST %v:\n", mod)
- for _, req := range list {
- fmt.Fprintf(os.Stderr, "\t%v\n", req)
- }
- }
return list, nil
}
+// ErrRequire is the sentinel error returned when Require encounters problems.
+// It prints the problems directly to standard error, so that multiple errors
+// can be displayed easily.
+var ErrRequire = errors.New("error loading module requirements")
+
func (*mvsReqs) Max(v1, v2 string) string {
if v1 != "" && semver.Compare(v1, v2) == -1 {
return v2
diff --git a/vendor/cmd/go/mod_test.go b/vendor/cmd/go/mod_test.go
index bd3d8db..bb303c0 100644
--- a/vendor/cmd/go/mod_test.go
+++ b/vendor/cmd/go/mod_test.go
@@ -842,7 +842,7 @@
require rsc.io/quote v1.2.0
`), 0666))
tg.cd(tg.path("x"))
-
+
tg.run("list", "-m", "-f={{.Main}}: {{.Dir}}")
tg.grepStdout(`^true: `, "expected main module to have Main=true")
tg.grepStdout(regexp.QuoteMeta(tg.path("x")), "expected Dir of main module to be present")
@@ -856,7 +856,10 @@
tg.run("list", "-m", "-f={{.Dir}}", "rsc.io/quote") // now module list should find it too
tg.grepStdout(`mod[\\/]rsc.io[\\/]quote@v1.2.0`, "expected cached copy of code")
-}
+
+ tg.run("list", "std")
+ tg.grepStdout("^math/big", "expected standard library")
+}
func TestModInitLegacy(t *testing.T) {
testenv.MustHaveExternalNetwork(t)
