"Futurize" foxy-future branch (#687)

* Fix/futurize build against Foxy (#780) * Kludge: add transitive dependencies for rosbag2_cpp * Kludge: add transitive dependencies for rosbag2_compression_zstd * Kludge: add transitive dependencies for rosbag2_compression * Kludge: add transitive dependencies for rosbag2_transport * Kludge: add transitive dependencies for rosbag2_tests * Action CI build as overlay on Foxy rosbag2 binaries * Kludge: fix pybind include path for rosbag2_py (#783) Signed-off-by: Emerson Knapp <[email protected]> Co-authored-by: Alexander <[email protected]>
ros2 · Jun 18, 2021 · b7c3b21 · b7c3b21
1 parent 1aba66f
commit b7c3b21
Show file tree

Hide file tree

Showing 16 changed files with 247 additions and 59 deletions.
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -11,7 +11,7 @@ jobs:
       image: rostooling/setup-ros-docker:ubuntu-focal-ros-foxy-ros-base-testing-latest
     steps:
     - uses: actions/checkout@v2
-    - uses: ros-tooling/action-ros-lint@0.0.6
+    - uses: ros-tooling/action-ros-lint@v0.1
       with:
         linter: copyright
         distribution: foxy
@@ -32,7 +32,7 @@ jobs:
       image: rostooling/setup-ros-docker:ubuntu-focal-ros-foxy-ros-base-testing-latest
     steps:
     - uses: actions/checkout@v2
-    - uses: ros-tooling/action-ros-lint@0.0.6
+    - uses: ros-tooling/action-ros-lint@v0.1
       with:
         linter: xmllint
         distribution: foxy
@@ -59,7 +59,7 @@ jobs:
       image: rostooling/setup-ros-docker:ubuntu-focal-ros-foxy-ros-base-testing-latest
     steps:
     - uses: actions/checkout@v2
-    - uses: ros-tooling/action-ros-lint@0.0.6
+    - uses: ros-tooling/action-ros-lint@v0.1
       with:
         linter: ${{ matrix.linter }}
         distribution: foxy
@@ -83,7 +83,7 @@ jobs:
           linter: [pep257, flake8]
     steps:
     - uses: actions/checkout@v2
-    - uses: ros-tooling/action-ros-lint@0.0.6
+    - uses: ros-tooling/action-ros-lint@v0.1
       with:
         linter: ${{ matrix.linter }}
         distribution: foxy

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -3,32 +3,36 @@ on:
   pull_request:
   push:
     branches:
-      - master
-  schedule:
-    # Run every hour. This helps detect flakiness,
-    # and broken external dependencies.
-    - cron:  '0 * * * *'
+      - foxy-future
+  # NOTE: build schedules are only supported for the default branch - this wouldn't work anyways
+  # schedule:
+  #   # Run every hour. This helps detect flakiness,
+  #   # and broken external dependencies.
+  #   - cron:  '0 * * * *'
 
 jobs:
   build_and_test:
     runs-on: ubuntu-latest
     container:
-      image: rostooling/setup-ros-docker:ubuntu-focal-latest
+      # Use the ros-base container, so that the rosbag2 binaries are already installed
+      image: rostooling/setup-ros-docker:ubuntu-focal-ros-foxy-ros-base-latest
     steps:
-    - uses: ros-tooling/action-ros-ci@0.1.0
+    - uses: ros-tooling/action-ros-ci@v0.2
       with:
         package-name: |
           ros2bag
           rosbag2_compression
+          rosbag2_compression_zstd
           rosbag2_cpp
           rosbag2_storage
           rosbag2_storage_default_plugins
+          rosbag2_test_common
+          rosbag2_tests
           rosbag2_transport
           shared_queues_vendor
           sqlite3_vendor
-          rosbag2_test_common
-          rosbag2_tests
-        target-ros2-distro: rolling
+          zstd_vendor
+        target-ros2-distro: foxy
     - uses: actions/upload-artifact@v1
       with:
         name: colcon-logs

diff --git a/README.md b/README.md
@@ -1,18 +1,28 @@
+# Foxy Improvement Backport Branch
+
+This branch contains a special-purpose version of rosbag2 that is intended to be used in a Foxy application.
+
+It contains performance improvements and new features from Galactic development up to March 23, 2021 - with minor tweaks to maintain build and test compatibility against the stable Foxy distribution.
+
+The popularly-requested improvements in this branch were not possible to backport to Foxy in an API/ABI-maintaining way. Instead, this "Foxy Future" branch is provided for users to build from source and get the performance benefits from nearly a year of development after Foxy.
+
+For more context on this conversation, see https://discourse.ros.org/t/fast-forward-merging-rosbag2-master-api-to-foxy/18927 or reach out to the ros-tooling Working Group.
+
+## Usage instructions
+
+* Check out this branch `foxy-future` from the repository `github.com/ros2/rosbag2` into your ROS workspace (or an overlay workspace)
+* Build like the other ROS packages you use
+
+This branch of rosbag2 will mask the binary-released version, allowing to you to enjoy this very improved version.
+
+NOTE: Any code that built against the rosbag2 C++ API may not build out of the box against this version, the main reason for the branch is that the APIs changed in some ways that could not be fixed into a backport.
+
 # rosbag2
 ![License](https://img.shields.io/github/license/ros2/rosbag2)
 [![GitHub Action Status](https://github.com/ros2/rosbag2/workflows/Test%20rosbag2/badge.svg)](https://github.com/ros2/rosbag2/actions)
 
 Repository for implementing rosbag2 as described in its corresponding [design article](https://github.com/ros2/design/blob/f69fbbd11848e3dd6866b71a158a1902e31e92f1/articles/rosbags.md).
 
-## Foxy Backport Release
-
-NOTE: this branch will contain the sources for the package `ros-foxy-rosbag2-future`, an updated package containing new performance improvements and features that could not be safely backported to `ros-foxy-rosbag2` in the Foxy Fitzroy distribution without breaking API/ABI. This branch is NOT YET STABLE and will be force-pushed during the release process.
-
-This to-do list will be removed as the updates are made to this branch:
-* [ ] Rename all `package.xml`s with `-future` package name suffix and `<conflicts>` tag
-* [ ] Update this README to ensure correct installation and usage instructions
-* [ ] Finalize the `master` commit that this will be based on
-
 ## Installation instructions
 
 ## Debian packages

diff --git a/ros2bag/ros2bag/command/bag.py b/ros2bag/ros2bag/command/bag.py
@@ -19,6 +19,8 @@
 class BagCommand(CommandExtension):
     """Various rosbag related sub-commands."""
 
+    FUTURE_NOTICE = 'Running rosbag2 "futurized" source release, not the official Foxy binary version.'  # NOQA
+
     def add_arguments(self, parser, cli_name):
         self._subparser = parser
 
@@ -27,6 +29,8 @@ def add_arguments(self, parser, cli_name):
             parser, cli_name, '_verb', 'ros2bag.verb', required=False)
 
     def main(self, *, parser, args):
+        print(self.FUTURE_NOTICE)
+        print()
         if not hasattr(args, '_verb'):
             # in case no verb was passed
             self._subparser.print_help()

diff --git a/rosbag2_compression/CMakeLists.txt b/rosbag2_compression/CMakeLists.txt
@@ -75,6 +75,8 @@ if(BUILD_TESTING)
     test/rosbag2_compression/fake_compressor.cpp
     test/rosbag2_compression/fake_decompressor.cpp)
   target_link_libraries(fake_plugin ${PROJECT_NAME})
+  # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+  ament_target_dependencies(fake_plugin rosbag2_cpp rosbag2_storage)
   install(
     TARGETS fake_plugin
     ARCHIVE DESTINATION lib
@@ -86,22 +88,28 @@ if(BUILD_TESTING)
   ament_add_gmock(test_compression_factory
     test/rosbag2_compression/test_compression_factory.cpp)
   target_link_libraries(test_compression_factory ${PROJECT_NAME})
+  # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+  ament_target_dependencies(test_compression_factory rosbag2_cpp rosbag2_storage)
 
   ament_add_gmock(test_compression_options
     test/rosbag2_compression/test_compression_options.cpp)
   target_include_directories(test_compression_options PUBLIC include)
   target_link_libraries(test_compression_options ${PROJECT_NAME})
+  # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+  ament_target_dependencies(test_compression_options rosbag2_cpp rosbag2_storage)
 
   ament_add_gmock(test_sequential_compression_reader
     test/rosbag2_compression/test_sequential_compression_reader.cpp)
   target_include_directories(test_sequential_compression_reader PUBLIC include)
   target_link_libraries(test_sequential_compression_reader ${PROJECT_NAME})
+  # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
   ament_target_dependencies(test_sequential_compression_reader rosbag2_cpp rosbag2_storage)
 
   ament_add_gmock(test_sequential_compression_writer
     test/rosbag2_compression/test_sequential_compression_writer.cpp)
   target_include_directories(test_sequential_compression_writer PUBLIC include)
   target_link_libraries(test_sequential_compression_writer ${PROJECT_NAME})
+  # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
   ament_target_dependencies(test_sequential_compression_writer rosbag2_cpp rosbag2_storage)
 endif()
 

diff --git a/rosbag2_compression/src/rosbag2_compression/sequential_compression_reader.cpp b/rosbag2_compression/src/rosbag2_compression/sequential_compression_reader.cpp
@@ -67,7 +67,8 @@ void SequentialCompressionReader::preprocess_current_file()
      * Because we have no way to check whether the bag was written correctly,
      * check for the existence of the prefixed file as a fallback.
      */
-    const rcpputils::fs::path base{base_folder_};
+    // Foxy compatibility. Adapts for https://github.com/ros2/rcpputils/pull/119
+    rcpputils::fs::path base{base_folder_};
     const rcpputils::fs::path relative{get_current_file()};
     const auto resolved = base / relative;
     if (!resolved.exists()) {

diff --git a/rosbag2_compression_zstd/CMakeLists.txt b/rosbag2_compression_zstd/CMakeLists.txt
@@ -38,7 +38,10 @@ target_include_directories(${PROJECT_NAME}
 ament_target_dependencies(${PROJECT_NAME}
   rcpputils
   rosbag2_compression
-  zstd)
+  zstd
+  # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+  rosbag2_cpp
+  rosbag2_storage)
 target_compile_definitions(${PROJECT_NAME} PRIVATE ROSBAG2_COMPRESSION_ZSTD_BUILDING_DLL)
 pluginlib_export_plugin_description_file(rosbag2_compression plugin_description.xml)
 
@@ -69,9 +72,11 @@ if(BUILD_TESTING)
 
   ament_add_gmock(test_zstd_compressor
     test/rosbag2_compression_zstd/test_zstd_compressor.cpp)
-  target_include_directories(test_zstd_compressor PUBLIC include)
   target_link_libraries(test_zstd_compressor ${PROJECT_NAME})
-  ament_target_dependencies(test_zstd_compressor rclcpp rosbag2_test_common)
+
+  ament_target_dependencies(test_zstd_compressor rclcpp rosbag2_test_common
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    rosbag2_cpp rosbag2_storage zstd rosbag2_compression)
 endif()
 
 ament_package()
diff --git a/rosbag2_cpp/CMakeLists.txt b/rosbag2_cpp/CMakeLists.txt
@@ -140,43 +140,52 @@ if(BUILD_TESTING)
   ament_add_gmock(test_converter_factory
     test/rosbag2_cpp/test_converter_factory.cpp)
   if(TARGET test_converter_factory)
-    target_include_directories(test_converter_factory PRIVATE include)
     target_link_libraries(test_converter_factory ${PROJECT_NAME})
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    ament_target_dependencies(test_converter_factory rosbag2_storage)
   endif()
 
   ament_add_gmock(test_typesupport_helpers
     test/rosbag2_cpp/test_typesupport_helpers.cpp
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   if(TARGET test_typesupport_helpers)
     target_link_libraries(test_typesupport_helpers ${PROJECT_NAME})
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    ament_target_dependencies(test_typesupport_helpers rosbag2_storage rosbag2_test_common)
   endif()
 
   ament_add_gmock(test_info
     test/rosbag2_cpp/test_info.cpp
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   if(TARGET test_info)
     target_link_libraries(test_info ${PROJECT_NAME})
-    ament_target_dependencies(test_info rosbag2_test_common)
+    ament_target_dependencies(test_info rosbag2_test_common
+      # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+      rosbag2_storage)
   endif()
 
   ament_add_gmock(test_sequential_reader
     test/rosbag2_cpp/test_sequential_reader.cpp)
   if(TARGET test_sequential_reader)
-    ament_target_dependencies(test_sequential_reader rosbag2_storage)
     target_link_libraries(test_sequential_reader ${PROJECT_NAME})
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    ament_target_dependencies(test_sequential_reader rosbag2_storage)
   endif()
 
   ament_add_gmock(test_storage_without_metadata_file
     test/rosbag2_cpp/test_storage_without_metadata_file.cpp)
   if(TARGET test_storage_without_metadata_file)
-    ament_target_dependencies(test_storage_without_metadata_file rosbag2_storage)
     target_link_libraries(test_storage_without_metadata_file ${PROJECT_NAME})
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    ament_target_dependencies(test_storage_without_metadata_file rosbag2_storage)
   endif()
 
   ament_add_gmock(test_message_cache
     test/rosbag2_cpp/test_message_cache.cpp)
   if(TARGET test_message_cache)
     target_link_libraries(test_message_cache ${PROJECT_NAME})
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    ament_target_dependencies(test_message_cache rosbag2_storage)
   endif()
 
   # If compiling with gcc, run this test with sanitizers enabled
@@ -207,15 +216,17 @@ if(BUILD_TESTING)
   ament_add_gmock(test_sequential_writer
     test/rosbag2_cpp/test_sequential_writer.cpp)
   if(TARGET test_sequential_writer)
-    ament_target_dependencies(test_sequential_writer rosbag2_storage)
     target_link_libraries(test_sequential_writer ${PROJECT_NAME})
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    ament_target_dependencies(test_sequential_writer rosbag2_storage)
   endif()
 
   ament_add_gmock(test_multifile_reader
     test/rosbag2_cpp/test_multifile_reader.cpp)
   if(TARGET test_multifile_reader)
-    ament_target_dependencies(test_multifile_reader rosbag2_storage)
     target_link_libraries(test_multifile_reader ${PROJECT_NAME})
+    # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+    ament_target_dependencies(test_multifile_reader rosbag2_storage)
   endif()
 endif()
 

diff --git a/rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp b/rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp
@@ -110,6 +110,8 @@ void SequentialWriter::open(
   }
 
   bool dir_created = rcpputils::fs::create_directories(db_path);
+  // Foxy compatibility. Adapts for https://github.com/ros2/rcpputils/pull/98
+  dir_created &= db_path.is_directory();
   if (!dir_created) {
     std::stringstream error;
     error << "Failed to create database directory (" << db_path.string() << ").";

diff --git a/rosbag2_py/CMakeLists.txt b/rosbag2_py/CMakeLists.txt
@@ -65,42 +65,61 @@ function(clean_windows_flags target)
   endif()
 endfunction()
 
+function(reorder_pybind_include_directories _target)
+  # Hack to fix overlay workspace include directories order
+  # Needs to be removed/revised after https://github.com/ros2/ros2/issues/1150 gets fixed
+  # Needs to be called instead of ament_target_dependencies for all targets created via pybind11_add_module
+  #
+  # Ros uses its own pybind11 library. It is installed to /opt/ros/foxy folder, so INCLUDE_DIRECTORIES property
+  # of target created via pybind11_add_module is polluted with /opt/ros/foxy/include
+  # All includes found in that directory are used instead of local workspace
+
+  # Get all values from INCLUDE_DIRECTORIES property
+  get_property(_pybind_include_directories TARGET ${_target} PROPERTY INCLUDE_DIRECTORIES)
+  # Clear all values from INCLUDE_DIRECTORIES property
+  set_property(TARGET ${_target} PROPERTY INCLUDE_DIRECTORIES "")
+  # Add ament dependencies
+  ament_target_dependencies(${_target} PUBLIC ${ARGN})
+  # Restore INCLUDE_DIRECTORIES property
+  set_property(TARGET ${_target} APPEND PROPERTY INCLUDE_DIRECTORIES ${_pybind_include_directories})
+endfunction()
+
 ament_python_install_package(${PROJECT_NAME})
 
-pybind11_add_module(_reader SHARED
+pybind11_add_module(_reader SHARED SYSTEM
   src/rosbag2_py/_reader.cpp
 )
-ament_target_dependencies(_reader PUBLIC
+reorder_pybind_include_directories(_reader
   "rosbag2_compression"
   "rosbag2_cpp"
   "rosbag2_storage"
 )
 clean_windows_flags(_reader)
 
-pybind11_add_module(_storage SHARED
+pybind11_add_module(_storage SHARED SYSTEM
   src/rosbag2_py/_storage.cpp
   src/rosbag2_py/format_bag_metadata.cpp
 )
-ament_target_dependencies(_storage PUBLIC
+reorder_pybind_include_directories(_storage
   "rosbag2_cpp"
   "rosbag2_storage"
 )
 clean_windows_flags(_storage)
 
-pybind11_add_module(_writer SHARED
+pybind11_add_module(_writer SHARED SYSTEM
   src/rosbag2_py/_writer.cpp
 )
-ament_target_dependencies(_writer PUBLIC
+reorder_pybind_include_directories(_writer
   "rosbag2_compression"
   "rosbag2_cpp"
   "rosbag2_storage"
 )
 clean_windows_flags(_writer)
 
-pybind11_add_module(_info SHARED
+pybind11_add_module(_info SHARED SYSTEM
   src/rosbag2_py/_info.cpp
 )
-ament_target_dependencies(_info PUBLIC
+reorder_pybind_include_directories(_info
   "rosbag2_cpp"
   "rosbag2_storage"
 )

diff --git a/rosbag2_storage/src/rosbag2_storage/metadata_io.cpp b/rosbag2_storage/src/rosbag2_storage/metadata_io.cpp
@@ -232,12 +232,8 @@ BagMetadata MetadataIo::read_metadata(const std::string & uri)
     YAML::Node yaml_file = YAML::LoadFile(get_metadata_file_name(uri));
     auto metadata = yaml_file["rosbag2_bagfile_information"].as<rosbag2_storage::BagMetadata>();
     rcutils_allocator_t allocator = rcutils_get_default_allocator();
-    if (RCUTILS_RET_OK !=
-      rcutils_calculate_directory_size(uri.c_str(), &metadata.bag_size, allocator))
-    {
-      throw std::runtime_error(
-              std::string("Exception on calculating the size of directory :") + uri);
-    }
+    // Foxy compatibility. Adapts for https://github.com/ros2/rcutils/pull/306
+    metadata.bag_size = rcutils_calculate_directory_size(uri.c_str(), allocator);
     return metadata;
   } catch (const YAML::Exception & ex) {
     throw std::runtime_error(std::string("Exception on parsing info file: ") + ex.what());