Wrong logic when looking for Qt

[krop]

This was mentioned in #830 but deserves its own report

https://github.com/KDAB/GammaRay/blob/3.0/CMakeLists.txt#L424-L439 has multiple issues.

1 - It can't find Qt 6 2 - the set package properties line does nothing

To verify both, we'll use a trimmed test:

cmake_minimum_required(VERSION 3.16)
project(test CXX)

include(FeatureSummary)
find_package(QT NAMES Qt6 Qt5 COMPONENTS Core CONFIG REQUIRED)
message(STATUS "Control: ${QT_VERSION_MAJOR} / ${Qt_VERSION_MAJOR} / ${QT_VERSION} / ${Qt_VERSION}")

set_package_properties(Qt${QT_VERSION_MAJOR}Core PROPERTIES TYPE REQUIRED PURPOSE "Testing what feature_summary would print")
feature_summary(WHAT ALL INCLUDE_QUIET_PACKAGES FATAL_ON_MISSING_REQUIRED_PACKAGES)

(the status message will print Control: 5 / / 5.15.11 / )

1- This can't find Qt6. Unlike find_library, find_file and so on, the CMake config modules specified with NAME are sorted. It will always look for Qt5ConfigVersion.cmake first.

This can be verified with cmake --trace:

/data/kde/src/TESTS/find_package_with_NAME/CMakeLists.txt(5):  find_package(QT NAMES Qt6 Qt5 COMPONENTS Core CONFIG REQUIRED )
/usr/lib64/cmake/Qt5/Qt5ConfigVersion.cmake(2):  set(PACKAGE_VERSION 5.15.11 )

Qt6/Qt6Config* is never tested. There's no way around that. NO_MODULE is also unneeded, this signature can't work with anything but cmake config files.

2- the feature_summary line will only print:

-- The following REQUIRED packages have been found:

 * QT

Invalid entries are ignored. Since Qt${QT_VERSION_MAJOR}Core is never evaluated, feature_summary() won't complain. Adding something with a totally bogus name would behave the same way.

[ferdnyc]

@krop

I don't think this is correct. On my Fedora 38 system, the logic supplied always finds Qt6, regardless of the order the NAMES are listed in (Qt5 Qt6 or Qt6 Qt5).

There's a variable the Qt CMake configurations for both Qt5Core and Qt6Core will look for, QT_DEFAULT_MAJOR_VERSION. They'll both set it (to their own major version) if it's not set, so I'm still not 100% clear on what determines which config will be picked up if it's unset. It may be CMake's (complex) discovery rules that ultimately decide, but at least on my system they don't seem to prevent Qt6 being chosen over Qt5.

[ferdnyc]

I'm speaking of discovery in isolation, here. In the GammaRay CMakeLists.txt, there's also the cmake/ECM/modules files from Extra CMake Modules in play — primarily QtVersionOption.cmake and ECMQueryQt.cmake — those will default to Qt5, if its discovery is triggered without first setting an explicit version.

I had to jump through some hoops, in the Fedora packaging definition, to avoid accidentally triggering ECM auto-versioning of Qt too early. Since, IMHO, the GammaRay CMakeLists.txt leaves the Qt version decision until way too late in the process, leaving ample time for ECM's version selection to be triggered.

Specifically, ECM will choose a Qt version (always defaulting to Qt5) as soon as¹ the include(ECMGeneratePriFile) command is executed, which comes way before the find_package(QT ...) command that's meant to perform auto-discovery.

Notes

1. Edit: Actually, turns out that's not quite correct. ECMGeneratePriFile will trigger Qt autodetection (with Qt5 default) on load, if the variable KDE_INSTALL_USE_QT_SYS_PATHS is either unset or set to TRUE. The GammaRay config explicitly sets it to FALSE to try and mitigate that, but it doesn't help if you actually want that variable set TRUE. IMHO, it would be far simpler to just do the find_package(QT ...) stuff early in the config, before anything like include(ECMGeneratePriFile) is run.

[ferdnyc]

The other problem / unfortunate disparity is that ECM supports Qt version pre-selection by setting the variable QT_MAJOR_VERSION — not the QT_VERSION_MAJOR that the GammaRay configs support/set.

