Add ament_set_default_language_standards

[InvincibleRMC]

Can use one point as a single source of truth for C++/C version.

[InvincibleRMC]

Pulls: #621
Gist: https://gist.githubusercontent.com/InvincibleRMC/a32f79bfa73772f87acda6151763baaf/raw/d14f40e48a2cd7025a44da55e4a0e6821ab454e2/ros2.repos
BUILD args: --continue-on-error --packages-above-and-dependencies ament_cmake_core ament_cmake_gen_version_h
TEST args: --packages-above ament_cmake_core ament_cmake_gen_version_h
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18597

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

rhel

[ahcorde]

Pulls: ros2/rcutils#548, #621
Gist: https://gist.githubusercontent.com/ahcorde/625e52fa23a766ff8fae9be37ad3ad47/raw/d820803942e8a156af42db30a04fcb6309347102/ros2.repos
BUILD args: --packages-above-and-dependencies ament_cmake_core rcutils
TEST args: --packages-above ament_cmake_core rcutils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18622

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[sloretz]

Since the language standards are specific to the ROS distro release, I recommend putting this into ament_cmake_ros instead of ament_cmake_core

https://github.com/ros2/ament_cmake_ros

[emersonknapp]

Seems great, nice time and space saver, will simplify distro upgrades.

ament_cmake_ros makes sense hypothetically from @sloretz comment, though that package currently only contains some ros-specific cmake testing helpers. A bit unclear what the real intended scope of the package is, and how it differentiates from ament_cmake_ros_core - I was even under an impression it was "semi-deprecated" as we continue to focus on use of modern CMake targets and have fewer ament magic calls per package - but that could be a really old and misinformed thought.

[jmachowinski]

I just realized, this is the old style version of setting the C++ standard, and not the modern one.

The modern version would be to generate an interface target, and to link against it.

add_library(default_cpp_standard INTERFACE)

target_compile_features(default_cpp_standard INTERFACE cxx_std_20)

target_link_libraries(my_app PRIVATE default_cpp_standard)

You can play the same game with targets for more warnings etc...

I think this is more what we want, as it propagates down to consuming packages of rclcpp if we link public and export correctly.

[InvincibleRMC]

Also while we are here do we want to include something for cmake_minimum_required? Upon some quick googling. There doesn't seem to be a good way for bumping cmake versions from a single source of truth :(.

[fujitatomoya]

i do like this refactoring to avoid the duplication and keep the consistency over repositories.

[wjwwood]

I had one comment that I think should be resolved before merging, and an open question about the pr in general:

Should we do this here or in ament_cmake_ros/ament_cmake_ros_core (https://github.com/ros2/ament_cmake_ros)? I feel like these settings are part of "ROS's policies", not necessarily everyone who may use ament_cmake. On the other hand, there could be room for a "default for everyone who uses ament_cmake" too, but then I'd wonder why the CMake defaults are not sufficient.

[wjwwood]

I'm not sure we should do this, since Windows does not implement this standard. I would keep it at C11, since Windows (mostly) implements that one.

[InvincibleRMC]

@sloretz I know you proposed C17, do you have thoughts?

[wjwwood]

I just read the other comments, sorry. I agree with @sloretz.

[InvincibleRMC]

Sorry I haven't had time to update this ticket. We discussed at pmc today to move into ament_cmake_ros_core

[wjwwood]

A bit unclear what the real intended scope of the package is, and how it differentiates from ament_cmake_ros_core - I was even under an impression it was "semi-deprecated" as we continue to focus on use of modern CMake targets and have fewer ament magic calls per package - but that could be a really old and misinformed thought.

I think the important lines are here:

https://github.com/ros2/ament_cmake_ros/blob/1ebf5450912a971d70638a0713a2c18b649a4d1d/ament_cmake_ros_core/ament_cmake_ros_core-extras.cmake.in#L17-L18

Which default us to shared libraries and add the ROS_PACKAGE_NAME compiler directive for all ROS packages, again, just saving us some boilerplate in ROS specific packages. While we do want to move away from ament magic, there will always be tension between that ideal and saving on duplication. DRY always feels nice when changes like updating the minimum CMake version or C++ standard come up, but I also think there is value in the DAMP idea too, especially as automating these kinds of changes becomes easier. I like the distinction "DRY code, DAMP DSLs" (from http://blog.jayfields.com/2006/05/dry-code-damp-dsls.html, sorry their certificate seems to have expired), the only question is, whether each of these bits of ament magic are more code or more dsl.

[jmachowinski]

Don't set the standard globally, but do it via an interface target

[InvincibleRMC]

Moved to ros2/ament_cmake_ros#62