-
Notifications
You must be signed in to change notification settings - Fork 703
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,GCCcore/11.2.0] Qt6 v6.2.3, PySide6 v6.2.3 w/ Python 3.8.6 + 3.9.6 #15096
Conversation
….2.0.eb, PySide6-6.2.3-GCCcore-10.2.0.eb, PySide6-6.2.3-GCCcore-11.2.0.eb
@kosl I see you the CI is complaining about style issues. Of course you can push new commits, and have the CI check if that resolves the style issues. However, that means you have to wait for the CI to trigger and wait for the result. It's generally much faster to try it yourself by running In this case, I think most style issues would be resolved if you remove all commented items, which I guess you were planning on doing at some point anyway :) |
Thank you @casparvl for taking care of the checks. This PR might not be complete at this point but it is a start for many of us. |
@boegelbot please test @ generoso |
@kosl: I noticed your comment, but I only dance when @akesandgren or @bartoldeman or @bedroge or @boegel or @branfosj or @casparvl or @jfgrimm or @lexming or @Micket or @migueldiascosta or @ocaisa or @SebastianAchilles or @smoors or @verdurin or @robert-mijakovic tells me (for now), I'm sorry... - notification for comment with ID 1065943345 processed Message to humans: this is just bookkeeping information for me, |
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on login1 PR test command '
Test results coming soon (I hope)... - notification for comment with ID 1066056965 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot |
@boegelbot please test @ generoso |
@casparvl: Request for testing this PR well received on login1 PR test command '
Test results coming soon (I hope)... - notification for comment with ID 1066175783 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot |
@kosl does this PR build for you locally? It probably only succeeds because it is picking up on various system libraries. I compared your EasyConfig to the one for
I'd suggest to go through the list of libraries from the Qt5 EasyConfig and check the gist to see if Qt6 is still looking for them. If it is, I'd suggest to keep including them as dependencies. Btw, I think your error comes from the fact that you commented out |
Dependencies are back now and it should build. |
LGTM |
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.
Qt5 build used to use a dedicated EasyBlock. I think we should keep doing the same for Qt6, and we can probably do that by expanding the Qt5 EasyBlock. See the suggestions in my comments.
] | ||
sources = ['qt-everywhere-src-%(version)s.tar.xz'] | ||
checksums = ['f784998a159334d1f47617fd51bd0619b9dbfe445184567d2cd7c820ccb12771'] | ||
|
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.
Please check which of the patches from
patches = [ |
- I think the 'Qt5-5.13.1_fix-avx2.patch' was only relevant when compiled with lower GCC versions (it refers to this bugreport which in turn refers to this one, which claims it was solved in GCC 9.3)
- I'm pretty sure 'Qt5-5.13.1_fix-qmake-libdir.patch' is still needed, but please check
- Similar for 'Qt5-5.14.1_fix-OF-Gentoo.patch'
- When I look at the Qt 6.2.3 sources, I think 'Qt5-5.12.2_fix-gcc11.patch' is no longer needed. The only change from that patch that seems to not have been upstreamed 1-to-1 is this, but still that code section changed slightly, so I guess they did resolve the underlying bug.
@@ -0,0 +1,74 @@ | |||
easyblock = 'ConfigureMake' |
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.
We used to have a dedicated EasyBlock for Qt: https://github.com/easybuilders/easybuild-easyblocks/blob/develop/easybuild/easyblocks/q/qt.py Now, I found that the build system for Qt6 changed to CMake https://www.qt.io/blog/qt-6-build-system so it makes sense that we can't use the old EasyBlock as it was.
However, I think we should make a new one, since a) the build procedure seems to be pretty peculiar and b) we don't want to loose the 'good stuff' from the previous EasyBlock. E.g. automatic selection of the target platform that was done here, custom sanity check step here, etc. We'll need to figure out what's still relevant for Qt6, as not everything might be.
If I understand correctly, the build procedure is something like
configure <qt_arguments> -- <CMake_arguments>
cmake --build .
cmake --install .
right? Isn't the old syntax of configure, make, make install
also not still supported then? It might be easier to use that, since that is already used by the ConfigureMake
EasyBlock (from which the Qt
EasyBlock inherits)
I think it would make most sense to simply expand the old Qt EasyBlock and use version checks (e.g. like this) to make it behave differently for Qt6. You'll need to figure out which parts of the Qt5 EasyBlock are no longer applicable to the Qt6 build (e.g. I think setting the QMAKE
variables is no longer needed?).
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.
I have Opened pull request: #17902 with newer compilers and all dependencies includes.
Build procedure with CMake is quite stable and no longer a problem as it was with Qt5. Qt5 EasyBlock are no longer applicable to the Qt6 build.
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.
closing this since see also: |
(created using
eb --new-pr
)