-
Notifications
You must be signed in to change notification settings - Fork 7.5k
[curl] http3 (with ngtcp2) non experimental feature #46459
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| { | ||
| "name": "curl", | ||
| "version": "8.15.0", | ||
| "port-version": 1, | ||
| "description": "A library for transferring data with URLs", | ||
| "homepage": "https://curl.se/", | ||
| "license": "curl AND ISC AND BSD-3-Clause", | ||
|
|
@@ -33,7 +34,7 @@ | |
| ] | ||
| }, | ||
| "gnutls": { | ||
| "description": "SSL support (gnutls)", | ||
| "description": "TLS support (gnutls)", | ||
| "dependencies": [ | ||
| { | ||
| "name": "libgnutls", | ||
|
|
@@ -71,6 +72,26 @@ | |
| "nghttp2" | ||
| ] | ||
| }, | ||
| "http3": { | ||
| "description": "HTTP3 support with ngtcp2 on openssl", | ||
| "dependencies": [ | ||
| { | ||
| "name": "curl", | ||
| "default-features": false, | ||
| "features": [ | ||
| "openssl" | ||
| ] | ||
| }, | ||
| "nghttp3", | ||
| { | ||
| "name": "ngtcp2", | ||
| "default-features": false, | ||
|
Comment on lines
+87
to
+88
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it even possible to set default-features to false for other dependencies than "self" (i.e. curl trying to set ngtcp2 to non-default)? I read somewhere in the issue tracker that this can only be explicitly set by the root project and otherwise has no effect. My Gradle build script wrapper around vcpkg at least complains about this now. Or is my check too strict?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having |
||
| "features": [ | ||
| "openssl" | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| "httpsrr": { | ||
| "description": "enable support for HTTPS RR" | ||
| }, | ||
|
|
@@ -119,16 +140,16 @@ | |
| ] | ||
| }, | ||
| "mbedtls": { | ||
| "description": "SSL support (mbedTLS)", | ||
| "description": "TLS support (mbedTLS)", | ||
| "dependencies": [ | ||
| "mbedtls" | ||
| ] | ||
| }, | ||
| "non-http": { | ||
| "description": "Enables protocols beyond HTTP/HTTPS/HTTP2" | ||
| "description": "Enables protocols beyond HTTP/HTTPS/HTTP2/HTTP3" | ||
| }, | ||
| "openssl": { | ||
| "description": "SSL support (OpenSSL)", | ||
| "description": "TLS support (OpenSSL)", | ||
| "dependencies": [ | ||
| "openssl" | ||
| ] | ||
|
|
@@ -145,19 +166,6 @@ | |
| "librtmp" | ||
| ] | ||
| }, | ||
| "schannel": { | ||
| "description": "SSL support (Secure Channel)", | ||
| "supports": "windows & !uwp", | ||
| "dependencies": [ | ||
| { | ||
| "name": "curl", | ||
| "default-features": false, | ||
| "features": [ | ||
| "sspi" | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| "ssh": { | ||
| "description": "SSH support via libssh2", | ||
| "dependencies": [ | ||
|
|
@@ -179,17 +187,19 @@ | |
| ] | ||
| }, | ||
| "ssl": { | ||
| "description": "Default SSL backend", | ||
| "description": "Default SSL / TLS implementation.", | ||
| "dependencies": [ | ||
| { | ||
| "$comment": "Defaults to schannel on Windows, unless http3 is also enabled", | ||
| "name": "curl", | ||
| "default-features": false, | ||
| "features": [ | ||
| "schannel" | ||
| "sspi" | ||
| ], | ||
| "platform": "(windows & !uwp) | mingw" | ||
| }, | ||
| { | ||
| "$comment": "Otherwise, defaults to OpenSSL.", | ||
| "name": "curl", | ||
| "default-features": false, | ||
| "features": [ | ||
|
|
@@ -213,7 +223,7 @@ | |
| }, | ||
| "sspi": { | ||
| "description": "SSPI support", | ||
| "supports": "windows & !uwp" | ||
| "supports": "(windows & !uwp) | mingw" | ||
| }, | ||
| "tool": { | ||
| "description": "Builds curl executable", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,6 +94,9 @@ crashpad:x64-linux=fail | |
| ctemplate:x64-linux=fail | ||
| ctemplate:x64-osx=fail | ||
| cuda:x64-osx=fail | ||
| curl[core,http3,gnutls]=options | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note for other reviewers: This has the effect of testing http3 and no longer testing the other TLS backends in our 'combined' feature test.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do test the other tls (multissl) in the vcpkg-ci-curl. |
||
| curl[core,http3,mbedtls]=options | ||
| curl[core,http3,wolfssl]=options | ||
| dbghelp=skip # Since pipeline cannot automatically install dbghelp dependency, skip this detection | ||
| dimcli(windows & static)=fail # VS2019 version 16.9.4's project system changes where PDBs are placed in a way that breaks the upstream build script of this port. See https://developercommunity.visualstudio.com/t/Toolset-169-regression-vcxproj-producin/1356639 | ||
| discord-game-sdk:x64-windows-static-md=fail | ||
|
|
@@ -750,7 +753,6 @@ cppzmq:arm64-uwp = cascade | |
| cppzmq:x64-uwp = cascade | ||
| cub(!((windows & x64 & !uwp) | (linux & x64) | (linux & arm64))) = cascade | ||
| cuda-api-wrappers(uwp | osx) = cascade | ||
| curl[wolfssl](uwp) = cascade | ||
| cutelyst2:arm64-uwp = cascade | ||
| cutelyst2:arm64-windows = cascade | ||
| cutelyst2:x64-uwp = cascade | ||
|
|
@@ -2102,7 +2104,6 @@ cgns[fortran](windows | android) = feature-fails # No fortran compiler installed | |
| coroutine(osx) = fail # requires c++20 | ||
| crashrpt(windows) = fail # precompiled header errors. See https://github.com/microsoft/vcpkg/issues/33470 | ||
| ctbench(osx) = fail # requires C++ 20 | ||
| curl[brotli,c-ares,http2,idn,idn2,mbedtls,non-http,openssl,sectransp,ssh,ssl,tool,websockets,wolfssl](osx) = combination-fails | ||
| date[remote-api](uwp) = feature-fails # error C2065: 'FOLDERID_ProgramFiles': undeclared identifier. See https://github.com/microsoft/vcpkg/issues/33610 | ||
| dcmtk[core,iconv,icu,openssl,png,tiff,tools,xml2,zlib](osx) = combination-fails # missing symbols from libtiff. See https://github.com/microsoft/vcpkg/issues/33512 | ||
| dcmtk[xml2](windows) = feature-fails # fatal error C1083: Cannot open include file: 'iconv.h'. See https://github.com/microsoft/vcpkg/issues/33473 | ||
|
|
||
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.
Shouln't
http3also be wired to control the use ofnghttp3?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.
It adding nghttp3 inside curl cmake automatic.
I don't want it will be confusion with openssl + nghttp3 the experimental way for http3 with openssl.
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.
"automatic" without context doesn't answer the question.
I looked it up now:
USE_NGTCP2is (almost*) sufficient to control the use of the nghttp3 dependency (USE_QUICHEleft aside):https://github.com/curl/curl/blob/cfbfb65047e85e6b08af65fe9cdbcf68e9ad496a/CMakeLists.txt#L1087-L1131
*) There is the multi-ssl problem again: curl could support this with wolfssl, but the port won't allow that combination - feature "http3" can depend only on one particlar SSL option.
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.
Given that
USE_NGHTTP3does not appear to be an input I'm happy with this as @talregev has written