Skip to content

[curl] Add http3 feature on linux only #37146

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.

Sign up for GitHub

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

Closed
talregev wants to merge 1 commit into from
Closed

[curl] Add http3 feature on linux only #37146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ports/curl/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ vcpkg_check_features(OUT_FEATURE_OPTIONS FEATURE_OPTIONS
FEATURES
# Support HTTP2 TLS Download https://curl.haxx.se/ca/cacert.pem rename to curl-ca-bundle.crt, copy it to libcurl.dll location.
http2 USE_NGHTTP2
http3 USE_OPENSSL_QUIC
wolfssl CURL_USE_WOLFSSL
openssl CURL_USE_OPENSSL
mbedtls CURL_USE_MBEDTLS
Expand Down
19 changes: 19 additions & 0 deletions ports/curl/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"name": "curl",
"version": "8.7.1",
"port-version": 1,
"description": "A library for transferring data with URLs",
"homepage": "https://curl.se/",
"license": "curl AND ISC AND BSD-3-Clause",
Expand Down Expand Up @@ -45,6 +46,24 @@
"nghttp2"
]
},
"http3": {
"description": "HTTP3 support",
"supports": "(uwp | !windows) & !(osx | ios) & !mingw",
Copy link
Copy Markdown
Contributor

@dg0yt dg0yt Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is complex and probably wrong. Are mingw and osx really unsupported? Is uwp really supported, and regular windows isn't?

Copy link
Copy Markdown
Contributor Author

@talregev talregev Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copy that from the ssl support for openssl. It exact the same.
https://github.com/microsoft/vcpkg/pull/37146/files#diff-a91e0d663e8719fb2cb62552c202144c9be4e9a2548b555644fd3399673cddb0R171

Copy link
Copy Markdown
Contributor Author

@talregev talregev Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dg0yt As I mention and you keep ignore my comment.
When you install all the features in the excluded platforms, it will cause build failure.

Copy link
Copy Markdown
Contributor

@dg0yt dg0yt Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copy that from the ssl support for openssl. It exact the same.

Note: This is the selection of openssl as the default. The use of openssl is not restricted. You can vcpkg install curl[core,openssl]:x64-windows if you don't want schannel.

Copy link
Copy Markdown
Contributor Author

@talregev talregev Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vcpkg don't let you truly to use ssl as you want. The choise of ssl also backed in the features.
A simple example that will represent common use that represent the problem I am talking about. On windows:
vcpkg install curl[core,http3,http2] ->
a. from http3 -> curl[http3,openssl]
b. from http2 -> curl[http2,ssl] -> curl[http2,ssl,schannel]
together: curl[http3,http2,openssl,schannel,ssl] -> enable multissl feature then cause build error with http3.

Beside that simple example, you cannot use curl[http3] on excluded platforms with other ports that use openssl, because they install ssl feature by default, and that make curl[http3] useless for them.

So until curl port will really let choose ssl backend on user choice and not by os type, I am put that restrictions. You can try to fix curl port after my merge, and see the problems yourself.

Copy link
Copy Markdown
Member

@BillyONeal BillyONeal Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vcpkg maintainers talked about this today and we agree with removing the TLS backend features in favor of adding an HTTP3 feature; @BillyONeal to test that upgrade does the right thing (we are 90% sure it does)

Copy link
Copy Markdown
Contributor Author

@talregev talregev Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BillyONeal I already have PR for that. #37450

Copy link
Copy Markdown
Contributor Author

@talregev talregev Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will put this one on draft, and activate the other one.

Copy link
Copy Markdown
Contributor

@dg0yt dg0yt Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vcpkg maintainers talked about this today and we agree with removing the TLS backend features in favor of adding an HTTP3 feature; @BillyONeal to test that upgrade does the right thing (we are 90% sure it does)

What does this mean? openssl everywhere?

Copy link
Copy Markdown
Contributor

@dg0yt dg0yt Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backend comparision: https://curl.se/docs/ssl-compared.html

"dependencies": [
{
"name": "curl",
"default-features": false,
"features": [
"openssl"
]
},
"nghttp3",
{
"name": "openssl",
"version>=": "3.2.0"
}
]
},
"idn": {
"description": "Default IDN support",
"dependencies": [
Expand Down
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2070,7 +2070,7 @@
},
"curl": {
"baseline": "8.7.1",
"port-version": 0
"port-version": 1
},
"curlpp": {
"baseline": "2018-06-15",
Expand Down
5 changes: 5 additions & 0 deletions versions/c-/curl.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "6c39ca59950b3ac90efbf1acab800f83e279beb5",
"version": "8.7.1",
"port-version": 1
},
{
"git-tree": "4f3aa7f4fd142a1c5822e4f36e0a4c45c031134a",
"version": "8.7.1",
Expand Down