Skip to content

Add ament_ros_defaults target#62

Merged
skyegalaxy merged 8 commits intorollingfrom
rmc/ament_ros_defaults
Apr 17, 2026
Merged

Add ament_ros_defaults target#62
skyegalaxy merged 8 commits intorollingfrom
rmc/ament_ros_defaults

Conversation

@InvincibleRMC
Copy link
Copy Markdown
Contributor

@InvincibleRMC InvincibleRMC commented Mar 25, 2026 •
edited
Loading

Description

Done so a grouped standard can more easily be achieved and ease migrations to future versions.
Started out ament/ament_cmake#621, but moved here instead.

Currently bumps C++ to C++20 and leaves C at C11. Since @wjwwood mentioned there might be some compatabilty problems on Windows. @sloretz original propsed C17. To discuss.

Is this user-facing behavior change?

Users will now be able to link against the following.

ament_cmake_ros_core::ament_ros_defaults
ament_cmake_ros_core::ament_ros_cxx_standard
ament_cmake_ros_core::ament_ros_c_standard
ament_cmake_ros_core::ament_ros_warnings

Did you use Generative AI?

Additional Information

@InvincibleRMC
Init
7ad436b 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Move into seperate file
1cfe7a5 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC InvincibleRMC mentioned this pull request Mar 25, 2026
Closed
@InvincibleRMC
Copy link
Copy Markdown
Contributor Author

InvincibleRMC commented Mar 25, 2026

Pulls: #62
Gist: https://gist.githubusercontent.com/InvincibleRMC/96f8f8ecb9ebc4afef81c0d99804a800/raw/826b6e3b8958c1d59f93cbf3a0a3512d568fdb3c/ros2.repos
BUILD args: --continue-on-error --packages-above-and-dependencies ament_cmake_ros_core rmw_test_fixture_implementation
TEST args: --packages-above ament_cmake_ros_core rmw_test_fixture_implementation
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18662

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

@InvincibleRMC InvincibleRMC changed the title Add ament_ros_defaults Add ament_ros_defaults target Mar 25, 2026
@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Mar 26, 2026

Pulls: ros2/rcutils#548, #62
Gist: https://gist.githubusercontent.com/ahcorde/3e7ab21988cdd588ff217c5561a8ba80/raw/bd52d1d2df3a4da8b2da4f06ef3ebf0a122e7a70/ros2.repos
BUILD args: --packages-above-and-dependencies ament_cmake_ros rcutils rmw_test_fixture_implementation
TEST args: --packages-above ament_cmake_ros rcutils rmw_test_fixture_implementation
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18675

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

@ahcorde ahcorde mentioned this pull request Mar 26, 2026
Merged
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Mar 26, 2026

I spent some time looking into the Windows-c11 thing, and it looks like it's no longer an issue (sort of): https://devblogs.microsoft.com/cppblog/c11-and-c17-standard-support-arriving-in-msvc/

There has been support for /std:c11 and /std:c17 for years now, which is good, but it's still missing key features we use like atomics. So we can probably use c17 now, but the code we have to ifdef for windows probably still needs to stay.

wjwwood
wjwwood reviewed Mar 26, 2026
View reviewed changes
Comment thread rmw_test_fixture_implementation/test/CMakeLists.txt
Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
@InvincibleRMC
nits + c17
118fc8b 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Add other defaults
4626b1f 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Add missing targets to export
8bea02e 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Mar 31, 2026

Pulls: ros2/rcutils#548, #62
Gist: https://gist.githubusercontent.com/ahcorde/9559d38bc1bba1117521e3b1ca1486d7/raw/bd52d1d2df3a4da8b2da4f06ef3ebf0a122e7a70/ros2.repos
BUILD args: --packages-above-and-dependencies ament_cmake_ros rcutils rmw_test_fixture_implementation
TEST args: --packages-above ament_cmake_ros rcutils rmw_test_fixture_implementation
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18748

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

ahcorde
ahcorde requested changes Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see these messages when compiling rmw_test_fixture_implementation

<command-line>: warning: "ROS_PACKAGE_NAME" redefined
<command-line>: note: this is the location of the previous definition

Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
@InvincibleRMC @ahcorde
Apply suggestion from @ahcorde
e8b166e 
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Copy link
Copy Markdown
Contributor Author

InvincibleRMC commented Apr 2, 2026

I can see these messages when compiling rmw_test_fixture_implementation

<command-line>: warning: "ROS_PACKAGE_NAME" redefined
<command-line>: note: this is the location of the previous definition

Is there some modern way for doing conditional here to keep both? Or should we drop the interface target for now?

@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Apr 2, 2026

@wjwwood or @mjcarroll any ideas here ?

@ahcorde ahcorde requested a review from wjwwood April 3, 2026 19:16
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Apr 4, 2026

