Qt 5.15 Deprecation fixes #975
Conversation
LouisStAmour
force-pushed
the
compatibility/qt515
branch
from
01a1f1e to
092b8d7
Compare
LouisStAmour
changed the title
|
Hi, I see that you are doing a lot of pull requests based on Qt 5.15. When we open sourced Sourcetrail we decided to stick with Qt 5.12 for a while because it has long term support, so the pre-built downloads will be around much longer. Is Qt 5.15 also an LTS release? If not, I'm not sure if we should update. |
|
I'm pretty sure I could have set the deprecations to 5.12 and most of these would still be required. I haven't interactively tested this branch yet, however. I picked 5.15 because it was the first to support MSVC 2019: https://doc-snapshots.qt.io/qt5-5.15/supported-platforms.html -- Previous versions do not: https://doc-snapshots.qt.io/qt5-5.14/supported-platforms.html 5.15 is indeed an LTS release, and is currently in the Preview section of the installer. LTS releases are being restricted to commercial customers going forward, however, so if and when 5.16 is released, they aren't going to open source any newer releases though they will still make patches available, it sounds like: https://www.qt.io/blog/qt-offering-changes-2020 I've focused primarily on staying up-to-date, my goal in all the software I write is that it should be based off trunk as much as reliably possible. For example, automatic updating of dependency versions: https://dependabot.com/#how-it-works Ideally we can confirm compatibility with visual regression (snapshot) tests, performance regression tests, done in relatively controlled rendering environments like Docker, etc. I'll admit, C++ doesn't have the tooling for this that I've come to expect from other language platforms. So I may have to re-evaluate some of my thoughts in light of this. But I think there's a lot the C++ community can learn from the web, and you're seeing this in projects from larger companies, such as the instrumentation, telemetry and testing that goes into the Chrome browser shipping, or new releases and testing channels of Microsoft operating systems and software. For example, though it's years out of date: https://www.infoq.com/articles/how-google-tests-software/ (How Google Tests Software, a book) That said, if there's a bug (or more than one) in any of my PRs or approaches, I'd be happy to backtrack -- but ideally from the perspective of investigating a solution. I'd rather not stick to older versions for too long, in my experience it makes your software less forward-compatible, more likely to be forked or require major revisions at a later date, etc. In this particular case, the deprecations generally highlight areas where a simplified view of text layout, mousewheel or some other Qt concept from Qt 4 or earlier was holding back matching how the underlying OS was receiving data or how things would be rendered. What I mean is that by looking at deprecations a bit closer, we can actually come up with better code, better ways of doing things. Eliminating toil is a key part of this - it should be easy to update to new versions as much as possible, and confirm things haven't broken. https://landing.google.com/sre/workbook/chapters/eliminating-toil/ |
|
Regarding versions and dependencies, #954 comes to mind. In corporate projects, I tend to prefer fully isolated builds like Bazel supports, where you download all the dependencies first, then run your build, then produce and run some result, and replace it when it needs updating like a static containerized app. But this is the real world, and people want to link to system dependencies, so find_package in CMake is more useful than Bazel at this time, I suppose. And the only alternative involves a fair amount of work in creating wrapper scripts to isolate a build process that's more open, or open up a build process that's more isolated, etc. |
|
Updated with additional fixes. |
|
Wow, that's really bad news from Qt. The reason we chose to stick with LTS releases (e.g. 5.12) is that once a new Qt version (e.g. 5.13) is released, the LTS version is still available for download. So if we didn't manage to update to that new Qt version right away (maybe we had build issues or we just didn't have the time), users can still download the LTS release of Qt and build Sourcetrail anyways. LTS will become commercial-only. So if we update to a Qt version past 5.12, it won't be guaranteed that Sourcetrail contributers will find a pre-built Qt version for download, thus they need to build it on their own, which may be some kind of road block for a number of those potential contributers. All in all, I'm not sure if we should update Qt. |
|
Given #981 I can see us trying to maintain support for both older and newer releases of Qt at the same time, testing in both Qt 5.9.9 say, as well as Qt 5.12.x and Qt 5.15 (or HEAD). We could say we support the HEAD release of the last 3-4 public branches of Qt, currently 5.9, 5.12 and 5.14-5.15. I could also see us eventually forking and hosting our own Qt releases if we only want LTS patch releases similar to what larger Java apps do today. Generally it’s a matter of setting up a build system and automatic patches, then running releases and tests automatically, maybe a bit of code signing. This assumes Qt Installer builds are easy to maintain open source, of course. |
| @@ -23,8 +24,8 @@ std::function<Ret(Args...)> loadFunctionFromLibrary( | |||
| const FilePath& libraryPath, const std::string& functionName, std::string& errorString) | |||
| { | |||
| #ifdef _WIN32 | |||
| const std::string libraryPathString = libraryPath.getBackslashedString(); | |||
| HINSTANCE handle = LoadLibrary(libraryPathString.c_str()); | |||
| auto libraryPathString = QDir::toNativeSeparators(QString::fromStdWString(libraryPath.wstr())); | |||
There was a problem hiding this comment.
Please don't use "auto", as we try to keep our code base explicit so that it is easily readable, even when using tools that don't infer a type for "auto".
|
Honestly, I don't think that its a good idea to tie Sourcetrail so closely to the Qt release cycle, such that whenever they release a new update, we also need to change some things. Even if most of it can be automated, there will be issues that need manual work and attention. Sourcetrail is not funded well enough to justify putting our time into maintaining multiple build configs for different Qt versions. @LouisStAmour: thank you very much for handing in this pull request. I would really like to accept all those deprication fixes. They should also be compatible with Qt 5.12. Can you please change that in the CMake again? And the one thing I commented on? |
|
Sure, I'll update the PR shortly as you describe. I do think we should keep in mind our dependencies and maybe build out our build system -- I've been meaning to play around with GitHub Actions, for example -- but my opinions are sourced from Google's SRE guidance as much as anything -- that it's better to have an automated system tell you when you screw up than to let the public tell you when things break, or worse, patch it for their distro and not tell you. :) Building out "supported configs" for testing in the cloud would be very much a nice to have, but a useful one, I think. |
|
We decided to close this PR, because we don't want to upgrade to Qt 5.15 at the moment. But @mlangkabel took parts of this PR and adapted it for Qt 5.12 with #1003. I still need to review and fix those changes for Linux & macOS. Nonetheless, thanks for implementing! |
No description provided.