cmd/vet: could not import C error when used with -v

[mvdan]

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

go version devel +45a4609c0a Thu Jul 27 05:04:28 2017 +0000 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/mvdan/go/land:/home/mvdan/go"
GORACE=""
GOROOT="/home/mvdan/tip"
GOTOOLDIR="/home/mvdan/tip/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build516844053=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

package main

import "C"

func main() {}

 $ go vet
 $ go vet -v
vet: bug.go:3:8: could not import C (can't find import: "C")
Checking file bug.go
exit status 1

What did you expect to see?

Success, even with -v.

What did you see instead?

The unexpected import error.

go vet -v doesn't have this issue on 1.8.3, so I'm marking as a release blocker for 1.9.

Something to note is that the catalyst for this bug was the fix for #19350 by @robpike. In 1.8, when running go vet -v, the verbose flag wasn't passed to go tool vet, and the bug was hidden away.

But if we run the tool manually in both 1.8.3 and master, it's broken on both:

 $ go tool vet -v .
vet: bug.go:3:8: could not import C (can't find import: "C")
Checking file bug.go
 $ go1 tool vet -v .
vet: bug.go:3:8: could not import C (can't find import: )
Checking file bug.go

The obvious, faster option is to revert 62aeb77. Given that we have a bit of time left until 1.9 is supposed to go out, I'll poke around go tool vet to see if I can fix the bug there.

[mvdan]

To clarify - go vet -v on 1.8.3 doesn't actually enable the verbose option, so it doesn't have the Checking file... lines either. Reverting the commit would essentially be replacing one bug with another.

[mvdan]

What I've found so far - the reason that -v makes it fail is this:

// Type check the package.
err := pkg.check(fs, astFiles)
if err != nil && *verbose {
        warnf("%s", err)
}

Should -v affect whether vet returns exit code 0 or 1? That seems unexpected.

And it seems like the underlying cause is go/importer and go/types not playing well with import "C". So perhaps the underlying bug is just a dup of #6774.

I still see the change in behavior as a regression, though. An error that was being ignored is now being surfaced by -v, changing whether go vet is happy or not.

Edit: I did not intend to mark this issue as a duplicate.

[josharian]

Maybe warnf shouldn't cause vet to return 1.

[mvdan]

It certainly seems that way, given the name. That fix would be ok for 1.9, as far as I'm concerned.

[josharian]

Here's where I pass the buck. :)

Maybe warnf shouldn't cause vet to return 1.

It certainly seems that way, given the name.

This decision belongs to @robpike.

That fix would be ok for 1.9, as far as I'm concerned.

That call is above my pay grade. @bradfitz or @ianlancetaylor?

[robpike]

I can't speak to the underlying issue in the importer. That should be fixed.

But I do believe that the exit code should be 1. The purpose of warnf is to report an error but not exit the program, as further tests are possible. It indicates a problem, in this case that it can't import one of the packages, and so vet cannot say that there are no errors in the program.

In short, warnf should cause an exit with code 1. Vet should exit with 0 only when the whole program has been checked successfully and without problems.

[mvdan]

That call is above my pay grade.

Oh, above mine too - I was simply giving my opinion :)

But I do believe that the exit code should be 1.

My point here was not so much whether an import error should make the program fail, or whether a warning should make it fail. It was more the fact that -v is changing the exit code. Depending on whether you were using go vet with -v on your CI with 1.8.x, it will suddenly break for no good reason when upgrading to 1.9rc1 (this is what happened to us).

[bcmills]

The purpose of warnf is to report an error but not exit the program,

Doesn't that imply that warnf calls should never be conditional on the verbose flag? It seems strange to detect an error that indicates a real problem but not to report it.

[ianlancetaylor]

The vet tool runs the type checker in go/types. The warning being reported is a type checking failure. We only report that warning when using -v, because vet is supposed to work on packages that do not type check. So as @bcmills suggests I think that this output is best characterized as not being a warning. I will send a CL.

[gopherbot]

Change https://golang.org/cl/52851 mentions this issue: cmd/vet: don't exit with failure on type checking error