Skip to content

[curl] fix vcpkg-cmake-wrapper missing ZLIB find_package call#10715

Closed
cenit wants to merge 3 commits intomicrosoft:masterfrom
cenit:dev/cenit/curl
Closed

[curl] fix vcpkg-cmake-wrapper missing ZLIB find_package call#10715
cenit wants to merge 3 commits intomicrosoft:masterfrom
cenit:dev/cenit/curl

Conversation

@cenit
Copy link
Copy Markdown
Contributor

@cenit cenit commented Apr 6, 2020

Fixes: #10714

Describe the pull request

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented Apr 6, 2020

I had to insert a fix that is also on #10703 in order to have the green check

list(REMOVE_ITEM ARGS "CONFIG")
list(REMOVE_ITEM ARGS "MODULE")

_find_package(ZLIB)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why? CURLConfig.cmake has a find_dependency(ZLIB) call

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep, but it does not work when placed under the vcpkg-cmake-wrapper and CMake 3.17 :)

Please try it yourself, it was not so nice when I discovered pipelines failing after CMake 3.17 upgrade

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe related to this warning:

CMake Warning (dev) at D:/qt2/installed/x64-windows/share/curl/CURLConfig.cmake:31 (if):
  if given arguments:

    "ON"

  An argument named "ON" appears in a conditional statement.  Policy CMP0012
  is not set: if() recognizes numbers and boolean constants.  Run "cmake
  --help-policy CMP0012" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Builds fine if you change the ON to 1. Will probably also build fine if the policy is set.

CMake Deprecation Warning at D:/qt2/installed/x64-windows/share/curl/CURLConfig.cmake:26 (cmake_policy):
  The OLD behavior for policy CMP0012 will be removed from a future version
  of CMake.

Setting:
cmake_policy(SET CMP0012 NEW)
Also works.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And yeah curlpp is at fault here since it does not call cmake_minimum_required() before any project() call. If I add cmake_minimum_required(VERSION 2.8) to the top of curlpp CMakeLists.txt the build also succeeds.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what do you think would be the best solution? adding the cmake_policy(SET CMP0012 NEW) to curl vcpkg-cmake-wrapper.cmake or adding cmake_minimum_required(VERSION 2.8) to curlpp CMakeLists.txt?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

patching curlpp and submitting the patch to upstream. Curl is not at fault here

@PhoebeHui
Copy link
Copy Markdown
Contributor

PhoebeHui commented Apr 7, 2020

Related to #10535, the 2 PRs seems to fix same issue.

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented Apr 7, 2020

since mine is newer, I will close this one.

@cenit cenit closed this Apr 7, 2020
@cenit cenit deleted the dev/cenit/curl branch May 1, 2020 15:52
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.

[curlpp] build failure

4 participants