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

bump CMake minimum version to use new behavior of CMP0048 #1052

Merged
merged 2 commits into from
Jan 22, 2020

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Jan 22, 2020

CMP0048 NEW behavior sets PROJECT_VERSION variables to empty strings if
project() is called with no VERSION argument. This could be a problem
for packages that set VERSION variables prior to project() because
project() would then clear them, but catkin does not do this.

edit: This bumps the minimum CMake version to 3.0.2, which is the minimum supported by ROS Kinetic and new enough to default to the NEW behavior of CMP0048. This avoids a CMake warning when building and testing this package in Debian Buster and Ubuntu Focal.

Fixes this CMake warning: http://build.ros.org/job/Npr__catkin__ubuntu_focal_amd64/4/warnings2Result/new/

@sloretz sloretz requested a review from dirk-thomas January 22, 2020 20:04
@sloretz sloretz self-assigned this Jan 22, 2020
@dirk-thomas
Copy link
Member

This needs a rebase to pass CI.

@dirk-thomas
Copy link
Member

While this should work I am worried to require this change in every ROS package. Maybe we should explicitly set the policy (to enabled?)?

CMP0048 NEW behavior sets PROJECT_VERSION variables to empty strings if
project() is called with no VERSION argument. This could be a problem
for packages that set VERSION variables prior to project() because
project() would then clear them, but catkin does not do this.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Copy link
Contributor Author

sloretz commented Jan 22, 2020

While this should work I am worried to require this change in every ROS package. Maybe we should explicitly set the policy (to enabled?)?

I'm not sure how to do that automatically for downstream projects. IIUC it looks like the warning is issued in the project() command itself. The policy would have to be set before then, but I think the earliest catkin could set something is in catkinConfig.cmake when find_package(catkin) is called. That is expected to happen after calling project().

@dirk-thomas
Copy link
Member

I bumped the CMake minimum version to something at least 3.0 (which is when CMP0048 was introduced) but not higher than 3.0.2 which is the CMake version on Debian Jessie targeted by ROS Kinetic.

@dirk-thomas dirk-thomas changed the title Use CMP0048 New behavior bump CMake minimum version to use new behavior of CMP0048 Jan 22, 2020
@sloretz
Copy link
Contributor Author

sloretz commented Jan 22, 2020

I bumped the CMake minimum version to something at least 3.0 (which is when CMP0048 was introduced) but not higher than 3.0.2 which is the CMake version on Debian Jessie targeted by ROS Kinetic.

✔️ works on my machine

Kukanani added a commit to ros-perception/vision_msgs that referenced this pull request Jul 18, 2020
tdenewiler pushed a commit to tdenewiler/node_example that referenced this pull request Sep 21, 2020
* Bump CMake version to avoid CMP0048

This bumps the minimum CMake version to 3.0.2, which is the minimum supported by ROS Kinetic and new enough to default to the NEW behavior of CMP0048. This avoids a CMake warning when building and testing this package in Debian Buster and Ubuntu Focal.

See ros/catkin#1052

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Use catkin_install_python

Uses catkin_install_python() so scripts are installed with the correct
shebang for the given version of ROS.

http://wiki.ros.org/UsingPython3/SourceCodeChanges#Changing_shebangs

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
srediske pushed a commit to srediske/robotiq that referenced this pull request Feb 4, 2022
v4hn added a commit to TAMS-Group/pr2_robot that referenced this pull request Mar 6, 2023
about VERSION being managed by project() since 3.0
Keep the version consistent with other packages in ROS that
applied similar fixes, e.g.,
ros/catkin#1052
fmauch added a commit to fmauch/Universal_Robots_ROS_Driver that referenced this pull request Oct 11, 2023
RobertWilbrandt pushed a commit to UniversalRobots/Universal_Robots_ROS_Driver that referenced this pull request Oct 11, 2023
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.

2 participants