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

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

Closed
wants to merge 3 commits into from
Closed

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

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
d4f7248
[curl] fix vcpkg-cmake-wrapper missing ZLIB find_package call
cenit Apr 6, 2020
c0bfefe
[replxx] remove failure on ci baseline
cenit Apr 6, 2020
1585ba4
[curl] bump CONTROL
cenit Apr 6, 2020
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
4 changes: 2 additions & 2 deletions ports/curl/CONTROL
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: curl
Version: 7.68.0-2
Version: 7.68.0-3
Build-Depends: zlib
Homepage: https://github.com/curl/curl
Description: A library for transferring data with URLs
Expand Down Expand Up @@ -47,4 +47,4 @@ Feature: sspi
Description: SSPI support

Feature: brotli
Description: brotli support (brotli)
Description: brotli support (brotli)
1 change: 1 addition & 0 deletions ports/curl/vcpkg-cmake-wrapper.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ list(REMOVE_ITEM ARGS "NO_MODULE")
list(REMOVE_ITEM ARGS "CONFIG")
list(REMOVE_ITEM ARGS "MODULE")

_find_package(ZLIB)
Copy link
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
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
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
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
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
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
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

_find_package(${ARGS} CONFIG)

if(TARGET CURL::libcurl)
Expand Down
1 change: 0 additions & 1 deletion scripts/ci.baseline.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,6 @@ redis-plus-plus:arm64-windows=fail
replxx:arm-uwp=fail
replxx:x64-uwp=fail
replxx:arm64-windows=fail
replxx:x86-windows=fail
reproc:arm-uwp=fail
reproc:x64-uwp=fail
restbed:arm-uwp=fail
Expand Down