-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[curl] fix vcpkg-cmake-wrapper missing ZLIB find_package call #10715
Conversation
I had to insert a fix that is also on #10703 in order to have the green check |
@@ -2,6 +2,7 @@ list(REMOVE_ITEM ARGS "NO_MODULE") | |||
list(REMOVE_ITEM ARGS "CONFIG") | |||
list(REMOVE_ITEM ARGS "MODULE") | |||
|
|||
_find_package(ZLIB) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
Related to #10535, the 2 PRs seems to fix same issue. |
since mine is newer, I will close this one. |
Fixes: #10714
Describe the pull request
What does your PR fix? Fixes issue [curlpp] build failure #10714
Does your PR follow the maintainer guide?
yes