-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
ngtcp2: update to 1.15.1 #29376
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
base: master
Are you sure you want to change the base?
ngtcp2: update to 1.15.1 #29376
Conversation
Notifying maintainers: |
--with-gnutls=yes | ||
|
||
variant openssl description "Enable OpenSSL" { | ||
depends_lib-append port:openssl3 |
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.
This should not be here, PortGroup already adds the dependency.
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.
Oops, I suppose I need to either do openssl.branch no_version
as with php*
port, or remove PortGroup openssl 1.0
as with 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.
Why though? If you want to use openssl3
, just keep the PG. If configure does not find it, then check the portgroup file for related variables.
P. S. You could move the PG inside the variant, if it is not used otherwise. (I do not remember now if it was needed or remained as an artifact.)
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.
Let me first try dropping it entirely. Semantically it doesn't make sense if OpenSSL isn't even needed in the default case.
net/ngtcp2/Portfile
Outdated
variant openssl description "Enable OpenSSL" { | ||
depends_lib-append port:openssl3 | ||
configure.args-append \ | ||
--with-openssl=yes |
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.
Just to make sure, can openssl and gnutls be used here simultaneously?
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.
Yes. Except those cases stated explicitly in the readme file (https://github.com/ngtcp2/ngtcp2?tab=readme-ov-file#crypto-helper-library), crypto library backends could coexist and are only leveraged when linked specifically by a program.
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.
Maybe use it by default then? Or the variant is still justified?
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.
As stated in the PR's description, support for OpenSSL is very new and has limitations not present in other backends, meaning that current programs cannot be easily migrated and new programs must meet those extra conditions. AFAIK neither Fedora Rawhide nor Debian Sid ship this backend, but I do see its value. Maybe even GnuTLS could be made as a variant (still the default one, though) so those who prefer dropping it could do so.
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.
Ok, do what you consider useful, I will test it on my end. And thank you for improving the port!
Added patch for detecting vanilla OpenSSL (for use with Picotls) without actually building the OpenSSL backend (for the Automake build process, with CMake it should behaves like this already). Patch already submitted to upstream. |
@CL-Jeremy Does building recent versions of |
--with-wolfssl=yes | ||
} | ||
|
||
variant picotls description "Enable Picotls backend" { |
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.
@CL-Jeremy This seems to imply that +picotls
requires +openssl
as well: ngtcp2/ngtcp2#1789 (comment)
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.
Nope, the patch (already accepted by upstream) deals with exactly this. With this patch (or using upstream 1.16+), it merely checks for vanilla OpenSSL and will proceed to build Picotls backend regardless of QUIC support inside OpenSSL (which is determined along with LibreSSL, wolfSSL and QuicTLS, just enabling Picotls by itself skips these checks). Of course, OpenSSL would be needed as a dependency build dependency (Picotls is currently only built statically, Picotls upstream has plans, but dynamic libs are not yet ready) in this case.
Oh and by the way, how does PortGroup openssl
handle build deps? Picotls should not need OpenSSL at runtime.
Description
I'm doing this mainly for my
samba4
port. Will follow up oncurl
, which supports the other backends (except Picotls, but since it supports OpenSSL, one could doport install ngtcp2 -- -gnutls +openssl +picotls
). Will see if more ports are affected.Type(s)
Tested on
macOS 10.15.7 19H2026 x86_64
Command Line Tools 12.4.0.0.1.1610135815
Verification
Have you
port lint
?sudo port test
?sudo port -vst install
?