-
Notifications
You must be signed in to change notification settings - Fork 179
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 newer Protobuf #1187
Comments
Looks like C# doesn't like the new protobuf. Maybe we just need to re-generate the files?
|
The c# files are already generated as part of the build. Which proto version did you upgrade to? |
Yep, that's the exact version that I used, too. git describe tells me it is |
Now that I think about it, it is more likely that something with restoring the nuget protobuf references is going wrong now. |
OK, I managed to fix the build. It turns out that there are numerous hardcoded protobuf versions that one need to change manually and that need to match the version that is used in the build. I missed one of them. |
So it compiles now, but at least executing the eCAL Samples fails with the following error mesage. The reason is that we have pre-generated pb2.py files checked in. The files are generated with protoc 3.0, if I recall correctly. I think the only proper way is to generate the pb2.py files at build time and not push them to the repo.
Edit: after re-generating the pb2.py files with protoc.exe that came from the eCAL Installer that I tested, everything worked fine. |
Possible solutions:
We decided to leave the pb2.py files as they are at the moment. The user can re-generate them if required. |
Context
Currently, we have Protobuf 3.11 in the submodule, which is used for Windows. That version is quite old and newer versions would have better support for C#. Also, the old version of protobuf currently forces users to install old python protobuf wheels, as we ship the eCAL wheel with pre-generated pb2.py files.
Proposal
The goal here is to update the protobuf submodule to something more recent.
Tasks and updates
The text was updated successfully, but these errors were encountered: