Differential D15310

Search for the Qt5Platform/ThemeSupport in the default includes first
AbandonedPublic
Actions

Authored by sdepiets on Sep 6 2018, 8:04 AM.

Details

Reviewers
lbeltrame
krop
Group Reviewers
Plasma
Summary

Currently, the FindQt5PlatformSupport and FindQt5ThemeSupport cmake modules use pkg-config to search for the includes files without searching if those files would be available in the default path first (for instance if building against a custom Qt installation and using CMAKE_PREFIX_PATH, and not the version installed on the system).

This becomes a blocking issue with Qt5.11.1 as the signature of the constructor of some classes changed in QtThemeSupport vs Qt 5.9. The library found would be 5.11.1 (CMAKE_PREFIX_PATH) but the header files would be 5.9 (installed on the system and found by pkg-config).

This change searches for the header files in the default path first, then defaults to the current behavior of using pkg-config.

Diff Detail

Repository
R135 Integration for Qt applications in Plasma
Branch
cmake_qt5_default (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2554
Build 2572: arc lint + arc unit
sdepiets created this revision.Sep 6 2018, 8:04 AM
Restricted Application added a project: Plasma. · View Herald TranscriptSep 6 2018, 8:04 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sdepiets requested review of this revision.Sep 6 2018, 8:04 AM
sdepiets edited the summary of this revision. (Show Details)Sep 6 2018, 8:11 AM
sdepiets added reviewers: lbeltrame, Plasma.
sdepiets edited the summary of this revision. (Show Details)
sdepiets edited the summary of this revision. (Show Details)Sep 6 2018, 8:16 AM
davidedmundson added a subscriber: davidedmundson.Sep 6 2018, 8:24 AM

I have this in my dev setup

PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig:/opt/kde5/lib64/pkgconfig

If you had the equivalent, would you need this?

krop added inline comments.Sep 6 2018, 8:24 AM
cmake/modules/FindQt5PlatformSupport.cmake
75

This CMake module doesn't know Qt5Core_VERSION, this only works for you because there's a find_package(Qt5 ...) call before looking for Qt5PlatformSupport

79

Same here

cmake/modules/FindQt5ThemeSupport.cmake
76

Same here

80

Same here

krop added a comment.Sep 6 2018, 8:25 AM
In D15310#321210, @davidedmundson wrote:

I have this in my dev setup

PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig:/opt/kde5/lib64/pkgconfig

If you had the equivalent, would you need this?

Agreed, that's an issue with your environment not being set correctly.

sdepiets added a comment.Sep 6 2018, 8:57 AM
In D15310#321216, @cgiboudeaux wrote:
In D15310#321210, @davidedmundson wrote:

I have this in my dev setup

PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig:/opt/kde5/lib64/pkgconfig

If you had the equivalent, would you need this?

Agreed, that's an issue with your environment not being set correctly.

It does work with PKG_CONFIG_PATH pointing to the custom Qt directory.
However, if the PREFIX is set and works for the rest of the modules, it should be able to work without this additional configuration.

krop added a comment.Sep 6 2018, 9:02 AM
In D15310#321232, @sdepiets wrote:
In D15310#321216, @cgiboudeaux wrote:
In D15310#321210, @davidedmundson wrote:

I have this in my dev setup

PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig:/opt/kde5/lib64/pkgconfig

If you had the equivalent, would you need this?

Agreed, that's an issue with your environment not being set correctly.

It does work with PKG_CONFIG_PATH pointing to the custom Qt directory.
However, if the PREFIX is set and works for the rest of the modules, it should be able to work without this additional configuration.

you set QTDIR, CMAKE_PREFIX_PATH and a couple other environment variables to use custom builds, that's exactly the same reasoning.
If you don't want to set them, use system locations.

sdepiets updated this revision to Diff 41092.Sep 6 2018, 9:03 AM

Checking that Qt5Core version is initialized

krop added a comment.Sep 6 2018, 9:04 AM
In D15310#321234, @sdepiets wrote:

Checking that Qt5Core version is initialized

Sorry, but the whole change makes no sense, a big -1.

davidedmundson added a comment.Sep 6 2018, 9:08 AM

From the docs:

By default, if CMAKE_MINIMUM_REQUIRED_VERSION is 3.1 or later, or if PKG_CONFIG_USE_CMAKE_PREFIX_PATH is set, the CMAKE_PREFIX_PATH, CMAKE_FRAMEWORK_PATH, and CMAKE_APPBUNDLE_PATH cache and environment variables will be added to pkg-config search path. The

I'd be ok with bumping our cmake version / setting PKG_CONFIG_USE_CMAKE_PREFIX_PATH

sdepiets abandoned this revision.Sep 6 2018, 10:57 AM
In D15310#321238, @davidedmundson wrote:

From the docs:

By default, if CMAKE_MINIMUM_REQUIRED_VERSION is 3.1 or later, or if PKG_CONFIG_USE_CMAKE_PREFIX_PATH is set, the CMAKE_PREFIX_PATH, CMAKE_FRAMEWORK_PATH, and CMAKE_APPBUNDLE_PATH cache and environment variables will be added to pkg-config search path. The

I'd be ok with bumping our cmake version / setting PKG_CONFIG_USE_CMAKE_PREFIX_PATH

It looks much nicer indeed.

Apologies for the commit.

Revision Contents
Changeset List

Diff 41092

View Options

cmake/modules/FindQt5PlatformSupport.cmake

Loading...
View Options

cmake/modules/FindQt5ThemeSupport.cmake

Loading...