Make OpenSSL auto-enable by default#2407
Conversation
Require OpenSSL v3 library. Older versions still supported, but require LIBOPENSSL_LIBS="-lssl -lcrypto"
Command:
git grep -l USE_OPENSSL | while read f; do
sed -i -e 's/[ ]USE_OPENSSL/ HAVE_LIBOPENSSL/g' \
-e 's/!USE_OPENSSL/!HAVE_LIBOPENSSL/g' $f
done
The prefix character is important to avoid changing
other macros which contain the substring verbatim.
OpenSSL library was the only use of this autoconf macro. We can finally drop it.
This test case used to be folded into the layer-00-default becasue OpenSSL was auto-disabled. Now that OpenSSL is auto-enabled we need to check it explicitly to ensure that GnuTLS-only environments still build properly.
yadij
added
feature
|
See individual commit messages for details on the relevant step of the total upgrade. |
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
I continue to beg you to stop posting new pull requests until the growing backlog is dealt with.
|
|
||
| srcdir=`dirname $0` | ||
|
|
||
| # Everything is supposed to work when OpenSSL is not available. |
There was a problem hiding this comment.
I agree that "everything is supposed to work when OpenSSL is not available", but "everything but OpenSSL" case is not special enough to deserve adding a whole buildtest layer for it IMO. We do not have a layer dedicated to testing "everything but GnuTLS" builds. Same for all other optional libraries. Let's not start adding such layers.
There was a problem hiding this comment.
The "everything but GnuTLS" is folded into the maximus layer on our CI nodes - by way of how OpenSSL enabled overrides the #elif HAVE_LIBGNUTLS.
When OpenSSL is auto-enabled the "default" and "maximus" layers start doing the same things. We loose the "everything but OpenSSL" which was, until this PR, folded into "default". Thus the new layer.
| ## For some OS pkg-config is broken or unavailable. | ||
| ## Detect libraries the hard way. | ||
|
|
||
| PKG_CHECK_MODULES([LIBOPENSSL],[openssl >= 3],[:],[ |
There was a problem hiding this comment.
PR description: Require OpenSSL v3 library minimum due to GPL license compatibility with older OpenSSL versions.
Regardless of whether there is such (in)compatibility, I do not see enough reasons to restrict OpenSSL version like this. If folks install OpenSSL v1 but do not want to build with it, they can use --without-openssl or equivalent. A simpler/less-restrictive default is better for developers and in most deployment cases.
There was a problem hiding this comment.
I choose not to surprise our users with potential legal problems by default.
As mentioned in the PR description it is easy for the (few already and decreasing) builders who do not have v3 available to enable the legacy openssl support.
| #include "auth/UserRequest.h" | ||
| #endif | ||
| #if USE_OPENSSL | ||
| #if HAVE_LIBOPENSSL |
There was a problem hiding this comment.
Modifying 80+ files to rename this frequently used macro is not a good idea, especially with our backlog of changes.
There was a problem hiding this comment.
Meh. Everything you put off "because backlog" is just another entry on that backlog of yours.
I spent a few hours trying workarounds, but none worked well enough so I have left the collisions for a later pre-merge decision.
There was a problem hiding this comment.
Meh. Everything you put off "because backlog" is just another entry on that backlog of yours.
"That backlog" is Squid Project backlog; it is not just "mine" or "yours". It has very serious negative effects on nearly all Squid development, by all core developers, including this pull request. My numerous attempts to resolve that problem have failed so far, but ignoring it and posting more pull requests like this one only makes things worse.
2 participants
Require OpenSSL v3 library minimum due to GPL license
compatibility with older OpenSSL versions.
Older versions still supported, but require
LIBOPENSSL_LIBS="-lssl -lcrypto"