jhump/protoreflect

clarity on v2 api support via `MessageV2`

timruffles opened this issue · 3 comments

The docs for this repo suggest using MessageV2 to achieve some interoperability with proto's v2 API. I realise there's a new repo now, which looks great! But it'd be great to get clarity on how much of the V2 functionality users of this repo can expect to be supported.

For example:

package main

import (
	protov1 "github.com/golang/protobuf/proto"
	"github.com/jhump/protoreflect/desc/protoparse"
	"github.com/jhump/protoreflect/dynamic"
)

func main() {
	p := protoparse.Parser{
		/* setup */
	}
	fds, err := p.ParseFiles(files)
	if err != nil {
		panic(err)
	}

	for _, fd := range fds {
		for _, mt := range fd.GetMessageTypes() {
			msg := dynamic.NewMessage(mt)
			asV2 := protov1.MessageV2(msg)
			// unclear what can safely be done with asV2? 
			// some things appear not to work, but fail silently
			
			// e.g. this prints nil
			fmt.Println(asV2.ProtoReflect().Descriptor().Fields())
		}
	}
}

(Thanks again for this repo!)

jhump commented

@timruffles, as of v1.15.0, the descriptor types in github.com/jhump/protoreflect/desc are now fully interoperable with the stuff in the v2 API -- i.e. google.golang.org/protobuf/reflect/protoreflect. You can d.Unwrap() a desc.Descriptor to get back a protoreflect.Descriptor. Similarly, you can desc.Wrap(...) to turn a protoreflect.Descriptor into a desc.Descriptor.

I think that largely means that code can just use google.golang.org/protobuf/types/dynamicpb instead of github.com/jhump/protoreflect/dynamic -- even if you have a desc.Descriptor (such as from the builder or protoparse packages), you can easily convert to a protoreflect.Descriptor and then do dynamicpb.NewMessage(msgDescriptor).

Also, that is unfortunate that the code snippet is unable to examine the fields that way - I thought that would work. I don't know what else to tell you -- those are limitations in the upstream's implementation of MessageV2. For that one in particular, I don't see why it wouldn't work. But there are other operations, like msg.ProtoReflect().Range(...) that I would expect to fail.

I don't know if/when I'd have the time to explore what all works and doesn't work; sorry. My hope was to instead get people to upgrade to v1.15.0, which then gives a much more clear path to using stuff in the v2 API. And then I'd just create a v2 of this repo, and get people to eventually switch to that (and just never have to deal with the old v1 API stuff again...).

jhump commented

@timruffles, I've updated the notice at the top of the README with a little more guidance. I hope this is sufficient.

Thanks! And 1.15 is great 👏