Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with gogo/protobuf #359

Closed
ditsuke opened this issue Jun 22, 2022 · 2 comments
Closed

Compatibility with gogo/protobuf #359

ditsuke opened this issue Jun 22, 2022 · 2 comments

Comments

@ditsuke
Copy link

ditsuke commented Jun 22, 2022

I want to use Twirp with structures generated by gogo's gofast compiler for idiomatic go structs. As of v8.1.2, twirp generated code seems to not be compatible with gogo's gofast compiler (twirp seemed to have a compatibility test as of v7.2.0). Are there any plans for gofast compatibility and are there any generator options I'm missing?

@rhysh
Copy link
Contributor

rhysh commented Jul 25, 2022

What commands did you run, and what failing output did you see?

I tried downloading the latest version of https://github.com/gogo/protobuf (v1.3.2) and of Twirp, and using libprotoc 3.21.3 and gogo's gofast to regenerate the code in the ./example directory. After an ad-hoc go mod init, I ran go build ./... and got errors like this:

example/service.twirp.go:118:72: cannot use in (variable of type *Size) as type protoreflect.ProtoMessage in argument to doProtobufRequest:
	*Size does not implement protoreflect.ProtoMessage (missing ProtoReflect method)

Is that what you're reporting?


That function call uses arguments of type "google.golang.org/protobuf/proto".Message value, which is an alias to "google.golang.org/protobuf/reflect/protoreflect".ProtoMessage, which is:

type ProtoMessage interface{ ProtoReflect() Message }

That's a change from "github.com/golang/protobuf/proto".Message, which is https://pkg.go.dev/google.golang.org/protobuf@v1.28.0/runtime/protoiface#MessageV1:

type MessageV1 interface {
	Reset()
	String() string
	ProtoMessage()
}

The code that gofast generates appears to implement protoiface.MessageV1, so it may be usable with the deprecated github.com/golang/protobuf/proto package, but not with the newer google.golang.org/protobuf/proto package.

func (m *Size) Reset()         { *m = Size{} }
func (m *Size) String() string { return proto.CompactTextString(m) }
func (*Size) ProtoMessage()    {}

It looks like Twirp v8 included a move from the deprecated github.com/golang/protobuf module to its successor google.golang.org/protobuf in #304. It looks like github.com/gogo/protobuf can only interop with the deprecated module. So this doesn't look Twirp-specific to me. Maybe the gogo maintainers have ideas on how to interop with the current version of Google's protobuf+Go support? But I could be missing something.

Twirp v8 requires that its generated code can use a message type Msg like this:

package example

import "google.golang.org/protobuf/proto"

var _ proto.Message = (*Msg)(nil)

@wmatveyenko
Copy link
Contributor

Hi @ditsuke, just checking in here.
Since we view Google as the main vendor of protobuf, we want to stay up to date with that implementation.
If gogo/protobuf is not keeping up with those releases, we would prioritize tracking the canonical implementation of protobuf from Google.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants