-
Notifications
You must be signed in to change notification settings - Fork 283
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
Support arbitrary version strings in OpenSSL wrapper and add minimum_openssl_version option #2475
Conversation
This comment has been minimized.
This comment has been minimized.
I have some concerns. I suggested in slack
Because I think this approach has some issues
I think having the separate "minimum_wrapped_version" parameter sorts out these issues. |
@Micket with this PR as it is, it will indeed be complicated to push an existing installation using the v1.1 wrapper to v1.1.1. However, I think that it is not necessary to go that way. Existing installations are fine in my opinion. We do not currently have any easyconfig in the tree that needs OpenSSL v1.1.1. So I would not touch the current v1.1 wrapper used in 2021a. If there is an agreement to set a minimum version with an extra option such as
I would follow the same rules that we have for other software, we set a version per toolchain and we stick to it. In this case, v1.1 for 2021a (which is equivalent to a minimum |
Just to mention this in the PR for posterity; as far as I know OpenSSL is pretty far from semver. We have actually already messed in the 4.4.0 release here since it does already include an implicit assumption on >=1.1.1 with Qt5 5.15.2, so something needs changing, and I don't think we ever want 1.1.0. I would also find it a bit strange if versions don't actually match exactly if we go with "1.1.1x/1.1.1" versions compared to the intentionally vague 1.1. Perhaps some third/fourth/fifth opinions on all of this. @boegel maybe? |
I saw that
Oh right, I missed that merge. Then it's already an issue for 2021a. It's not ideal, but in that case the best solution might indeed be to add that internal minimum version to the 1.1 wrapper. I'll look into it.
For Java it's different because it is very unlikely that anything breaks if the Java installation is not kept up to date. With OpenSSL, software will start to not build if not updated properly. |
@Micket added the extra option Updated the PR description and table with example cases. Ready for tests. |
Test report by @lexming Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) CentOS 7.9 with OpenSSL 1.0.2k as main install and 1.1.1g on the side
|
Test report by @lexming Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) CentOS 7.9 with OpenSSL 1.0.2k as main install and 1.1.1g partially installed on the side without headers
|
Test report by @lexming Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) CentOS 7.9 with OpenSSL 1.0.2k
|
Test report by @lexming Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) Fedora 33 with OpenSSL 1.1.1k and compatibility library for 1.0.2o that lacks the openssl executable and headers
|
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.
lgtm
This comment has been minimized.
This comment has been minimized.
Test report by @Micket Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
As discussed in Slack, software needing OpenSSL v1.1.1(eg Qt5) will not work with v1.1.0. Therefore we have to make our wrapper aware of the full version of OpenSSL.
Since the soname of
libssl
is limited to a major minor version, the system will only provide a singlelibssl.so.1.1
in any case. So the existing code to find the library file still applies. All that is needed is an extra check on the version string contained in the shared object.Changelog:
opensslv.h
headerupdate in 6d5124e
libssl
andlibcrypto
as some distros shiplibssl
without any version stringupdate in 044ef73, 70d0186, 5656949
minimum_openssl_version
to explicitly set the minimum version of OpenSSL required in the systemminimum_openssl_version
can only increase the depth of the wrapper easyconfig versionExisting easyconfigs with just
major.minor
versions continue to work.Example of the results obtained with different combinations of OpenSSL versions: