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

go/build: "cannot find package <name> in any of:" message misleading #39281

Closed
jkopczyn opened this issue May 27, 2020 · 6 comments
Closed

go/build: "cannot find package <name> in any of:" message misleading #39281

jkopczyn opened this issue May 27, 2020 · 6 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Milestone
Unplanned

Comments

@jkopczyn
Copy link

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

go1.14.2 linux/amd64

What did you do?

I try to build (well, vet, but the message comes from src/go/build/build.go) a file which imports a module. That module is not in my $GOROOT, or in my multiple-entry $GOPATH.

The error message I got was

foo.go:12:2: cannot find package "api/test/metadata/v1" in any of:
    /golang-root/src/api/test/metadata/v1 (from $GOROOT)
    /path/to/home/dir/foo/bar/baz/src/api/test/metadata/v1 (from $GOPATH)
    /path/to/home/dir/foo/bar/src/config/src/api/test/metadata/v1
    /path/to/home/dir/foo/bar/src/platform/quux/src/api/test/metadata/v1

A naive reading of this message is that /golang-root/src/api/test/metadata/v1 is sourced from my $GOROOT, /path/to/home/dir/foo/bar/baz/src/api/test/metadata/v1 is sourced from my $GOPATH, and the other two are sourced from somewhere else, no explanation given.

The actual answer is that the first line is from $GOROOT and all the others are from $GOPATH. This is highly unintuitive from the format the message is given in. To properly convey this information, I suggest this format:

foo.go:12:2: cannot find package "api/test/metadata/v1" in any of:
    (from $GOROOT:) /golang-root/src/api/test/metadata/v1
    (from $GOPATH:) /path/to/home/dir/foo/bar/baz/src/api/test/metadata/v1
        /path/to/home/dir/foo/bar/src/config/src/api/test/metadata/v1
        /path/to/home/dir/foo/bar/src/platform/quux/src/api/test/metadata/v1

Note the additional indents for the second and further entries from $GOPATH; IIUC $GOROOT should never have multiple entries so this would not apply to that.
Alternate suggestion, IMO yet more clear but with the drawback of taking up more screen real estate:

foo.go:12:2: cannot find package "api/test/metadata/v1" in any of:
    (from $GOROOT:)
    /golang-root/src/api/test/metadata/v1
    (from $GOPATH:)
    /path/to/home/dir/foo/bar/baz/src/api/test/metadata/v1
    /path/to/home/dir/foo/bar/src/config/src/api/test/metadata/v1
    /path/to/home/dir/foo/bar/src/platform/quux/src/api/test/metadata/v1

This format could optionally indent all the paths or outdent the "(from $GO....:)" lines.
jkopczyn added a commit to jkopczyn/went that referenced this issue May 27, 2020
@jkopczyn
Add "from $GOPATH tag" to all entries of GOPATH
d74e9a9 
simplest possible mitigation for golang#39281
@jkopczyn jkopczyn mentioned this issue May 27, 2020
Open
@mdempsky mdempsky changed the title "cannot find package <name> in any of:" message misleading go/build: "cannot find package <name> in any of:" message misleading May 27, 2020
@andybons
Copy link
Member

@bcmills @jayconrod @matloob

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2020
@andybons andybons added this to the Unplanned milestone May 28, 2020
@bcmills
Copy link
Contributor

bcmills commented May 28, 2020

As far as I can tell, this only affects users in GOPATH mode with multiple GOPATH entries.

We are not making significant changes to GOPATH mode at this point, and users with multiple GOPATH entries are rare even compared to GOPATH users in general. So I agree that this could be made clearer, but at this point I don't think it's worth the effort (to implement or to review).

If there is an analogous problem in module mode, please file a separate issue and we can address that.

@bcmills bcmills closed this as completed May 28, 2020
@jkopczyn
Copy link
Author

I am unclear on how you could possibly use GOPATH without multiple entries, but that's not that important. Even a rare subset of a rare subset is still a few hundred users at minimum. The PR I attached is one line, a 1-minute review at most. This is really not worth 1 minute of your time to save, conservatively, a few hundred people an hour of their time?

@jkopczyn
Copy link
Author

@davecheney , who added the original logic being modified and therefore has the context to judge whether the 1-line change is in fact as simple as it appears.

@jayconrod
Copy link
Contributor

@jkopczyn A code of conduct reminder: please be patient and respectful.

In particular, don't minimize the time and effort needed to review these kinds of changes. We routinely go through several rounds of review on changes just like this. Changes to error messages need to be tested in many environments, and existing tests need to be updated. The process takes time, and even after a change is approved, it needs to be tracked, updated, and merged in the next release cycle.

I agree with @bcmills here. While GOPATH is not quite deprecated yet, it is no longer the way most developers manage packages. We don't plan to make significant changes to that functionality.

@jkopczyn
Copy link
Author

I am not asking for a significant change. I am asking for a one-line deletion which makes an existing code path get used in all nontrivial cases instead of only in the first nontrivial case. It adds no additional testing surface, and in fact reduces testing surface. I am not exaggerating or minimizing when I say that this would be a 1-minute review, whether the decision in that review was to approve it or to reject it. I already checked the rest of the repo to see if there were any test files or expectations which it would affect; I couldn't find any. If I've missed some, it will be a simple matter of deleting a test case which covered the deleted path, though I admit that would make it a slightly larger PR and a slightly longer review.

Further, if GOPATH is not deprecated, then it's still supported and should be treated as such. This generates misleading messages and the mitigation is dead simple. I can see why you wouldn't want to do the larger change to a great format when it's in maintenance mode, so I'm not asking for that. But changing from a misleading format to a reasonably-good one is a deletion of one short line.

@golang golang locked and limited conversation to collaborators May 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@jayconrod @andybons @jkopczyn @bcmills @gopherbot