Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
cmd/go: make hints in error messages more consistent
* All commands the user can run to fix the problem now appear alone on
  a separate line after a tab.
* Removed -d from 'go get' commands.
* Replaced 'go mod tidy' with 'go mod download $modpath' when a
  package might be provided by a module missing a sum.
* Errors about 'path@version' syntax are more explicit.

Fixes #29415
Fixes #42087
Fixes #43430
Fixes #43523

Change-Id: I4427c2c4506a727a2c727d652fd2d506bb134d3b
Reviewed-on: https://go-review.googlesource.com/c/go/+/282121
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
  • Loading branch information
Jay Conrod committed Jan 8, 2021
1 parent 25886cf commit 6192b98
Show file tree
Hide file tree
Showing 17 changed files with 42 additions and 36 deletions.
4 changes: 2 additions & 2 deletions src/cmd/go/go_test.go
Expand Up @@ -2655,12 +2655,12 @@ func TestBadCommandLines(t *testing.T) {
tg.tempFile("src/@x/x.go", "package x\n")
tg.setenv("GOPATH", tg.path("."))
tg.runFail("build", "@x")
tg.grepStderr("invalid input directory name \"@x\"|cannot use path@version syntax", "did not reject @x directory")
tg.grepStderr("invalid input directory name \"@x\"|can only use path@version syntax with 'go get' and 'go install' in module-aware mode", "did not reject @x directory")

tg.tempFile("src/@x/y/y.go", "package y\n")
tg.setenv("GOPATH", tg.path("."))
tg.runFail("build", "@x/y")
tg.grepStderr("invalid import path \"@x/y\"|cannot use path@version syntax", "did not reject @x/y import path")
tg.grepStderr("invalid import path \"@x/y\"|can only use path@version syntax with 'go get' and 'go install' in module-aware mode", "did not reject @x/y import path")

tg.tempFile("src/-x/x.go", "package x\n")
tg.setenv("GOPATH", tg.path("."))
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/get/get.go
Expand Up @@ -202,7 +202,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
func downloadPaths(patterns []string) []string {
for _, arg := range patterns {
if strings.Contains(arg, "@") {
base.Fatalf("go: cannot use path@version syntax in GOPATH mode")
base.Fatalf("go: can only use path@version syntax with 'go get' and 'go install' in module-aware mode")
continue
}

Expand Down
6 changes: 1 addition & 5 deletions src/cmd/go/internal/load/pkg.go
Expand Up @@ -769,11 +769,7 @@ func loadPackageData(path, parentPath, parentDir, parentRoot string, parentIsStd
}

if strings.Contains(path, "@") {
if cfg.ModulesEnabled {
return nil, false, errors.New("can only use path@version syntax with 'go get'")
} else {
return nil, false, errors.New("cannot use path@version syntax in GOPATH mode")
}
return nil, false, errors.New("can only use path@version syntax with 'go get' and 'go install' in module-aware mode")
}

// Determine canonical package path and directory.
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modget/get.go
Expand Up @@ -1558,7 +1558,7 @@ func (r *resolver) checkPackagesAndRetractions(ctx context.Context, pkgPatterns
}
}
if retractPath != "" {
fmt.Fprintf(os.Stderr, "go: run 'go get %s@latest' to switch to the latest unretracted version\n", retractPath)
fmt.Fprintf(os.Stderr, "go: to switch to the latest unretracted version, run:\n\tgo get %s@latest", retractPath)
}
}

Expand Down
14 changes: 8 additions & 6 deletions src/cmd/go/internal/modload/import.go
Expand Up @@ -134,6 +134,7 @@ func (e *AmbiguousImportError) Error() string {
// for its .zip file.
type ImportMissingSumError struct {
importPath string
modPaths []string
found, inAll bool
}

Expand All @@ -145,7 +146,7 @@ func (e *ImportMissingSumError) Error() string {
message = fmt.Sprintf("missing go.sum entry for module providing package %s", e.importPath)
}
if e.inAll {
return message + "; to add it:\n\tgo mod tidy"
return message + fmt.Sprintf("; to add it:\n\tgo mod download %s", strings.Join(e.modPaths, " "))
}
return message
}
Expand Down Expand Up @@ -238,7 +239,7 @@ func importFromBuildList(ctx context.Context, path string, buildList []module.Ve
// Check each module on the build list.
var dirs []string
var mods []module.Version
haveSumErr := false
var sumErrModPaths []string
for _, m := range buildList {
if !maybeInModule(path, m.Path) {
// Avoid possibly downloading irrelevant modules.
Expand All @@ -251,8 +252,9 @@ func importFromBuildList(ctx context.Context, path string, buildList []module.Ve
// We are missing a sum needed to fetch a module in the build list.
// We can't verify that the package is unique, and we may not find
// the package at all. Keep checking other modules to decide which
// error to report.
haveSumErr = true
// error to report. Multiple sums may be missing if we need to look in
// multiple nested modules to resolve the import; we'll report them all.
sumErrModPaths = append(sumErrModPaths, m.Path)
continue
}
// Report fetch error.
Expand All @@ -273,8 +275,8 @@ func importFromBuildList(ctx context.Context, path string, buildList []module.Ve
if len(mods) > 1 {
return module.Version{}, "", &AmbiguousImportError{importPath: path, Dirs: dirs, Modules: mods}
}
if haveSumErr {
return module.Version{}, "", &ImportMissingSumError{importPath: path, found: len(mods) > 0}
if len(sumErrModPaths) > 0 {
return module.Version{}, "", &ImportMissingSumError{importPath: path, modPaths: sumErrModPaths, found: len(mods) > 0}
}
if len(mods) == 1 {
return mods[0], dirs[0], nil
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modload/init.go
Expand Up @@ -380,7 +380,7 @@ func LoadModFile(ctx context.Context) {

if f.Module == nil {
// No module declaration. Must add module path.
base.Fatalf("go: no module declaration in go.mod.\n\tRun 'go mod edit -module=example.com/mod' to specify the module path.")
base.Fatalf("go: no module declaration in go.mod. To specify the module path:\n\tgo mod edit -module=example.com/mod")
}

if err := checkModulePathLax(f.Module.Mod.Path); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modload/load.go
Expand Up @@ -868,7 +868,7 @@ func loadFromRoots(params loaderParams) *loader {
// base.Errorf. Ideally, 'go list' should not fail because of this,
// but today, LoadPackages calls WriteGoMod unconditionally, which
// would fail with a less clear message.
base.Errorf("go: %[1]s: package %[2]s imported from implicitly required module; try 'go get -d %[1]s' to add missing requirements", pkg.path, dep.path)
base.Errorf("go: %[1]s: package %[2]s imported from implicitly required module; to add missing requirements, run:\n\tgo get %[2]s@%[3]s", pkg.path, dep.path, dep.mod.Version)
}
ld.direct[dep.mod.Path] = true
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modload/vendor.go
Expand Up @@ -214,6 +214,6 @@ func checkVendorConsistency() {
}

if vendErrors.Len() > 0 {
base.Fatalf("go: inconsistent vendoring in %s:%s\n\nrun 'go mod vendor' to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory", modRoot, vendErrors)
base.Fatalf("go: inconsistent vendoring in %s:%s\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor", modRoot, vendErrors)
}
}
2 changes: 1 addition & 1 deletion src/cmd/go/internal/test/testflag.go
Expand Up @@ -325,7 +325,7 @@ func testFlags(args []string) (packageNames, passToTest []string) {
if !testC {
buildFlag = "-i"
}
fmt.Fprintf(os.Stderr, "flag %s is not a 'go test' flag (unknown flags cannot be used with %s)\n", firstUnknownFlag, buildFlag)
fmt.Fprintf(os.Stderr, "go test: unknown flag %s cannot be used with %s\n", firstUnknownFlag, buildFlag)
exitWithUsage()
}

Expand Down
10 changes: 7 additions & 3 deletions src/cmd/go/testdata/script/mod_get_promote_implicit.txt
Expand Up @@ -6,17 +6,21 @@ cp go.mod.orig go.mod
go list -m indirect-with-pkg
stdout '^indirect-with-pkg v1.0.0 => ./indirect-with-pkg$'
! go list ./use-indirect
stderr '^go: m/use-indirect: package indirect-with-pkg imported from implicitly required module; try ''go get -d m/use-indirect'' to add missing requirements$'
stderr '^go: m/use-indirect: package indirect-with-pkg imported from implicitly required module; to add missing requirements, run:\n\tgo get indirect-with-pkg@v1.0.0$'

# We can promote the implicit requirement by getting the importing package,
# as hinted.
# We can promote the implicit requirement by getting the importing package.
# NOTE: the hint recommends getting the imported package (tested below) since
# it's more obvious and doesn't require -d. However, that adds an '// indirect'
# comment on the requirement.
go get -d m/use-indirect
cmp go.mod go.mod.use
cp go.mod.orig go.mod

# We can also promote implicit requirements using 'go get' on them, or their
# packages. This gives us "// indirect" requirements, since 'go get' doesn't
# know they're needed by the main module. See #43131 for the rationale.
# The hint above recommends this because it's more obvious usage and doesn't
# require the -d flag.
go get -d indirect-with-pkg indirect-without-pkg
cmp go.mod go.mod.indirect

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/testdata/script/mod_get_retract.txt
Expand Up @@ -11,7 +11,7 @@ cp go.mod.orig go.mod
go mod edit -require example.com/retract/self/prev@v1.9.0
go get -d example.com/retract/self/prev
stderr '^go: warning: example.com/retract/self/prev@v1.9.0: retracted by module author: self$'
stderr '^go: run ''go get example.com/retract/self/prev@latest'' to switch to the latest unretracted version$'
stderr '^go: to switch to the latest unretracted version, run:\n\tgo get example.com/retract/self/prev@latest$'
go list -m example.com/retract/self/prev
stdout '^example.com/retract/self/prev v1.9.0$'

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/testdata/script/mod_invalid_path.txt
Expand Up @@ -3,7 +3,7 @@
# Test that go list fails on a go.mod with no module declaration.
cd $WORK/gopath/src/mod
! go list .
stderr '^go: no module declaration in go.mod.\n\tRun ''go mod edit -module=example.com/mod'' to specify the module path.$'
stderr '^go: no module declaration in go.mod. To specify the module path:\n\tgo mod edit -module=example.com/mod$'

# Test that go mod init in GOPATH doesn't add a module declaration
# with a path that can't possibly be a module path, because
Expand Down
8 changes: 6 additions & 2 deletions src/cmd/go/testdata/script/mod_sum_ambiguous.txt
Expand Up @@ -25,13 +25,17 @@ cp go.sum.a-only go.sum
! go list example.com/ambiguous/a/b
stderr '^missing go.sum entry needed to verify package example.com/ambiguous/a/b is provided by exactly one module$'
! go list -deps .
stderr '^use.go:3:8: missing go.sum entry needed to verify package example.com/ambiguous/a/b is provided by exactly one module; to add it:\n\tgo mod tidy$'
stderr '^use.go:3:8: missing go.sum entry needed to verify package example.com/ambiguous/a/b is provided by exactly one module; to add it:\n\tgo mod download example.com/ambiguous/a/b$'

cp go.sum.b-only go.sum
! go list example.com/ambiguous/a/b
stderr '^missing go.sum entry for module providing package example.com/ambiguous/a/b$'
! go list -deps .
stderr '^use.go:3:8: missing go.sum entry for module providing package example.com/ambiguous/a/b; to add it:\n\tgo mod tidy$'
stderr '^use.go:3:8: missing go.sum entry for module providing package example.com/ambiguous/a/b; to add it:\n\tgo mod download example.com/ambiguous/a$'

cp go.sum.buildlist-only go.sum
! go list -deps .
stderr '^use.go:3:8: missing go.sum entry for module providing package example.com/ambiguous/a/b; to add it:\n\tgo mod download example.com/ambiguous/a example.com/ambiguous/a/b$'

-- go.mod --
module m
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/testdata/script/mod_sum_readonly.txt
Expand Up @@ -47,7 +47,7 @@ stderr '^missing go.sum entry for module providing package rsc.io/quote$'
# a package that imports it without that error.
go list -e -deps -f '{{.ImportPath}}{{with .Error}} {{.Err}}{{end}}' .
stdout '^m$'
stdout '^rsc.io/quote missing go.sum entry for module providing package rsc.io/quote; to add it:\n\tgo mod tidy$'
stdout '^rsc.io/quote missing go.sum entry for module providing package rsc.io/quote; to add it:\n\tgo mod download rsc.io/quote$'
! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.zip

# go.sum should not have been written.
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/go/testdata/script/mod_vendor_auto.txt
Expand Up @@ -66,7 +66,7 @@ stderr '^go: inconsistent vendoring in '$WORK[/\\]auto':$'
stderr '^\texample.com/printversion@v1.0.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt'
stderr '^\texample.com/unused: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
stderr '^\texample.com/version@v1.2.0: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
stderr '\n\nrun .go mod vendor. to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory$'
stderr '^\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor$'

# Module-specific subcommands should continue to load the full module graph.
go mod graph
Expand Down Expand Up @@ -135,7 +135,7 @@ stderr '^go: inconsistent vendoring in '$WORK[/\\]auto':$'
stderr '^\texample.com/printversion@v1.0.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt'
stderr '^\texample.com/unused: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
stderr '^\texample.com/version@v1.2.0: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
stderr '\n\nrun .go mod vendor. to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory$'
stderr '^\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor$'

# If -mod=vendor is set, limited consistency checks should apply even when
# the go version is 1.13 or earlier.
Expand All @@ -151,7 +151,7 @@ cp $WORK/modules-bad-1.13.txt vendor/modules.txt
! go list -mod=vendor -f {{.Dir}} -tags tools all
stderr '^go: inconsistent vendoring in '$WORK[/\\]auto':$'
stderr '^\texample.com/printversion@v1.0.0: is explicitly required in go.mod, but vendor/modules.txt indicates example.com/printversion@v1.1.0$'
stderr '\n\nrun .go mod vendor. to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory$'
stderr '^\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor$'

# If the go version is still 1.13, 'go mod vendor' should write a
# matching vendor/modules.txt containing the corrected 1.13 data.
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/go/testdata/script/mod_versions.txt
@@ -1,14 +1,14 @@
# Test rejection of pkg@version in GOPATH mode.
env GO111MODULE=off
! go get rsc.io/quote@v1.5.1
stderr 'cannot use path@version syntax in GOPATH mode'
stderr '^go: can only use path@version syntax with ''go get'' and ''go install'' in module-aware mode$'
! go build rsc.io/quote@v1.5.1
stderr 'cannot use path@version syntax in GOPATH mode'
stderr '^package rsc.io/quote@v1.5.1: can only use path@version syntax with ''go get'' and ''go install'' in module-aware mode$'

env GO111MODULE=on
cd x
! go build rsc.io/quote@v1.5.1
stderr 'can only use path@version syntax with ''go get'''
stderr '^package rsc.io/quote@v1.5.1: can only use path@version syntax with ''go get'' and ''go install'' in module-aware mode$'

-- x/go.mod --
module x
6 changes: 3 additions & 3 deletions src/cmd/go/testdata/script/test_flag.txt
Expand Up @@ -9,13 +9,13 @@ go test -count=1 -custom -args -v=7
# However, it should be an error to use custom flags when -i or -c are used,
# since we know for sure that no test binary will run at all.
! go test -i -custom
stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -i\)$'
stderr '^go test: unknown flag -custom cannot be used with -i$'
! go test -c -custom
stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -c\)$'
stderr '^go test: unknown flag -custom cannot be used with -c$'

# The same should apply even if -c or -i come after a custom flag.
! go test -custom -c
stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -c\)$'
stderr '^go test: unknown flag -custom cannot be used with -c$'

-- go.mod --
module m
Expand Down

0 comments on commit 6192b98

Please sign in to comment.