search

onsi / gomega

Ginkgo's Preferred Matcher Library
http://onsi.github.io/gomega/
MIT License
2.16k stars 282 forks source link

Migrate github.com/golang/protobuf to google.golang.org/protobuf #743

Closed jbduncan closed 6 months ago

jbduncan commented 6 months ago

Fixes https://github.com/onsi/gomega/pull/470.

This PR fixes the merge conflicts in https://github.com/onsi/gomega/pull/490 and uses the latest protoc and protoc-gen-go to fix simple_message.proto with command protoc --go_out=ghttp/protobuf ghttp/protobuf/simple_message.proto.

onsi commented 6 months ago

hey - thanks. just wanted to check (I'm not super familiar with the protobuf ecosystem so bear with me if this is a dumb question): can I safely assume this is a backward compatible change?

Thanks!

jbduncan commented 6 months ago

@onsi Good question! I'm just as unfamiliar with the protobuf ecosystem as you are, but I had to regenerate ghttp/protobuf/simple_message.pb.go with the latest protoc compiler to prevent Go compilation errors when passing it to gomega.VerifyProtoRepresenting, so I think it is incompatible, sadly.

It looks like it's possible to pass instances of old *.pb.go files like the master version of simple_message.pb.go to VerifyProtoRepresenting with VerifyProtoRepresenting(protoadapt.MessageV2Of(simpleMessage)), but existing code will still be broken and I don't know what the recommended way forward for libraries like Gomega is.

jbduncan commented 6 months ago

@onsi I think I've found a way to maintain source backwards compatibility (see e61836c).

On master (which uses github.com/golang/protobuf v1.5.3), VerifyProtoRepresenting accepts proto.Message which is an alias for protoiface.MessageV1. But as of this PR changing things to google.golang.org/protobuf v1.33.0, Protobuf broke backwards compatibility by changing proto.Message to be an alias for protoreflect.ProtoMessage, which seems to be Protobuf's "message V2" API.

Therefore, I've changed VerifyProtoRepresenting to accept protoiface.MessageV1, which is structurally the same interface as what proto.Message was before. Also the new version of simple_message.pb.go implements it, so V2 message types are backwards-compatible too, thankfully.

I decided not to revert simple_message.pb.go back to its old implementation because it used the old package github.com/golang/protobuf, and it didn't feel right to leave Gomega's tests stuck with that package given it's been deprecated.

Any thoughts? Otherwise, I think we're good to go.

onsi commented 6 months ago

thanks @jbduncan - you're now the expert :wink:

I'll merge this in and cut a release. If folks open up issues I'll tag you on them so we can figure out what to do together if need be. Sound ok?

jbduncan commented 6 months ago

thanks @jbduncan - you're now the expert 😉

I'll merge this in and cut a release. If folks open up issues I'll tag you on them so we can figure out what to do together if need be. Sound ok?

@onsi Haha! I suppose I am! Yep, that's totally fine with me. Thanks for merging this in so quickly. 👍