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

Support new oneof feature #95

Open
GoogleCodeExporter opened this issue Apr 7, 2015 · 11 comments
Open

Support new oneof feature #95

GoogleCodeExporter opened this issue Apr 7, 2015 · 11 comments

Comments

@GoogleCodeExporter
Copy link

Currently, trying to compile a .proto file using new "oneof" feature with 
protogen will make protoc display the following errors:

meta_data2.proto:8:5: Expected "required", "optional", or "repeated".
meta_data2.proto:8:19: Expected field number.

This is of course because the protoc version used by protobuf-csharp-port is 
old and the feature is new. Since oneof is a very desirable feature, it would 
be nice if protobuf-csharp-port would support it.

Original issue reported on code.google.com by [email protected] on 14 Nov 2014 at 3:58

@GoogleCodeExporter
Copy link
Author

Yes, agreed - there's a bunch of work I want to do to bring the project up to 
speed with the original project, but I don't have time right now. It's not just 
a matter of updating protoc... there's the code to support it in C# that needs 
writing too :)

I don't have a time frame for this right now, but it *is* on my list...

Original comment by [email protected] on 14 Nov 2014 at 4:01

@GoogleCodeExporter
Copy link
Author

Most appreciated Jon

Original comment by [email protected] on 25 Nov 2014 at 7:22

@GoogleCodeExporter
Copy link
Author

Any news on getting 2.6 support?

Original comment by [email protected] on 6 Feb 2015 at 9:09

@GoogleCodeExporter
Copy link
Author

There are plans to go straight to 3.0. I'm not in a position to give details 
about anything right now, but I definitely intend it to happen. Sorry I can't 
be any more specific.

Original comment by jonathan.skeet on 6 Feb 2015 at 10:24

@Camuvingian
Copy link

Is there any news on this as one of my team has implemented oneof in Java and C++ and I am in charge doing this for C#; it is looking like there is not currently support for this anywhere? Thanks for your time.

@jskeet
Copy link
Owner

jskeet commented Feb 2, 2016

I have no plans to add features to this codebase. oneof is fully supported in the proto3 implementation at https://github.com/google/protobuf, along with all other proto3 features (maps, official JSON representation etc)... but be aware that that does not support proto2.

Given that oneof doesn't require any wire changes, you could fake part of it reasonably easily against proto2 codegen if you only need it in a couple of very specific situations - just manually write partial classes against the generated code to add a property which checks which value has been set. That's fine for reading, but you wouldn't get any benefits on the writing side though... you might want to add a method on the builder which sets a field and clears the others in the oneof... it depends on your situation.

@dvescovi1
Copy link

We, like the other commenters, would like oneof support in our proto2 implementation.
Its just not practical for us to migrate all the way to proto3.
Judging by your comments it looks like you have at least given it some thought. I guess it is already done in proto3 under the Google/protobuf project (probably under your direction?).
Do you believe a back port is possible? If so, what would the scope of work involve? Is it something we could undertake without knowing all the nitty-gritty internal details?

@jskeet
Copy link
Owner

jskeet commented Jun 28, 2016

It's certainly not something I would consider doing in this version - as I've said before, this version is really in "serious bugs only" mode. I simply don't have the time to put any very significant effort into it.

I'm sure it would be feasible to add oneof support to this version, but it's definitely more work than I'd be willing to take on. Even reviewing someone else's code on it is likely to be quite a bit of work, to be honest - because there are quite a few potential corner cases.

@dvescovi1
Copy link

I understand the status of the project and I was not proposing any work on your part. I was just trying to understand just how one might go about adding oneof support knowing you are probably privy to how they may have done it in the Google/protobuf 3 project. You dropped some proposals earlier in the comments. I was just digging for a little more info. Thanks.

@jskeet
Copy link
Owner

jskeet commented Jun 28, 2016

To be honest, anyone else adding support would still require work on my part - to review it and then support it afterwards.

In terms of how it's implemented in the proto3 project, the simplest approach is probably to look at the output generated for a oneof... although it's worth bearing in mind that the messages in the proto3 port are mutable, which will change things somewhat.

@TheMightyPope
Copy link

Thanks for the framework Jon. I understand you don't have the time, and this discussion is pretty stale anyway. I think I have the same problem that others will have, which is that we work with partners that have, for whatever reason, implemented using proto2. This old project can't handle oneof, and the new Google protoc can't compile proto2 code. For me, I can't reasonable ask other companies I work with to change their proto definitions, and meanwhile Google says proto2 will be supported for a long time. My best case scenario would be that the new protoc would support proto2 as well, and we could have everything in one place. But I know I'm dreaming, so I'll try and sort out the oneof problem now from my generated C# code manually. Thanks for all the great contributions!

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

No branches or pull requests

5 participants