x/tools/cmd/vet: does not return the same results across workstations

[stevekuznetsov]

We are attempting to integrate go vet's static analysis into our continuous integration pipeline for OpenShift Origin but have noticed some interesting behavior.

Different developers with the same go version (1.4.2) and golang.org/x/tools/ at the same HEAD (c262de8 tools/cmd/vet: Sort checks list alphabetically.) will see wildly different results from running go vet. We run the tool on a subset of directories within our repository, as seen here.

All of the output that we see does indeed result from defects in the code, so none of it is spurious, but some developers do not see many errors while others see ~50.

What factors affect the output of go vet? I think the people that have tested this have all been on different operating systems (OSX, Fedora 21,22). What can we do to ensure deterministic results?

[cespare]

Different developers with the same go version (1.4.2) and golang.org/x/tools/ at the same HEAD

Are you sure that you're running the same installed version of vet?

Can you give a reproducible example of code for which vet gives different output on different machines?

[adg]

Do you have GOPATH set correctly on all the machines?

On 16 September 2015 at 12:30, Steve Kuznetsov [email protected]
wrote:

We are attempting to integrate go vet's static analysis into our
continuous integration pipeline for OpenShift Origin
https://github.com/openshift/origin but have noticed some interesting
behavior.

Different developers with the same go version (1.4.2) and
golang.org/x/tools/ at the same HEAD (c262de8 tools/cmd/vet: Sort checks
list alphabetically.) will see wildly different results from running go
vet. We run the tool on a subset of directories within our repository, as
seen here
https://github.com/openshift/origin/blob/master/hack/verify-govet.sh.

All of the output that we see does indeed result from defects in the code,
so none of it is spurious, but some developers do not see many errors while
others see ~50.

What factors affect the output of go vet? I think the people that have
tested this have all been on different operating systems (OSX, Fedora
21,22). What can we do to ensure deterministic results?

—
Reply to this email directly or view it on GitHub
#12636.

[stevekuznetsov]

@cespare I do not know why the installed version of go vet would differ than that in $GOPATH/src but I do think we have the same version installed, we all ran go get -u golang.org/x/tools/cmd/vet today before testing.

@adg Yes, I believe we have $GOPATH set correctly.

@cespare For example. client.go returns no errors for me (on my local machine and a brand new CentOS Docker container with only golang, git and wget installed) and some other developers, but

pkg/cmd/experimental/diagnostics/client.go:37: github.com/openshift/origin/pkg/diagnostics/client.ConfigContext composite literal uses unkeyed fields

For yet other developers. Checking the file, you can see that there are indeed unkeyed fields.

We are tracking our outputs at this issue: openshift/origin#4682

[stevekuznetsov]

While we are running go vet on directories, I do not believe that there are masking effects in play here - the results on my local machine, the empty Docker container, our Travis and Jenkins jobs, etc, all are minimal - one or two errors for the entire codebase, and none in the directory containing the above client.go.

[davecheney]

@stevekuznetsov Depending on how you obtained Go 1.4.x you may be hitting the included copy of the vet tool which will override any go vet tool in GOPATH.

[stevekuznetsov]

@davecheney I see. On the test I did with a clean CentOS image, I got the image with wget from the official website:

$ docker run -it centos:latest bash
# yum install -y git wget 
# wget https://storage.googleapis.com/golang/go1.4.2.linux-amd64.tar.gz
# tar -C /usr/local/ -xzf go1.4.2.linux-amd64.tar.gz

Will this method get a stale version of go vet? This installation path results in a version of go vet that greatly under-reports errors in the code.

[davecheney]

@stevekuznetsov installing go 1.4.2 gives you the version of vet that shipped with go 1.4.2. I believe that version of Go will always prefer $GOROOT/pkg/tools/linux_amd64/vet

[stevekuznetsov]

@davecheney Can I force it to use the version installed in $GOPATH with go tool vet or some other invocation?

[davecheney]

@stevekuznetsov I think, but have not tested, you can remove the version in $GOROOT/pkg/tools/..., that should let the version in GOPATH bubble up.

[stevekuznetsov]

@davecheney If I delete that binary, go tool vet fails and says that go vet is not installed. Once I've deleted that binary and re-installed using go get, however, my output does not change. I do not think that the version that shipped with go1.4.2 is being run, as the error I am seeing is about unknown identifiers in an Example, and the commit that brought that error into go vet (golang/tools@a25a8d5) pushed into the git repo yesterday.

[stevekuznetsov]

@davecheney By forcing myself to use the old version of go vet - that is, a blank CentOS container, only installing go using wget - I am able to reproduce the large amounts of error messages that some of the other developers were seeing. Somewhere between the version that shipped with go1.4.2 and the current HEAD there must have been a regression. I am seeing eighty-seven errors and by a quick poll, they all seem legitimate. They are all composite literal errors (unkeyed fields) that do not show up in the new version.

[stevekuznetsov]

@davecheney @cespare Any thoughts? The older version of go get does not adequately explain what we are seeing.

[stevekuznetsov]

Bump. I've now checked that we are all running the same build of go1.4.2 as well as the latest go vet as well as invoking the command in the same manner (no variation in bash interpretation across machines), still seeing different results.

[ianlancetaylor]

Unfortunately I don't think we are going to be able to fix this unless we can recreate it. Can you give us exact and complete instructions for how to recreate the problem?

[stevekuznetsov]

@ianlancetaylor I am looking right now for you guys to tell me what factors affect the operation of go vet so I can isolate this better. Right now I have isolated: go version, go vet version, exact execution (commands are the same from bash).

Instance, the invocation:

$ go tool vet -shadow=false pkg/cmd/experimental/diagnostics/client.go

