Support preservation of unknown fields for proto3

[stevvooe]

In protobuf 2, fields unknown to a particular schema were generally passed through under Read-Modify-Write or passthrough conditions. This allowed use cases that supported uncoordinated upgrade paths where older intermediates could operate with knowing about updated schemas. This feature was silently dropped in proto3 and I am not sure it was ever supported in gogo.

Likely, gogo protobuf support can be controlled by enabling a plugin or option. This determination is dependent on the design details of the upstream protobuf project.

Please see protocolbuffers/protobuf#272 for details. The maintainers have also released a document describing the plan.

There may be nothing to do in this project at this time. Please let me know if this has already been visited.

[awalterschulze]

I was just following golang/protobuf and the proto3 spec.
So if golang/protobuf brings it back, which I am gathering they will, then gogo/protobuf will also bring it back.

gogoprotobuf has an option goproto_unrecognized to turn this generation off in proto2.

[awalterschulze]

I am closing this, but just comment if you would like it reopened.

[stevvooe]

There is still discussion on this upstream. This is a wider "protobuf" issue more than a language issue.

[awalterschulze]

Technically we could support this with goproto_unrecognized=true, but that will require some work. Would you like this issue reopened?

[stevvooe]

@awalterschulze They made some upstream changes regarding this but I am not sure what the path forward here is.

[awalterschulze]

@stevvooe
This is a huge piece of work, of which I have completed the biggest/hardest part.
This work is available on the upstream branch.
At some point (given time) I plan to merge the upstream branch in a dev branch and then start merging their dev branch into ours. One commit at a time.
It would be a great help if you could test the gogoprotobuf's upstream branch on your code base.

[stevvooe]

@awalterschulze I gave it a shot and the generated code looks okay.

I did get the following error when building:

~/g/s/g/c/containerd ❯❯❯ make                                                                                                                                                                                                                                                                                  ⏎ master ✭ ✚ ◼
🇩 bin/ctr
# github.com/containerd/containerd/vendor/k8s.io/apimachinery/pkg/api/resource
vendor/k8s.io/apimachinery/pkg/api/resource/quantity_proto.go:26:7: undefined: proto.Sizer
Makefile:168: recipe for target 'bin/ctr' failed
make: *** [bin/ctr] Error 2

Looks like a definition is missing from the gogo/protobuf/proto package.

I have the changes in a branch in https://github.com/stevvooe/containerd/tree/try-gogo-upstream.

[awalterschulze]

I will look into this. I also started merging upstream into dev and more commits from golang dev into gogo dev, to mimic golang protobuf.

…

On Sat, 7 Apr 2018, 00:01 Stephen Day, ***@***.***> wrote: @awalterschulze <https://github.com/awalterschulze> I gave it a shot and the generated code looks okay. I did get the following error when building: ~/g/s/g/c/containerd ❯❯❯ make ⏎ master ✭ ✚ ◼ 🇩 bin/ctr github.com/containerd/containerd/vendor/k8s.io/apimachinery/pkg/api/resource vendor/k8s.io/apimachinery/pkg/api/resource/quantity_proto.go:26:7: undefined: proto.Sizer Makefile:168: recipe for target 'bin/ctr' failed make: *** [bin/ctr] Error 2 Looks like a definition is missing from the gogo/protobuf/proto package. I have the changes in a branch in https://github.com/stevvooe/containerd/tree/try-gogo-upstream. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#275 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvsLexaOswaO3CxLe5kfyG5utDfixriks5tl-WggaJpZM4Mc9WN> .

[awalterschulze]

I found it. Here it seems to have been deleted
https://github.com/gogo/protobuf/pull/399/files#diff-91571d85fc4075309e506f26c5941fc5L47

[awalterschulze]

@stevvooe I have just added proto.Sizer back on the dev branch
2c90c88

[awalterschulze]

The dev branch has progressed to a place where XXX_unrecognized is now being generated for proto3. But goproto_unrecognized=false now turns of generated of XXX_unrecognized for both proto2 and proto3 :)
Please let me know if this works for you?

Also I wanted to know whether you had any problems with the XXX_NoUnkeyedLiteral field being generated?