Skip to content

Updates for Qt6#1058

Closed
EricAtORS wants to merge 6 commits intocommontk:masterfrom
EricAtORS:qt6-with-qregexp
Closed

Updates for Qt6#1058
EricAtORS wants to merge 6 commits intocommontk:masterfrom
EricAtORS:qt6-with-qregexp

Conversation

@EricAtORS
Copy link

This is different from #1023 in 2 ways:

  1. You can compile with Qt6 from this changelist.
  2. QRegExpis taken from theQt5CoreCompat` rather than being aliased to QRegularExpression, minimizing the changes.

Just some additional notes:
I've only tested it for CTKCore and CTKWidgets.
I'm also using Conan to manage the compilation and so not all Qt macros are there.
This is something I'm looking to bring up with the conan maintainers.

I am migrating to Qt6, and so as a first pass, I want to see that the CI will build for older versions as well.

Copy link
Contributor

@jamesobutler jamesobutler Dec 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you've switch from Qt5::Test to Qt::Test. This versionless target support would only be possible with Qt 5.15 and Qt 6+.

Re https://doc.qt.io/qt-6/cmake-qt5-and-qt6-compatibility.html#versionless-targets

set(QT_NO_CREATE_VERSIONLESS_FUNCTIONS ON)  # Versionless functions requires Qt 5.15 and newer
set(QT_NO_CREATE_VERSIONLESS_TARGETS ON)  # Versionless targets requires Qt 5.15 and newer

Re https://doc.qt.io/qt-6/cmake-qt5-and-qt6-compatibility.html#supporting-older-qt-5-versions
Following

find_package(QT NAMES Qt6 Qt5 REQUIRED COMPONENTS Core)
find_package(Qt${CTK_QT_VERSION} REQUIRED COMPONENTS Core)

Could you instead utilize Qt${CTK_QT_VERSION}::Test in this case? And then apply the same logic for other targets?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know that it was only for qt 5.15+ for the versionless targets. I'll fix them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you've made some changes with the versioned target, though I still see this specific place in the code and some other places with only a versionless target being used like Qt::

@EricAtORS EricAtORS force-pushed the qt6-with-qregexp branch 2 times, most recently from f5974cc to 2b5bc19 Compare December 8, 2022 15:58
@EricAtORS
Copy link
Author

I don't understand why it fails with Qt5. For now, I'm going to close the PR.

@EricAtORS EricAtORS closed this Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments