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

Custom options? #256

Open
lquerel opened this issue Dec 26, 2019 · 16 comments · May be fixed by #591
Open

Custom options? #256

lquerel opened this issue Dec 26, 2019 · 16 comments · May be fixed by #591

Comments

@lquerel
Copy link

lquerel commented Dec 26, 2019

Are the custom options supported?
I tried options and uninterpreted_option without success.
Thanks
Laurent

@killercup
Copy link

I'm into this as well at the moment. I followed the proto3 docs to add a custom option and prost fails to read in the files when I don't import the correct defintion so I know it's doing something; but nevertheless uninterpreted_option stays empty.

@killercup
Copy link

So looking at a test.proto like this

syntax = "proto3";
package test;

import "google/protobuf/descriptor.proto";

extend google.protobuf.MessageOptions {
  int32 message_opt1 = 7739036;
}

message Test {
  option (message_opt1) = 420000;
  option no_standard_descriptor_accessor = true;
  
  string content = 1;
}

and calling protoc --json_out=./ ./test.proto (libprotoc 3.11.4, using protoc-gen-json), the only output I get for the Test message is this:

{
  "name": "Test",
  "field": [
    {
      "name": "content",
      "number": 1,
      "label": 1,
      "type": 9,
      "json_name": "content"
    }
  ],
  "options": {
    "no_standard_descriptor_accessor": true
  }
}

To me, this looks like there are no uninterpreted_options collected? Is this a bug in protoc?


Full JSON output -- quite long

@danburkert
Copy link
Collaborator

prost currently doesn't expose message descriptors in any interface except ServiceBuilder, so I'm curious what the usecase for options is? What does supporting them mean in the context of prost?

@killercup
Copy link

killercup commented Feb 27, 2020

Ah, I should've probably explained that. I've patched prost-build locally to also give me a Vec<prost_types::FileDescriptorProto> to generate some helper data structures (I'll make a PR when it works in my project and you're intested). I'd like to custom options to have some metadata for this on my messages.

@danburkert
Copy link
Collaborator

I see. It's a longer-term goal to expose message descriptors in generated code in some way, but it's going to need some design. Having a PR to go off might be very helpful!

Back to the question at hand - prost isn't doing anything that might strip options from the descriptors, afaik. You can look and see exactly how prost-build is invoking protoc to try and isolate where they might be getting dropped.

@wildarch
Copy link

I looked into this for project-oak/oak#668, and the issue seems to be that whatever parses the FileDescriptorSet needs to be aware that the extension exists for it not to be dropped. I have a messy but working sample repo here: https://github.com/wildarch/handle_extract.

Judging from docs here, no uninterpreted_option fields should ever be set within options.

It seems to me that for this to work prost would need to support extensions, which I don't think it currently does?

@wildarch
Copy link

I was able to extract field options with a very small patch to the prost-types crate. As long as the tag number matches the one defined in the extension on FieldOptions it parses just fine.

I was talking to @tiziano88 about generalizing this, and we thought perhaps a flag can be added to prost-build that enables extension support for particular types. I imagine this would then generate code at the end of the struct similar to:

#[derive(Clone, PartialEq, ::prost::Message)]
struct MyProtoMessage {
  // ...

  #[prost(unknown_fields)]
  pub unknown_fields: HashMap<u64, Bytes>,
}

Note that this is slightly more generic than just extensions, but those would be the primary use case. Enabling this flag when generating the code for prost-types should be enough to expose custom options.

The invocation of prost-build would look something like:

Config::new().unknown_fields("MyProtoMessage").compile_protos(...)

I'm planning to write a PR that implements this, happy to hear any feedback on the design above 😄.

@killercup
Copy link

@wildarch I'm slightly confused (maybe I just need more coffee) -- what is MyProtoMessage here? A message or a custom type for representing the structure of proto messages? Can your approach cover what I wrote in https://github.com/danburkert/prost/issues/256#issuecomment-591901770 or would I need to special-case every single one of my messages?

@wildarch
Copy link

wildarch commented Apr 30, 2020

MyProtoMessage is just an example message here for which the unknown_fields flag is enabled. For your case it would look like this:

Changes to MessageOptions:

struct MessageOptions {
  ... (all the same)

  #[prost(unknown_fields)]
  pub unknown_fields: HashMap<u64, Bytes>,
}

