Lint failing with compilation warnings on go-libedit; what's going on?

[Andrei Matei]

Does anyone know what's going on with this pestilence that seems to be affecting random TC builds recently? I see some slack chatter about it but can't make heads or tails.

The logs from the failure end with the following. My first reading would suggest that it's not TestVet that's failing but another one (cause it says "PAUSE  TestLint/TestVet", not "CONT TestVet"), but line 1087 is TestVet.
I think TestVet will fail if `go vet` prints anything to stdout or stderr other than a bunch of crap we ignore. So I guess the question is - is it surprising that `go vet` is trying to compile `go-libedit`? Perhaps the order of subtests in TestLint is non-deterministic (there's a bunch of t.Parallel() around) and perhaps sometimes TestVet is the first to compile go-libedit, where other times it's compiled (and the warnings ignored) before it? Maybe the update to go1.11 changed the order further?
Does anyone know anything?

=== PAUSE TestLint/TestVet

        lint_test.go:1087: 

            In file included from wrap-common.c:1:

        lint_test.go:1087: 

            In file included from vendor/github.com/knz/go-libedit/unix/src/libedit-common.c:1:

        lint_test.go:1087: 

            vendor/github.com/knz/go-libedit/unix/src/c-libedit/common.c:371:21: warning: passing 'wint_t *' (aka 'unsigned int *') to parameter of type 'wchar_t *' (aka 'int *') converts between pointers to integer types with different sign [-Wpointer-sign]

        lint_test.go:1087: 

            vendor/github.com/knz/go-libedit/unix/src/c-libedit/histedit.h:267:37: note: passing argument to parameter here

        lint_test.go:1087: 

            In file included from wrap-search.c:1:

        lint_test.go:1087: 

            In file included from vendor/github.com/knz/go-libedit/unix/src/libedit-search.c:1:

        lint_test.go:1087: 

            vendor/github.com/knz/go-libedit/unix/src/c-libedit/search.c:607:20: warning: passing 'wint_t *' (aka 'unsigned int *') to parameter of type 'wchar_t *' (aka 'int *') converts between pointers to integer types with different sign [-Wpointer-sign]

        lint_test.go:1087: 

            vendor/github.com/knz/go-libedit/unix/src/c-libedit/histedit.h:267:37: note: passing argument to parameter here

[Andrei Matei]

An example TC failure: https://teamcity.cockroachdb.com/viewLog.html?buildId=1097059&buildTypeId=Cockroach_UnitTests

[Peter Mattis]

I'm not sure what is happening, but I can reproduce those warnings when building within the builder image. Why those warnings go on to affect TestLint/TestVet is still a mystery to me.

--
You received this message because you are subscribed to the Google Groups "CockroachDB" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cockroach-db...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/cockroach-db/CAPqkKgmR6Y2%3DPzxt1a%2Be58jz7yBwUEW_3Rw7sfzVGf1yxK9Rjw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Jordan Lewis]

Let's fix this by moving TestVet to be after the parallel steps for now.

To view this discussion on the web visit https://groups.google.com/d/msgid/cockroach-db/CANKgOKgQcP3XqGfZeDsK6DpdqZGqX%2B-pLgvO2U3ACqqK8A0YzQ%40mail.gmail.com.

[Peter Mattis]

I'm not sure that fixes it. Note that we `go build -i` the world before running `TestLint`. Nothing should be built by `go vet`. The fact it is happening in parallel with the rest of the linters should be immaterial.

[Peter Mattis]

I suspect something changed with the go1.11 upgrade, but I'm at a loss for what. I've verified that `go vet` has this same complaint about github.com/knz/go-libedit in go1.10 and go1.11. We haven't changed the C compiler recently which would also explain the new warnings. In fact, I'm quite perplexed as to why TestLint/TestVet doesn't always see this problem. There is some magic that sometimes stops the build warnings from appearing. Something to do with the difference between "go build -i" (which "installs" the output) and "go build -i -v" (which caches the output, including the warnings).

https://github.com/cockroachdb/cockroach/pull/34069 will fix the problem by silencing the upstream warnings. I'm feeling it isn't useful to further understand the intricacies of `go vet` and build caching.

[Andrei Matei]

Thanks for fixing!