(QT_VERSION_MAJOR is meant to be set by the Qt configs themselves when they're loaded. So I actually kind of understand why KDE chose a different input variable for selection using QtVersionOption.cmake.)

[dantti]

If you don't want it to look for Qt5 just set what you want cmake -DQT_VERSION_MAJOR else provide the Qt you want using the standard method of CMAKE_PREFIX_PATH, we just check for QT_VERSION_MAJOR being defined so we don't need yet another variable like GAMMARAY_QT6 to have it explicit on the configure.

[krop]

That's not what this report is about, the code is broken. find_package(QT NAMES ...) doesn't do what you think.

For some reason, on fedora, the sorting is done differently than on openSUSE, but the issue is the same.

[ferdnyc]

That's not what this report is about, the code is broken. find_package(QT NAMES ...) doesn't do what you think.

Well, it does correctly discover either Qt5 or Qt6, if one of them is installed on the system. It's just that when they're both installed, the behavior is ambiguous.

@krop is correct, though, that setting QT_VERSION_MAJOR to 5 or 6 before running find_package(QT 5.5 NAMES Qt6 Qt5 COMPONENTS Core NO_MODULE) has absolutely no effect on which Qt version will be found. QT_VERSION_MAJOR is an output variable for the configuration.

The ECM scripts that were included into the dist could allow Qt selection, by setting QT_MAJOR_VERSION (note the difference in variable name), if they were explicitly called before running find_package(QT ...).

[dantti]

has absolutely no effect on which Qt version will be found.

It does, setting QT_VERSION_MAJOR will skip the find_package(NAMES), and will instead try to find QtX -DQT_VERSION_MAJOR=6, if NOT defined then it will find the first Qt that it can, which you can force to the one of your liking using what QtCreator does that is setting CMAKE_PREFIX_PATH.

ECM handling is another story tho...

[dantti]

find_package(QT .. NAMES) is what produces QT_VERSION_MAJOR, so if one set it before running cmake it is defined and will do what find_package(QT NAMES) would do that is setting this variable.

[ferdnyc]

@dantti

It does, setting QT_VERSION_MAJOR will skip the find_package(NAMES),

OK, technically in that sense, yes I concede it does have an effect.

find_package(QT .. NAMES) is what produces QT_VERSION_MAJOR

That's true, but check this out, on Fedora with both installed:

My CMakeLists.txt:

cmake_minimum_required(VERSION 3.16)

project(test VERSION 0.1 LANGUAGES CXX)

set(QT_VERSION_MAJOR 5)
find_package(QT${QT_VERSION_MAJOR} 5.5 NAMES Qt5 Qt6 COMPONENTS Core NO_MODULE REQUIRED)

message(STATUS "QT_VERSION_MAJOR: ${QT_VERSION_MAJOR}")
message(STATUS "QT6_VERSION_MAJOR: ${QT6_VERSION_MAJOR}")
message(STATUS "QT5_VERSION_MAJOR: ${QT5_VERSION_MAJOR}")

Using it...

$ cmake -B _test -S .
-- The CXX compiler identification is GNU 13.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib64/ccache/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- QT_VERSION_MAJOR: 5
-- QT6_VERSION_MAJOR: 
-- QT5_VERSION_MAJOR: 6
-- Configuring done (0.5s)
-- Generating done (0.0s)
-- Build files have been written to: /tmp/_test

QT5_VERSION_MAJOR = 6!??!

It seems find_package(<foo> NAMES Qt5 Qt6 ...) will blindly just set <foo>_VERSION_MAJOR, and it really won't be told which of the NAMES to prefer. (The outcome is the same if I explicitly set QT5_VERSION_MAJOR to 5, as well.)

But, hey, that's OK, I can't even get ECM's Qt tools to properly force a Qt version anymore. It used to work, but I think Kitware have tightened up CMake's scoping recently. Because now, I can load the ECM modules, watch them default to Qt5 (with --trace --trace-expand), and still both QT_MAJOR_VERSION and QT_VERSION_MAJOR remain unset in my calling scope, which will instead continue to default to Qt6 if at all permitted to.

[krop]

There's more than the random behavior for default builds: Considering both the Qt5 and 6 plugins must be built to load any Qt app, gammaray should look for both. and the choice should only be about which Qt version to use for the UI.

Currently, it has to be built twice to have /usr/lib64/gammaray/3.0/qt5_xx-x86_64/* and /usr/lib64/gammaray/3.0/qt6_xx-x86_64/*

[dantti]

Please understand that you are doing it wrong, you don't call find_package(NAMES) if you have your QT_VERSION_MAJOR set, it will simply ignore that, and GammaRay doesn't do that, instead we only find_package( without names) to get the required modules.

[dantti]

QtCreator used to create projects like: find_package(QT NAMES Qt5 Qt6) find_package(Qt{$QT_VERSION_MAJOR) here you provide CMAKE_PREFIX_PATH to the Qt you want, but it might still find the wrong Qt if your provided CMAKE_PREFIX_PATH is bogus

This is why I added the check to only try to find whatever version it can if QT_VERSION_MAJOR is not defined, this allows you to still pick a Kit in QtCreator and it will use the Qt version from the KIT.