search

icerpc / icerpc-csharp

A C# RPC framework built for QUIC, with bidirectional streaming, first-class async/await, and Protobuf support.
https://docs.icerpc.dev
Apache License 2.0
107 stars 13 forks source link

IceRpc.Protobuf.Tools and Google.Protobuf.Tools #3716

Closed bernardnormier closed 1 year ago

bernardnormier commented 1 year ago

It would be nicer if IceRpc.Protobuf.Tools could "pull" a specific version of Google.Protobuf.Tools.

This way, in your IceRPC + Protobuf project file, you would need to reference only (a specific version of) IceRpc.Protobuf.Tools. However, that's not the case. The GreeterProtobuf examples shows you need to reference both Google.Protobuf.Tools and IceRpc.Protobuf.Tools.

Why this setup?

pepone commented 1 year ago

One issue is that Google.Protobuf.Tools doesn't include buildTransive properties. Therefore, even if IceRpc.Protobuf.Tools references the Google.Protobuf.Tools package, the properties defined there – used to locate the protoc compiler and Protobuf well-known files – wouldn't be defined when the ProtoCompile task runs in the user project that only references IceRpc.Protobuf.Tools.

IceRpc.Protobuf.Tools already includes the required Google.Protobuf.dll for IceRpc.ProtocGen.dll. Similarly, we can bundle the protoc compiler and the Protobuf well-known files when we build the IceRpc.ProtocGen package.

One thing I didn't realize before is that because we include Google.Protobuf.dll we should update the LICENSE accordingly

bernardnormier commented 1 year ago

Therefore, even if IceRpc.Protobuf.Tools references the Google.Protobuf.Tools package, the properties defined there – used to locate the protoc compiler and Protobuf well-known files – wouldn't be defined when the ProtoCompile task runs in the user project that only references IceRpc.Protobuf.Tools.

I assume we could add fallback properties in IceRpc.Protobuf.Tools for this situation.

pepone commented 1 year ago

I assume we could add fallback properties in IceRpc.Protobuf.Tools for this situation.

I need to test if that is enough, it is not clear to me if the dependencies of packages defining DevelopmentDependency are pulled. Here IceRpc.Protobuf.Tools package is a development dependency.

pepone commented 1 year ago

If we add Google.Protobuf.Tools dependency to IceRpc.Protobuf.Tools it doesn't help. When a project adds IceRpc.Protobuf.Tools it will not pull Google.Protobuf.Tools, this is how dev dependencies work.

I think it will be simpler to repackage Google.Protobuf.Tools inside IceRpc.Protobuf.Tools assuming the licenses are compatible.

bernardnormier commented 1 year ago

Ok.