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

cmd/doc: no error for invalid one-letter package path #24462

Closed
willfaught opened this issue Mar 20, 2018 · 13 comments
Closed

cmd/doc: no error for invalid one-letter package path #24462

willfaught opened this issue Mar 20, 2018 · 13 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@willfaught
Copy link
Contributor

This felt vaguely familiar, like maybe I'd reported it before, but searched and couldn't find anything, so apologies if it's a dupe.

What version of Go are you using (go version)?

go version go1.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/willfaught/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/willfaught/Developer/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_1/ggvd2t1x7hz_185crsb36zlr0000gp/T/go-build967928832=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

~ $ echo $GOPATH  
/Users/willfaught/Developer/go
~ $ ls $GOPATH/src
cloud.google.com      collectd.org          go.mercari.io         golang.org            gopkg.in              k8s.io                sourcegraph.com
code.cloudfoundry.org github.com            go.uber.org           google.golang.org     honnef.co             rsc.io                t
~ $ go doc z

What did you expect to see?

~ $ go doc z
doc: no buildable Go source files in /Users/willfaught
exit status 1
~ $

What did you see instead?

~ $ go doc z
package z // import "."

~ $

It seems to be related to the package path being one letter:

~ $ go doc zz # zz, not z
doc: no buildable Go source files in /Users/willfaught
exit status 1
~ $
@jimmyfrasche
Copy link
Member

@willfaught does go list produce similar errors or is this specific to go doc?

@mvdan
Copy link
Member

mvdan commented Mar 20, 2018

Seems to be exculsive to go doc. go list, godoc and all other tools error. I can reproduce it on my machine too.

@mvdan mvdan added this to the Go1.11 milestone Mar 20, 2018
@mvdan mvdan added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 20, 2018
@mvdan
Copy link
Member

mvdan commented Mar 20, 2018

Ah, this is because of how go doc is designed. It will try to find a package by suffix, so that go doc json will match encoding/json.

However, this has a funny result; the Go tree has many one-letter packages. For example, the one that OP was finding was likely /home/mvdan/tip/src/cmd/go/testdata/testvendor/src/q/z. And if one uses d, it finds /home/mvdan/tip/src/cmd/go/testdata/src/canonical/d.

Seems like the easy fix here is to teach go doc the basics of when directories in $GOPATH cannot contain importable packages, such as those under testdata or under directories starting with _.

@mvdan mvdan self-assigned this Mar 20, 2018
@willfaught
Copy link
Contributor Author

It doesn't work for go list z:

~ $ go list z
can't load package: package z: cannot find package "z" in any of:
	/usr/local/Cellar/go/1.10/libexec/src/z (from $GOROOT)
	/Users/willfaught/Developer/go/src/z (from $GOPATH)

However, this has a funny result; the Go tree has many one-letter packages. For example, the one that OP was finding was likely /home/mvdan/tip/src/cmd/go/testdata/testvendor/src/q/z. And if one uses d, it finds /home/mvdan/tip/src/cmd/go/testdata/src/canonical/d.

That appears to be correct. y, w, x, v, a, b, c, d all work for me, but e does not:

~ $ go doc e                      
doc: no buildable Go source files in /Users/willfaught
exit status 1

The output doesn't indicate the absolute import path, so I have no idea where these one-letter packages are. Perhaps go doc should print the import path to disambiguate a partial match.

I didn't know about the pattern matching. If that's indeed correct behavior, then I guess this isn't a bug.

Thanks for looking into it!

@willfaught
Copy link
Contributor Author

@mvdan I didn't see you assigned it to yourself until I closed it. Re-opening, sorry.

@willfaught willfaught reopened this Mar 21, 2018
@mvdan
Copy link
Member

mvdan commented Mar 21, 2018

Perhaps go doc should print the import path to disambiguate a partial match.

Maybe. I simply added a debug print myself. That seems like a discussion/feature for another issue, though.

The fix is simple, but the tests rely on a package in the testdata directory, so keeping the tests working is being non-trivial. Will send a CL soon-ish.

@willfaught
Copy link
Contributor Author

willfaught commented Mar 21, 2018 via email

@mvdan
Copy link
Member

mvdan commented Apr 13, 2018

Completely removing the suffix matching is unlikely what we want, as that's a useful feature. The point is fixing this in a way that will keep the tests working.

@gopherbot
Copy link

Change https://golang.org/cl/106935 mentions this issue: cmd/doc: skip directories like other go tools

@mvdan
Copy link
Member

mvdan commented Apr 13, 2018

@willfaught @jimmyfrasche I finally found a way to keep the tests working that I'm happy with. Reviews welcome.

@willfaught
Copy link
Contributor Author

willfaught commented Apr 14, 2018 via email

@mvdan
Copy link
Member

mvdan commented Apr 17, 2018

Reverted, because I botched the test code and broke some builders. Will try again next week.

@mvdan mvdan reopened this Apr 17, 2018
@gopherbot
Copy link

Change https://golang.org/cl/109216 mentions this issue: cmd/doc: skip directories like other go tools

@golang golang locked and limited conversation to collaborators May 1, 2019
@rsc rsc unassigned mvdan Jun 23, 2022
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
Go1.11
Development

No branches or pull requests
4 participants
@willfaught @jimmyfrasche @mvdan @gopherbot