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

go/types: TestStdlib fails due failed import in vendored pprof #25367

Closed
ysmolsky opened this issue May 13, 2018 · 9 comments
Closed

go/types: TestStdlib fails due failed import in vendored pprof #25367

ysmolsky opened this issue May 13, 2018 · 9 comments

Comments

@ysmolsky
Copy link
Member

@ysmolsky ysmolsky commented May 13, 2018

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

~/golang/src (master=) % go version
go version devel +6876447952 Sat May 12 16:26:36 2018 +0000 darwin/amd64

How to reproduce

/golang/src % go test go/types
--- FAIL: TestStdlib (6.66s)
	stdlib_test.go:223: /Users/thorn/golang/src/cmd/vendor/github.com/google/pprof/pprof.go:24:2: could not import github.com/chzyer/readline (can't find import: "github.com/chzyer/readline")
	stdlib_test.go:223: /Users/thorn/golang/src/cmd/vendor/github.com/google/pprof/pprof.go:79:18: invalid operation: r.rl (variable of type *invalid type) has no field or method Stderr
	stdlib_test.go:223: /Users/thorn/golang/src/cmd/vendor/github.com/google/pprof/pprof.go:69:18: invalid operation: r.rl (variable of type *invalid type) has no field or method Stderr
	stdlib_test.go:223: /Users/thorn/golang/src/cmd/vendor/github.com/google/pprof/pprof.go:58:7: invalid operation: r.rl (variable of type *invalid type) has no field or method SetPrompt
	stdlib_test.go:223: /Users/thorn/golang/src/cmd/vendor/github.com/google/pprof/pprof.go:59:14: invalid operation: r.rl (variable of type *invalid type) has no field or method Readline
FAIL
FAIL	go/types	7.313s

Introduced in 46047e6 by @hyangah
Library "github.com/chzyer/readline" is not installed on my system and not vendored as well.

@agnivade
Copy link
Contributor

@agnivade agnivade commented May 13, 2018

Wonder how the try-bots passed this.

Loading

@ysmolsky
Copy link
Member Author

@ysmolsky ysmolsky commented May 13, 2018

Maybe this test is not run by try-bots?

% go test -short go/types
ok  	go/types	0.370s

Loading

@agnivade
Copy link
Contributor

@agnivade agnivade commented May 13, 2018

That's right. Try-bots just run -short tests if I am not mistaken.

Loading

@mvdan
Copy link
Member

@mvdan mvdan commented May 13, 2018

I was under the impression that Go's pprof vendor wouldn't use readline, so we wouldn't have to pull it in as a dependency. If so, this is hopefully just a mistake with an easy fix.

Loading

@hyangah
Copy link
Contributor

@hyangah hyangah commented May 15, 2018

We don't use the file, so we can delete it.

Loading

@mvdan
Copy link
Member

@mvdan mvdan commented May 15, 2018

We should come up with a strategy to keep this from happening again, though. Even if the go/types test gets run by the builders, it wouldn't be part of the trybots, so I'm sure that the file would get re-added sooner than later.

If we were using a dependency management tool, this would be trivial - it's a main package, so it should not be vendored. Perhaps a better short-term solution is to add src/cmd/vendor/github.com/google/pprof/*.go to some gitignore file.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented May 15, 2018

Change https://golang.org/cl/113295 mentions this issue: cmd/vendor/.../pprof: delete pprof.go file

Loading

@hyangah
Copy link
Contributor

@hyangah hyangah commented May 15, 2018

Currently there is no formal procedure to update this, so I will just write a script for update.

Loading

@mvdan
Copy link
Member

@mvdan mvdan commented May 15, 2018

Do you mean that vendor.json has been getting updated manually? I assumed that it was done with a tool like govendor, but I failed to get that one to work properly.

Loading

@gopherbot gopherbot closed this in 750a42f May 17, 2018
@golang golang locked and limited conversation to collaborators May 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants