Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/go/packages: 'go list' driver gives complex error if 'go' is missing #29552

Closed
mvdan opened this issue Jan 4, 2019 · 3 comments
Closed

x/tools/go/packages: 'go list' driver gives complex error if 'go' is missing #29552

mvdan opened this issue Jan 4, 2019 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@mvdan
Copy link
Member

mvdan commented Jan 4, 2019 

$ go env
GOARCH="amd64"
GOBIN="/home/mvdan/go/bin"
GOCACHE="/home/mvdan/go/cache"
GOEXE=""
GOFLAGS="-ldflags=-w"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/mvdan/go/land:/home/mvdan/go"
GOPROXY=""
GORACE=""
GOROOT="/home/mvdan/tip"
GOTMPDIR=""
GOTOOLDIR="/home/mvdan/tip/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build044844774=/tmp/go-build -gno-record-gcc-switches"
$ go version
go version devel +30c0a0d33f Thu Jan 3 18:23:07 2019 +0000 linux/amd64
$ cd $(go list -f {{.Dir}} golang.org/x/tools/go/packages)
$ git describe --always
8a605119
$ go install ./gopackages
$ gopackages io
Go package "io":
        package io
        has no exported type info
        file /home/mvdan/tip/src/io/io.go
        file /home/mvdan/tip/src/io/multi.go
        file /home/mvdan/tip/src/io/pipe.go
        import "errors"
        import "sync"
        import "sync/atomic"

$ bin=$(which gopackages)
$ PATH= $bin io
gopackages: couldn't exec 'go [list -e -json -compiled -test=false -export=false -deps=true -find=false -- io]': exec: "go": executable file not found in $PATH *exec.Error

I was expecting an error like go/packages: 'go list' driver requires 'go' to be installed. I understand that the error given basically says the same, but it exposes way too much internal detail, and can be confusing to the end user.

For instance, I'm developing a tool that just got switched to go/packages, so it now requires Go to be installed to use. Which is fine - but I want to give a short, helpful error message if someone forgets about that new requirement. Most of these users won't know or care that we internally use go/packages or go list, so the current error is bad.

Note that I'm not suggesting that we should add an extra go invocation at the beginning of the driver's implementation. It should be enough to wrap all go subcmd ... calls with a wrapper that replaces the executable file not found errors with more user-friendly ones.

/cc @ianthehat @matloob
@mvdan mvdan added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 4, 2019
@mvdan mvdan mentioned this issue Jan 4, 2019
Closed
@mvdan
Copy link
Member Author

mvdan commented Jan 4, 2019

I also used the NeedsFix label optimistically; if this is by design, or the actual fix should be different, please do speak up.

@dmitshur dmitshur changed the title go/packages: 'go list' driver gives complex error if 'go' is missing x/tools/go/packages: 'go list' driver gives complex error if 'go' is missing Jan 6, 2019
@gopherbot gopherbot added this to the Unreleased milestone Jan 6, 2019
@gopherbot
Copy link

Change https://golang.org/cl/168777 mentions this issue: go/packages: give a better error if 'go' is missing

@gopherbot
Copy link

Change https://golang.org/cl/168897 mentions this issue: go/packages: detect missing binary via exec.ErrNotFound error

gopherbot pushed a commit to golang/tools that referenced this issue Mar 22, 2019
@dmitshur
go/packages: detect missing binary via exec.ErrNotFound error
8f249a1 
exec.Command already runs exec.LookPath when given a name that contains
no path separators. There's no need to call exec.LookPath a second time
to detect that cmd.Run failed because of a missing executable file.
It can be detected from the returned error. Do so because it's cleaner.

Also improve the error text to say that the problem was that the go
executable file was not found in $PATH (or %PATH%, etc., depending on
the underlying operating system). In the general case, we can't know if
Go is or isn't installed.

Example error text on macOS:

	gopackages: 'go list' driver requires 'go', but executable file not found in $PATH

Updates golang/go#29552

Change-Id: I769553f125240dccd02098c22641f6a1ed10063c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168897
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@golang golang locked and limited conversation to collaborators Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
2 participants
@mvdan @gopherbot