all: use go run for go-bindata go generate commands #4134
Conversation
### WHAT Use go run for go-bindata rather than install go-bindata. ### WHY There are many variations of go-bindata. The install script checks if a binary named go-bindata is present in the PATH. Because there are many variations of go-bindata a dev can have another installed that generates different files. There's not really any need for us to install go-bindata and use it in the way we are. The go run command is now able to run a specific version of a tool without it being installed on the users PATH. We use this pattern for goxdr generation, and the pattern works here too.
| @@ -5,7 +5,7 @@ import ( | |||
| migrate "github.com/rubenv/sql-migrate" | |||
| ) | |||
|
|
|||
| //go:generate go-bindata -nometadata -ignore .+\.(go|swp)$ -pkg dbmigrate -o dbmigrate_generated.go ./migrations | |||
| //go:generate go run github.com/kevinburke/go-bindata/[email protected]+incompatible -nometadata -ignore .+\.(go|swp)$ -pkg dbmigrate -o dbmigrate_generated.go ./migrations | |||
There was a problem hiding this comment.
Instead of specifying the version in every usage, I would use a tools.go file (see https://marcofranssen.nl/manage-go-tools-via-go-modules for instance) and delegate the dependency versioning to go.mod
There was a problem hiding this comment.
Much more context at golang/go#25922
There was a problem hiding this comment.
I've used that pattern before and it carries some complexity that isn't as intuitive or simple as a go run command, and I don't think that additional complexity is outsized by the benefit of shortening these comments by a few characters.
That tools.go pattern was proposed in 2018 before the go run command supported running specific versions as it does today. I think it will be interesting to see if it continues to be common.
In regards to the complexity, adding tools to the module expands the dependency graph of the module, even if they are in a 'tools' tag and not compiled or used at runtime. The dependency graph of our module impacts the dependency graph of any importer of our module, i.e. any importer of the Go SDK. This tool is used in development, not by the compiler, so I don't think we should introduce them into the dependency graph.
There was a problem hiding this comment.
I'll leave it up to you, but embedding the version in every invocation isn't great. I wonder if there is a way to have a different go module just for this, so that it doesn't expand the main module's graph.
There was a problem hiding this comment.
It is possible to use a separate mod file with some build commands, maybe run, I'll give it a go. It may mean there is yet another bespoke thing about our repo though that people need to learn and it'd be nice if we could be less bespoke rather than more. I'll see what it's like.
There was a problem hiding this comment.
I had a go of using a separate go.mod file.
I named it go.tools.mod initially and placed it in the root directory. Every call site had this long go run -modfile=../../../../../../go.tools.mod. Running go mod tidy -modfile=go.tools.mod added all the regular dependencies though and got rid of the dependency I needed.
So I created a directory tools-deps and moved the mod file there, and added a tools.go, following the tools.go pattern. Every call site had this go run -modfile=../../../../../../tools-deps/go.mod.
It was all pretty fiddly, not intuitive to me, and much more fiddly than setting versions in a few places.
but embedding the version in every invocation isn't great.
Assuming you're referring to the inconvenience, I agree, it's inconvenient to have the version in a few places rather than in a single place. I think it is less inconvenient to the multi-mod file tools.go setup though. I don't see any other stand out costs. A little copying isn't so bad? Let me know if I'm missing anything.
There was a problem hiding this comment.
I took a look at how another repo does tools.go, and I'm curious to explore this more, but I think we should merge this as is for now as it gets benefits over what we have today, and tools.go if we add it can be an improvement on this.
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
WHAT
Use go run for go-bindata rather than install go-bindata.
WHY
There are many variations of go-bindata. The install script checks if a binary named go-bindata is present in the PATH. Because there are many variations of go-bindata a dev can have another installed that generates different files.
There's not really any need for us to install go-bindata and use it in the way we are. The go run command is now able to run a specific version of a tool without it being installed on the users PATH.
We use this pattern for goxdr generation, and the pattern works here too.
Known limitations
N/A