x/tools/cmd/goyacc: Please add "version" command or flag

ehashman commented 4 years ago

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

bash-4.2$ go version
go version go1.12.15 linux/amd64

Does this issue reproduce with the latest release?

Can't tell, see below for details.

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

go env Output

bash-4.2$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/prow/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/prow/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build991634038=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Ran goyacc -version:

bash-4.2$ goyacc -version
flag provided but not defined: -version

What did you expect to see?

I expected a version to be output.

What did you see instead?

I received an error message.

I'm working on a project that uses goyacc to generate files. There are differences in the generated files between versions. Experentially, I can tell that I am using a different version of goyacc in my local environment than is being used in my CI container because the output is different, but I have no way to tell what version is being used where because this flag is not defined.

I spent hours today trying to pin down the source of the differences, and which version of goyacc I needed to be using in order to get a clean build that matched CI. I ended up needing to use git bisect on the goyacc tool to determine a known good version. I even tried using readelf -x and strings on the goyacc binary to determine version information, but I could not find anything there.

Please consider adding a -version flag to the goyacc tool so it is clear and easy to determine differences in output between different versions of the tool. Thank you!

robpike commented 4 years ago

While there's nothing wrong with adding a version flag, I don't understand why you would care since the generated parser should be semantically the same. If the parser, as opposed to its source, has changed behavior, that's a genuine bug that we should fix.

ianlancetaylor commented 4 years ago

I don't know what would keep the version flag up to date. goyacc hardly ever changes: there were no changes between September 2018 and July 2020. It seems risky to rely on human memory to update the -version output.

ehashman commented 4 years ago

Hm, strange. I have a file that is being generated by goyacc that shows whitespace changes if I check out today's HEAD but not an earlier commit (~Jun. 16). I will see if I can bisect to a specific breaking commit tomorrow.

Honestly, I wouldn't mind if you hid a git commit in the ELF file or embedded some identifying version information directly the binary. Doesn't have to be a -version flag. I'm stuck because it's impossible to tell what source a shipped binary in a container was built from.

robpike commented 4 years ago

It's not a "breaking commit". Depending on the exact format of the yacc output (or almost any output, honestly) is not a great idea. It's what the generated code does that matters, not what it looks like.

ehashman commented 4 years ago

When generated artifacts are being checked into source control, as is common, any diffs---semantic or not---result in a dirty build.

I don't mind having to update said artifacts when upstream tools make changes, but I do need to be able to predict and differentiate between what tools produce what outputs for reproducibility.

beoran commented 4 years ago

If you want to get a stable build for generators, you should to the following:

Run your generator by using go generate and a generate.go file containing :

//go:generate go run golang.org/x/tools/goyacc <options>

To keep the version of goyacc stable, also add a tools.go file to the top of your repository which contains:

//build +tools
include (
        _ "golang.org/x/tools"
)

This file serves to make sure the dependency to the tool is maintained in your module.

Finally add the following to your go.mod file

require golang.org/x/tools@v<desired_version> 
replace golang.org/x/tools => golang.org/x/tools@v<desired_version>

The replace statement prevents inadvertent upgrades until you are ready to perform them.

This is a somewhat complex procedure to get a stable build using generators, but that is how it is supposed to work nowadays. Perhaps this procedure should be documented somewhere or explained in a Go Blog post?

ehashman commented 4 years ago

This is one possible solution and I think it would certainly be worth documenting!

However, I don't think this addresses my concern. It avoids the problem, but it does not solve the problem. In that sense, it feels more like a workaround rather than a fix.

Consider that it is possible for there to be multiple versions of goyacc installed on a given system. As an operator using this tool, the installation of which I may or may not control (i.e. in this case I'm working with common CI images that I might not control the builds of), I would like some way to determine what version of the tool is being used without having access to the full build chain for the system.

A feature flag or embedded commit isn't a perfect solution for determining what source a given binary corresponds to (e.g. it doesn't address security concerns without e.g. byte for byte reproducibility), but it would be better than nothing. Golang itself and most other golang binaries that I've used have a -version flag or similar capability, so I didn't expect this to be seen as a novel request; it would be nice for that to be standardized across the toolchain.

ianlancetaylor commented 4 years ago

I'm not opposed to adding a -version flag to goyacc.

But I don't understand how that will solve your problem. It is in the nature of goyacc that essentially any change will change the generated file. After all, the only thing that goyacc does is generate a file. So if it essential that you avoid changes to the generated file, then your goal should be to use a consistent version of goyacc. And the way to do that is to lock to a specific git revision, as @beoran suggests.

To put it another way, if a -version flag will solve your problem, then I suggest that you treat the git revision of your goyacc sources as the version of goyacc.

beoran commented 4 years ago

I see your concern abiut stable builds, but the nice thing about //go:generate go run module/tool is that, in conjunction with my other suggestions, the correct version of the tool will be downloaded by the go compiler and executed during the call to go generate. Like that the tool doesn't need to be installed in the CI or build image, only the go compiler is enough. Combine this with a makefile or even better, a magefile (https://magefile.org/) to coordinate the build.

ehashman commented 4 years ago

@ianlancetaylor My problem is that:

I want to be able to determine what version of goyacc is being run,
because I encountered a situation where two different versions of goyacc on two different systems generated different output
but currently, it is not possible to identify the version delta from the binaries alone, because a given build of goyacc does not contain any version information.

Locking things to a specific git revision by vendoring the tool/putting limitations on it within the project codebase is a workaround. It does not solve the problem in other situations where goyacc is being used as an independent utility: for example, in a continuous integration environment, where goyacc is shipped as part of the base image independently from the code because it is used solely for verification and should not be included in deployed binaries.

(While I do appreciate the gomod advice, this is unfortunately an older project that is in the process of being deprecated that doesn't use gomod, so I wouldn't be able to apply that in this case!)

ianlancetaylor commented 4 years ago

I wonder if we can build on go version -m here.

rsc commented 4 years ago

Like @ianlancetaylor said, it really seems like the solution is to run go version -m .../path/to/goyacc. Every binary has the version information embedded. We shouldn't add a flag to every program.

golang / go