Bump curl from 8.19 to 8.20#106077
Conversation
Track upstream `curl-8_20_0` release. Adapt the ClickHouse CMake integration for the source layout changes in this release: - Add `cf-dns.c`, `dnscache.c`, `protocol.c`, `thrdpool.c`, `thrdqueue.c` to the source list (DNS cache split out of `hostip.c`; new protocol table; new thread pool / queue helpers). - Drop `curl_rtmp.c` (removed upstream). - In `curl_config.h`, rename `USE_THREADS_POSIX` to `HAVE_THREADS_POSIX` (now drives `USE_MUTEX` for `curl_share` locks), add `USE_RESOLV_ARES` to wire c-ares as the resolver under the new naming, and define `CURL_ENABLE_NTLM` to preserve the previous (implicit) NTLM build. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
clickhouse-gh
Bot
added
pr-build
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
| #define USE_THREADS_POSIX | ||
| #define HAVE_THREADS_POSIX | ||
| #define USE_ARES | ||
| #define USE_RESOLV_ARES |
There was a problem hiding this comment.
This changes the resolver backend, not just the macro spelling. With the old curl_config.h, USE_THREADS_POSIX made 8.19 take the threaded resolver path in curl_setup.h before it ever considered USE_ARES, so name resolution went through system getaddrinfo. Defining USE_RESOLV_ARES here makes 8.20 use c-ares instead, which can stop honoring NSS-backed names that getaddrinfo resolves, such as entries from non-DNS nsswitch.conf providers.
If the bump is meant to preserve ClickHouse's existing resolver behavior, this should be USE_RESOLV_THREADED with HAVE_THREADS_POSIX. If switching to c-ares is intentional, the PR should call it out as a behavior change and cover at least one resolver case that matters for ClickHouse deployments.
There was a problem hiding this comment.
This seems legitimate. The PR description states "add USE_RESOLV_ARES to wire c-ares as the resolver under the new naming", why are we making this change?
There was a problem hiding this comment.
Apparently USE_ARES was a noop previously, and agent "fixed it" along the way. I'd say we should switch to USE_RESOLV_ARES if it correcly makes curl use ares resolver. I'll take a look into if we still need USE_ARES.
There was a problem hiding this comment.
My agent claims that there is no reason to switch to c-ares since we don't use curl widely anyway, and it could result in some random thing not resolving. 🤷
There was a problem hiding this comment.
| #define HAVE_THREADS_POSIX | ||
| #define USE_ARES | ||
| #define USE_RESOLV_ARES | ||
| #define CURL_ENABLE_NTLM |
There was a problem hiding this comment.
I think we don't use NTLM at all, right? Maybe we should drop it and the files relevant to it. Perhaps out of scope for this PR.
| #define USE_THREADS_POSIX | ||
| #define HAVE_THREADS_POSIX | ||
| #define USE_ARES | ||
| #define USE_RESOLV_ARES |
There was a problem hiding this comment.
This seems legitimate. The PR description states "add USE_RESOLV_ARES to wire c-ares as the resolver under the new naming", why are we making this change?
george-larionov
left a comment
george-larionov
left a comment
There was a problem hiding this comment.
LGTM overall, up to you what to do with c-ares.
|
It's ok, we can switch to c-ares and see what happens. PS. I wanted to remove the curl dependency altogether, but it is still used in some obscure places. |
robot-ch-test-poll4
added
the
pr-synced-to-cloud
thevar1able
added
the
pr-must-backport
robot-ch-test-poll4
added
the
pr-must-backport-synced
Cherry pick #106077 to 25.8: Bump `curl` from 8.19 to 8.20
Cherry pick #106077 to 26.3: Bump `curl` from 8.19 to 8.20
Cherry pick #106077 to 26.4: Bump `curl` from 8.19 to 8.20
Cherry pick #106077 to 26.5: Bump `curl` from 8.19 to 8.20
robot-ch-test-poll
added
the
pr-backports-created
Backport #106077 to 25.8: Bump `curl` from 8.19 to 8.20
Backport #106077 to 26.3: Bump `curl` from 8.19 to 8.20
Backport #106077 to 26.5: Bump `curl` from 8.19 to 8.20
Backport #106077 to 26.4: Bump `curl` from 8.19 to 8.20
6 participants
Bump
curlto upstream release curl-8_20_0.Adapts the ClickHouse CMake integration for the source layout changes in this release:
cf-dns.c,dnscache.c,protocol.c,thrdpool.c,thrdqueue.cto the source list (DNS cache split out ofhostip.c; new protocol table; new thread pool / queue helpers).curl_rtmp.c(removed upstream).curl_config.h, renameUSE_THREADS_POSIXtoHAVE_THREADS_POSIX(now drivesUSE_MUTEXforcurl_sharelocks), addUSE_RESOLV_ARESto wire c-ares as the resolver under the new naming, and defineCURL_ENABLE_NTLMto preserve the previous (implicit) NTLM build.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Update bundled
curlto 8.20.0.Version info
26.6.1.25726.5.3.15,26.4.5.17,26.3.14.7,25.8.25.11