Restore ability to regenerate merkledag.pb.go #5012
Conversation
Due to the IPLD-PB format relying on a mis-feature of gogo-protobuf, the process of (re)generating the golang protobuf bindings is way harder than it ought to be :( What I do here is compile-and-execute a patched version of the gogofast encoder, and present it as a temporary generator-plugin to the stock `protoc` binary. This ensures we still get the effect we want, but the "nonstandard" generator does not get installed anywhere. All tests verified to pass despite the seemingly substantial changes in the generated code License: MIT Signed-off-by: Mib Kd743naq <mib.kd743naq@gmail.com>
|
@whyrusleeping hrm... the jenkins failure seems to be something unrelated with your (originally ) name on |
|
It is because Windows Jenkins was not yet updated to Go 1.10. |
merkledag/pb/regen_pb.sh
Outdated
| set -e | ||
|
|
||
| if ! [ -f merkledag.proto ] ; then | ||
| echo 1>&2 "You must run the regenerator wihile in the directory containing merkledag.proto" |
There was a problem hiding this comment.
Typo, did you mean: "while in the"?
There was a problem hiding this comment.
You are correct, fixed
|
@Kubuxu any thoughts on this PR? It is pretty straightforward as a workaround. If you prefer to re-vendor a patched-up version into gx - I sadly couldn't figure out how to do that :( |
|
@mib-kd743naq does this patch apply on a master HEAD of gogo-protobuf? |
|
@Kubuxu yeah, with offsets: |
| export PATH=".:$PATH" | ||
| protoc -I. -I$GOPATH/src/$GOGO_GX -I$GOPATH/src/$GOGO_GX/protobuf --gofast_gxpin_out=. merkledag.proto | ||
|
|
||
| perl -p -i -e "s{github.com/gogo/protobuf}{$GOGO_GX}g" merkledag.pb.go merkledagpb_test.go |
There was a problem hiding this comment.
would be great to have a couple comments here describing what this perl does. In case it needs to be rewritten in the future because some platforms wont have perl installed
There was a problem hiding this comment.
Note: This is actually a fairly simple one liner that is just replacing github.com/gogo/protobuf with $GOGO_GX (after being expanded by the shell) in the files merkledag.pb.go and merkledagpb_test.go. 's{}{}' is used instead of 's///' to avoid unnecessary escaping.
|
Also, ideally, this would allow us to update to a newer version of the protobuf library. |
|
@whyrusleeping I guess you maen #3214. I can give it a shot ( will close this PR and try a new one later tonight to reduce churn ). Is the desire to throw away the tests to make the gx-snapshot smaller still valid? |
|
@mib-kd743naq Yeah, having 40MB worth of code in a dependency is pretty annoying. I would love to have that fixed. |
|
Hey guys, if u manage to figure it out, also have no idea, would be amazing if there could be a new rule in the Makefile new Protobuf section, aside Build like: For now I installed protobuf globally then run into few errors regarding version as it's old but it works. Good job with the project btw! |
|
@EnchanterIO, in general, you can run |
While performing merkledag.proto-related experiments I ran into rather unexpected difficulties regenerating the damn thing. Pushing my work upstream without making any functional changes, to hopefully save several hours to the next person ending up looking there
The TLDR is that one now can get the stack to do the right thing by simply:
I would not be in any way opposed to this PR being rejected and a better pin being created within gx with all the necessary changes that go with this