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

[poly2tri] Set policy CMP0063 to NEW #27263

Merged
merged 3 commits into from
Oct 19, 2022
Merged

[poly2tri] Set policy CMP0063 to NEW #27263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ddf1f1
Set policy CMP0063 to NEW
kduske Oct 16, 2022
06c684f
fixup! Set policy CMP0063 to NEW
kduske Oct 17, 2022
d30d6c9
amend! Set policy CMP0063 to NEW
kduske Oct 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ports/poly2tri/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
cmake_minimum_required(VERSION 3.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just push this to 3.15 or higher. (vcpkg internally has 3.24)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean instead of or in addition to setting the policy?

Copy link
Contributor

@dg0yt dg0yt Oct 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Do you mean instead of or in addition to setting the policy?

"instead".
By requiring a particular version, CMake sets all policies introduced up to that version to NEW. Version 3.15 implicitly includes CMP0063 from CMake 3.3.
(PS Bumping cmake_minimum_required may come with side effects, but @Neumann-A probably hopes for vcpkg-CI to catch them.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately that does not fix the issue. As per the documentation, this particular policy defaults to OLD if not set even on 3.25.

Copy link
Contributor

Choose a reason for hiding this comment

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

CMake version 3.24.2 warns when the policy is not set and uses OLD behavior

do you get the warning ? Are the properties even getting set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get the warning if I set

set(CMAKE_CXX_VISIBILITY_PRESET hidden)
set(CMAKE_VISIBILITY_INLINES_HIDDEN TRUE)

in poly2tri's CMakeList.txt, but not the policy. And this reveals an error I made in the initial PR, where I did not set the visibility preset properties. I'm having a bit of trouble with my setup locally so I did not notice that it doesn't work when I only set the policy.

Here's the exact warning I get:

CMake Warning (dev) at /Users/kristian/Documents/Code/TrenchBroom/vcpkg/scripts/buildsystems/vcpkg.cmake:613 (_add_library):
  Policy CMP0063 is not set: Honor visibility properties for all target
  types.  Run "cmake --help-policy CMP0063" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.

  Target "poly2tri" of type "STATIC_LIBRARY" has the following visibility
  properties set for CXX:

    CXX_VISIBILITY_PRESET
    VISIBILITY_INLINES_HIDDEN

  For compatibility CMake is not honoring them for this target.
Call Stack (most recent call first):
  CMakeLists.txt:38 (add_library)
This warning is for project developers.  Use -Wno-dev to suppress it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll push some fixup to address all these issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, 06c684f should fix these issues now. I have not updated the required cmake version to keep the changes minimal. I think it's not necessary to bump it for this. Let me know if you think otherwise and I'll change it.


if(POLICY CMP0063)
cmake_policy(SET CMP0063 NEW)
endif()

set(CMAKE_CXX_VISIBILITY_PRESET hidden)
set(CMAKE_VISIBILITY_INLINES_HIDDEN TRUE)

project(poly2tri LANGUAGES C CXX)

set(INSTALL_BIN_DIR "bin" CACHE PATH "Path where exe and dll will be installed")
Expand Down
4 changes: 2 additions & 2 deletions ports/poly2tri/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "poly2tri",
"version-string": "2020-07-21",
"port-version": 2,
"version-date": "2020-07-21",
"port-version": 3,
"description": "The Clipper library performs clipping and offsetting for both lines and polygons. All four boolean clipping operations are supported - intersection, union, difference and exclusive-or. Polygons can be of any shape including self-intersecting polygons.",
"homepage": "https://github.com/greenm01/poly2tri",
"supports": "!uwp",
Expand Down
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5798,7 +5798,7 @@
},
"poly2tri": {
"baseline": "2020-07-21",
"port-version": 2
"port-version": 3
},
"polyclipping": {
"baseline": "6.4.2",
Expand Down
5 changes: 5 additions & 0 deletions versions/p-/poly2tri.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "6f490bcfed9bb8b55036006a4389bfa7e94c73ff",
"version-date": "2020-07-21",
"port-version": 3
},
{
"git-tree": "03cdd793a8f279b18df99f74bf4eef1e24ad5809",
"version-string": "2020-07-21",
Expand Down