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

cmd/go: typo in flag causes panic when running a test file in GOROOT/src #35176

Closed
josharian opened this issue Oct 26, 2019 · 7 comments
Closed
Assignees
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@josharian
Copy link
Contributor

At tip:

$ go test -timeoout=100h fmt
panic: loadPackageData called with empty package path

goroutine 21 [running]:
cmd/go/internal/load.loadPackageData(0x0, 0x0, 0x0, 0x0, 0xc0000240c4, 0x16, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/josh/go/tip/src/cmd/go/internal/load/pkg.go:648 +0x625
cmd/go/internal/load.(*preload).preloadMatches.func1(0xc000068640, 0x0, 0x0)
	/Users/josh/go/tip/src/cmd/go/internal/load/pkg.go:841 +0x80
created by cmd/go/internal/load.(*preload).preloadMatches
	/Users/josh/go/tip/src/cmd/go/internal/load/pkg.go:839 +0x165

Note the typo: timeoout instead of timeout.

cc @bcmills @jayconrod

@josharian josharian added this to the Go1.14 milestone Oct 26, 2019
@wade-welles
Copy link

Just pulled down the newest master and checked and was unable to reproduce, was this already fixed? I wanted to fix it :D

./go test -timeoout=100h fmt
flag provided but not defined: -timeoout
Usage of /tmp/go-build643614696/b001/go.test:
  -flaky
...

@josharian
Copy link
Contributor Author

On my phone, but I wonder whether you have to be in dir $GOROOT/src to reproduce.

@mvdan
Copy link
Member

mvdan commented Oct 27, 2019

I equally cannot reproduce this on go version devel +0f559941fb Sat Oct 26 15:17:28 2019 +0000 linux/amd64. I tried inside GOROOT/src and with various GO111MODULE values. I wonder if I missed any piece of the puzzle.

@josharian
Copy link
Contributor Author

Sorry about that. I believe to reproduce you need a test file in GOROOT/src, and then run from that dir. Which means this is pretty low priority. Will re-milestone.

@josharian josharian modified the milestones: Go1.14, Unplanned Oct 28, 2019
@bcmills bcmills changed the title cmd/go: typo in flag causes panic cmd/go: typo in flag causes panic when running a test file in GOROOT/src Oct 28, 2019
@bcmills bcmills added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 28, 2019
@bcmills bcmills modified the milestones: Unplanned, Backlog Oct 28, 2019
@jayconrod
Copy link
Contributor

Reproduced with the instructions from the last comment.

It looks like go test -timeoout fmt is being interpreted as go test . with some arguments to pass to the tests, since -timeoout is not a flag that go test itself recognizes. So it tries to load the package in the current directory, $GOROOT/src.

The bug is that modload.ImportPaths is returning an empty string as one of the import paths. Which is kind of correct because normally a package path in std is its subdirectory under $GOROOT/src, and since there is no subdirectory in this case, we get "".

Maybe we should detect and reject this case specifically? It's such a small corner case though.

@quantonganh
Copy link
Contributor

Has it been fixed?

$ go test -timeoout=100h fmt
GOROOT/src is not an importable package

@bcmills
Copy link
Contributor

bcmills commented Nov 3, 2023

Yep, looks like this was fixed in https://go.dev/cl/185345 (March 2020). Thanks for checking!

@bcmills bcmills closed this as completed Nov 3, 2023
@bcmills bcmills self-assigned this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants