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

Fixes policy CMP0135 warning for CMake >= 3.24 #35

Merged
merged 12 commits into from
Sep 9, 2022
Merged

Fixes policy CMP0135 warning for CMake >= 3.24 #35

merged 12 commits into from
Sep 9, 2022

Conversation

Crola1702
Copy link
Contributor

@Crola1702 Crola1702 commented Sep 8, 2022 •
edited
Loading

Warning fix
Signed-off-by: Cristóbal Arroyo [email protected]

This warning started appearing on new machines (windows-container-bd99e2f0 and others). It’s caused by a new CMake version (3.24) that expects this policy to be set.

Solution with this PR changes:

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

This last windows build wasn't run right (probably the agent didn't had the CMake3.24 version)

Crola1702 and others added 10 commits August 31, 2022 18:29
@Crola1702
Sets CMP0135 policy
07973fe 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Changed command to set policy
9bf68c4 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Set download_extract_timestamp to true
29aec65 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Put policy set inside macro
f134d96 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Changed true -> TRUE
b81f5b6 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Unsettled DOWNLOAD_EXTRACT_TIMESTAMP value
59db9bc 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Changed cmake_policy() location
5fab026 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Changed cmake_policy() location to top of file
f4ce91b 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Updated DOWNLOAD_EXTRACT_TIMESTAMP
9b2869d 
Signed-off-by: Crola1702 <[email protected]>
@Crola1702
Changed location of CMP0135 policy set
b4052ed 
Signed-off-by: Crola1702 <[email protected]>
clalancette
clalancette reviewed Sep 9, 2022
View reviewed changes
Copy link
Contributor

@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.

This seems like a strange file to set the policy in. That is, the problem is in the main CMakeLists.txt, where we actually call ExternalProject_Add, but this is the cmake file that we export to downstream consumers. So I don't quite understand why it is in here; can you explain more?

@Crola1702
Copy link
Contributor Author

Crola1702 commented Sep 9, 2022 •
edited
Loading

This seems like a strange file to set the policy in. That is, the problem is in the main CMakeLists.txt, where we actually call ExternalProject_Add, but this is the cmake file that we export to downstream consumers. So I don't quite understand why it is in here; can you explain more?

Yes. I was trying to apply the solution we used in osrf_testing_tools_cpp (osrf/osrf_testing_tools_cpp#71) by setting the policy in cmake.in

New builds:

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

Now it's working fine on all three builds.

@Crola1702 Crola1702 marked this pull request as ready for review September 9, 2022 17:24
Blast545
Blast545 approved these changes Sep 9, 2022
View reviewed changes
Copy link

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

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

LGTM

@Crola1702 Crola1702 merged commit 5565d5b into rolling Sep 9, 2022
@delete-merged-branch delete-merged-branch bot deleted the Crola1702/fix-cmp0135-policy-warning branch September 9, 2022 19:02
@MichaelOrlov
Copy link

https://github.com/Mergifyio backport humble

@mergify
Copy link

mergify bot commented Nov 23, 2022

backport humble

❌ Command disallowed due to command restrictions in the Mergify configuration.

  • sender-permission>=write

@MichaelOrlov
Copy link

@cottsay @clalancette Could you please backport this PR to the Humble?
It turns out that Mergify refuses to create PR since I don't have write permissions to this repo.
Without this PR our CI throwing warning and reporting that build unstable due to the new warnings.
One of the warnings from https://ci.ros2.org/job/ci_windows/18281/

@MichaelOrlov MichaelOrlov mentioned this pull request Nov 23, 2022
Merged
@Crola1702
Copy link
Contributor Author

https://github.com/Mergifyio backport humble

mergify bot pushed a commit that referenced this pull request Nov 23, 2022
@Crola1702 @mergify
Fixes policy CMP0135 warning for CMake >= 3.24 (#35)
c0571ab 
* Sets CMP0135 policy

Signed-off-by: Crola1702 <[email protected]>
(cherry picked from commit 5565d5b)
@mergify mergify bot mentioned this pull request Nov 23, 2022
Merged
@mergify
Copy link

mergify bot commented Nov 23, 2022

backport humble

✅ Backports have been created

@Crola1702
Copy link
Contributor Author

Could you please backport this PR to the Humble?

@MichaelOrlov check #41

@MichaelOrlov
Copy link

@Crola1702 Thanks, looking forward to be merged.

clalancette pushed a commit that referenced this pull request Nov 28, 2022
@mergify @Crola1702
Fixes policy CMP0135 warning for CMake >= 3.24 (#35) (#41)
8d286b2 
* Sets CMP0135 policy

Signed-off-by: Crola1702 <[email protected]>
(cherry picked from commit 5565d5b)

Co-authored-by: Cristóbal Arroyo <[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

@Blast545 Blast545 Blast545 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.
4 participants
@Crola1702 @MichaelOrlov @clalancette @Blast545