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

[20328] Add support for building tests within an ament context #200

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Feb 21, 2024

This PR adds support for building Fast CDR within an ament context (as is the ROS 2 CI).

Since Fast CDR v2.0.0, the CMake option to build the tests is the CMake standard -DBUILD_TESTING=ON. When trying to introduce Fast CDR v2 in ROS 2, ROS 2 CI was failing because that CI builds all packages with -DBUILD_TESTING=ON, and in that context Fast CDR could not find GTest because is not installed in the system.

In the ROS 2 ecosystem, there are vendor packages within ament for GTest, which are the ones the CI is using. However, Fast CDR's CMakeLists.txt was doing a plain find_package(GTest REQUIRED), and thus failing. In order to find GTest in that context, ament's infrastructure needs to be used. This PR:

  1. Adds a package.xml to Fast CDR for ROS 2 packaging. This means that when Fast CDR v2.2.0 in included in ROS 2 Rolling, we could dispose of this patch file.
  2. Extends the existing colcon.pkg to included the ament dependencies so the build order is correctly set by colcon. We cannot simply dispose of this file, as then colcon does not build GTest before Fast CDR when building tests.
  3. Adds a CMake macro find_or_add_gtest to correctly include GTest both in and outside an ament context.
  4. Forces C++14 when building tests, as in at the moment ROS 2 Rolling uses GTest 1.14, which requires it, and our CI system supports it. For building the library, C++11 is still the minimum required and is still enforced by default.

Thanks to @clalancette and @mjcarroll for their help on this

@EduPonz EduPonz marked this pull request as ready for review February 21, 2024 15:14
@JesusPoderoso JesusPoderoso added this to the v2.2.0 milestone Feb 21, 2024
@clalancette
Copy link
Contributor

Here's a quick CI job on ROS 2 infrastructure using this PR (only building up to fastcdr):

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@EduPonz EduPonz merged commit e1cef73 into master Feb 22, 2024
15 checks passed
@EduPonz EduPonz deleted the feature/ros2_ci_support branch February 22, 2024 17:35
EduPonz added a commit that referenced this pull request Feb 22, 2024
* Refs #20328: Add package.xml and colcon.pkg compatible with ROS 2 CI

Signed-off-by: EduPonz <[email protected]>

* Refs #20328: Add macro to find gtest both in and outside ament context

Signed-off-by: EduPonz <[email protected]>

* Refs #20328: Use new macro to find gtest

Signed-off-by: EduPonz <[email protected]>

* Refs #20328: Apply suggestions

Signed-off-by: EduPonz <[email protected]>

* Refs #20328: Fix typo

Signed-off-by: EduPonz <[email protected]>

---------

Signed-off-by: EduPonz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants