Open jhump opened 3 years ago
Is there any real code outside Google that actually uses MessageSet? It's an artifact of proto1 and it's a source of significant technical debt. In open-source, we ideally should treat it as if it doesn't exist.
I received several bugs against protoparse related to handling of message set wire format. So someone is using it:
FWIW, I believe it is possible (likely even?) that teams inside of Google use protoparse.
Also, as mentioned in the description, this is strangely inconsistent. I can generate such code to Go using protoc-gen-go
and load the descriptor just fine.
Also, as mentioned in the description, this is strangely inconsistent. I can generate such code to Go using protoc-gen-go and load the descriptor just fine.
I agree it's inconsistent, and I'm inclined to make protoc-gen-go
not be able to generate MessageSet code in open-source.
I should note that legacy features like MessageSets and weak fields are guarded by the protolegacy
build tag. We have an implementation for them, but there's no guarantee that we'll keep them around once all the technical debt relying on them is cleared out.
I have a repo that attempts to be a pure Go replacement for protoc. It only deals with descriptors. It would be a rather huge shame if I were unable to use the Go protobuf runtime because of these decisions.
In my case, the code I care about is just dealing with descriptors -- not other aspects of runtime behavior. Not even being able to create a descriptor seems incredibly draconian.
I actually don't have a problem if protoc-gen-go
refused to generate code for the MessageSet, but I think it's a reasonable request that I at least be able to construct and inspect such a descriptor. After all, it's a valid descriptor; why would the runtime not allow me to create it?
Like I originally said, I think the errors belong elsewhere -- like actually trying to marshal/unmarshal such a message.
As I mentioned above, I had to make changes to my own protoreflect for these to work, because there is some need in the community. It's not like I'm asking for any extra surface area or new API, so the actual carrying cost of such a change would be de minimis.
I should note that legacy features like MessageSets and weak fields are guarded by the protolegacy build tag.
Are you suggesting that I should just build with this tag, in lieu of any other kind of change being made?
I think it's reasonable to say that protodesc
should accept any valid descriptors, including ones where MessageOptions.message_set_wire_format
is set. It doesn't need to do anything with the value aside from preserving the contents of the MessageOptions
message.
Trying to marshal/unmarshal such a message would still produce an error, of course.
What version of protobuf and what language are you using? v1.25.0
What did you do? Example proto file
tmp.proto
We can successfully generate Go code for this:
And the resulting code can successfully retrieve the descriptor:
However, if we try to convert the descriptor to a proto and then create a descriptor from that (which would also happen if, for example, we loaded the descriptor from a protoset produced by
protoc
or from a gRPC server via server reflection), we get an error:What did you expect to see? The second, longer program succeeds and prints "tmp.proto".
What did you see instead? Running the new program actually fails:
I can understand an error like that if we tried to use such a message, to serialize or de-serialize. But it seems really weird that we can't even create the descriptor, especially since we can successfully create the descriptor if it is packaged in the
protoc-gen-go
-generated code.