CMake config files: use as min dep version the Qt version we built against
AbandonedPublic

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

Details

Reviewers
None
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.
While the current code of KCoreAddons 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.

Test Plan

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

Diff Detail

Repository
R244 KCoreAddons
Branch
setusetqtversionasmindep
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15785
Build 15803: arc lint + arc unit
kossebau created this revision.Aug 28 2019, 11:01 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 28 2019, 11:01 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
kossebau requested review of this revision.Aug 28 2019, 11:01 PM
kossebau added a comment.EditedAug 28 2019, 11:03 PM

If you think this makes sense, I would do the same change to all KDE Frameworks modules (and hope others pick up for their code with public library API).

See also D23551 as a variant where private Qt API is used, where then the proposal is to find EXACT the version the library was built against.

Target: KF 5.63 earliest.

ping :) No-one any opinion?

krop added a subscriber: krop.Nov 29 2019, 8:54 PM

ping :) No-one any opinion?

Sure :)
-1. The Config file is supposed to look for the lowest supported version.

In D23550#569552, @cgiboudeaux wrote:

ping :) No-one any opinion?

Sure :)
-1. The Config file is supposed to look for the lowest supported version.

So, which is meant to be the lowest supported version for a built kcoreaddons which was compiled against e.g. Qt 5.13 (and thus uses its additional API)?

krop added a comment.Nov 29 2019, 9:12 PM
In D23550#569552, @cgiboudeaux wrote:

ping :) No-one any opinion?

Sure :)
-1. The Config file is supposed to look for the lowest supported version.

So, which is meant to be the lowest supported version for a built kcoreaddons which was compiled against e.g. Qt 5.13 (and thus uses its additional API)?

The current code already tells what the minimum Qt version is. (5.11.0)
Even if you start building *all* the frameworks with a higher Qt version, that doesn't change the minimum version.

Just, the built binary no longer has that theoretical minimum dependency, it needs at least the Qt version the very instance of kcoreaddons was built against.

kossebau abandoned this revision.Dec 16 2019, 12:42 PM

Oh well, seems no-one else sees the light yet ;) Given this IMHO wrong data in the current CMake config files does not do actual harm in 99.9 % cases I will discard this for now, and just do it correctly (for what I see) only in my projects :)
Just have to wait for more people running in the non-99.9 % cases perhaps ;)