If you have a reference to the DescriptorProto for your Test message, you can do something like test_msg_desc.options.unwrap().unknown_fields.get(7739036).unwrap() and the returned value will be a Bytes containing 420000. I hope that makes sense?

I should note that while the DescriptorProto type is public I don't think prost currently exposes an API to retrieve it. For our use case we would need it during code generation for fields, so I'm thinking of proposing an API like:

impl Config {
  pub fn override_field(&mut self, field_descriptor: &FieldDescriptorProto) -> Option<(Name, Type)>
}

We could have similar things for messages, would that cover your use case? Alternatively, once we have runtime introspection you could use that to get the option value.

@Frando
Copy link

Frando commented Jul 15, 2020

I am using prost with a ServiceGenerator and have to support a custom option on services and methods. I could not get it to work.
In my .proto file, I have:

syntax = "proto2";

package myapp;

import "google/protobuf/descriptor.proto";
extend google.protobuf.ServiceOptions {
  optional uint32 service_id = 50000;
}
extend google.protobuf.MethodOptions {
  optional uint32 id = 50001;
}

service MyŚervice {
  option (myapp.service_id) = 1;

  rpc Open (Req) returns (Res) { option (myapp.id) = 1; 
  rpc Close (Req) returns (Res) { option (myapp.id) = 1; 
}

If I compile the file with protoc to a FileDescriptorSet and decode that with protoc --decode_raw, I see that the custom options and values are included, and protoc doesnt show erros.

If I understand things correctly, the options should appear in Service.options.uninterpreted_option in my ServiceGenerator. However, that vector is empty. Do I miss something, or does someone have a pointer on how to get this working?

Edit: I think this needs extension support (#100 )?
Edit 2: It took some time to dive into this. I've found #317 now which looks much simpler than full extension support, with #287 I think options could be supported at least in the ServiceGenerator

@Frando
Copy link

Frando commented Jul 16, 2020

OK, with #317 plus the changes I linked in my comment, I have working custom options for my service generator!

With #317, in my build.rs, I still have to set the tags manually (because prost doesn't support extensions), and the value has to be parsed manually, but that's allright for me so far.

For reference, and for others following along, this is what I arrived at to parse an uint64 "id" option on both services and methods (depending on #317):

pub trait MyOptions {
    fn get_unknown_fields(&self) -> &Vec<prost::UnknownField>;
    fn id_tag(&self) -> u32;
    fn id(&self) -> Option<u64> {
        let tag = self.id_tag();
        let fields = self.get_unknown_fields();
        let field = fields.iter().filter(|f| f.tag == tag).nth(0)?;
        let mut id = 0u64;
        varinteger::decode(&field.value[..], &mut id);
        Some(id)
    }
}

impl MyOptions for prost_types::MethodOptions {
    fn get_unknown_fields(&self) -> &Vec<prost::UnknownField> {
        &self.protobuf_unknown_fields
    }
    fn id_tag(&self) -> u32 {
        50001
    }
}

impl MyOptions for prost_types::ServiceOptions {
    fn get_unknown_fields(&self) -> &Vec<prost::UnknownField> {
        &self.protobuf_unknown_fields
    }
    fn id_tag(&self) -> u32 {
        50000
    }
}

And then, in my service generator, I can access the id on service.options.id() and method.options.id().

So - would be great to get #317 in 🚀

@autodidaddict
Copy link

I'm having this problem as well - I have custom options in my .proto file and the uninterpreted options vec is always empty. I need access to the custom options when generating my service..

@mkaes
Copy link

mkaes commented Jun 27, 2021

Is there any update on this?
I have gathered all the patches by @Frando, @wildarch and some others in my own branch and after some fixes they do work as expected for custom types. Since I use this a lot in some projects it would be nice to get this in the main branch.

@nswarm nswarm linked a pull request Feb 5, 2022 that will close this issue
@cemoktra
Copy link

cemoktra commented Aug 5, 2022

Pushing this one. Would be really nice, FieldDescriptorProto knows there is an option but has no information about it. Would be really nice to get this

@m1cr0man
Copy link

I would also like to see this resolved. I am trying to define custom options on a service method, and I ended up here whilst trying to figure out why uninterpreted_options was empty. Fingers crossed for #591

@Sculas
Copy link

Sculas commented Apr 9, 2023

The prost repo is still very active, so why is this issue pretty much abandoned? Really hoping to see this merged someday as I also really need this for one of my projects.

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

Successfully merging a pull request may close this issue.

10 participants