Let Kross be useable w/o searching for private deps

Review Request #123031 - Created March 18, 2015 and updated

Information
Hrvoje Senjan
kross
Reviewers
buildsystem, kdeframeworks
alexmerry

Only search for public deps in cmake config.

Open question - shall we search all public deps, or minimal ones? e.g. target 1 link Qt5Core, target 2 links Qt5Gui. Do we search for both, or just Qt5Core?


  

Issues

  • 1
  • 0
  • 0
  • 1
Description From Last Updated
Are you sure ? ./ui/actioncollectionview.h:27:#include <QWidget> Christophe Giboudeaux Christophe Giboudeaux
Christophe Giboudeaux
Hrvoje Senjan
Hrvoje Senjan
Review request changed

Description:

~  

Only search for public deps in cmake config

  ~

Only search for public deps in cmake config.

  +
  +

Open question - shall we search all public deps, or minimal ones? e.g. target 1 link Qt5Core, target 2 links Qt5Gui. Do we search for both, or just Qt5Core?

Groups:

+buildsystem

People:

+alexmerry
Alex Merry

Sorry for the slow reply!

I think you should be finding QtWidgets, as linking to the GUI target will fail otherwise. From a practical point of view, Kross isn't going to be installed if QtWidgets isn't present, so it shouldn't be an issue. If it is (or becomes) possible to build Kross without the GUI part, then the find_dependency call for QtWidgets should be omitted only when Kross is built in that way.

Alexander Potashev

Please add "find_dependency(Qt5Core @REQUIRED_QT_VERSION@)" for completeness.

Build of KrossUi currently cannot be disabled. If we wanted to aim server environments without QtWidgets, then we first need to make KrossUi optional or move it into a separate framework. Until we are done with making KrossUi optional, there is almost no point in dropping find_dependency(Qt5Widgets [...]).

  1. Please add "find_dependency(Qt5Core @REQUIRED_QT_VERSION@)" for completeness.

    I disagree ;-)
    Qt5Core is nowhere to be found in link libraries (offcourse it's used, but if we want to be literal...).
    I can add it though if we agree to also add the Qt5Core target explicitly in link_libraries.

  2. Yeah, my view is that you should be searching for anything that is in the list of interface link libraries for exported targets, but not for anything else. If you drag it in implicitly via target_link_libraries, you can assume it's brought in implicitly via find_dependency.

  3. Agreed. Is this patch ready to push then?

Alex Merry
Ship It!
Albert Astals Cid

This patch does not apply anymore.

Loading...