| stikonas |
qAsConst() is only available since Qt 5.7.0, but the specified minimum Qt version is 5.2.0.
This partially reverts commit d671f62febfe2bedeae9c427c58a02675dabd9cb in case of building with a lower Qt version to avoid the usage of qAsConst() and make it compile.
Builds fine now with Qt 5.5.1, 5.6.1, 5.6.2, and 5.9.1.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
Well it's fine with me but do we still want to keep support for old Qt versions? Qt 5.7 is required for almost everything now, current Plasma versions, current Applications, etc...
Well, Qt 5.6 is LTS, and the latest openSUSE Leap 42.3 (released in July) only comes with 5.6.2 therefore.
Not all applications require 5.7.0 as a minimum, and Plasma 5.8 LTS does support 5.6 as well.
In any case, if libktorrent requires 5.7.0, it should say so in the CMakeLists.txt.
Well, of course CMakeLists.txt needs to reference new Qt version, it was forgotten... I just wasn't sure if anybody still ships Qt 5.6, but apparently OpenSUSE does, strange that they are older than Debian stable...
Maybe we still want to bump it to 5.5 at least if it wasn't tested with older versions.
If you push it now I can try to mention it in the libktorrent release notes...
openSUSE Leap is partly based on the commercial SUSE Linux Enterprise, and the SUSE maintainers decided to stick with 5.6.x for now (mainly because it's LTS)...
Maybe we still want to bump it to 5.5 at least if it wasn't tested with older versions.
I would try to build it with older versions too, but I would need to build the Frameworks first too (openSUSE 13.2 came with Qt 5.2 and 5.4, but only has Frameworks 5.11...)
I suppose requiring 5.5 at least would be fine though.
If you push it now I can try to mention it in the libktorrent release notes...
Pushed to the 2.1 branch, should I merge it to master too?
Hm, I just noticed that this is kind of moot anyway because ktorrent itself does require Qt 5.7.0 now.
(I couldn't compile it earlier because I wasn't able to download the tarball, there was an "insufficient permissions" error)
From what I see it would be quite some work to change that too, and probably not worth it...
FWIW, I patched ktorrent to build with Qt < 5.7.0 too.
Would you be interested in that patch? Just asking as I did it already anyway...
The patch is quite big though, nearly 1000 lines (and that's without version checks which I would have yet to add).
https://build.opensuse.org/package/view_file/home:wolfi323:branches:KDE:Extra/ktorrent/fix-build-with-qt5.6.patch?expand=1
Hmm, it's a fairly big patch then... Not sure whether it's worth adding so many ifdefs. Maybe let's ask somebody else, I might ask tosky...
Just to avoid a misunderstanding: the patch file is about 1000 lines.
The actual changes boil down to a little more than 100 lines that are modified.
Hmm, I think people on kde-devel think that it's not worth pushing to ktorrent repo if it's just opensuse...
That's fine, and I can fully understand it.
And it is of course ok that ktorrent requires Qt 5.7.0. Though we have to patch it then if we want to offer it for current openSUSE Leap 42.x... (the next one will come with Qt 5.9 anyway)
But, as it is only about qAsConst(), it also works to define it manually when building against Qt < 5.7.0. That makes the patch much less intrusive and easier to maintain as distribution-specific patch.