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

fix: add cxx_standard to avoid c++ check error #30

Open
wants to merge 3 commits into
base: rolling
Choose a base branch
from

Conversation

homalozoa
Copy link

As the solution at orocos/orocos_kinematics_dynamics#441 , I add an arg to set the CXX_STANDARD to 11 for building project "orocos_kdl'.

Signed-off-by: homalozoa <nx.tardis@gmail.com>
@cottsay
Copy link
Member

cottsay commented Sep 27, 2024

It would be nice if the developer still had the ability to change the CXX_STANDARD explicitly without modifying the source code. What would you think of setting CMAKE_CXX_STANDARD to 11 on the project only if it isn't otherwise defined, and then passing that along to the orocos_kdl build?

@clalancette
Copy link
Contributor

It would be nice if the developer still had the ability to change the CXX_STANDARD explicitly without modifying the source code. What would you think of setting CMAKE_CXX_STANDARD to 11 on the project only if it isn't otherwise defined, and then passing that along to the orocos_kdl build?

Yeah, agreed. In point of fact, I'd make the default C++17 (that's what we use anywhere), but conditional on it not being set by the user.

@homalozoa
Copy link
Author

It would be nice if the developer still had the ability to change the CXX_STANDARD explicitly without modifying the source code. What would you think of setting CMAKE_CXX_STANDARD to 11 on the project only if it isn't otherwise defined, and then passing that along to the orocos_kdl build?

Agreed. Good idea! I'll modify to that.

Signed-off-by: homalozoa <nx.tardis@gmail.com>
@clalancette
Copy link
Contributor

Pulls: #30
Gist: https://gist.githubusercontent.com/clalancette/1321d88724b4cb30559b927293800f01/raw/71f05fc797d5090a995cc6249c0512315bc7d174/ros2.repos
BUILD args: --packages-above-and-dependencies orocos_kdl_vendor
TEST args: --packages-above orocos_kdl_vendor
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14724

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

@clalancette
Copy link
Contributor

So, unfortunately, on Windows this ended up causing a regression. I guess it is probably because we specified C++17 instead of C++11, and orocos_kdl is not ready for that? I guess we can set this back to C++11 by default, but let's add a comment saying why it is different than every other core package.

Signed-off-by: homalozoa <nx.tardis@gmail.com>
@homalozoa
Copy link
Author

So, unfortunately, on Windows this ended up causing a regression. I guess it is probably because we specified C++17 instead of C++11, and orocos_kdl is not ready for that? I guess we can set this back to C++11 by default, but let's add a comment saying why it is different than every other core package.

Sure. I already changed it back. Let's check it again.

@clalancette
Copy link
Contributor

Pulls: #30
Gist: https://gist.githubusercontent.com/clalancette/a6815558ddb29f472b38b508851298ae/raw/71f05fc797d5090a995cc6249c0512315bc7d174/ros2.repos
BUILD args: --packages-above-and-dependencies orocos_kdl_vendor
TEST args: --packages-above orocos_kdl_vendor
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14727

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

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.

3 participants