-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[curl] Update to 8.18.0 #48563
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] Update to 8.18.0 #48563
Conversation
| ) | ||
| # The on-the-fly tarballs do not carry the details of release tarballs. | ||
| vcpkg_replace_string("${SOURCE_PATH}/include/curl/curlver.h" [[-DEV"]] [["]]) | ||
| vcpkg_replace_string("${SOURCE_PATH}/include/curl/curlver.h" [[LIBCURL_TIMESTAMP "[unreleased]"]] [[LIBCURL_TIMESTAMP "[vcpkg]"]]) |
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.
Should this maybe include the port version like vcpkg-2?
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.
- The port version isn't readily available as a CMake variable.
(I don't want to read the manifest for an optional property.) - If we were using official tarballs, the value would also be const for one version.
- If there was a strong desire for details, we might use (a fragment of) the directory name of
SOURCE_PATH(e.g.c-8_18_0-1-f780f9182e.clean->f780f9182e). It is unique for a combination of sources and patches.
|
This is the infamous CMake-breaks-link-library-order-for-traditional-linker. |
Only linking |
A: Upstream links using the cmake variables (expecting filepaths), not the cmake targets. |
This the result of Now that the CURL config package has been around for so long, I tend to simply follow upstream and let |
ports/curl/vcpkg-cmake-wrapper.cmake
Outdated
| message(WARNING "CURL_LIBRARIES list at least one target. This will not work for use cases where targets are not resolved.") | ||
| endif() | ||
| # leave CURL_LIBRARIES as set by upstream (imported target) | ||
| message(WARNING "Cannot easily unroll CURL_LIBRARIES (\"${CURL_LIBRARIES}\") to filepaths. Exported CMake config must use \"find_dependency(CURL)\".") |
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.
I think this warning could be moved into a watch() action for read access. Then it is only emitted when the variable is actually read, not when a project uses the target directly.
|
Using
Cascaded from a dependency which uses CURL_LIBRARIES. Check again after dealing with first list.
|
Adapt to upstream changes.
I expect that the port needs to keep the build system patches in order to satisfy static linkage and multi-config requirements across CMake, pkg-config and curl-config.
LIBCURL_VERSIONandLIBCURL_TIMESTAMP#48495CURL_LIBRARIESwarning, [curl] Update to 8.18.0 #48563 (comment)Exported configuration is provided in four ways, and this is why the port keeps significant patching vs. pristine CURL:
curl-configscripts: non-Windows only. Single config.All link libs and lib dirs must be listed explicitly.
Our patch reduces duplicate link libs.
pkg-config: Single config.It is best practice to refer to depedencies via
Requiresinstead ofLibsif possible. But upstream does both, and it also forwardsprivateto the regular field for static builds.Our patch removes redundancy (causing extra work for dealing with
curl-config). This trimming significantly facilitates downstream analysis of linking issues.CURL::libcurl. Multi-config in vcpkg only.Our patching ensures that link libraries are found without requiring pkg-config and that multi-config use is possible. (It would be fairly easy to supply pkg-config, but this would not support multi-config.
As before, this is achieved by using the LINK_LIBRARIES results from pkg_check_modules.
CURL_LIBRARIES. Multi-config in vcpkg only.This is the legacy interface provided by the Find module. Such variables are often consumed in the expectation that they don't contain imported targets, i.e. not requiring
find_dependencyin downstream exports. The port used to try hard to satisfy this condition. With this PR, this will only be done in trivial cases (shared library linkage, or just OpenSSL and ZLIB dependencies). For other dependencies,CURL_LIBRARIESwill just point toCURL::libcurlas implemented upstream.(i.e. patching is reduced a little bit now.)