Add Mockgen Version Flag #362
Conversation
Add -version flag that shows a version provided by Go modules. This strategy works well for module aware invocations of `go get` and `go run` (which means using GO111MODULE=on). If we ever plan to distribute built binaries, it is necessary to amend the strategy with build flags. GOPATH mode versioning is unsupported.
|
|
||
| func printVersion() { | ||
| if bi, exists := debug.ReadBuildInfo(); exists { | ||
| fmt.Println(bi.Main.Version) |
There was a problem hiding this comment.
I haven't seen this pattern before (usually see a const str). Does this return the version metadata of the go module? Does that mean it won't work unless installed via module support? I'm guessing that's what the build checks and error messaging are about.
There was a problem hiding this comment.
I am not leaning strongly towards the implementation in this PR, but I tend to think it's the best out of the three options that I have considered:
- using constant string
- using build flags
- using debug.ReadBuildInfo
Each option has pros and cons.
Problem with constant string is that any time somebody pulls master version X (not the latest version) of mockgen with go get, they will always end up with mockgen that includes all the commits since X but the version string is unchanged (the binary would report same version for binaries with different commits, which is exactly what we dont want).
Problem with build flags is that they won't work unless we distribute binaries. And I see that current assumption is that people use go get to get the mockgen
Problem with debug.ReadBuildInfo is that it relies on using modules.
I lean towards using debug.ReadBuildInfo. I think it should be the best practice for the users of mockgen to always use predictable version, especially for CI builds. That was the original problem in #250. Thus I think that users should not run go get in GOPATH mode and they should use @ to specify which version of mockgen they want to use. Note that the version info is embedded in the binary, so after go get is used to download the binary the clients can copy the mockgen out of the GOPATH and -version should still work.
As I mentioned in the description of this PR (the GOPATH mode versioning is unsupported) this wont work unless installed via module support
WDYT?
There was a problem hiding this comment.
Problem with constant string is that any time somebody pulls master version X (not the latest version) of mockgen with go get, they will always end up with mockgen that includes all the commits since X but the version string is unchanged (the binary would report same version for binaries with different commits, which is exactly what we dont want).
I've always used a constant string in projects but I haven't thought about the UX of pulling master in that case. I don't think I have a strong opinion and I don't disagree with the point you've made.
I don't think there is anything wrong with making module support a requirement for the version command to work; as long as we fall back to some other behavior when module support is not enabled.
There was a problem hiding this comment.
It looks like there is some nice fallback behavior using the build tags for different versions of go :👍
There was a problem hiding this comment.
@cvgw wrt build tags. Are you referring to the // +build go1.12 tag that has been in this PR?
There was a problem hiding this comment.
I've always used a constant string in projects but I haven't thought about the UX of pulling master in that case. I don't think I have a strong opinion and I don't disagree with the point you've made.
Yeah I agree. Constant string is a good solution if you build and distribute the binary. But! :-) If we did build and distribute the binary, it think there is even better solution than version string. The better solution is to use -ldflags with go build command. That solution allows you to set a value to global property in your golang program during build time.
There was a problem hiding this comment.
@cvgw wrt build tags. Are you referring to the
// +build go1.12tag that has been in this PR?
yes
The better solution is to use
-ldflagswithgo buildcommand. That solution allows you to set a value to global property in your golang program during build time.
I realized that's what we do during compile on another project I help on https://github.com/GoogleContainerTools/kaniko/blob/master/Makefile#L30
Co-Authored-By: Cody Oss <6331106+codyoss@users.noreply.github.com>
Co-Authored-By: Cody Oss <6331106+codyoss@users.noreply.github.com>
Add -version flag that shows a version provided by Go modules.
This strategy works well for module aware invocations of
go getandgo run(which means using GO111MODULE=on).If we ever plan to distribute built binaries, it is necessary to amend
the strategy with build flags.
GOPATH mode versioning is unsupported.