Change https://golang.org/cl/126256 mentions this issue: Reland "godoc: skip build tag annotations when displaying examples"

@andybons: what are the reproduction steps for this?

I don’t understand the issue. Is the problem that vet is complaining about it? Or something else? cc @mvdan just in case it is vet.

Vet complains about misplaced build tags. However, it doesn't use the Go parser to look at actual comments, it does a simple text search on the file. Thus, a multi-line string containing what looks like a build tag can trigger a vet warning. Since go test nowadays runs vet, it causes hard failure.

OK, so go vet godoc/*.go with the test in https://go-review.googlesource.com/c/tools/+/125040 shows the issue. I can try to take a look later today.

Re-titling it as a vet issue.

EDIT: Also, I'm surprised we don't have a single test which has build tags as normal text ? How are we even testing build tags ?

Unless my memory fails me, I improved the vet check so that it would have zero false positives in Go source files, even in the presence of multiline strings. So this sounds like a bug - perhaps an edge case that I missed.

I am clearly missing something here - this only happens with vet from Go 1.10 and not tip, as one would expect after my vet improvement in https://go-review.googlesource.com/c/go/+/111415:

$ git rev-parse HEAD # right before the revert
faa8a71ab532725a7fd9d901a97f74c5038c409b
$ go version
go version devel +5f5402b723 Tue Jul 24 23:54:08 2018 +0000 linux/amd64
$ go vet
$ go1 version
go version go1.10.3 linux/amd64
$ go1 vet
# golang.org/x/tools/godoc
./godoc_test.go:327: +build comment must appear before package clause and be followed by a blank line
./godoc_test.go:328: +build comment must appear before package clause and be followed by a blank line

So as far as I can tell, this is a duplicate of #13533 and has been fixed for 1.11 since May. Do we need 1.10's vet to also be fixed for some reason? The change in question is quite large, so it's likely too invasive to backport into 1.10.x.

How are we even testing build tags?

See src/cmd/vet/testdata/buildtag/.

this only happens with vet from Go 1.10 and not tip

That seems correct. Although it is very hard to say which version of Go is being tested against for the tools repo here https://build.golang.org/?repo=golang.org%2fx%2ftools which caused the test to fail.

Do we need 1.10's vet to also be fixed for some reason?

Assuming we are testing using 1.10, then we cannot add the test done in CL 125040 because the build dashboard becomes red. I am not sure what is to be done here. Maybe we can put the test behind a build tag or something ? (Funny that we are using a build tag to prevent detection of a build tag)

See src/cmd/vet/testdata/buildtag/.

Thanks. Makes sense now since it is fixed at tip. :)

Although it is very hard to say which version of Go is being tested against for the tools repo here

I see three Go versions per each tools repo commit, presumably Go master, 1.10.x, and 1.9.x. So I guess there's our answer.

I am not sure what is to be done here.

One temporary solution, until all 1.12 is out and 1.10 can be dropped, is to "hide" the build tag in the multiline string, like:

const str = `
`+`// +build foo
etc etc
`

@mvdan: updated the reland with a similar workaround: https://go-review.googlesource.com/c/tools/+/126256

@mvdan - Perhaps this should be closed since this is fixed at tip ?

A workaround was used in the CL above, so yes, I agree that this can be closed. Thanks for the ping!