libcurl: fix dependencies handling in CMake build #15460
Conversation
therefore min conan version is bumped to 1.54.0
SpaceIm
force-pushed
the
fix/libcurl-cmake-deps
branch
from
2fc9e5d to
1304462
Compare
ghost
mentioned this pull request
|
I detected other pull requests that are modifying libcurl/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
ghost
mentioned this pull request
…nan-center is greater than 7.72.0
|
I've also removed logic based on libcurl versions not maintained anymore. |
This comment has been minimized.
This comment has been minimized.
| # TODO: check this patch, it's suspicious | ||
| replace_in_file(self, cmakelists, | ||
| "include(CurlSymbolHiding)", "") | ||
|
|
||
| # brotli | ||
| replace_in_file(self, cmakelists, "find_package(Brotli QUIET)", "find_package(brotli REQUIRED CONFIG)") |
There was a problem hiding this comment.
Why not use the Conan 1.55 feature and override this before the generate?
There was a problem hiding this comment.
It wouldn't work except for libnghttp2 and wolfssl.
There was a problem hiding this comment.
I wouldn't even do that for libnghttp2 because we want to link only nghttp2 component, so a patch is required anyway.
There was a problem hiding this comment.
🤮 that's sucky can you share your changes so I can flag it too the team please?
There was a problem hiding this comment.
Sorry I don't understand what you want me to share exactly. For me the reason why it can't work in this specific case is obvious when you know how CMakeDeps works and looking at what is replaced here in libcurl CMakeLists.
It's a combination of cases that just can't work with CMakeDeps or anything you could try with set_property() during CMakeDeps generation:
- casing inconsistency. Example:
find_package(foo)followed byif(FOO_FOUND)orFOO_LIBRARIES, instead ofif(foo_FOUND)orfoo_LIBRARIES. Indeed CMakeDeps always generates variables with a prefix whose casing is the one ofcmake_file_name. If you overridecmake_file_namewithfoo, it will not defineFOO_FOUND&FOO_LIBRARIESso it can lead to several issues (feature unexpectedly disabled, link to dependency fails etc). If you overridecmake_file_namewithFOO,find_package(foo)won't be able to findFOOConfig.cmake - non-standard variables names CMakeDeps never generates. Example:
FOO_LIBRARY
There was a problem hiding this comment.
I'd love to see this put into action https://blog.conan.io/2022/12/20/New-conan-release-1-55.html
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
WTF? I've solved merge conflicts, what's wrong? It's not the first time I see these incorrect merge conflicts after a merge of master branch. |
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 11 (
Conan v2 pipeline (informative, not required for merge) ❌
The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future. See details:Failure in build 14 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
* fix interaction of CMakeDeps & CMakeLists of libcurl * remove mingw workaround for autotools install therefore min conan version is bumped to 1.54.0 * zstd option always available since we oldest version maintained in conan-center is greater than 7.72.0 * cleanup more stuff not related to versions currently maintained * map with_libpsl option to CMake build since 7.84.0 * properly handle with_libpsl option * no self.info in validate() * small change
closes #15457
#14902 led to a side effect which is important to understand for conan contributors: CMakeDeps is not a silver bullet, there is no magic when you bypass custom Find modules of upstream with slightly different casing.