Why is ABCI's types.pb.go moved into the abci package?

[thanethomson]

I'm looking for some historical context here, or even some educated guesses.

Why is types.pb.go placed in the ABCI types directory, instead of being left next to its types.proto file as is the case for all the other protos?

This follows from this discussion regarding the use of buf in Tendermint.

[creachadair]

I don't know the history, but can do some archaeology.

abci/types/types.pb.go was first added by commit 4bee228 in June 2018. From the commit log, it appears this was an import from a presumably-separate ABCI repository.

It appears to have been there ever since, so I am guessing it's not that this one was singled out, but that the others were made less-idiosyncratic. Most of the stuff in types/tendermint seems to have gained its current layout from #5025 and #5039 in June 2020. There was some further renaming in #5102, but mostly the directory layout seems to have been preserved after that point.

After that the history gets harder to read because of the move of most of the proto files over to the spec repo. The earliest commit I find moving things to the spec repo is from September 2019 (95cf253), but I did not find when those files might have been moved there. Possibly they were always in the spec repo, and we had been cloning them all along? It is not clear.

[thanethomson]

Ah interesting. Thanks for doing that digging!

Is there any reason it needs to be kept there? I can understand the convenience of course, having the proto-generated ABCI types colocated with all the other ABCI-related Go types. Also, moving it would probably be a substantial breaking change in the API - I can investigate and quantify this at some point when I've got some time.

[creachadair]

Is there any reason it needs to be kept there? I can understand the convenience of course, having the proto-generated ABCI types colocated with all the other ABCI-related Go types. Also, moving it would probably be a substantial breaking change in the API - I can investigate and quantify this at some point when I've got some time.

There is no fundamental technical reason for it, mainly at this point it would require updating all the import paths that depend on it (not only in our own code, but in anyone depending on it, e.g., the SDK). If we split the proto generated code from the non-generated code, we'd also have to split the import paths. We can certainly do that, it's mostly just inconvenient.

There are a couple of places where we do add additional wrapper code to the generated stubs, notably some JSON encoding wrappers, probably to work around the fact that protobuf types don't marshal correctly to JSON out of the box. Those would have to live in the same package, since you can't add methods to things in other packages.

For folks accustomed to the Bazel outlook, where proto generated code is an intermediate artifact of the build and is not checked in, this tactic is sometimes frowned upon. I see it as a reasonable compromise, though, once we've decided to commit our generated code to the repo, and it plays more nicely than the alternatives with the standard go get and go install behaviour. The alternative is to hand-curate a bunch of parallel shim types and manually convert to and from them.

I think those are the main points we'd have to address.