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.

Sign up for GitHub

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

Jump to bottom

Upgrade cmake minimum versions to 2.8.12 #11

Merged
merged 2 commits into from
Dec 1, 2020
Merged

Upgrade cmake minimum versions to 2.8.12 #11

merged 2 commits into from
Dec 1, 2020

Conversation

brawner
Copy link

@brawner brawner commented Nov 23, 2020 •
edited
Loading

In the most recent release of cmake (3.19), they have started deprecating cmake versions < 2.8.12. While a similar change has been proposed, it has not yet been added to the upstream repo (google#3094).

https://ci.ros2.org/view/nightly/job/nightly_win_rel/1760/cmake/

Build all of ros2, testing up to rcutils

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Signed-off-by: Stephen Brawner [email protected]

@sloretz
Copy link

sloretz commented Nov 24, 2020

CI ros/urdfdom_headers#67 (comment)

@brawner
Copy link
Author

brawner commented Nov 24, 2020 •
edited
Loading

CI results:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Based on these results, it looks like cmake warnings due to googletest in the ament directory have been resolved. I think the googletest failures in the above windows job are all coming from osrf_testing_tools_cpp (osrf/osrf_testing_tools_cpp#61)

@brawner
Copy link
Author

brawner commented Nov 30, 2020

Testing again with only this branch, --packages-up-to rcutils

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner
Copy link
Author

brawner commented Nov 30, 2020

Only one cmake warning, which is due to mimick and a PR is already open. ros2/Mimick#15

clalancette
clalancette reviewed Nov 30, 2020
View reviewed changes
Copy link

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I'm not 100% sure of this, but I think you should be able to get away with just patching the CMakeLists.txt.install files. As far as I understand how this works, those are the only ones that are used. Mostly I want to do this to keep the "diff" between CMakeLists.txt.upstream and the upstream CMakeLists.txt down.

@brawner
Copy link
Author

brawner commented Nov 30, 2020

Ok, trying just patching .install files.

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner
Copy link
Author

brawner commented Dec 1, 2020

@clalancette It looks like reducing the scope of the patch worked. The only cmake warning on windows was due to mimick, which has now been addressed in ros2/mimick_vendor#14

@clalancette clalancette merged commit b5b29a8 into ament:ros2 Dec 1, 2020
@clalancette clalancette mentioned this pull request Dec 11, 2020
Merged
jacobperron pushed a commit that referenced this pull request Dec 14, 2020
@brawner @jacobperron
Upgrade cmake minimum versions to 2.8.12 (#11)
629bd56 
* Upgrade cmake minimum versions to 2.8.12

Signed-off-by: Stephen Brawner <[email protected]>
@jacobperron jacobperron mentioned this pull request Dec 14, 2020
Merged
clalancette pushed a commit that referenced this pull request Feb 25, 2021
@jacobperron @brawner
Upgrade cmake minimum versions to 2.8.12 (#11) (#12)
69ff0aa 
* Upgrade cmake minimum versions to 2.8.12

Signed-off-by: Stephen Brawner <[email protected]>

Co-authored-by: Stephen Brawner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@brawner @sloretz @clalancette