Skip to content

Commit b2a5893

Browse files
author
Jay Conrod
committed
cmd/go: reduce redundancy in direct mode lookup error messages
get.RepoRootForImportPath now returns errors that satisfy load.ImportPathError in cases where the import path appears in the messages. (The import path probably should appear in all errors from this function, but this CL does not change these errors). Changed modfetch.notExistError to be a wrapper (with an Unwrap method) instead of a string. This means errors.As works with notFoundError and ImportPathError. ImportMissingError no longer prints the package path if it wraps an ImportPathError. TestMissingImportErrorRepetition no longer counts the package path within a URL (like https://...?go-get=1). Fixes golang#35986 Change-Id: I38f795191c46d04b542c553e705f23822260c790 Reviewed-on: https://go-review.googlesource.com/c/go/+/210338 Run-TryBot: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
1 parent a6c8fac commit b2a5893

File tree

5 files changed

+34
-12
lines changed

5 files changed

+34
-12
lines changed

‎src/cmd/go/internal/get/vcs.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"cmd/go/internal/base"
2323
"cmd/go/internal/cfg"
24+
"cmd/go/internal/load"
2425
"cmd/go/internal/web"
2526
)
2627

@@ -661,7 +662,7 @@ func RepoRootForImportPath(importPath string, mod ModuleMode, security web.Secur
661662
if err == errUnknownSite {
662663
rr, err = repoRootForImportDynamic(importPath, mod, security)
663664
if err != nil {
664-
err = fmt.Errorf("unrecognized import path %q: %v", importPath, err)
665+
err = load.ImportErrorf(importPath, "unrecognized import path %q: %v", importPath, err)
665666
}
666667
}
667668
if err != nil {
@@ -676,7 +677,7 @@ func RepoRootForImportPath(importPath string, mod ModuleMode, security web.Secur
676677
if err == nil && strings.Contains(importPath, "...") && strings.Contains(rr.Root, "...") {
677678
// Do not allow wildcards in the repo root.
678679
rr = nil
679-
err = fmt.Errorf("cannot expand ... in %q", importPath)
680+
err = load.ImportErrorf(importPath, "cannot expand ... in %q", importPath)
680681
}
681682
return rr, err
682683
}
@@ -700,7 +701,7 @@ func repoRootFromVCSPaths(importPath string, security web.SecurityMode, vcsPaths
700701
m := srv.regexp.FindStringSubmatch(importPath)
701702
if m == nil {
702703
if srv.prefix != "" {
703-
return nil, fmt.Errorf("invalid %s import path %q", srv.prefix, importPath)
704+
return nil, load.ImportErrorf(importPath, "invalid %s import path %q", srv.prefix, importPath)
704705
}
705706
continue
706707
}

‎src/cmd/go/internal/modfetch/coderepo.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e
359359
Path: r.modPath,
360360
Err: &module.InvalidVersionError{
361361
Version: info2.Version,
362-
Err: notExistError(err.Error()),
362+
Err: notExistError{err: err},
363363
},
364364
}
365365
}

‎src/cmd/go/internal/modfetch/repo.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,9 @@ func (lookupDisabledError) Error() string {
250250
var errLookupDisabled error = lookupDisabledError{}
251251

252252
var (
253-
errProxyOff = notExistError("module lookup disabled by GOPROXY=off")
254-
errNoproxy error = notExistError("disabled by GOPRIVATE/GONOPROXY")
255-
errUseProxy error = notExistError("path does not match GOPRIVATE/GONOPROXY")
253+
errProxyOff = notExistErrorf("module lookup disabled by GOPROXY=off")
254+
errNoproxy error = notExistErrorf("disabled by GOPRIVATE/GONOPROXY")
255+
errUseProxy error = notExistErrorf("path does not match GOPRIVATE/GONOPROXY")
256256
)
257257

258258
func lookupDirect(path string) (Repo, error) {
@@ -264,7 +264,7 @@ func lookupDirect(path string) (Repo, error) {
264264
rr, err := get.RepoRootForImportPath(path, get.PreferMod, security)
265265
if err != nil {
266266
// We don't know where to find code for a module with this path.
267-
return nil, notExistError(err.Error())
267+
return nil, notExistError{err: err}
268268
}
269269

270270
if rr.VCS == "mod" {
@@ -408,11 +408,22 @@ func (l *loggingRepo) Zip(dst io.Writer, version string) error {
408408
}
409409

410410
// A notExistError is like os.ErrNotExist, but with a custom message
411-
type notExistError string
411+
type notExistError struct {
412+
err error
413+
}
414+
415+
func notExistErrorf(format string, args ...interface{}) error {
416+
return notExistError{fmt.Errorf(format, args...)}
417+
}
412418

413419
func (e notExistError) Error() string {
414-
return string(e)
420+
return e.err.Error()
415421
}
422+
416423
func (notExistError) Is(target error) bool {
417424
return target == os.ErrNotExist
418425
}
426+
427+
func (e notExistError) Unwrap() error {
428+
return e.err
429+
}

‎src/cmd/go/internal/modload/import.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ func (e *ImportMissingError) Error() string {
4343
if str.HasPathPrefix(e.Path, "cmd") {
4444
return fmt.Sprintf("package %s is not in GOROOT (%s)", e.Path, filepath.Join(cfg.GOROOT, "src", e.Path))
4545
}
46+
if i := load.ImportPathError(nil); errors.As(e.QueryErr, &i) {
47+
return fmt.Sprintf("cannot find module: %v", e.QueryErr)
48+
}
4649
if e.QueryErr != nil {
4750
return fmt.Sprintf("cannot find module providing package %s: %v", e.Path, e.QueryErr)
4851
}

‎src/go/build/build_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,14 +515,21 @@ func TestMissingImportErrorRepetition(t *testing.T) {
515515
os.Setenv("GO111MODULE", "on")
516516
defer os.Setenv("GOPROXY", os.Getenv("GOPROXY"))
517517
os.Setenv("GOPROXY", "off")
518+
defer os.Setenv("GONOPROXY", os.Getenv("GONOPROXY"))
519+
os.Setenv("GONOPROXY", "none")
518520

519521
ctxt := Default
520522
ctxt.WorkingDir = tmp
521523

522524
pkgPath := "example.com/hello"
523-
if _, err = ctxt.Import(pkgPath, tmp, FindOnly); err == nil {
525+
_, err = ctxt.Import(pkgPath, tmp, FindOnly)
526+
if err == nil {
524527
t.Fatal("unexpected success")
525-
} else if n := strings.Count(err.Error(), pkgPath); n != 1 {
528+
}
529+
// Don't count the package path with a URL like https://...?go-get=1.
530+
// See golang.org/issue/35986.
531+
errStr := strings.ReplaceAll(err.Error(), "://"+pkgPath+"?go-get=1", "://...?go-get=1")
532+
if n := strings.Count(errStr, pkgPath); n != 1 {
526533
t.Fatalf("package path %q appears in error %d times; should appear once\nerror: %v", pkgPath, n, err)
527534
}
528535
}

0 commit comments

Comments
 (0)