search

keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
21.43k stars 1.48k forks source link

Qt 6 upgrade #7774

Open c4rlo opened 2 years ago

c4rlo commented 2 years ago

Summary

Qt 6 was released in Dec 2020. We should ultimately port KeePassXC to it from Qt 5. I'd be happy to help out with this.

Details

Before completely switching to Qt 6, we should introduce an opt-in build mode for it.

The Porting to Qt 6 guide notes:

Before upgrading to Qt 6, make sure that your Qt 5 application is updated to Qt 5.15. The latest Qt 5 version has the least amount of changes when porting to Qt 6.

Currently, when building KeePassXC with -DWITH_DEV_BUILD=ON, there are a lot of warnings about usage of deprecated Qt features. For example:

src/cli/keepassxc-cli.cpp:236:71: warning: ‘QTextStream& QTextStreamFunctions::endl(QTextStream&)’ is deprecated: Use Qt::endl [-Wdeprecated-declarations]
  236 |         err << QObject::tr("Invalid command %1.").arg(commandName) << endl;
      |                                                                       ^~~~

However, Qt::endl is only available from Qt 5.14.

INSTALL.md currently lists "Qt 5 (>= 5.9.5)" as a requirement.

Question: Should we increase our minimum Qt 5 version requirement, to make porting to Qt 6 easier? Or would the maintainers prefer to keep the current minimum requirement? If the latter, then adding Qt 6 support would involve a few more #if QT_VERSION >= ... checks.

droidmonkey commented 2 years ago

Moving to qt6 is easy for Mac and windows.... almost impossible on Linux right now. At least it would require a lot more work. We want to go to Qt6, but there is really no reason to do that right now.

c4rlo commented 2 years ago

What makes it so hard on Linux?

If it's opt-in as I suggested (e.g. behind -DWITH_QT6=ON), that should let Linux distro packagers upgrade when they are ready.

I'm also still curious about how feasible it would be to raise the minimum Qt 5 version.

droidmonkey commented 2 years ago

Omg no build flag, that would be a testing nightmare. The qt minimum version aligns with the oldest supported Ubuntu release. Qt6 has very low availability on Linux, only Ubuntu Jammy has it in the package repository at this time.

hifi commented 2 years ago

This is a list of the current status as of April 2022 of some relevant distribution channels.

The Good

The Bad

These are unlikely to get a backport of Qt 6 and we can't realistically wait this long. We should just make sure the Flatpak and AppImage run on these.

The Ugly

Relevant distribution channels that don't have Qt 6 yet.

Polynomial-C commented 2 years ago

The Good

* Gentoo **[Qt 6.2.4](https://github.com/gentoo/qt/tree/master/dev-qt/qtbase)**

Unfortunately this is not Gentoo's main package repository you were pointing to but a separate package repository mainly exisiting for qt testing. Gentoo's main package repository (the one which the average user installs packages from) still is at qt-5.12.3

hifi commented 2 years ago

Ah, my bad. I'll move it, thanks for pointing that out!

c4rlo commented 2 years ago

@droidmonkey wrote:

The qt minimum version aligns with the oldest supported Ubuntu release.

That would be Ubuntu 18.04 (LTS): it has Qt 5.9.5. However, it also only has Botan 2.4.0, less than the required version (2.11.0).

Ubuntu 20.04 (the next LTS) has Qt 5.12.8 and Botan 2.12.1, so it's fine (and I've just confirmed keepassxc builds there).

Similarly, the oldest Fedora version that has a sufficiently recent Botan is Fedora 31; it has Botan 2.11.0 and Qt 5.13.2.

So assuming we are happy keeping the Botan 2.11 requirement, it would seem to make sense to:

Any thoughts from the maintainers?

droidmonkey commented 2 years ago

We support 18.04 through our ppa. It's for easier to provide the right version of Botan than the right version of qt5 through a ppa.

c4rlo commented 2 years ago

I see – you've got libbotan-kpxc-2-12 in that repo. Makes sense. Although, aren't the Snap packages sufficient there?

droidmonkey commented 2 years ago

Snaps are built... differently.

c4rlo commented 2 years ago

What I meant was that since Ubuntu 18.04 users can already get KeePassXC from the Snap store (and as an AppImage), perhaps it could make sense to eventually remove the PPA and hence support for Ubuntu 18.04. This should allow us increase the minimum required Qt version.

