Skip to content
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

WIP: Qt6 transition #10267

Closed
wants to merge 2 commits into from
Closed

WIP: Qt6 transition #10267

wants to merge 2 commits into from

Conversation

varjolintu
Copy link
Member

@varjolintu varjolintu commented Feb 4, 2024

Transition to Qt6. A work in progress. Continues the work from: #7783

Fixes #7774.

TODO:

  • Test Auto-Type with macOS
  • All test pass with Windows
  • All test pass with Linux
  • All tests pass with macOS
  • MacPasteBoard
  • Fix FdoSecrets
  • QXmlWriter::setCodec() no longer exists. UTF-16 support is needed in SSHAgent. Affects to TextStream also.

Type of change

  • ✅ Refactor (significant modification to existing code)

@varjolintu varjolintu added this to the v2.8.0 milestone Feb 4, 2024
@varjolintu varjolintu marked this pull request as draft February 4, 2024 08:09
@varjolintu
Copy link
Member Author

varjolintu commented Feb 4, 2024

There is still lot to fix. One main headache is related to how QByteArray's are handled in Qt6:
https://doc.qt.io/qt-6/qstring.html#QString-9. This breaks Base32 among other classes.

Note: : any null ('\0') bytes in the byte array will be included in this string, converted to Unicode null characters (U+0000). This behavior is different from Qt 5.x.

The easiest way to deal this is to replace QString(byteArray) calls with QString(byteArray.toStdString().c_str()) but it's not very convenient. Although it's mostly used in tests, we should make sure there are no places where this is actually used with some relevant data.

@varjolintu varjolintu added pr: refactoring Pull request that refactors code Qt labels Feb 4, 2024
@droidmonkey
Copy link
Member

droidmonkey commented Feb 4, 2024

If we need to convert a non-string byte array (as in actual raw bytes) to qstring for comparison or printing, then we should convert it to hex or base64 first. Ideally we would just compare with the desired bytestring though.

@varjolintu
Copy link
Member Author

varjolintu commented Feb 4, 2024

If we need to convert a non-string byte array (as in actual raw bytes) to qstring for comparison or printing, then we should convert it to hex or base64 first. Ideally we would just compare with the desired bytestring though.

QString::fromUtf8() is the most problematic one here. It's used in multiple relevant places, like in KeePassReader classes. Even after changing those, the tests fail. There's also something strange about the Endian functions. Currently I'm out of ideas.

@shmerl
Copy link

shmerl commented Aug 16, 2024

Is this still gradually moving or got stuck?

@droidmonkey
Copy link
Member

Want to merge other major refactoring first

@oniGino
Copy link

oniGino commented Oct 22, 2024

any work that can be carved off here into other tickets?, I'm willing to submit PRs, failing tests, etc.

@droidmonkey
Copy link
Member

droidmonkey commented Oct 22, 2024

I think its the CMake cleanup first, then this one. Wrapping up 2.7.10 development first which should carry us through next year for 2.8.0. Gotta do all this prep to work on the big new features like entry layouts and FIDO auth.

@fkobi
Copy link

fkobi commented Jan 1, 2025

pinging as this is important

@droidmonkey
Copy link
Member

Yes agree, was looking at this earlier this week. We do have an EOS date for qt 5.15, and we are pretty strict about that stuff.

@varjolintu can you rebase this onto #11003?

@varjolintu
Copy link
Member Author

Yes agree, was looking at this earlier this week. We do have an EOS date for qt 5.15, and we are pretty strict about that stuff.

@varjolintu can you rebase this onto #11003?

Maybe that should be rebased as well first?

@droidmonkey
Copy link
Member

droidmonkey commented Jan 2, 2025

Good point

@varjolintu rebase done

@varjolintu
Copy link
Member Author

FYI: Still working on this. Started with KPXC_MINIMAL build. macOS side already works and all tests pass. When everything seems fine with Linux and Windows as well, I'll move to a full build.

@varjolintu
Copy link
Member Author

Work continues in #11651.

@varjolintu varjolintu closed this Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: refactoring Pull request that refactors code Qt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Qt 6 upgrade
6 participants