proto: deprecate {Unm,M}arshalMessageSet{JSON} #741
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
According to a quick GitHub code search, there are no known external MessageSet messages: |
dsnet
force-pushed
the
deprecate-messageset
branch
from
dd64bd8 to
c231619
Compare
|
\cc @jmarais @donaldgraham (maintainers over at gogo/protobuf). Let me know if you have reason to believe people depend on these. MessageSets were already deprecated upon the first open source release of protobufs in 2008 and I'm aware of zero real usages externally. |
Despite the naming, these are "internal-only" functions that are intended to
only be called from generated code for MessageSets.
Furthermore, MessageSets are themselves a deprecated feature from proto1 days,
such that descriptor.proto even warns against their use since the initial
open-source release of protocol buffers in 2008. Within Google,
there are no direct usages of these functions, and all existing
usages of MessageSets go through the new table-driven implementation.
In addition to deprecating the {Unm,M}arshalMessageSet{JSON} top-level functions,
also modify the generator to stop emitting MarshalJSON and UnmarshalJSON methods
for messages sets. The UnmarshalJSON method is not implemented and the
MarshalJSON method does not seem to be called anywhere in Google (verified by
making the method panic and doing a global test). The jsonpb package continues
to work with MessageSets.
I should note that when the table-driven implementation was open sourced
in late 2017 (see 8cc9e46), it accidentally
removed generation of the Marshal and Unmarshal method. However, no one seemed
to have noticed that those methods were no longer generated.
dsnet
force-pushed
the
deprecate-messageset
branch
from
c231619 to
3a1a64b
Compare
neild
approved these changes
Oct 31, 2018
dsnet
added a commit
that referenced
this pull request
Nov 27, 2018
Despite the naming, these are "internal-only" functions that are intended to
only be called from generated code for MessageSets.
Furthermore, MessageSets are themselves a deprecated feature from proto1 days,
such that descriptor.proto even warns against their use since the initial
open-source release of protocol buffers in 2008. Within Google,
there are no direct usages of these functions, and all existing
usages of MessageSets go through the new table-driven implementation.
In addition to deprecating the {Unm,M}arshalMessageSet{JSON} top-level functions,
also modify the generator to stop emitting MarshalJSON and UnmarshalJSON methods
for messages sets. The UnmarshalJSON method is not implemented and the
MarshalJSON method does not seem to be called anywhere in Google (verified by
making the method panic and doing a global test). The jsonpb package continues
to work with MessageSets.
I should note that when the table-driven implementation was open sourced
in late 2017 (see 8cc9e46), it accidentally
removed generation of the Marshal and Unmarshal method. However, no one seemed
to have noticed that those methods were no longer generated.
Change-Id: I5fa3ddc452ff1906a4d7c31c6e0a2902702784ca
Cherry-Pick: github.com/golang/protobuf@951a149f90371fb8858c6c979d03bb2583611052
Original-Author: Joe Tsai <joetsai@digital-static.net>
Reviewed-on: https://go-review.googlesource.com/c/151435
Reviewed-by: Damien Neil <dneil@google.com>
thaJeztah
added a commit
to thaJeztah/swarmkit
that referenced
this pull request
Oct 20, 2019
full diff: golang/protobuf@v1.2.0...v1.3.2 protobuf v1.3.2: - golang/protobuf#785: grpc code generation: add an UnimplementedServer type implementing each server interface, returning an unimplemented error for each method - golang/protobuf#851: convert prints to os.Stderr to use log.Printf - golang/protobuf#883: jsonpb: fix marshaling of Duration with negative nanoseconds protobuf v1.3.1: - The set of dependencies specified in go.mod has now been reduced to only the standard library. protobuf v1.3.0: - golang/protobuf#699: add a go.mod module file - golang/protobuf#701: stop generating package "// import" comment - golang/protobuf#741: deprecate {Unm,M}arshalMessageSet{JSON} - golang/protobuf#760: different internal implementation of oneofs - `.pb.go` files generated by protoc-gen-go@v1.3.0 will require the proto@v1.3.0 package to work - various minor changes to code generation Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Despite the naming, these are "internal-only" functions that are intended to
only be called from generated code for MessageSets.
Furthermore, MessageSets are themselves a deprecated feature from proto1 days,
such that descriptor.proto even warns against their use since the initial
open-source release of protocol buffers in 2008. Within Google,
there are no direct usages of these functions, and all existing
usages of MessageSets go through the new table-driven implementation.
In addition to deprecating the {Unm,M}arshalMessageSet{JSON} top-level functions,
also modify the generator to stop emitting MarshalJSON and UnmarshalJSON methods
for messages sets. The UnmarshalJSON method is not implemented and the
MarshalJSON method does not seem to be called anywhere in Google (verified by
making the method panic and doing a global test). The jsonpb package continues
to work with MessageSets.
I should note that when the table-driven implementation was open sourced
in late 2017 (see 8cc9e46), it accidentally
removed generation of the Marshal and Unmarshal method. However, no one seemed
to have noticed that those methods were no longer generated.