Closed brian-scb closed 2 years ago
I just encountered the same issue. There are IMO 2 solutions to this problem, I'm fine with either if @Arshia001 is fine with them.
Change the
let Descriptor(): global.Google.Protobuf.Reflection.FileDescriptor = descriptorBackingField.Value
let Descriptor: global.Google.Protobuf.Reflection.FileDescriptor = descriptorBackingField.Value
This makes the generator compatible with the Google.Api.CommonProtos
NuGet package and other packages that are generated by the C# generator
The user of the plugin just compiles the google/apis
protos them selves and then depend on them.
I tried this approach but since the annotations.proto
is using extensions the plugin failed to generate correct code.
I'm currently working on fixing the extensions support in the plugin, will link a PR once I have something working.
@Arshia001 What do you think?
It it proving to be a bit tricky to implement the extension support. I'll be going route 1. to unblock my work. I'll open a PR tomorrow with the changes.
@Arshia001 Do you have any ideas on how best to implement extension support? Extension are not that widely used so it's not a huge problem if route 1. is acceptable since it allows users to use the googleapis protos.
The code generator wasn't really meant to be compatible with the C# code generator. It's meant to be its own thing, in much the same way the code generator for any other language (Java, Go, etc.) is incompatible with the C# code generator. This is, in fact, the reason why there's a separate Protobuf.FSharp
package; we could have used Google's (MS's?) official package if the code generator was indeed compatible.
It may be possible to make the codegen compatible with the C# codegen to some degree (though it will be hard at least, if not impossible), but there'll always be the fact that code generated by the C# codegen is fundamentally different. The C# codegen generates classes and properties with presence APIs and whatnot, while the F# codegen generates records of ValueOption fields. If we were OK with using the C# codegen from F#, we wouldn't need this repo in the first place. One can always simply put the .proto
s in a C# library and reference that from F#. We're already doing this with EF (when we use it, which is rare) and the WCF client libraries.
To fix this situation, I believe we need to fix the edge cases where the F# codegen doesn't work, and use it to generate code from referenced .proto
s. If those .proto
s come with supporting logic inside their library, the logic has to be reimplemented in F#. This is actually the case for the built-in timestamp type.
Now, for this particular case, I don't know how extensions work. I'm sure we can work together to figure something out though. Can you give me a few pointers on extensions and the API's you're having trouble with? If extensions are really rare (haven't seen them used myself, yet) and this isn't likely to be a problem with many other API's, we could even try to fix the generated code by hand, though that's the very last option.
I'm not 100% following. I was using the C# code gen and I'm trying to get away from that to keep it F# to take advantage of using record types instead of the class based interface from the C# version. Is there something I should have done different that made you think I was trying use C# for something.
This extension I'm using is for using the gRPC gateway. I believe this is also supported by the C# code (might be wrong about this though)
edit: adding references to the protos for annotations and http
You're not using the C# codegen, but the Google library you're probably using is. Code generated by the two code generators is incompatible which is why you're getting an error.
I was trying to add those as links in the fsproj it found the annotations file, but ended up throwing an error finding the http file.
[edit]: I see now that code gen for the Annotations file is looking a little odd with a module at the end that doesn't seem complete and some other errors. Can see in a gist here
Well, something is clearly missing there, both on line 27 Api..google
and at the end. I'll have to take a look.
I'm trying to fix the extension generation but I'm having some problems with ValueOptions at the moment:
Currently the generated code in my branch is:
// <auto-generated>
// Generated by the F# GRPC code generator. DO NOT EDIT!
// source: google/api/annotations.proto
// </auto-generated>
namespace rec Google.Api
#nowarn "40"
module AnnotationsReflection =
let private descriptorBackingField: global.System.Lazy<global.Google.Protobuf.Reflection.FileDescriptor> =
let descriptorData = global.System.Convert.FromBase64String("\
Chxnb29nbGUvYXBpL2Fubm90YXRpb25zLnByb3RvEgpnb29nbGUuYXBpGhVnb29nbGUvYXBpL2h0dHAu\
cHJvdG8aIGdvb2dsZS9wcm90b2J1Zi9kZXNjcmlwdG9yLnByb3RvOksKBGh0dHASHi5nb29nbGUucHJv\
dG9idWYuTWV0aG9kT3B0aW9ucxiwyrwiIAEoCzIULmdvb2dsZS5hcGkuSHR0cFJ1bGVSBGh0dHBCbgoO\
Y29tLmdvb2dsZS5hcGlCEEFubm90YXRpb25zUHJvdG9QAVpBZ29vZ2xlLmdvbGFuZy5vcmcvZ2VucHJv\
dG8vZ29vZ2xlYXBpcy9hcGkvYW5ub3RhdGlvbnM7YW5ub3RhdGlvbnOiAgRHQVBJYgZwcm90bzM=")
global.System.Lazy<_>(
(fun () ->
global.Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode(
descriptorData,
[|
global.Google.Api.HttpReflection.Descriptor()
global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()
|],
new global.Google.Protobuf.Reflection.GeneratedClrTypeInfo(
null,
[|
global.Google.Api.AnnotationsExtensions.http
|],
null
)
)
),
true
)
let Descriptor(): global.Google.Protobuf.Reflection.FileDescriptor = descriptorBackingField.Value
module AnnotationsExtensions =
let http = global.Google.Protobuf.Extension<global.Google.Protobuf.Reflection.MethodOptions,ValueOption<global.Google.Api.HttpRule>>(72295728, global.Google.Protobuf.FieldCodec.ForMessage(578365826u, global.Google.Api.HttpRule.Parser))
// End of auto-generated code
The error I'm getting is:
error FS0001: A generic construct requires that the type 'ValueOption<HttpRule>' have reference semantics, but it does not, i.e. it is a struct
The WIP is here: https://github.com/Arshia001/FSharp.GrpcCodeGenerator/pull/8
@purkhusid the error indicates that Google.Protobuf.Extension
expects a reference type (i.e. class in C#, types not annotated with [<Struct>]
in F#). ValueOption
is a value type, hence the name value option.
Are you sure the generic argument needs to be an option? I think it needs to be the type itself, e.g. global.Google.Protobuf.Extension<global.Google.Protobuf.Reflection.MethodOptions,global.Google.Api.HttpRule>
.
@Arshia001 Yeah, I wasn't sure if the extensions should be ValueOption or not, but it seems like it's not possible.
I updated the PR and now everything compiles. I'm not familiar enough with Extensions to know if the generated code is doing anything wrong though. It looks correct if I compare it to the C# code.
@Arshia001 The error is a bit more complex than anticipated. After testing out the code in my PR I got a runtime exception related to the descriptor.proto
:
Unhandled exception. System.TypeInitializationException: The type initializer for '<StartupCode$ingestion_api-service>.$Service' threw an exception.
---> System.NullReferenceException: Object reference not set to an instance of an object.
at Google.Protobuf.Reflection.SingleFieldAccessor..ctor(PropertyInfo property, FieldDescriptor descriptor)
at Google.Protobuf.Reflection.FieldDescriptor.CreateAccessor()
at Google.Protobuf.Reflection.FieldDescriptor.CrossLink()
at Google.Protobuf.Reflection.MessageDescriptor.CrossLink()
at Google.Protobuf.Reflection.FileDescriptor.CrossLink()
at Google.Protobuf.Reflection.FileDescriptor.BuildFrom(ByteString descriptorData, FileDescriptorProto proto, FileDescriptor[] dependencies, Boolean allowUnknownDependencies, GeneratedClrTypeInfo generatedCodeInfo)
at Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode(Byte[] descriptorData, FileDescriptor[] dependencies, GeneratedClrTypeInfo generatedCodeInfo)
at <StartupCode$Protobuf-FSharp>.$Descriptor.clo@192-7.Invoke()
at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
at System.Lazy`1.CreateValue()
at System.Lazy`1.get_Value()
at Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()
at <StartupCode$google-api-protos>.$Annotations.clo@26-27.Invoke() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/google/api/fsharp_annotations_proto.dll_pb/Annotations.fs:line 27
at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
at System.Lazy`1.CreateValue()
at System.Lazy`1.get_Value()
at Google.Api.AnnotationsReflection.Descriptor() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/google/api/fsharp_annotations_proto.dll_pb/Annotations.fs:line 48
at <StartupCode$ingestion_api-service>.$Service.clo@19.Invoke() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 20
at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
at System.Lazy`1.CreateValue()
at System.Lazy`1.get_Value()
at Src.Protobuf.IngestionApi.Service.ServiceReflection.Descriptor() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 35
at <StartupCode$ingestion_api-service>.$Service.Descriptor@47-1() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 47
at <StartupCode$ingestion_api-service>.$Service.clo@47-10.Invoke(Unit arg00@) in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 47
at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
at System.Lazy`1.CreateValue()
at System.Lazy`1.get_Value()
at Microsoft.FSharp.Control.LazyExtensions.Force[T](Lazy`1 ) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\prim-types.fs:line 6629
at <StartupCode$ingestion_api-service>.$Service..cctor() in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 47
--- End of inner exception stack trace ---
at <StartupCode$ingestion_api-service>.$Service.get___Method_Bootstrap@46()
at Src.Protobuf.IngestionApi.Service.BootstrapService.BootstrapServiceMethodBinder.BindService(BootstrapServiceBase serviceImpl) in /home/danielp/dannidev/monorepo/bazel-bin/src/protobuf/ingestion_api/service/ingestion_api_service_proto.dll_pb/Service.fs:line 50
at Main.main(String[] argv) in /home/danielp/dannidev/monorepo/src/dotnet/apis/ingestion_api/Main.fs:line 60
I debugged the issue and it comes down the following:
The annotations.proto
depends on the descriptor.proto
and the generator resolves the descriptor for descriptor.proto
to global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()
Since we depend on the Google.Protobuf
package to parse the descriptors we get an null reference exception here: https://github.com/protocolbuffers/protobuf/blob/6b0ff74ecf63e26c7315f6745de36aff66deb59d/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs#L90-L90
The descriptor.proto
is proto2 and the Google.Protobuf
package assumes that there is an Has<property name>
property on all fields on a proto2 message(Also applies for optional
in proto3). The global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()
in Protobuf.FSharp
does not have these properties and thus we get a null reference exception.
I tried editing the generated code manually to use global.Google.Protobuf.Reflection.DescriptorReflection.Descriptor
instead of global.Google.Protobuf.FSharp.Reflection.DescriptorReflection.Descriptor()
and the runtime errors were no longer present.
Since this is also an error when using the optional
keyword in proto3
I think the correct approach is to fix the generator so that if the proto file is proto2 or the field is optional
we add the Has<field name>
and Clear<field name>
properties to generated code.
I have a fix ready for review in #8 along with some other generation fixes
That's amazing! I'll check it out as soon as I can. Thanks!
Fixed in #16.
When using a proto file with Goole API Annotations for a gateway I get an error about a function that cannot be applied:
snippet of the generated code:
Seems if I remove the
()
fromglobal.Google.Api.AnnotationsReflection.Descriptor()
that particular error goes away, not sure if it's generating any other problems as I'm not using the dotnet version of the gateway at this time.I used the following proto file for this example: