Support multiple major Qt versions in ECM
Open, Needs TriagePublic

Description

In very few places, ECM has the major Qt version hard-coded, and isn't properly versioned to be co-installable (unlike KF libraries). Can we support multiple major Qt versions in the same ECM code base (and thus avoid the co-installability problem)? Do we want that? If no, how do we want to distinguish ECM 5 and 6 then?

vkrause created this task.Mar 27 2021, 1:03 PM
ervin moved this task from Needs Input to In Discussion on the KF6 board.Mar 27 2021, 2:33 PM

Notes from the KF6 sprint (2021-03-27):

Possibilities: either create ECM6, or make sure ECM can work with both Qt versions.

In the latter case, the problem is: we need a settings very early whether to use Qt5 or Qt6

Even setting compatibility target (as it was done during Qt4->Qt5 transition, and which in fact what is done by https://invent.kde.org/frameworks/extra-cmake-modules/-/merge_requests/53/diffs) requires that

In fact it follow the pattern use already use (find_package then invoke some command)

Solution 1: So maybe force find Qt before include would solve this and solve the few places that depend on the Qt version,
or at least have the program set which Qt version should be used.
In the long term, Just make it an error if Qt is not set.
So basically: what we need is an ECM macro needed to be called with the Qt version required. During the transitional time, the compatibility will be kept until at some point the program requires the new ECM which sets the new behavior (i.e. which doesn't assume a specific Qt version)

Solution 2: make sure ECM doesn't call find_package(Qt<n>) at all and make that explicit in each program. It requires us to touch basically all programs but it's more of a mechanical change. We keep compatibility with older projects depending on the version of ECM required. This would mean that the find_package(Qt<n>) would be independent (in terms of order) of finding ECM and including its files. This will mean that depending on how you find_package(ECM VERSION xxx) you will get a different behaviour.

Decision : try Solution 2 first.

ervin moved this task from In Discussion to Backlog on the KF6 board.Mar 27 2021, 5:00 PM
dfaure added a subscriber: dfaure.Mar 27 2021, 10:30 PM

I'm confused. ECM does not call find_package(Qt<n>) at all right now. So no change there. For KF6 all we need is search/replace on the REQUIRED_QT_VERSION variable.

So why "This will mean that depending on how you find_package(ECM VERSION xxx) you will get a different behaviour." ?

Oh it does! In about 10 places outside tests. Examples are the designer plugin, qch, qm, and query qmake macros as well as parts of the Android support.

It would appear that I grepped wrongly, sorry about that.

One possibility would be to standardize the name REQUIRED_QT_VERSION and let apps/libs set that before including ECM modules.
Imposed ordering sucks a bit, but it's the same with the suggested solution ("you must find_package(Qt5/Qt6) before including ECMQueryQmake").
Between the two "imposed ordering" solutions, setting a var seems less drastic, many people like to include all the ECM stuff in one block, then do all the find_package in a separate block.
Just a thought.

That's "solution 1", we ended up preferring the find_package() removal as that seemed overall cleaner (cf. problems with implicit find_package calls for Python in the past), and both options require large scale changes anyway. Technically this could be done in a very similar fashion though, with enforcing that variable set for a sufficiently recent ECM version.

vkrause moved this task from Backlog to Needs Input on the KF6 board.Apr 3 2021, 1:08 PM
vkrause moved this task from Needs Input to Backlog on the KF6 board.Apr 3 2021, 1:26 PM
dfaure added a comment.Apr 3 2021, 2:11 PM

My thoughts during the meeting: extracting the qt-specific install dirs from KDEInstallDirs. Then apps would be able to choose between include(QtInstallDirs5) and include(QtInstallDirs6).

But actually, the same issue exists with include(KDEInstallDirs) itself. ECM should keep working for KF5 users, right?
So actually shouldn't we offer both include(KDEInstallDirs5) and include (KDEInstallDirs6)?
And then we don't actually need to split out the Qt stuff.....

IOW this is solution 3: offer both KF5/Qt5 and KF6/Qt6 modules in ECM, all we need to port the apps is append a number.

I can see how this creates more problems for the transitional period though. But the day we branch, that problem goes away....

Maybe we can have a KDEInstallDirs that forwards to KDEInstallDirs5 or KDEInstallDirs6 based on a cmake variable (command-line usage for the few people who want to try qt6 before we branch) ?

krop added a subscriber: krop.Apr 6 2021, 4:06 PM

IOW this is solution 3: offer both KF5/Qt5 and KF6/Qt6 modules in ECM, all we need to port the apps is append a number.

Shouldn't be needed if KDEInstallDirs becomes a wrapper:

if (ECM_VERSION_MAJOR VERSION_EQUAL 6 OR ECM_VERSION_MINOR VERSION_GREATER XX) # XX to be defined for 5.XX for the pre-alpha, beta, rc... releases)
  include(KDEInstallDirs6)
else()
  include(KDEInstallDirs5)
endif()
dfaure added a comment.Apr 6 2021, 4:56 PM

Ah, I see. I was thinking of ECM as a completely independent module, but you're right, as part of frameworks it will get a KF6 branch.

However the request here (AFAIU) is for a transitional period where code can be compiled with both Qt5 and Qt6. Maybe all we need is a cmake option (to switch between KF5/Qt5 and KF6/Qt6, for a bit). This doesn't require removing all find_package(Qt...) from ECM, it only means checking the option there.

However the request here (AFAIU) is for a transitional period where code can be compiled with both Qt5 and Qt6.

As ECM isn't co-installable in two major versions (unlike anything else in KF), I think this about more than just a transitional period. I'd expect we need to provide a working Qt 5 support in ECM for several years after Qt 6 branching, until Qt 5 is largely phased out for all ECM users.

dfaure added a comment.Apr 6 2021, 6:17 PM

Ah, co-installability, good point. Then we're back to: ECM can't know what the app wants, the app needs to ask for it, or (for the transitional period) the developer could ask for it, to test things. => KDEInstallDirs5, KDEInstallDirs6, and a KDEInstallDirs that forwards to KDEInstallDirs5 by default, and to KDEInstallDirs6 if a cmake option is set.

https://invent.kde.org/frameworks/extra-cmake-modules/-/merge_requests/108 prototypes the 5/6 split of KDEInstallDirs, but still lacks implementations for the interesting bits: how to decide in the version-less wrapper which one to include, and how to replace the ECMQueryQMake calls.

dfaure moved this task from Backlog to In Progress on the KF6 board.Jun 21 2021, 9:49 AM

I would say yes, mostly; what remains can be fixed as we go, I think (i.e. when building against Qt6 fails in a KF, we'll adjust the code in ECM to accommodate, e.g. https://invent.kde.org/frameworks/extra-cmake-modules/-/merge_requests/208).

We could put this in the Done column, or "in progress" (I think the former, as most of the heavy-lifting has been done already in ECM by Volker and David).

nicolasfella moved this task from In Progress to Done on the KF6 board.Apr 30 2022, 4:33 PM