--===============1172101378242124873== Content-Type: multipart/alternative; boundary="===============6696440415174873154==" --===============6696440415174873154== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Aug. 27, 2014, 6:05 p.m., Alex Merry wrote: > > modules/ECMQueryQmake.cmake, line 1 > > > > > > I think we need to be a bit more clever here. I'd go for a cache variable, and if (TARGET Qt5::qmake), set the default to that, otherwise set the default to just "qmake". You might also consider doing find_package(Qt5Core QUIET) if this is only going to be included when KDE_ECM_INSTALL_TO_QT_SYS_DIR is set. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119901/#review65382 ----------------------------------------------------------- On Aug. 27, 2014, 1:11 p.m., Rohan Garg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119901/ > ----------------------------------------------------------- > > (Updated Aug. 27, 2014, 1:11 p.m.) > > > Review request for Build System and KDE Frameworks. > > > Repository: extra-cmake-modules > > > Description > ------- > > Use qmake to query dirs for plugins and imports instead of hardcoding them in ECM. > > > Diffs > ----- > > modules/ECMQueryQmake.cmake PRE-CREATION > kde-modules/KDEInstallDirs.cmake 880539b > modules/ECMGeneratePriFile.cmake 34001d6 > > Diff: https://git.reviewboard.kde.org/r/119901/diff/ > > > Testing > ------- > > Seems to work on my system. > > > Thanks, > > Rohan Garg > > --===============6696440415174873154== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119901/

On August 27th, 2014, 6:05 p.m. UTC, Alex Merry wrote:

modules/ECMQueryQmake.cmake (Diff revision 6)
1
get_target_property(QMAKE_EXECUTABLE Qt5::qmake LOCATION)

I think we need to be a bit more clever here. I'd go for a cache variable, and if (TARGET Qt5::qmake), set the default to that, otherwise set the default to just "qmake".

You might also consider doing
find_package(Qt5Core QUIET)
if this is only going to be included when KDE_ECM_INSTALL_TO_QT_SYS_DIR is set.


- Alex


On August 27th, 2014, 1:11 p.m. UTC, Rohan Garg wrote:

Review request for Build System and KDE Frameworks.
By Rohan Garg.

Updated Aug. 27, 2014, 1:11 p.m.

Repository: extra-cmake-modules

Description

Use qmake to query dirs for plugins and imports instead of hardcoding them in ECM.

Testing

Seems to work on my system.

Diffs

  • modules/ECMQueryQmake.cmake (PRE-CREATION)
  • kde-modules/KDEInstallDirs.cmake (880539b)
  • modules/ECMGeneratePriFile.cmake (34001d6)

View Diff

--===============6696440415174873154==-- --===============1172101378242124873== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Kde-buildsystem mailing list Kde-buildsystem@kde.org https://mail.kde.org/mailman/listinfo/kde-buildsystem --===============1172101378242124873==--