This package attempts to install itself to GOROOT which will fail for
non-root users. Most users will have already installed the vet tool via a
system package, so it shouldn't be necessary to 'go get' here.
Moreover, the 'vet' make target already checks that it is installed before
running it, running 'go get' if necessary.
This is the output when running 'make updatedeps' as a regular user
without this change:
```
$ make updatedeps
go get -u github.com/mitchellh/gox
go get -u golang.org/x/tools/cmd/stringer
go get -u golang.org/x/tools/cmd/vet
go install golang.org/x/tools/cmd/vet: open /usr/local/go/pkg/tool/linux_amd64/vet: permission denied
make: *** [updatedeps] Error 1
```
After discussing with the very gracious @cespare over at
https://github.com/cespare/deplist/pull/2 I now understand that we can
pull off the same logic with just `go list`.
The logic is now simpler and more consistent:
* List out all packages in our repo
* For each of those packages, list their dependencies
* Filter out any dependencies that already live in this repo
* Remove duplicates
* And fetch the rest. `go get` will work out all transitive dependencies
from there
Currently when running `make updatedeps` from a branch, the dependency
list from master ends up getting used. We tried to work around this in
35490f7812, and got part way there, but
here's what was happening:
- record the current SHA
- run `go get -f -u -v ./...` which ends up checking out master
- master is checked out early in the `go get` process, which means all
subsequent dependencies are resolved from master
- re-checkout the recorded SHA
- run tests
This works in most cases, except when the branch being tested actually
changes the list of dependencies in some way.
Here we move away from letting `go get -v` walk through everything in
`./...`, instead building our own list of dependencies with the help of
`deplist`. We can then filter terraform packages out from the list, so
they don't get touched, and safely update the rest.
This should solve problems like those observed in #899 and #900.
__Note__: had to add a feature to deplist to make this work properly;
see 016ef97111
Working on getting it accepted upstream.
Add a vet target in order to catch suspicious constructs
reported by go vet.
Vet has successfully detected problems in the past,
for example, see
482460c4c8fc36b1cd9468a41035a97b704fb77d4f3f85b16595fa353ee94bfe18b40d
Some vet flags are noisy. In particular, the following flags
reports a large amount of generally unharmful constructs:
-assign: check for useless assignments
-composites: check that composite literals used field-keyed
elements
-shadow: check for shadowed variables
-shadowstrict: whether to be strict about shadowing
-unreachable: check for unreachable code
In order to skip running the flags mentioned above, vet is
invoked on a directory basis with 'go tool vet .' since package-
level type-checking with 'go vet' doesn't accept flags.
Hence, each file is vetted in isolation, which is weaker than
package-level type-checking. But nevertheless, it might catch
suspicious constructs that pose a real problem.
The vet target runs the following flags on the entire repo:
-asmdecl: check assembly against Go declarations
-atomic: check for common mistaken usages of the
sync/atomic package
-bool: check for mistakes involving boolean operators
-buildtags: check that +build tags are valid
-copylocks: check that locks are not passed by value
-methods: check that canonically named methods are canonically
defined
-nilfunc: check for comparisons between functions and nil
-printf: check printf-like invocations
-rangeloops: check that range loop variables are used correctly
-shift: check for useless shifts
-structtags: check that struct field tags have canonical format
and apply to exported fields as needed
-unsafeptr: check for misuse of unsafe.Pointer
Now and then, it might make sense to check the output of
VETARGS=-unreachable make vet
manually, just in case it detects several lines of dead code etc.
`go get -u` now checks that remote repo paths match the ones predicted
by the import paths.
So if working on TF from a forked repo, you cannot use `updatedeps` as
`go get` will complain about the differences between the import and the
path to your fork.
This is a refactored solution for PR #616. Functionally this is still
the same change, but it’s implemented a lot cleaner with less code and
less changes to existing parts of TF.
Mitchell Hashimoto