I don't know of any way to conditionally add the definition from either end. We may just have to exclude the definition in the interface target.

@InvincibleRMC
Copy link
Copy Markdown
Contributor Author

InvincibleRMC commented Apr 8, 2026 •
edited
Loading

For Lyrical I will remove the duped targets from the default will still expose them in case we decide to use them later.

@InvincibleRMC
Remove duped package name + build settings
f290f98 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Copy link
Copy Markdown
Contributor Author

InvincibleRMC commented Apr 8, 2026

Pulls: #62, ros2/rcutils#548
Gist: https://gist.githubusercontent.com/InvincibleRMC/fdfe0c78f32f762e6ac42154df9dce19/raw/324354195f82453e573c5173cddeed2b61497ae1/ros2.repos
BUILD args: --continue-on-error --packages-above-and-dependencies ament_cmake_ros_core rcutils
TEST args: --packages-above ament_cmake_ros_core rcutils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18877

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

@jmachowinski jmachowinski mentioned this pull request Apr 8, 2026
Merged
@jmachowinski
Copy link
Copy Markdown

jmachowinski commented Apr 8, 2026

https://github.com/ros2/rcutils/blob/0320f069924b0a31390fe5194c6e145293111173/CMakeLists.txt#L694

We are missing the export of ament_cmake_ros_core here

jmachowinski
jmachowinski requested changes Apr 8, 2026
View reviewed changes
Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
Comment thread ament_cmake_ros_core/cmake/ament_ros_defaults.cmake Outdated
@InvincibleRMC
Remove other targets and REQUIRED property
82c3513 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@jmachowinski
Copy link
Copy Markdown

jmachowinski commented Apr 8, 2026

Pulls: #62, ros2/rcutils#548
Gist: https://gist.githubusercontent.com/jmachowinski/5a5aeb4a178e96f09bcea09ea11d167e/raw/d2acee73ba7751b94dc0ac40bbc35d9d717aba24/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18893

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

@jmachowinski
Copy link
Copy Markdown

jmachowinski commented Apr 9, 2026

Pulls: #62, ros2/rcutils#548, ros2/rclcpp#3124
Gist: https://gist.githubusercontent.com/jmachowinski/1538e0f1074c2a0133830f9b6e76ee32/raw/31a05eaaf436bc349c5bf99faec772cce9b5f581/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18900

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

@jmachowinski jmachowinski mentioned this pull request Apr 9, 2026
Merged
mjcarroll
mjcarroll approved these changes Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI

@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Apr 13, 2026

Pulls: ros2/rclcpp#3124, #62, ros2/rcutils#548
Gist: https://gist.githubusercontent.com/ahcorde/308c095a49dc34449d63c4dc7eac9439/raw/31a05eaaf436bc349c5bf99faec772cce9b5f581/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp ament_cmake_ros_core rmw_test_fixture_implementation rcutils
TEST args: --packages-above rclcpp ament_cmake_ros_core rmw_test_fixture_implementation rcutils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18943

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

@skyegalaxy
Copy link
Copy Markdown
Member

skyegalaxy commented Apr 16, 2026

  • Linux-rhel Build Status
  • Windows Build Status

skyegalaxy
skyegalaxy approved these changes Apr 17, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@skyegalaxy skyegalaxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per client library WG meeting today: we'll run CI one more time and if linux is green, we'll merge this, ros2/rclcpp#3124 and ros2/rcutils#548

@skyegalaxy
Copy link
Copy Markdown
Member

skyegalaxy commented Apr 17, 2026

Pulls: ros2/rclcpp#3124, #62, ros2/rcutils#548
Gist: https://gist.githubusercontent.com/skyegalaxy/c1125cbe48fe911d3211fef54246d06d/raw/a2987278e6f5a5125f605d68dd0f3d7ff6a82dd6/ros2%2520repos%2520c++20
BUILD args: --packages-above-and-dependencies rclcpp ament_cmake_ros_core rmw_test_fixture_implementation rcutils
TEST args: --packages-above rclcpp ament_cmake_ros_core rmw_test_fixture_implementation rcutils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18991/

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

@skyegalaxy skyegalaxy merged commit 6a84b6f into rolling Apr 17, 2026
4 checks passed
@ahcorde ahcorde deleted the rmc/ament_ros_defaults branch April 20, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahcorde ahcorde ahcorde requested changes

@jmachowinski jmachowinski jmachowinski requested changes

@mjcarroll mjcarroll mjcarroll approved these changes

@skyegalaxy skyegalaxy skyegalaxy approved these changes

@wjwwood wjwwood Awaiting requested review from wjwwood

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@InvincibleRMC @ahcorde @wjwwood @jmachowinski @skyegalaxy @mjcarroll