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

Autogenerated protobuf code as public API #113

Open
azeey opened this issue Dec 14, 2020 · 3 comments
Open

Autogenerated protobuf code as public API #113

azeey opened this issue Dec 14, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request proposal scripting Scripting interfaces to Ignition

Comments

@azeey
Copy link
Contributor

azeey commented Dec 14, 2020

Currently the c++ code autogenerated by protobuf is used in the public API of ign-msgs. While this is convenient in that users of ign-msgs can just link against ign-msgs and immediately start using the message classes in their applications, it comes with some downsides.

  1. Adding a field to a message breaks ABI even though the wire protocol of protobuf is designed to be backward compatible. This this requires a major version bump whereas adding a completely new message can be done in a minor release of ign-msgs.
  2. Platforms like macOS with more frequent updates suffer from breakages caused by updates to libprotobuf. Even minor version updates need new bottles. Rebuild bottles for protobuf 3.14 osrf/homebrew-simulation#1205
  3. Incompatibility with other versions of libprotobuf installed on the system. protobuf error on ubuntu 18.04 gazebo-classic#2496

A potential solution for (1) would be to make ign-msgs only contain the .proto files and generate message classes in each downstream library. For (2) and (3), we could consider statically link against libprotobuf as recommended in
https://github.com/protocolbuffers/protobuf/tree/master/src#binary-compatibility-warning

@chapulina
Copy link
Contributor

make ign-msgs only contain the .proto files and generate message classes in each downstream library.

I suggest as a first step we package just the .proto files into a separate package from the generated C++ code. This would allow us to be in a transition stage while some downstream libraries generate the messages themselves, and others use the precompiled binaries.

This should also make it cleaner to use the proto files with languages other than C++.

@nkoenig
Copy link
Contributor

nkoenig commented Jun 2, 2022

Something to look into: https://docs.buf.build/introduction

@mjcarroll
Copy link
Contributor

The general approach I'm going to follow:

  1. Keep installing the current proto files
  2. Install our custom generator and python message generator script: https://github.com/gazebosim/gz-msgs/blob/gz-msgs9/tools/gz_msgs_generate.py
  3. Break the current compiled library into a separate component that users can optionally use.
  4. Create cmake scripts for locating installed files and python script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request proposal scripting Scripting interfaces to Ignition
Projects
None yet
Development

No branches or pull requests

4 participants