Generate messages in downstream builds #339

mjcarroll · 2023-05-11T20:33:45Z

🎉 New feature

Expose message generation functionality to downstream packages and remove message generation from the msgs package.

Summary

Background

Up until now, we have:

Distributed proto files - each describing a gz-msgs message
Generated .cc and .hh files in the gz-msgs package
The cc and hh files are compiled into a gz-msgs library that is also distributed in the binary package.

The issue with this approach is that it means that each change to a proto message will cause the ABI to break, which is inconvenient. We have multiple issues addressing this in

New Approach

This new approach will now distribute the proto files, but will no longer distribute a binary library that downstream users will link against. Instead, downstream users can generate the messages that they need themselves or add new message definitions in their own packages.

A summary of the changes required to make this happen:

Expose the mechanisms we use for message generation
- This requires installing the protoc generator as well as our python script.
- Add cmake functionality to let downstream users discover available messages and retrieve the installation path of the generator and python script (handled via cmake extras)
Create a core library that has the gz-msgs functionality minus the generated code.
Create a compiled library that maintains the CLI functionality.
Make conversions a header-only operation, so that the core library doesn't need to depend on the generated messages.

Downstream changes required:

gz-fuel-tools Use updated message generate pipeline gz-fuel-tools#354
gz-gui: Update gz-gui to use new message generation pipeline gz-gui#541
gz-sim: Update gz-sim to use new message generation pipeline gz-sim#2000
gz-sensors: Update gz-sensors to use new message generation pipeline gz-sensors#349
gz-launch: Update gz-launch to use new message generation pipeline gz-launch#219
gz-transport: Use the message generation framework gz-transport#398

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

caguero · 2023-05-19T16:13:09Z

Just checking, we can't compile the whole harmonic yet with only this PR, right?

caguero · 2023-05-19T16:14:44Z

Could you help me a bit with the expectations after this PR? What can I do and what can't I do yet (if any)?

caguero · 2023-05-19T16:25:53Z

Does this PR require https://github.com/gazebosim/gz-cmake/pull/345/files?

mjcarroll · 2023-05-19T17:38:25Z

Just checking, we can't compile the whole harmonic yet with only this PR, right?

Correct, I am working on the corresponding downstream modifications.

Could you help me a bit with the expectations after this PR? What can I do and what can't I do yet (if any)?

I still owe some documentation here. The general expectation is that gz-msgs will only install proto files as part of the debians, rather than generated headers and libraries. Downstream libraries will generate the cc/hh files from protos as part of their build process.

We should expect that with this PR, you should be able to add messages to your project, as demonstrated in this example: https://github.com/mjcarroll/mjcarroll-custom-msgs

This will also provide users a way to define custom messages downstream with simple cmake macros.

Does this PR require https://github.com/gazebosim/gz-cmake/pull/345/files?

Correct, it was just merged, and I'm going to cut a release shortly.

mjcarroll · 2023-05-22T12:45:44Z

@osrf-jenkins retest this please

caguero

I'm compiling Harmonic with the understanding that it won't successfully compile all libraries. After running colcon I see generated .pb.h files under ./build/gz-msgs10/gz_msgs_gen/gz/msgs. Is that expected?

caguero · 2023-05-22T16:08:06Z

I also see these two warning messages:

--- stderr: gz-msgs10                                                                                               
warning: tag INPUT: input source '/home/caguero/harmonic_ws/build/gz-msgs10/include/gz/msgs/details' does not exist
warning: tag INPUT: input source '/home/caguero/harmonic_ws/build/gz-msgs10/include/gz/msgs/details' does not exist

cmake/gz_msgs_protoc.cmake

core/generator/Generator.cc

compiled/CMakeLists.txt

test/integration/CMakeLists.txt

mjcarroll · 2023-05-22T17:43:56Z

I'm compiling Harmonic with the understanding that it won't successfully compile all libraries. After running colcon I see generated .pb.h files under ./build/gz-msgs10/gz_msgs_gen/gz/msgs. Is that expected?

