Switch to Qt 6 and C++17 #6516

davidebeatrici · 2024-07-20T23:10:18Z

Requires thorough testing, especially the regex changes.

Krzmbrzl

Haven't looked through all changes yet.

How did you manage the C++17 dependency though?

Krzmbrzl · 2024-07-21T05:43:22Z

src/Group.cpp

@@ -195,7 +195,7 @@ bool Group::appliesToUser(const Channel &currentChannel, const Channel &aclChann
 			channel = channel->cParent;
 		}

-		int requiredChannelIndex = currentChannelHierarchy.indexOf(contextChannel);
+		auto requiredChannelIndex = currentChannelHierarchy.indexOf(contextChannel);


Invalid use of auto

Krzmbrzl · 2024-07-21T05:43:43Z

src/Group.cpp

@@ -211,10 +211,10 @@ bool Group::appliesToUser(const Channel &currentChannel, const Channel &aclChann
 			return RET_FALSE;
 		}

-		const int minDepth = requiredChannelIndex + minDescendantLevel;
-		const int maxDepth = requiredChannelIndex + maxDescendantLevel;
+		const auto minDepth = requiredChannelIndex + minDescendantLevel;


Invalid use of auto

Krzmbrzl · 2024-07-21T05:43:57Z

src/Group.cpp

-		const int minDepth = requiredChannelIndex + minDescendantLevel;
-		const int maxDepth = requiredChannelIndex + maxDescendantLevel;
+		const auto minDepth = requiredChannelIndex + minDescendantLevel;
+		const auto maxDepth = requiredChannelIndex + maxDescendantLevel;


Invalid use of auto

Krzmbrzl · 2024-07-21T05:44:05Z

src/Group.cpp


-		const int totalDepth = homeChannelHierarchy.count() - 1;
+		const auto totalDepth = homeChannelHierarchy.count() - 1;


Invalid use of auto

Krzmbrzl · 2024-07-21T05:44:22Z

src/ProcessResolver.cpp

@@ -154,15 +154,15 @@ void ProcessResolver::doResolve() {
 				QByteArray cmdline = f.readAll();
 				f.close();

-				int nul = cmdline.indexOf('\0');
+				const auto nul = cmdline.indexOf('\0');


Invalid use of auto

Krzmbrzl · 2024-07-21T05:49:18Z

src/mumble/GlobalShortcut.cpp

@@ -580,7 +583,11 @@ QString ShortcutDelegate::displayText(const QVariant &item, const QLocale &loc)
 			}
 		}
 		default:
+#if QT_VERSION >= 0x060000


Keep only Qt6

Krzmbrzl · 2024-07-21T05:49:30Z

src/mumble/GlobalShortcut.cpp

@@ -849,7 +856,11 @@ QTreeWidgetItem *GlobalShortcutConfig::itemForShortcut(const Shortcut &sc) const
 	::GlobalShortcut *gs  = GlobalShortcutEngine::engine->qmShortcuts.value(sc.iIndex);

 	item->setData(0, Qt::DisplayRole, static_cast< unsigned int >(sc.iIndex));
+#if QT_VERSION >= 0x060000


Keep only Qt6

Krzmbrzl · 2024-07-21T05:49:38Z

src/mumble/GlobalShortcut.cpp

