cmd/go install build dependencies using +build tools with modules

[andig]

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

$ go version
go version go1.12.7 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/andig/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/andig/htdocs/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/opt/go/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/andig/htdocs/mbmd/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/w5/_c0nb6n90fn96tzw04dtc6240000gn/T/go-build898061171=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Situation: go modules enabled and go.mod populated. Consider build relying on go dependencies in some file:

//go:generate stringer -type=Measurement

It seems the proper way to do this is/was adding these as anonymous imports:

// +build tools

package main

import (
	_ "golang.org/x/tools/cmd/stringer"
)

During build, e.g. on CI, this will fail as stringer won't be found unless it is a) explicitly installed and b) $GOPATH/bin is in the path.
Running stringer using go:generate go run ... isn't a valid workaround either as that would break during cross-builds.

What did you expect to see?

No error during build without additional installation and path config

What did you see instead?

build failed.

[andig]

May be somewhat related to #30515

[zikaeroh]

And if you aren't already aware, @myitcv's gobin is a great way to run these tools dependencies.

(Though, I'm a bit confused why you'd want to run stringer in CI, not committing generated code leads to all sorts of issues for users of your code.)

[andig]

Was not aware of gobin, but doesn‘t that just move the problem of installing to gobin? The go build command is already aware of all relevant path, module and tools information and should be able to find the build tools it installed without external help. Otherwise the tools build comment seems of little help?

As for stringer not committed: for time being I‘ve doen it this way to make sure I don‘t forget regenerating locally and can ensure that ci and goreleaser will always have latest version of generated assets.

[zikaeroh]

You should be able to just do go run example.org/full/path/to/main, and have it run based on your currently active module. Since that causes a recompile each invocation, gobin is there to cache the result and recall it later (speeding things up). Marginally related, but I have a custom script in my dotfiles which uses a local module and gobin to "install" my global Go tooling, rather than going into GOPATH. It can be bootstrapped using go run to get gobin to install itself, then go from there. (And I'm not entirely certain I know what you mean by go run breaking during "cross-builds".)

As for committing your generated code, not doing so can cause issues depending on the scenario (like #32720 which ended up being maxbrunsfeld/counterfeiter#126). You could also rerun go generate in CI, then check that git status --porcelain is empty (which also works for verifying go mod tidy has been run). But, whatever works for you; this issue isn't about committing generated code. 🙂

[myitcv]

Running stringer using go:generate go run ... isn't a valid workaround either as that would break during cross-builds.

Can you explain what you mean here? go run is identical to any solution using go install, just minus the "install" bit.

but doesn‘t that just move the problem of installing to gobin?

This is indeed the main issue with gobin right now.

The advantage gobin has over go run is that it avoid the costly link step (which starts to add up if you have multiple invocations of gobin).

FWIW, I use gobin for all of my go:generate directives, e.g.

//go:generate gobin -m -run golang.org/x/tools/cmd/stringer -type=Pill

https://github.com/myitcv/gobin/blob/master/README.md

Regarding committing generated code/re-running go generate etc. My advice would be:

• always commit generated code
• re-run go generate (or equivalent) during CI
• fail the build if the result leaves you with an "unclean" git status, i.e. there are changes in the working tree:

test -z "$(git status --porcelain)" || (git status; git diff; false)

https://github.com/myitcv/gobin/blob/350ab1036b24c551fd9da4410be39d85eeaab02e/.travis.yml#L70

I‘ve doen it this way to make sure I don‘t forget regenerating locally and can ensure that ci and goreleaser will always have latest version of generated assets

I'm going to close this as a dup of #30515. All of the points you raise are totally valid but this issue (and many others) are effectively covered by that issue, specifically this comment: #30515 (comment).

[andig]

Running stringer using go:generate go run ... isn't a valid workaround either as that would break during cross-builds.

Can you explain what you mean here? go run is identical to any solution using go install, just minus the "install" bit.

What I meant is really #33354 (comment). I did use

//go:generate go run ...

in the past only to have that fail during cross-builds as the go:generate-invoked tool would end up being of the wrong architecture.

My advice would be ...

Understood, thank you for taking the time to lay out the best practices! Again a wonderful experience with the go community.

Looking at this once more I think this might even solve my problem from #33354 as the invocation of go generate would move out of the actual cross-build process. Downside: if there are any tools that need to produce different results during cross-build that, again, would break or need additional instrumentation. Stringer isn't so thats a non-issue here.

I'm going to close this as a dup of #30515.

I'll need to fully read it and maybe add //+build comments to the discussion.

[andig]

See #33518 (comment) for proposal