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

Jump to bottom

cmd/vet: starting in Go 1.10, output order is not deterministic for multiple packages #24309

Closed
nmiyake opened this issue Mar 8, 2018 · 5 comments
Closed

cmd/vet: starting in Go 1.10, output order is not deterministic for multiple packages #24309

nmiyake opened this issue Mar 8, 2018 · 5 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@nmiyake
Copy link
Contributor

nmiyake commented Mar 8, 2018

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

go version go1.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

darwin-amd64

What did you do?

  • Created a package with a vet error that also has a subpackage with a vet error
  • Ran go vet . ./bad2 to run go vet on both packages

What did you expect to see?

I expected that the output ordering of the packages would match that of the input -- the errors in the current package would be reported first, followed by those in bad2. This was the case up through Go 1.9.

What did you see instead?

The order of the output was not deterministic.

Output of multiple runs in Go 1.10:

➜ go vet . ./bad2
# github.com/org/repo/bad
./foo.go:9: Printf format %s has arg os.Stdout of wrong type *os.File
# github.com/org/repo/bad/bad2
bad2/bar.go:9: Printf format %s has arg os.Stdout of wrong type *os.File
➜ go vet . ./bad2
# github.com/org/repo/bad/bad2
bad2/bar.go:9: Printf format %s has arg os.Stdout of wrong type *os.File
# github.com/org/repo/bad
./foo.go:9: Printf format %s has arg os.Stdout of wrong type *os.File

Output of multiple runs in Go 1.9:

➜ /usr/local/go-1.9.2/bin/go vet . ./bad2
foo.go:9: Printf format %s has arg os.Stdout of wrong type *os.File
exit status 1
bad2/bar.go:9: Printf format %s has arg os.Stdout of wrong type *os.File
exit status 1
➜ /usr/local/go-1.9.2/bin/go vet . ./bad2
foo.go:9: Printf format %s has arg os.Stdout of wrong type *os.File
exit status 1
bad2/bar.go:9: Printf format %s has arg os.Stdout of wrong type *os.File
exit status 1
@nmiyake
Copy link
Contributor Author

nmiyake commented Mar 8, 2018

Based on the observed behavior, it looks like go vet in Go 1.10 processes packages in parallel and thus the output ordering does not necessarily reflect the order in which the packages were specified.

This has the unfortunate side effect that the output of failing runs of go vet may differ even when the inputs are the same. This makes it hard to do things like diffing the generated output. It would be nice to be able to force serial execution such that the output for a given run of go vet is deterministic given the same inputs.

@ALTree
Copy link
Member

ALTree commented Mar 8, 2018 •
edited

I think we don't make any guarantee about the order of the vet output on multiple packages.

This makes it hard to do things like diffing the generated output

Tests breaking because you are diffing assuming a specific output order is often a red flag; those test are bound to be brittle. The usual workaround (may not be applicable in your case, but still) is to stop diffing and start grepping for specific things that you don't want to be in the output.

cc @robpike @mvdan

@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 8, 2018
@mvdan
Copy link
Member

mvdan commented Mar 8, 2018

Forcing serial execution won't necessarily make the output deterministic. The old vet never made any promises about deterministic output that I am aware of. It would also make vet slower, and require an extra flag.

Why not sort the output before passing it to diff?

@mvdan
Copy link
Member

mvdan commented Mar 8, 2018

Also, as @ALTree mentioned, you should really be using a whitelist of vet warnings to ignore. The standard library does this too. With that method, the output order doesn't matter at all.

@nmiyake
Copy link
Contributor Author

nmiyake commented Mar 8, 2018

Thanks for the clarification -- I understand all the points, and don't think this is a major issue so I'm fine with this.

Also, I verified/confirmed that setting the GOMAXPROCS environment variable to 1 before execution (GOMAXPROCS=1 go vet . ./bad2) disables the parallelism, so it is still possible to get serial behavior by setting this before running, which I think is completely reasonable as a work-around (although obviously this would be at the expense of speed).

@nmiyake nmiyake closed this as completed Mar 8, 2018
@golang golang locked and limited conversation to collaborators Mar 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mvdan @ALTree @nmiyake @gopherbot