cmd/vet/all

147 views
Skip to first unread message

Alan Donovan

unread,
Nov 14, 2018, 9:29:03 AM11/14/18
to Russ Cox, Brad Fitzpatrick, Josh Bleecher Snyder, golan...@googlegroups.com


Hi Russ, Brad, Josh,

I hit a setback trying to make cmd/vet/all use "go vet" instead of "go tool vet". Because go vet is incorporated into the build and cmd/vet/all runs vet for every OS/ARCH combination, my change effectively does a build for every platform, which is slow (>10m); see https://go-review.googlesource.com/c/go/+/149097.

After much experimentation, I think the path forward is to make cmd/vet/all run a driver that loads the entire std lib from source.  We could vendor the go/packages-based driver, x/tools/go/analysis/cmd/vet, which runs go list, then loads, type-checks and analyzes all the packages in parallel. This takes 10s real, 60s user per OS/ARCH.

This would unfortunately mean we need to vendor these additional packages, something I had been trying to avoid throughout the design:

Please tell me which approach you prefer, or if there is another I have overlooked.

cheers
alan



Ian Lance Taylor

unread,
Nov 14, 2018, 10:02:33 AM11/14/18
to Alan Donovan, Russ Cox, Brad Fitzpatrick, Josh Bleecher Snyder, golan...@googlegroups.com
cmd/vet/all is a test that nobody runs by default, so failures
routinely catch people by surprise. It does not need to be run by
all.bash--it's already not run by all.bash--and it doesn't need to be
run by cmd/dist. All we need is a way for the builders to run it and
a way for people to run it. If that means that the builders and
people must have golang.org/x/tools somewhere on their system, that
seems a simple enough requirement.

Ian

Brad Fitzpatrick

unread,
Nov 14, 2018, 10:10:01 AM11/14/18
to Ian Lance Taylor, Alan Donovan, Josh Bleecher Snyder, Russ Cox, golan...@googlegroups.com
Agreed. We can arrange for the builder to always have tip x/tools available.

Alan Donovan

unread,
Nov 14, 2018, 2:05:55 PM11/14/18
to Brad Fitzpatrick, Ian Lance Taylor, Josh Bleecher Snyder, Russ Cox, golan...@googlegroups.com
OK, thanks.  I think I'm getting somewhere. PTAL at https://go-review.googlesource.com/c/go/+/149097, which now builds and runs x/tools' vet.

Alan Donovan

unread,
Nov 15, 2018, 12:51:13 PM11/15/18
to Brad Fitzpatrick, Ian Lance Taylor, Josh Bleecher Snyder, Russ Cox, golan...@googlegroups.com
OK, we are now just two approvals away from success:

https://go-review.googlesource.com/c/go/+/149097 changes cmd/dist to build x/tools/go/analysis/cmd/vet.
The builders are already ready for this change (they put x/tools on GOPATH).
It needs a +2 from Brad.

https://go-review.googlesource.com/c/go/+/149609 deletes cmd/vet and replaces it with a copy of x/tools/go/analysis/vet-lite/main.go.
It needs a +2 from Russ.

thanks
alan
Reply all
Reply to author
Forward
0 new messages