Yes, this is correct. They are being generated to run the tests.

mjcarroll · 2023-05-30T20:45:56Z

@caguero I have opened PRs for all downstream libraries and added examples/READMEs.

iche033

Update Migradtion.md guide?

tools/gz_msgs_generate_factory.py

examples/generating_custom_msgs/main.cc

examples/using_gz_msgs/main.cc

examples/generating_custom_msgs/main.cc

Signed-off-by: Michael Carroll <[email protected]>

Co-authored-by: Addisu Z. Taddese <[email protected]> Signed-off-by: Michael Carroll <[email protected]> Signed-off-by: Michael Carroll <[email protected]>

mjcarroll · 2023-06-23T13:42:15Z

Okay, all feedback addressed and rebased to get DCO happy. I think we are good here, and then a prerelease?

scpeters · 2023-06-30T01:43:39Z

gz-msgs-extras.cmake.in

+include(${@PROJECT_NAME@_DIR}/gz_msgs_generate.cmake)
+include(${@PROJECT_NAME@_DIR}/target_link_messages.cmake)
+
+set(@PROJECT_NAME@_INSTALL_PATH "${@PROJECT_NAME@_DIR}/../../..")


given the other issues I've been seeing with the /usr/lib/<arch> folder in gazebo-release/gz-msgs10-release#3 and gazebosim/gz-cmake#360, I'm a bit suspicious that this hard-coded ../../.. is wrong

This reverts commit 0c78b96. Signed-off-by: Michael Carroll <[email protected]>

* Revert "Use cmake extras path variables (#359)" This reverts commit 786b42a. Signed-off-by: Michael Carroll <[email protected]> * Revert "Unconditionally find the python interpreter (#356)" This reverts commit 16de9ef. Signed-off-by: Michael Carroll <[email protected]> * Revert "Generate messages in downstream builds (#339)" This reverts commit 0c78b96. Signed-off-by: Michael Carroll <[email protected]> --------- Signed-off-by: Michael Carroll <[email protected]>

github-actions bot added the 🎵 harmonic Gazebo Harmonic label May 11, 2023

mjcarroll force-pushed the mjcarroll/rework-message-generation2 branch 5 times, most recently from 8eca9ee to 9e55508 Compare May 11, 2023 20:50

mjcarroll mentioned this pull request May 18, 2023

Generate messages in downstream builds #338

Closed

mjcarroll marked this pull request as ready for review May 18, 2023 13:11

mjcarroll requested a review from caguero as a code owner May 18, 2023 13:11

mjcarroll force-pushed the mjcarroll/rework-message-generation2 branch from 79041c4 to 3a66601 Compare May 19, 2023 14:30

mjcarroll mentioned this pull request May 19, 2023

Prepare for 3.2.0 release gazebosim/gz-cmake#352

Merged

5 tasks

mjcarroll mentioned this pull request May 22, 2023

Use updated message generate pipeline gazebosim/gz-fuel-tools#354

Closed

9 tasks

caguero reviewed May 22, 2023

View reviewed changes

cmake/gz_msgs_protoc.cmake Outdated Show resolved Hide resolved

core/generator/Generator.cc Outdated Show resolved Hide resolved

compiled/CMakeLists.txt Outdated Show resolved Hide resolved

test/integration/CMakeLists.txt Outdated Show resolved Hide resolved

mjcarroll mentioned this pull request May 23, 2023

Use the message generation framework gazebosim/gz-transport#398

Closed

iche033 reviewed May 31, 2023

View reviewed changes

tools/gz_msgs_generate_factory.py Outdated Show resolved Hide resolved

examples/generating_custom_msgs/main.cc Outdated Show resolved Hide resolved

examples/using_gz_msgs/main.cc Outdated Show resolved Hide resolved

examples/generating_custom_msgs/main.cc Show resolved Hide resolved

mjcarroll and others added 15 commits June 23, 2023 13:36

Update CMake documentation

3ccce92

Signed-off-by: Michael Carroll <[email protected]>

