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

chore: remove cmake minium version deprecation warning #354

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

rickstaa
Copy link
Contributor

@rickstaa rickstaa commented Aug 14, 2023

Not sure if this is desirable given back-compatibility, but this pull request ensures that the cmake minimum version < 3.5 deprecation warning is no longer thrown:

Warnings   << catkin_tools_prebuild:cmake /home/ricks/development/work/franka_ros_ws/logs/catkin_tools_prebuild/build.cmake.000.log                                                                        
CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):                                                                                                                                    
  Compatibility with CMake < 3.5 will be removed from a future version of                                                                                                                                  
  CMake.                                                                                                                                                                                                   
                                                                                                                                                                                                           
  Update the VERSION argument <min> value or use a ...<max> suffix to tell                                                                                                                                 
  CMake that the project does not need compatibility with older versions. 

@rickstaa
Copy link
Contributor Author

We can also update to version 3.10 since this is what is suggested by the migration guide.

@rickstaa rickstaa force-pushed the fix_cmake_minimum_version_deprication_warning branch 2 times, most recently from 22627ef to 73d954d Compare September 1, 2023 11:37
franka_ros/CMakeLists.txt Outdated Show resolved Hide resolved
@rickstaa
Copy link
Contributor Author

rickstaa commented Sep 1, 2023

@Maverobot, @marcbone I just fixed the latest CI errors, and I think we are now suitable for a merge 👍🏻.

@Maverobot
Copy link
Contributor

@rickstaa Thank you for your report and the fix. We will take a loot at this PR.

@rickstaa rickstaa force-pushed the fix_cmake_minimum_version_deprication_warning branch from 73d954d to 3829763 Compare September 4, 2023 08:07
@rickstaa
Copy link
Contributor Author

rickstaa commented Sep 4, 2023

I rebased this PR onto the develop branch and added a CHANGELOG entry.

Copy link
Contributor

@gollth gollth left a comment

Choose a reason for hiding this comment

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

Generally looks good, but if possible remove the additions to models/

@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.1.3)
cmake_minimum_required(VERSION 2.8.3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not updated to 3.10 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @gollth, thanks for reviewing my PR! I got an error when I updated it to 3.10 since the ROS meta package specs currently require version 2.8.3 (see https://wiki.ros.org/catkin/package.xml#Metapackages and moveit/moveit#1619).

How to reproduce

  1. Checkout 22627ef#diff-64225d9dae606e899687341a27336f3cfff2889d702f52095a148a4e6d50cb24 or change the CMake version in the franka_ros/CMakeLists.txt file to VERSION 3.10.
  2. Use catkin_make to build the package.
  3. Be greeted by the following error:
WARNING: The CMakeLists.txt of the metapackage 'franka_ros' contains non standard content. Use the content of the following file instead: /home/ricks/development/work/franka_ros_ws/build/catkin_generated/metapackages/franka_ros/CMakeLists.txt

Note

Catkin-tools doesn't have this problem.

franka_gazebo/models/ambient_sun/model.config Outdated Show resolved Hide resolved
franka_gazebo/models/ambient_sun/model.sdf Outdated Show resolved Hide resolved
@rickstaa rickstaa force-pushed the fix_cmake_minimum_version_deprication_warning branch from 3829763 to a37aee8 Compare September 4, 2023 09:29
@rickstaa rickstaa requested a review from gollth September 4, 2023 09:40
This commit ensure that the cmake minimum version < 3.5 deprecation
warning is no longer thrown. The minimum cmake version was updated
to 3.10 as this is the version that is recommended by the
[ROS noetic migration guide](https://wiki.ros.org/noetic/Migration#Increase_required_CMake_version_to_avoid_author_warning).
The meta package cmake version has to be kept on 2.8.3 because of some
ROS requirements (see
https://wiki.ros.org/catkin/package.xml#Metapackages).
@rickstaa rickstaa force-pushed the fix_cmake_minimum_version_deprication_warning branch from a37aee8 to ca0394f Compare March 23, 2024 20:47
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