From our repository root (referencing client.go as linked above) returns no errors on my machine, but on some others it returns the unkeyed struct literal issue on line 37.

This file is 46 lines long and, while it does take some context from other files, it is the best I can do right now. Please let me know how I can make that example better for you guys. I have not been able to write a small file and get the same result, and I understand that makes it difficult because the issue with building the tree may result from having the definition of that struct in some specific file or configuration, but I do not know how to isolate that.

[minux]

any differences in $GOPATH setting among the machines?

[stevekuznetsov]

@minux $GOPATH is not literally the same between machines, but it is set correctly in that $GOPATH is the same as when any go get was used.

[ianlancetaylor]

@stevekuznetsov I am not aware of anything that affects go vet operation other than the code itself, how it is invoked, the GOROOT and GOPATH environment variables, and the code that go vet is examining.

[ianlancetaylor]

The original issue is about the -composites warning. The -composites warning is only issued if the vet tool can determine that the composite literal is a struct. In this file, the type is from an imported package. So here is another difference that may matter (I'm not able to build your package to test this): whether the user has run "go install" for the dependent packages may change the go vet output.

[minux]

Yeah, that's probably the cause. Try go tool vet -v and see if it outputs "could not import" warning on machines that doesn't show the composite literal error. See also #9439.

[stevekuznetsov]

@ianlancetaylor @minux I will look into that today. Here is a list of other issues that we see intermittently between machines:

returns Lock by value:
no formatting directive in Sprintf call

[ianlancetaylor]

"returns Lock by value" will depend on installed packages if the warning is about returning the value of a type defined in a different package, where the type is a struct that contains a lock field.

I have no explanation for the "no formatting directive" error, except for the rather unlikely case of writing fmt.Sprintf(otherpackage.constant).

[stevekuznetsov]

@ianlancetaylor As I continue to look into this, could you please clarify what you mean by

The -composites warning is only issued if the vet tool can determine that the composite literal is a struct.

What does a failure to determine that the composite literal is a struct look like? Does go vet determine that it is a different type of composite literal? Or if it can't find the struct typedef it won't issue the warning?

[ianlancetaylor]

If go vet sees T{v}, and can not determine that T is a struct type, then it will not issue a warning about unkeyed fields. It will only issue a warning about unkeyed fields if it knows that T is a struct type. If T is imported from an uninstalled package, then currently vet can not determine that T is a struct type, and it therefore will not issue a warning.

[stevekuznetsov]

@ianlancetaylor Do you know how well go vet plays with godeps?

[ianlancetaylor]

I have not used godeps.

[stevekuznetsov]

@ianlancetaylor On a fresh CentOS container, the difference between the version of go vet that shipped with go1.4.2 and the current version from go get is that the new version does not pick up on some -composites. Running both with -v doesn't show me any import errors for those packages or files. I think there may be an issue with godeps but I'd expect go vet to complain about that with -v set? I'll ask godeps authors if they know.

[griesemer]

@stevekuznetsov I think what @ianlancetaylor explained before should explain the discrepancy. go vet is a conservative tool, if it is not pretty certain about an error, it is not going to report it.

It may help to understand better how go vet works: If go vet has access to all files of a package (directory) it will consider all package files (including tests). If it only has access to one file, it will only consider that one file.

Keep in mind that if it sees only one file of a (multi-file) package, it obviously cannot know about the contents in those other files. If types are declared in those files and hence not available, errors that would be obvious if those types were known cannot be reported.

The same is true for imports: If go vet cannot find an imported package, errors that would be obvious if those imports were available, are not reported (go vet cannot guess, it's not possible in general).

Imports are only available, if the respected packages were installed in the right place. That is, in that respect, go vet operates like the compiler applied to a single package (e.g. go tool compile ). Specifically, go vet does not make an attempt to analyze the source code of imported packages - they have to be installed.

The reason for this as follows: go vet makes use of the go/types package to determine type information. go/types learns about imported packages by "importing" their information from the installed packages. If they are not installed, they cannot be imported at the moment.

It is possible to make an "importer" that can import from source code (open issue), but we haven't done that yet.

Not knowing more about your specific setup, I suspect strongly that you get different results because your different systems have different files installed (or not installed). Which is exactly what @ianlancetaylor already suggested.

I don't know of any dependency of go vet on godeps, or why it should matter.

Hope that helps. I am inclined to say go vet is working as intended.

[stevekuznetsov]

@griesemer Thanks for the explanation. We seem to have found a $GOPATH problem that appears to have solved the issue. Many thanks.

[minux]

What was the problem? Could you please give a brief explanation here to help future readers of the this issue?

[stevekuznetsov]

Absolutely.

We had to tweak our $GOPATH in the following ways:

• _pre_pend the godep path to original $GOPATH in order to capture internally version-locked installations of dependencies by godep so there would not be conflicts between those and other installations in the external $GOPATH
• add the output of our compilation to the $GOPATH - we invoke go tool vet per-file, and while most things that depended on definitions outside of the immediate package worked, some were missed by not explicitly including the compiled packages in $GOPATH

Some oddities that we were not able to find the reason for but nevertheless do not seem to be issues anymore were the printf warnings that were found on some machines but not others - counting the number of arguments to fmt.Errorf doesn't require package imports, so we're not clear as to why there were differences in output but again they do not seem to be present any longer.

The total number of vet errors we see with the current HEAD remains low compared to that which we see using the version of go vet that shipped with go1.4.2, and I'm still investigating that.

[freeformz]

FWIW: Until go 1.6 I really suggest people rewrite their dependencies (godep save -r <pkg spec>) instead of manipulating the $GOPATH. I have not removed those commands from godep because os many people still rely on them.