Refactor common string operations into functions

7cf741a

Signed-off-by: Michael Carroll <[email protected]>

Style nit

263bf11

Signed-off-by: Michael Carroll <[email protected]>

Document target_link_messages

9dfab82

Signed-off-by: Michael Carroll <[email protected]>

Lint

1684866

Signed-off-by: Michael Carroll <[email protected]>

Restore const char* setter

85c8ea6

Signed-off-by: Michael Carroll <[email protected]>

Change to array rather than map

60200f3

Signed-off-by: Michael Carroll <[email protected]>

Windows typo

48810d9

Signed-off-by: Michael Carroll <[email protected]>

Fix visibility logic

9e4ea2a

Signed-off-by: Michael Carroll <[email protected]>

Remove conflicting visibility

6c55def

Signed-off-by: Michael Carroll <[email protected]>

Completely remove visibility from target_link_messages

82331b3

Signed-off-by: Michael Carroll <[email protected]>

Drop template from conversions

f0fda37

Signed-off-by: Michael Carroll <[email protected]>

Address reviewer feedback

a3e0182

Signed-off-by: Michael Carroll <[email protected]>

Change to implptr

52f7e12

Signed-off-by: Michael Carroll <[email protected]>

Update core/generator/Generator.cc

a775cb5

Co-authored-by: Addisu Z. Taddese <[email protected]> Signed-off-by: Michael Carroll <[email protected]> Signed-off-by: Michael Carroll <[email protected]>

mjcarroll force-pushed the mjcarroll/rework-message-generation2 branch from d299cbb to a775cb5 Compare June 23, 2023 13:36

mjcarroll merged commit 0c78b96 into main Jun 23, 2023

mjcarroll deleted the mjcarroll/rework-message-generation2 branch June 23, 2023 17:04

mjcarroll mentioned this pull request Jun 27, 2023

Support standalone executable in gz-msgs11 #357

Merged

9 tasks

scpeters reviewed Jun 30, 2023

View reviewed changes

This was referenced Jul 6, 2023

Compute relative path for cmake extras gazebosim/gz-cmake#362

Merged

Use computed relative extras path #359

Merged

mjcarroll added a commit that referenced this pull request Jul 12, 2023

Revert "Generate messages in downstream builds (#339)"

85a2b5d

This reverts commit 0c78b96. Signed-off-by: Michael Carroll <[email protected]>

azeey mentioned this pull request Jul 15, 2023

Add python message generation #362

Merged

9 tasks

azeey mentioned this pull request Aug 24, 2023

Restore message generation pipeline and generate message libraries #368

Merged

mjcarroll mentioned this pull request Sep 25, 2023

Expose message generation framework #36

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate messages in downstream builds #339

Generate messages in downstream builds #339

mjcarroll commented May 11, 2023 •

edited

Loading

caguero commented May 19, 2023

caguero commented May 19, 2023

caguero commented May 19, 2023

mjcarroll commented May 19, 2023

mjcarroll commented May 22, 2023

caguero left a comment

caguero commented May 22, 2023

mjcarroll commented May 22, 2023

mjcarroll commented May 30, 2023

iche033 left a comment

mjcarroll commented Jun 23, 2023

scpeters Jun 30, 2023

Generate messages in downstream builds #339

Generate messages in downstream builds #339

Conversation

mjcarroll commented May 11, 2023 • edited Loading

🎉 New feature

Summary

Background

New Approach

Checklist

caguero commented May 19, 2023

caguero commented May 19, 2023

caguero commented May 19, 2023

mjcarroll commented May 19, 2023

mjcarroll commented May 22, 2023

caguero left a comment

Choose a reason for hiding this comment

caguero commented May 22, 2023

mjcarroll commented May 22, 2023

mjcarroll commented May 30, 2023

iche033 left a comment

Choose a reason for hiding this comment

mjcarroll commented Jun 23, 2023

scpeters Jun 30, 2023

Choose a reason for hiding this comment

mjcarroll commented May 11, 2023 •

edited

Loading