Fix compile flags to work on rosbag_storage:0.17.x

[asymingt]

This fixes the compile flags for rolling, which has two versions -- one that does not support read order (0.17.x) and one that does support read order (0.18.x). I can confirm that this now gives me a clean build on my workspace, which is locked to 0.17.x:

Previous behavior:

asymingt@machinehead:~/development/mcap_ws$ colcon build
Starting >>> mcap_vendor
Starting >>> rosbag2_storage_mcap_testdata
Finished <<< mcap_vendor [0.23s]                                                                                 
Finished <<< rosbag2_storage_mcap_testdata [1.43s]                     
Starting >>> rosbag2_storage_mcap
--- stderr: rosbag2_storage_mcap                             
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:181:46: error: ‘ReadOrder’ in namespace ‘rosbag2_storage’ does not name a type
  181 |   void set_read_order(const rosbag2_storage::ReadOrder&) override;
      |                                              ^~~~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:181:8: error: ‘void rosbag2_storage_plugins::MCAPStorage::set_read_order(const int&)’ marked ‘override’, but does not override
  181 |   void set_read_order(const rosbag2_storage::ReadOrder&) override;
      |        ^~~~~~~~~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:499:57: error: ‘ReadOrder’ in namespace ‘rosbag2_storage’ does not name a type
  499 | void MCAPStorage::set_read_order(const rosbag2_storage::ReadOrder& read_order) {
      |                                                         ^~~~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp: In member function ‘void rosbag2_storage_plugins::MCAPStorage::set_read_order(const int&)’:
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:501:22: error: request for member ‘sort_by’ in ‘read_order’, which is of non-class type ‘const int’
  501 |   switch (read_order.sort_by) {
      |                      ^~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:502:27: error: ‘rosbag2_storage::ReadOrder’ has not been declared
  502 |     case rosbag2_storage::ReadOrder::ReceivedTimestamp:
      |                           ^~~~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:503:22: error: request for member ‘reverse’ in ‘read_order’, which is of non-class type ‘const int’
  503 |       if (read_order.reverse) {
      |                      ^~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:509:27: error: ‘rosbag2_storage::ReadOrder’ has not been declared
  509 |     case rosbag2_storage::ReadOrder::File:
      |                           ^~~~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:510:23: error: request for member ‘reverse’ in ‘read_order’, which is of non-class type ‘const int’
  510 |       if (!read_order.reverse) {
      |                       ^~~~~~~
/usr/local/home/asymingt/development/mcap_ws/src/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp:516:27: error: ‘rosbag2_storage::ReadOrder’ has not been declared
  516 |     case rosbag2_storage::ReadOrder::PublishedTimestamp:
      |                           ^~~~~~~~~
make[2]: *** [CMakeFiles/rosbag2_storage_mcap.dir/build.make:63: CMakeFiles/rosbag2_storage_mcap.dir/src/mcap_storage.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:154: CMakeFiles/rosbag2_storage_mcap.dir/all] Error 2
make: *** [Makefile:141: all] Error 2
---
Failed   <<< rosbag2_storage_mcap [2.59s, exited with code 2]

Summary: 2 packages finished [4.34s]
  1 package failed: rosbag2_storage_mcap
  1 package had stderr output: rosbag2_storage_mcap

New behavior:

asymingt@machinehead:~/development/mcap_ws$ colcon build
Starting >>> mcap_vendor
Starting >>> rosbag2_storage_mcap_testdata
Finished <<< mcap_vendor [0.22s]                                                                                 
Finished <<< rosbag2_storage_mcap_testdata [1.46s]                     
Starting >>> rosbag2_storage_mcap
Finished <<< rosbag2_storage_mcap [7.16s]                     

Summary: 3 packages finished [8.92s]

[asymingt]

Side note: since there are two versions of rolling, we might want to add CI builds for both to catch any regressions that happen as a result of subtle rosbag2_storage API changes.

[jtbandes]

since there are two versions of rolling

Isn't the point of rolling that it is always moving forward and represents the "bleeding edge"? https://docs.ros.org/en/humble/Releases/Release-Rolling-Ridley.html

[asymingt]

since there are two versions of rolling

Isn't the point of rolling that it is always moving forward and represents the "bleeding edge"? https://docs.ros.org/en/humble/Releases/Release-Rolling-Ridley.html

The CI builds for this repo are amazingly fast. Since rosbag2_storage is version-tagged and appears to be API stable within a tag, perhaps a better way for me to have asked the question is, "since rosbag2_storage_mcap is tightly coupled to the API of rosbag2_storage then perhaps it makes sense to have CI builds that that correspond to specific versions, like 0.17.x and 0.18.x".

[emersonknapp]

There is only ever one version of rolling - however the latest sources may be ahead of what is released/synced to the apt repos. There should never be "multi-version support" for rolling, this is indeed a bleeding edge release that only ever has one version and no meaningful history. There is no api or abi stability guarantee in rolling, it may break anything at any time. Given that we plan to migrate this repo to the rosbag2 repo, their releases will get synced so there is no way for this plugin to get out of sync - but if you are building this plugin from source the only reasonable workflow is to also build rosbag2 from source.

You are correct however that we aim to use version tags meaningfully for stability. But given the v0 major tag, minor bumps may break API

That's my take on the situation

[jtbandes]

Related: ros2/rosbag2#1163

[asymingt]

There is only ever one version of rolling - however the latest sources may be ahead of what is released/synced to the apt repos. There should never be "multi-version support" for rolling, this is indeed a bleeding edge release that only ever has one version and no meaningful history. There is no api or abi stability guarantee in rolling, it may break anything at any time. Given that we plan to migrate this repo to the rosbag2 repo, their releases will get synced so there is no way for this plugin to get out of sync - but if you are building this plugin from source the only reasonable workflow is to also build rosbag2 from source.

You are correct however that we aim to use version tags meaningfully for stability. But given the v0 major tag, minor bumps may break API

That's my take on the situation

I have to unfortunately build from source, because I'm working on a development checkout of ROS2 rolling, where some packages are held back from their bleeding edge. I probably represent a small proportion of users. I'm glad that the fix was easy, though.