I don't feel very strongly about it though.

droidmonkey commented 2 years ago

I'm in favor of that, at least freeze 18.04 to 2.7.x if necessary

yonas commented 2 years ago

@hifi FYI: Qt6 was added to FreeBSD on Aug 22.

droidmonkey commented 2 years ago

@phoerious I think we should make the leap in 2.8.0

ya-isakov commented 1 year ago

@hifi Gentoo has 6.4.{0,2} in main repo, although still "masked" - https://packages.gentoo.org/packages/dev-qt/qtbase

jNullj commented 1 year ago

qt 5 LTS will EOL very soon about 3 months.

Polynomial-C commented 1 year ago

qt 5 LTS will EOL very soon about 3 months.

I'd not be too much concerned about this. AFAIK, KDE-people already maintain qt-5.15.x for quite a while and as long as there's no kde-6 available, they will continue to add (security) fixes to qt-5.15.x.

varjolintu commented 11 months ago

Some relevant links that helped my compile KeePassXC with Qt 6.6.0: https://doc.qt.io/qt-6/qtcore-changes-qt6.html https://doc.qt.io/qt-6/extras-changes-qt6.html https://doc.qt.io/qt-6/gui-changes-qt6.html#native-clipboard-integration https://doc.qt.io/qt-6/qtcore5-index.html

orsonteodoro commented 9 months ago

I want to make a pull request so I can move on to porting other qt6 packages.

I have three versions of the patch.

NSL (New Suffix List) is O(1) lookup, PSL [Public Suffix List from qtbase network] is O(log n) lookup with no difference in orders of magnitude. NSL is GPL-3 since it uses Wikipedia data but less TLD coverage.

The last one is ready to send.

The Test results can be found in in the two sections NSL: https://github.com/orsonteodoro/oiledmachine-overlay/blob/master/app-admin/keepassxc/keepassxc-9999.ebuild#L351 PSL: https://github.com/orsonteodoro/oiledmachine-overlay/blob/master/app-admin/keepassxc/keepassxc-9999.ebuild#L444

The qt6 support currently requires Qt 6.6.1. I can refine the patch so that the minimum is more accurate for each API change.

If your okay with the pull request, then give the okay. Other criticism or code review is welcomed.

droidmonkey commented 9 months ago

@varjolintu already did the port for us, just needs to be put in a pr. https://github.com/varjolintu/keepassxc/tree/qt6

orsonteodoro commented 9 months ago

Okay. I will move on. Next time close it or rename the title.

pg83 commented 9 months ago

@droidmonkey As I can see, @orsonteodoro patch have freedesktop's org.freedesktop.secrets support. And @varjolintu patch does not.

BTW, entirely not cool situation.

varjolintu commented 9 months ago

@droidmonkey As I can see, @orsonteodoro patch have freedesktop's org.freedesktop.secrets support. And @varjolintu patch does not.

BTW, entirely not cool situation.

As you can see, this PR is still just a draft. It's not nearly finished.

pg83 commented 9 months ago

As you can see, this PR is still just a draft. It's not nearly finished.

That is why @orsonteodoro patch is strictly better.

varjolintu commented 9 months ago

As you can see, this PR is still just a draft. It's not nearly finished.

That is why @orsonteodoro patch is strictly better.

I cannot see any Passkeys related changes or tests in that patch. But seems there are a couple of things that are missing from my PR.

droidmonkey commented 9 months ago

@droidmonkey As I can see, @orsonteodoro patch have freedesktop's org.freedesktop.secrets support. And @varjolintu patch does not.

BTW, entirely not cool situation.

"Not cool"? I don't understand this. I simply stated that we already put some work into this matter.

Throwing links to patches on GitHub is really not helpful. Just open a pull request. That kicks off continuous integration testing and also bounds discussion of code specific changes to the PR and not the issue.

In general, I recommend discussing code changes like PSL vs NSL before doing any work at all.

JohnLGalt commented 9 months ago

To be fair, he did ask if it was OK to do a pull request:

If your okay with the pull request, then give the okay. Other criticism or code review is welcomed.

I think he misunderstood that you were basically saying yes.

varjolintu commented 9 months ago

Now that we have a PR, please comment any recommendations, fixes and other suggestions there instead of this thread, thanks. If the previous patch has some important improvements, those are gladly welcome.