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

Set CMake policy CMP0074 to NEW #2671

Merged
merged 1 commit into from
Dec 4, 2018

Conversation

SergioRAgostinho
Copy link
Member

@SergioRAgostinho SergioRAgostinho commented Dec 1, 2018

I reviewed things and I believe we're compliant. We only use *_ROOT variables to specify prefixes so it's totally ok, if CMake uses them to try and locate libraries by default.

Closes #2446

# 1. Find*.cmake modules need to be individually verified.
# 2. PCLConfig.cmake needs to be changed.
cmake_policy(SET CMP0074 OLD)
# 1. Remove with 3.12.4.
Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the version in which the policy came into effect. So it means that when we set the minimum version of cmake to 3.12, this will no longer be needed.

Given your question I assume it's a good idea to rewrite that comment.

Copy link
Member

Choose a reason for hiding this comment

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

But if the policy is not set CMake will default to OLD?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me write a couple of cases to clarify:

  • Case 1:
    • Policy: Not Set
    • CMake minimum required version: 3.1
    • CMake version used: 3.1
    • Results: You get the OLD behavior. CMake 3.1 doesn't even know about this policy because it was only created in version 3.12 it he behaves with the OLD behavior.
  • Case 2:
    • Policy: Not Set
    • CMake minimum required version: 3.1
    • CMake version used: 3.12
    • Results: You get the OLD behavior by default and a warning saying to explicitly set the bhavior. CMake 3.12 wants you to know that things changed.
  • Case 3:
    • Policy: Not Set
    • CMake minimum required version: 3.12
    • CMake version used: 3.12
    • Results: You get the new behavior by default.
  • Case 4:
    • Policy: Explicitly set to NEW
    • CMake minimum required version: 3.1
    • CMake version used: 3.1
    • Results: CMake 3.1 doesn't even know about this policy and ignores it. It still behaves in the OLD behavior.
  • Case 5:
    • Policy: Explicitly set to NEW
    • CMake minimum required version: 3.1
    • CMake version used: 3.12
    • Results: You get the NEW behavior. You're in a situation in which you're compliant with CMake versions from the minimum required one all the way to 3.12+
  • Case 6:
    • Policy: Explicitly set to NEW
    • CMake minimum required version: 3.12
    • CMake version used: 3.12
    • Results: You get the new behavior by default but now you're explicitly setting it although it isn't required.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the detailed list, I wasn't aware that case 3 behaves this way. Although it's the most logical thing to do. And from CMake docs:

The cmake_minimum_required() command does more than report an error if a too-old version of CMake is used to build a project. It also sets all policies introduced in that CMake version or earlier to NEW behavior.

@SergioRAgostinho
Copy link
Member Author

One related detail on this PR: I don't seem to have permissions to rerun jobs on the CI (without committing any new code). Are you able to do it?

@taketwo
Copy link
Member

taketwo commented Dec 3, 2018

image

Yes, I have this menu item in the top-right corner of the build view page.

@SergioRAgostinho
Copy link
Member Author

image

Yep. I don't have that button. Let me confirm that the email for this account is correct.

@SergioRAgostinho
Copy link
Member Author

It is correct. Maybe you need to bump my permissions in the organization.
image

@SergioRAgostinho
Copy link
Member Author

Stray failure on one unit test in ubuntu. It's good to be merged.

@taketwo taketwo merged commit bb34563 into PointCloudLibrary:master Dec 4, 2018
@taketwo taketwo changed the title Update CMake policy 74 Set CMake policy CMP 0074 to NEW Dec 4, 2018
@SergioRAgostinho SergioRAgostinho deleted the policy-74 branch December 5, 2018 21:25
@taketwo taketwo changed the title Set CMake policy CMP 0074 to NEW Set CMake policy CMP0074 to NEW Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMake deprecation warning for OLD policies
2 participants