@@ -953,7 +964,7 @@ void GlobalShortcutEngine::remap() {
 			sk->gs          = gs;

 			foreach (const QVariant &button, sc.qlButtons) {
-				int idx = qlButtonList.indexOf(button);
+				auto idx = qlButtonList.indexOf(button);


Invalid use of auto

Krzmbrzl · 2024-07-21T05:49:46Z

src/mumble/GlobalShortcut.cpp

@@ -1026,7 +1037,7 @@ bool GlobalShortcutEngine::handleButton(const QVariant &button, bool down) {
 		}
 	}

-	int idx = qlButtonList.indexOf(button);
+	const auto idx = qlButtonList.indexOf(button);


Invalid use of auto

Krzmbrzl · 2024-07-21T05:50:14Z

src/mumble/GlobalShortcut_win.cpp

@@ -193,25 +193,33 @@ void GlobalShortcutWin::registerMetaTypes() {
 		registered = true;

 		qRegisterMetaType< InputHid >();
+#if QT_VERSION < 0x060000


Not needed if we fully switch

davidebeatrici · 2024-07-21T13:23:29Z

I would like to keep support for Qt 5 at least until the next stable release, but don't want to write a preprocessor block for every qsizetype (int64_t) vs int instance.

I followed Qt's recommendation:

Note: If you want to build against both Qt 5 and Qt 6, the auto keyword is a good solution to cover signature differences between the versions.

Krzmbrzl · 2024-07-21T15:01:05Z

Why do you want to keep Qt5 support? It is EOL already and Qt6 has been around for so long as to be available on all platforms that we want to support (afaik) 🤔

davidebeatrici · 2024-07-21T15:55:58Z

Right:

https://www.qt.io/blog/qt-5.15-extended-support-for-subscription-license-holders

In that case we have to prepare a Qt 6 vcpkg environment.

Krzmbrzl · 2024-07-21T16:02:26Z

In that case we have to prepare a Qt 6 vcpkg environment.

Yep

Coming back to the C++17 requirement though: how did you manage it so that nothing breaks? 🤔

davidebeatrici · 2024-07-21T16:44:16Z

Well, there is one thing "broken": we use the <codecvt> header to convert between UTF-8 and UTF-16 and it was deprecated in C++17, resulting in a warning.

Unfortunately there is no standard replacement, we'll have to rely on something like https://github.com/nemtrif/utfcpp.

As for ZeroC Ice: the headers that make use of std::binary_function and std::unary_function are patched in our fork (which we use in our vcpkg port):

mumble-voip/ice@bc7eb3e
mumble-voip/ice@e1e88d8

Upstream seems to have fixed the issue in v3.7.7: zeroc-ice/ice@bc2762e

In conclusion: before merging this PR we should build with Qt 6 to make sure that there are no issues on any platforms.

davidebeatrici · 2024-07-22T00:41:30Z

#6517

davidebeatrici · 2024-07-23T02:01:11Z

E: Unable to locate package qt6-svg-dev

The name was libqt6svg6-dev before...

davidebeatrici · 2024-07-23T02:02:43Z

src/mumble/PluginUpdater.h

-	UpdateEntry() = default;
-


I added this change by mistake, it's supposed to fix a separate issue:

src/mumble/PluginUpdater.cpp:49:18: error: no matching constructor for initialization of 'UpdateEntry' 49 | UpdateEntry entry = { plugin->getID(), updateURL, updateURL.fileName(), 0 }; | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/mumble/PluginUpdater.h:30:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 4 were provided 30 | struct UpdateEntry { | ^~~~~~~~~~~ src/mumble/PluginUpdater.h:30:8: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 4 were provided 30 | struct UpdateEntry { | ^~~~~~~~~~~ src/mumble/PluginUpdater.h:31:2: note: candidate constructor not viable: requires 0 arguments, but 4 were provided 31 | UpdateEntry() = default; | ^

OpenMandrivaAssociation/mumble@df75805

davidebeatrici · 2024-08-19T02:49:13Z

E: Unable to locate package qt6-svg-dev
The name was libqt6svg6-dev before...

Despite libqt6svg6-dev being installed just fine, CMake is unable to find Qt's "Svg" component.

The package seemingly contains the required files, which leads me to think something is broken.

Unfortunately we cannot switch to Ubuntu 24.04 in Azure Pipelines because not publicly available yet: actions/runner-images#9848 (comment)

davidebeatrici · 2024-08-19T22:28:25Z

I ended up adding 24.04's repository and installing the Qt packages from there.

Krzmbrzl · 2024-08-20T18:27:24Z

We can't exclusively use 24.04 though. At least we have to also have a CI for 22.04

davidebeatrici · 2024-08-20T22:28:23Z

The base system is still 22.04, but I can see why we should test the Qt packages that are normally provided with it.

Unfortunately we have no choice other than reporting the bug upstream and hoping for a quick fix.

Krzmbrzl · 2024-08-21T05:06:55Z

Yeah, we should report it. However, until this is fixed I think we should just wait with merging this PR. At least for a couple of weeks to see what the maintainers will do with the reported issue. 👀

davidebeatrici · 2024-08-21T22:50:27Z

I created a VM with a minimal Ubuntu 22.04 installation and quickly found out what the issue was: missing libgl-dev.

Our build log actually gave us a hint, but I missed it as I focused on the specific component (Svg). Turns out the find_pkg() error is inaccurate: NONE of the GUI-related components were found; or rather, they were ignored due to the missing dependency.

-- Could NOT find OpenGL (missing: OPENGL_INCLUDE_DIR) 
CMake Error at cmake/pkg-utils.cmake:87 (message):
  Qt6 component not found: Svg
Call Stack (most recent call first):
  src/mumble/CMakeLists.txt:76 (find_pkg)

A minimal CMake project with the standard find_package() call reveals a more accurate log:

-- Could NOT find OpenGL (missing: OPENGL_INCLUDE_DIR) 
-- Could NOT find WrapOpenGL (missing: WrapOpenGL_FOUND) 
CMake Warning at /usr/lib/x86_64-linux-gnu/cmake/Qt6/Qt6Config.cmake:219 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/Qt6Gui/Qt6GuiConfig.cmake

  but it set Qt6Gui_FOUND to FALSE so package "Qt6Gui" is considered to be
  NOT FOUND.  Reason given by package:

  Qt6Gui could not be found because dependency WrapOpenGL could not be found.

Call Stack (most recent call first):
  CMakeLists.txt:5 (find_package)


-- Could NOT find OpenGL (missing: OPENGL_INCLUDE_DIR) 
-- Could NOT find WrapOpenGL (missing: WrapOpenGL_FOUND) 
CMake Warning at /usr/share/cmake-3.22/Modules/CMakeFindDependencyMacro.cmake:47 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/Qt6Gui/Qt6GuiConfig.cmake

  but it set Qt6Gui_FOUND to FALSE so package "Qt6Gui" is considered to be
  NOT FOUND.  Reason given by package:

  Qt6Gui could not be found because dependency WrapOpenGL could not be found.

Call Stack (most recent call first):
  /usr/lib/x86_64-linux-gnu/cmake/Qt6/QtPublicDependencyHelpers.cmake:14 (find_dependency)
  /usr/lib/x86_64-linux-gnu/cmake/Qt6Svg/Qt6SvgDependencies.cmake:96 (_qt_internal_find_dependencies)
  /usr/lib/x86_64-linux-gnu/cmake/Qt6Svg/Qt6SvgConfig.cmake:50 (include)
  /usr/lib/x86_64-linux-gnu/cmake/Qt6/Qt6Config.cmake:219 (find_package)
  CMakeLists.txt:5 (find_package)


CMake Warning at /usr/lib/x86_64-linux-gnu/cmake/Qt6/Qt6Config.cmake:219 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/Qt6Svg/Qt6SvgConfig.cmake

  but it set Qt6Svg_FOUND to FALSE so package "Qt6Svg" is considered to be
  NOT FOUND.  Reason given by package:

  Qt6Svg could not be found because dependency Qt6Gui could not be found.

Call Stack (most recent call first):
  CMakeLists.txt:5 (find_package)


-- Could NOT find OpenGL (missing: OPENGL_INCLUDE_DIR) 
-- Could NOT find WrapOpenGL (missing: WrapOpenGL_FOUND) 
CMake Warning at /usr/share/cmake-3.22/Modules/CMakeFindDependencyMacro.cmake:47 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/Qt6Gui/Qt6GuiConfig.cmake

  but it set Qt6Gui_FOUND to FALSE so package "Qt6Gui" is considered to be
  NOT FOUND.  Reason given by package:

  Qt6Gui could not be found because dependency WrapOpenGL could not be found.

Call Stack (most recent call first):
  /usr/lib/x86_64-linux-gnu/cmake/Qt6/QtPublicDependencyHelpers.cmake:14 (find_dependency)
  /usr/lib/x86_64-linux-gnu/cmake/Qt6Widgets/Qt6WidgetsDependencies.cmake:96 (_qt_internal_find_dependencies)
  /usr/lib/x86_64-linux-gnu/cmake/Qt6Widgets/Qt6WidgetsConfig.cmake:50 (include)
  /usr/lib/x86_64-linux-gnu/cmake/Qt6/Qt6Config.cmake:219 (find_package)
  CMakeLists.txt:5 (find_package)


CMake Warning at /usr/lib/x86_64-linux-gnu/cmake/Qt6/Qt6Config.cmake:219 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/Qt6Widgets/Qt6WidgetsConfig.cmake

  but it set Qt6Widgets_FOUND to FALSE so package "Qt6Widgets" is considered
  to be NOT FOUND.  Reason given by package:

  Qt6Widgets could not be found because dependency Qt6Gui could not be found.

Call Stack (most recent call first):
  CMakeLists.txt:5 (find_package)


CMake Error at CMakeLists.txt:5 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/Qt6/Qt6Config.cmake

  but it set Qt6_FOUND to FALSE so package "Qt6" is considered to be NOT
  FOUND.  Reason given by package:

  Failed to find Qt component "Gui".

  Expected Config file at
  "/usr/lib/x86_64-linux-gnu/cmake/Qt6Gui/Qt6GuiConfig.cmake" exists

  Failed to find Qt component "Svg".

  Expected Config file at
  "/usr/lib/x86_64-linux-gnu/cmake/Qt6Svg/Qt6SvgConfig.cmake" exists

  Failed to find Qt component "Widgets".

  Expected Config file at
  "/usr/lib/x86_64-linux-gnu/cmake/Qt6Widgets/Qt6WidgetsConfig.cmake" exists

Please note that libgl-dev is pulled in automatically with the packages from Ubuntu 24.04, meaning that it's effectively a bug that should be reported.

davidebeatrici · 2024-09-17T00:30:50Z

Formatted with clang-format 10, in order for CI to succeed.

… for sizes https://doc.qt.io/qt-6/qtcore-changes-qt6.html#api-changes

davidebeatrici requested review from Hartmnt and Krzmbrzl July 20, 2024 23:10

This comment was marked as resolved.

Sign in to view

davidebeatrici force-pushed the qt6-support branch from 1edb151 to ef6f556 Compare July 21, 2024 02:52

Krzmbrzl requested changes Jul 21, 2024

View reviewed changes

davidebeatrici commented Jul 23, 2024

View reviewed changes

davidebeatrici mentioned this pull request Aug 1, 2024

WIP: FEAT(client): Add support to XDG Desktop Portal GlobalShortcuts #5976

Draft

1 task

davidebeatrici mentioned this pull request Aug 10, 2024

Remove deprecated QStandardPaths from config fallback chain #6459

Closed

davidebeatrici linked an issue Aug 11, 2024 that may be closed by this pull request

Remove deprecated QStandardPaths from config fallback chain #6459

Closed

davidebeatrici force-pushed the qt6-support branch 2 times, most recently from 09ed65d to 00bf616 Compare August 19, 2024 02:41

davidebeatrici changed the title ~~FEAT: Add Qt 6 support~~ Switch to Qt 6 and C++17 Aug 19, 2024

davidebeatrici force-pushed the qt6-support branch from 00bf616 to bfcebdb Compare August 19, 2024 22:27

davidebeatrici mentioned this pull request Aug 20, 2024

CI(github-actions): Update Ubuntu runners #6537

Closed

davidebeatrici force-pushed the qt6-support branch from bfcebdb to cc11c79 Compare August 22, 2024 00:28

Krzmbrzl added client server labels Aug 22, 2024

Krzmbrzl added the feature-request This issue or PR deals with a new feature label Aug 22, 2024

davidebeatrici closed this Aug 22, 2024

davidebeatrici deleted the qt6-support branch August 22, 2024 14:28

davidebeatrici restored the qt6-support branch August 22, 2024 14:28

davidebeatrici reopened this Aug 22, 2024

Hartmnt mentioned this pull request Sep 16, 2024

BUILD: Disable MSVC 17.8 warning for stdext::checked_array_iterator #6554

Merged

davidebeatrici force-pushed the qt6-support branch from cc11c79 to d51aa2e Compare September 17, 2024 00:29

davidebeatrici force-pushed the qt6-support branch 2 times, most recently from a1084c4 to f5f46d2 Compare September 17, 2024 03:37

davidebeatrici added 3 commits September 19, 2024 04:43

FEAT: Add Qt 6 support

c043779

FIX: Warnings due to Qt containers now using qsizetype instead of int…

ef97a5d

… for sizes https://doc.qt.io/qt-6/qtcore-changes-qt6.html#api-changes

BUILD: Switch to Qt 6 and C++17

de91daa

davidebeatrici force-pushed the qt6-support branch from f5f46d2 to de91daa Compare September 19, 2024 02:46

davidebeatrici merged commit 7101060 into mumble-voip:master Sep 19, 2024
13 checks passed

davidebeatrici deleted the qt6-support branch September 19, 2024 03:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to Qt 6 and C++17 #6516

Switch to Qt 6 and C++17 #6516

davidebeatrici commented Jul 20, 2024

This comment was marked as resolved.

Krzmbrzl left a comment

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

Krzmbrzl Jul 21, 2024

davidebeatrici commented Jul 21, 2024

Krzmbrzl commented Jul 21, 2024

davidebeatrici commented Jul 21, 2024

Krzmbrzl commented Jul 21, 2024

davidebeatrici commented Jul 21, 2024

davidebeatrici commented Jul 22, 2024

davidebeatrici commented Jul 23, 2024

davidebeatrici Jul 23, 2024 •

edited

Loading

davidebeatrici commented Aug 19, 2024

davidebeatrici commented Aug 19, 2024

Krzmbrzl commented Aug 20, 2024

davidebeatrici commented Aug 20, 2024

Krzmbrzl commented Aug 21, 2024

davidebeatrici commented Aug 21, 2024

davidebeatrici commented Sep 17, 2024


		const int totalDepth = homeChannelHierarchy.count() - 1;
		const auto totalDepth = homeChannelHierarchy.count() - 1;

Switch to Qt 6 and C++17 #6516

Switch to Qt 6 and C++17 #6516

Conversation

davidebeatrici commented Jul 20, 2024

This comment was marked as resolved.

Krzmbrzl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidebeatrici commented Jul 21, 2024

Krzmbrzl commented Jul 21, 2024

davidebeatrici commented Jul 21, 2024

Krzmbrzl commented Jul 21, 2024

davidebeatrici commented Jul 21, 2024

davidebeatrici commented Jul 22, 2024

davidebeatrici commented Jul 23, 2024

davidebeatrici Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

davidebeatrici commented Aug 19, 2024

davidebeatrici commented Aug 19, 2024

Krzmbrzl commented Aug 20, 2024

davidebeatrici commented Aug 20, 2024

Krzmbrzl commented Aug 21, 2024

davidebeatrici commented Aug 21, 2024

davidebeatrici commented Sep 17, 2024

davidebeatrici Jul 23, 2024 •

edited

Loading