Make sure to search for Qt5-based qmlplugindump
ClosedPublic

Authored by asturmlechner on Dec 2 2017, 6:22 PM.

Details

Summary

Without any hint, qmlplugindump version is whatever default is set by qtchooser.

Test Plan

ecm_find_qmlmodule now works properly for e.g. kirigami.

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
asturmlechner created this revision.Dec 2 2017, 6:22 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptDec 2 2017, 6:22 PM
Restricted Application added subscribers: Build System, Frameworks. · View Herald Transcript
asturmlechner requested review of this revision.Dec 2 2017, 6:22 PM
krop added a subscriber: krop.EditedDec 2 2017, 7:14 PM

-1 this will not work for everyone.

dfaure added a subscriber: dfaure.Dec 3 2017, 9:49 AM

Well, it's just a hint. If it's not found there, no harm done. But yeah, at least the paths for most distros could be there, to be fair ;)

For OpenSuSE no change needed, it's in /usr/bin.

Which distro was this commit for? Looks like a debian/ubuntu path? If so, how did this go unnoticed until now?

krop added a comment.Dec 3 2017, 10:10 AM

Well, it's just a hint. If it's not found there, no harm done. But yeah, at least the paths for most distros could be there, to be fair ;)

For OpenSuSE no change needed, it's in /usr/bin.

it's a packaging issue if executables that don't have a .desktop file are not in PATH. An alternative could be to use qtpaths but it adds a dependency on qttools (and wouldn't help if it's installed in the same dir).

Which distro was this commit for? Looks like a debian/ubuntu path? If so, how did this go unnoticed until now?

This is for Gentoo - now I don't do Qt packaging, but as far as I'm aware this is pretty vanilla.

In D9116#174871, @cgiboudeaux wrote:

it's a packaging issue if executables that don't have a .desktop file are not in PATH. An alternative could be to use qtpaths but it adds a dependency on qttools (and wouldn't help if it's installed in the same dir).

The point of this change is that ECM should not rely on the default set by qtchooser/default.conf which can be either Qt4 or Qt5. qmlplugindump is not a binary that is exclusive to Qt5.

aacid added a subscriber: aacid.Dec 3 2017, 10:56 AM

is this a problem caused by qmlplugindump being one of those magic things that are "symlinks" to the qt4 or qt5 depending on the QT_SELECT env var?

maybe makes more sense to fix the execute_process so that it sets QT_SELECT to 5?

In D9116#174894, @aacid wrote:

is this a problem caused by qmlplugindump being one of those magic things that are "symlinks" to the qt4 or qt5 depending on the QT_SELECT env var?

exactly

Can we move forward with this?

krop added a comment.Jan 10 2018, 3:13 PM

Can we move forward with this?

ECM provides ECMQueryQmake.cmake. you can try eg:

include("${ECM_MODULE_DIR}/ECMQueryQmake.cmake")
then calling query_qmake(qt_binaries_dir QT_INSTALL_BINS)

and use ${qt_binaries_dir} as HINT

Use ECMQueryQmake.cmake as suggested.

krop accepted this revision.Jan 31 2018, 2:46 PM
This revision is now accepted and ready to land.Jan 31 2018, 2:46 PM
asturmlechner edited the summary of this revision. (Show Details)Jan 31 2018, 2:46 PM

Want me to fix FindQtWaylandScanner.cmake in the same way?

This revision was automatically updated to reflect the committed changes.
krop added a comment.Jan 31 2018, 2:58 PM

Want me to fix FindQtWaylandScanner.cmake in the same way?

Sure.