CMake config files: use as dep version the Qt one built against (private API)
AbandonedPublic

Authored by kossebau on Aug 28 2019, 11:17 PM.

Details

Reviewers
aacid
Group Reviewers
Frameworks
Build System
Summary

Some developers have multiple versions of Qt installed on their system.
And source code often has some version-dependent code paths, e.g. using
newer API if found available. In that case the resulting build has an
effective dependency on the Qt version built against.

To properly reflect that, the Qt version set as minimally required version
of the listed dependencies in the generated CMake config files should use
that version instead.
More, KXmlGui uses private API with Qt5::CorePrivate, so also future
versions of Qt5::Core might not be compatible.

While the current code of KXmlGui does not have such version dependent
code variants, they are added now and then, so using the Qt version built
against as minimally required dependency is a good default, and should not
change anything for all the users with just one Qt version, and improve
things for those with, catching bad mixes early in the build time.
And given the use of private Qt::Core API, requiring exactly the same
version by using "find_dependency(Qt5Core @USED_QT_VERSION@ EXACT)"
should be more safe, even if the private API might be stable in reality
for a set of versions.

Test Plan

Generated CMake config files now uses with the find_dependency(Qt5) calls
the version of the Qt KXmlGui was built against.

Diff Detail

Repository
R263 KXmlGui
Branch
setusetqtversionasmindep
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15786
Build 15804: arc lint + arc unit
kossebau created this revision.Aug 28 2019, 11:17 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 28 2019, 11:17 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
kossebau requested review of this revision.Aug 28 2019, 11:17 PM
apol added a subscriber: apol.Aug 28 2019, 11:28 PM

This will only work one-way. If the "wrong" Qt is older, but it's not necessarily the case.
Also cmake may be linking against the "right" Qt but then use the "wrong" one at runtime.

I'm not convinced.

In D23551#521571, @apol wrote:

This will only work one-way. If the "wrong" Qt is older, but it's not necessarily the case.
Also cmake may be linking against the "right" Qt but then use the "wrong" one at runtime.

Yes, this will not fix everything, just make things a bit more correct, and help at least in some situations. There is nothing lost, besides having to maintain one more variable.

Because right now, in some builds the generated CMake config is wrong about the min version when building against latest Qt (KIconThemes & KIO at least, due to some #if QT_VERSION >= QT_VERSION_CHECK(5, 12, 0) dependent API usage).

kossebau retitled this revision from CMake config files: use as min dep version the Qt version we built against to CMake config files: use as dep version the Qt one built against (private API).Sep 12 2019, 12:11 PM
kossebau abandoned this revision.Dec 16 2019, 12:45 PM

Discarding with same reason as given here: D23550#578745 One day someone else will do this in a similar way, I am sure ;)