-
Notifications
You must be signed in to change notification settings - Fork 704
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
{devel}[GCCcore/10.2.0] Qt5 v5.14.2 #11555
{devel}[GCCcore/10.2.0] Qt5 v5.14.2 #11555
Conversation
This comment has been minimized.
This comment has been minimized.
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
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 714275739 processed Message to humans: this is just bookkeeping information for me, |
…asyconfigs into 20201021205418_new_pr_Qt55151
This comment has been minimized.
This comment has been minimized.
Note: I tried to build PyQt on top of this without success, but I can't spot anything wrong with this particular build, so, maybe it's some lack of support in PyQt yet(?) |
@Micket Any log you can share? |
Test report by @boegel |
Test report by @boegel |
Test report by @boegel |
Test report by @smoors |
Test report by @zao |
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
So, how do we deal with the CentOS7 (and similarly old) OS'es. Let them opt in to use the OpenSSL dependency? It's going to quietly "fail" and just skip the components that need OpenSSL, so people are going to discovered they needed to modify the easyconfig when they get around to building PyQt5, which sucks. There is also a openssl11 package in CentOS7, so, we could try to get it to use that with some extra build flags maybe to make it work by default for everyone, but, again, the dynamic behavior here is annoying. Customize easyblock to support openssl11? Should we introduce a "OS_PKG_OPENSSL11_DEV" ? Hard to find an optimal solution here. |
I think that we have two options (given that PyQtWebEngine needs OpenSSL):
My vote goes for option 1. |
The discussion is a bit split between this PR and #11563, but taking into account both I'm not sure what our best way forward is here... If we go with Qt5 5.15.1 then we should consider it incompatible with CentOS 7, where OpenSSL 1.0.x is provided by the OS... I don't consider using an OpenSSL 1.1.x provided as dependency via EasyBuild a good idea with security in mind, even if we only used it as a build dependency and statically link it in to Qt5. One option could be creating an Is this a good enough reason to stick to Qt5 5.14.x in the 2020b generation of easyconfigs? |
If we downgrade to 5.14.x again, and postpone this until 2021, then we could consider actually start to always build OpenSSL by default, using a wrapper. This would be easier if everything under the next GCCcore started to link with an EB-provided OpenSSL by default. |
From an armchair security perspective, I'd expect that TLS support in scientific software would be more for client-initiated connections and less about serving as it's quite unlikely that anything has actual certificates anyway, reducing the attack surface greatly as there's no real secrets to leak. How up to date does a client TLS stack have to be? It would assumedly still use the distro's CA certificate collection? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Test report by @Micket |
Test report by @boegel edit: fails with:
@Micket We need Python 2 as a build dep for QtWebEngine? |
Test report by @boegel |
Test report by @boegel |
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 735283532 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegel |
Test report by @boegel |
Test report by @boegel |
Test report by @boegel |
Test report by @boegel |
oh what the heck is this now |
Test report by @boegelbot |
Test report by @Micket |
Test report by @Micket |
Test report by @boegel |
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
Going in, thanks @Micket! |
(created using
eb --new-pr
)depends on
#11536