Repository navigation
Update libcurl to 8.23.0 and remove legacy workarounds - #1952
AustinBenoit wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the bundled libcurl dependency from version 7.73.0 to 8.2.1 (mistakenly specified as 8.23.0). It updates CMake configurations and integration tests to include new platform-specific system library dependencies, such as bcrypt, iphlpapi, and secur32 on Windows, and SystemConfiguration on macOS. Additionally, it updates transport_curl.cc to use modern curl options like CURLOPT_PROTOCOLS_STR and curl_off_t transfer info types. The review feedback points out a critical build issue where the curl version is incorrectly set to 8_23_0 instead of 8_2_1, which will cause a 404 download failure.
| endif() | ||
|
|
||
| set(version 7_73_0) | ||
| set(version 8_23_0) |
There was a problem hiding this comment.
The version 8_23_0 (curl 8.23.0) does not exist. The SHA256 hash d0605d5883124ef5ae64c6d2b307e1417c482b362583b57491fdf37d9a99a792 corresponds to curl version 8.2.1 (8_2_1). Using 8_23_0 will cause the download URL to return a 404 Not Found error, which will break the build. Please update the version to 8_2_1.
set(version 8_2_1)
There was a problem hiding this comment.
- Update curl version in cmake/external/curl.cmake from 7_73_0 to 8_23_0
with SHA-256 verification and explicit DOWNLOAD_NAME.
- Update CMake configuration options in CMakeLists.txt for curl 8.23.0:
- Replace removed HTTP_ONLY option with explicit CURL_DISABLE_* protocol
flags so only HTTP/HTTPS are enabled.
- Replace renamed CMAKE_USE_SCHANNEL / CMAKE_USE_LIBSSH2 options with
CURL_USE_SCHANNEL / CURL_USE_LIBSSH2.
- Disable optional third-party libraries (libpsl, nghttp2, libidn2, and
cache-forced CURL_ZLIB, CURL_BROTLI, CURL_ZSTD), docs/manpages, and
install/CPack rules (CURL_DISABLE_INSTALL).
- Enable USE_APPLE_SECTRUST and set _curl_stdlib_cxx="c++" on Apple
platforms for native certificate verification and static BoringSSL +
libc++ linking.
- Remove obsolete APPLE cross-compiling CMAKE_TOOLCHAIN_FILE=/dev/null
workaround (fixed upstream in curl).
- Unset curl-specific BUILD_STATIC_LIBS and BUILD_EXAMPLES after
add_external_library(curl) so they do not linger in root directory scope.
- Update app/rest/CMakeLists.txt to pack libcurl_static (since libcurl is
now an ALIAS target) and link required Windows system libraries
(bcrypt, iphlpapi, secur32).
- Update app/rest/transport_curl.cc to use CURLOPT_PROTOCOLS_STR instead
of deprecated CURLOPT_PROTOCOLS, CURLINFO_*_T (curl_off_t) instead of
deprecated double-based CURLINFO_CONTENT_LENGTH_* and CURLINFO_SIZE_*,
and #pragma comment(lib) directives for MSVC static library consumers.
- Update consumer-facing documentation (release_build_files/readme.md,
AGENTS.md), third-party notices (release_build_files/NOTICES), and
integration test CMakeLists.txt files for the new macOS
SystemConfiguration.framework and Windows (bcrypt, iphlpapi, secur32)
link requirements.
2112e79 to
b3a1601
Compare
AustinBenoit
added
the
tests-requested: full
Integration test with FLAKINESS (succeeded after retry)Requested by @AustinBenoit on commit 67379e8
Add flaky tests to go/fpl-cpp-flake-tracker |
github-actions
Bot
added
the
tests: succeeded
a-maurice
left a comment
a-maurice
left a comment
There was a problem hiding this comment.
If you haven't yet, it'd be good to run the Unity tests against this. Will also need to update this page: https://firebase.google.com/docs/cpp/setup?platform=ios#additional-info-desktop
|
Unity run: I have the internal docs updates ready |
Firebase Unity SDK Verification (
|
Summary
Updates the bundled
libcurldependency from7.73.0(7_73_0) to the latest release8.23.0(8_23_0) and removes legacy curl workarounds.Changes
cmake/external/curl.cmake):7_73_0to8_23_0with explicitDOWNLOAD_NAMEandURL_HASH(SHA256).CMakeLists.txt,app/rest/CMakeLists.txt):HTTP_ONLYoption with explicitCURL_DISABLE_*protocol flags so onlyhttpandhttpsare built.CURL_USE_SCHANNEL,CURL_USE_LIBSSH2).CURL_USE_LIBPSL,USE_NGHTTP2,USE_LIBIDN2, and cache-forcedCURL_ZLIB,CURL_BROTLI,CURL_ZSTD), docs/manpages, and install/CPack rules (CURL_DISABLE_INSTALL).USE_APPLE_SECTRUSTand set_curl_stdlib_cxx="c++"on Apple platforms for native certificate verification and static BoringSSL +libc++linking.CMAKE_TOOLCHAIN_FILE=/dev/nullworkaround (fixed upstream in curl).BUILD_STATIC_LIBSandBUILD_EXAMPLESafteradd_external_library(curl)so they do not linger in root directory scope.libcurl_staticinapp/rest/CMakeLists.txt(sincelibcurlis anALIAStarget in curl 8.23.0) and add required Windows system libraries (bcrypt,iphlpapi,secur32).app/rest/transport_curl.cc):CURLOPT_PROTOCOLSwithCURLOPT_PROTOCOLS_STR("http,https").double-basedCURLINFO_CONTENT_LENGTH_*andCURLINFO_SIZE_*selectors withcurl_off_t-basedCURLINFO_*_T.#pragma comment(lib, ...)directives on MSVC for Windows system libraries required by staticlibcurl.-framework SystemConfiguration): Inlibcurl8.x (lib/macos.c),Curl_macos_init()(called fromcurl_global_init()when IPv6 is enabled withoutc-ares) callsSCDynamicStoreCopyProxies(NULL)from Apple'sSystemConfiguration.frameworkto initialize Darwin's dynamic store on the main thread before threadedgetaddrinfo()lookups run.bcrypt,iphlpapi,secur32):bcrypt(bcrypt.lib):libcurl8.x (lib/rand.c) uses Windows CNGBCryptGenRandom()for random number generation (replacing legacyCryptGenRandomfromadvapi32).iphlpapi(iphlpapi.lib):libcurl8.x callsif_nametoindex()from the IP Helper API to resolve IPv6 scope/zone IDs.secur32(secur32.lib):libcurl8.x (lib/curl_sspi.c) links directly againstInitSecurityInterface()insecur32.libfor Windows SSPI/Schannel instead of dynamically loadingsecur32.dllviaLoadLibrary()at runtime as in 7.73.0.firebase_app.lib,#pragma comment(lib, ...)directives embedded intransport_curl.ccautomatically link these Windows system libraries without requiring manual project file changes.