[CMake] Code and API review request for Qt5 CMake files

[Michael Hertling]

On 02/24/2012 03:34 PM, Stephen Kelly wrote:
> 
> Just forwarding to the cmake users list.
> 
> 
> 
> Stephen Kelly wrote:
> 
>>
>> Hi there,
>>
>> Qt5 generates its own CMake files, which you will be able to use to find
>> Qt5 and build with it.
>>
>> That is, you will port from, eg
>>
>> find_package(Qt4 REQUIRED Core Gui Xml)
>>
>> to
>>
>> find_package(Qt5Widgets REQUIRED)
>> find_package(Qt5Xml REQUIRED)
>>
>> find_package(Qt5Core) is also possible but is not needed because it is a
>> dependency of at least one of the other requirements already in this case.
>>
>> find_package(Qt5) will not work currently (though it can be made to work
>> now or after Qt 5.0).
>>
>> You will then port
>>
>> target_link_libraries(foo ${QT_QTCORE_LIBRARIES})
>>
>> to
>>
>> target_link_libraries(foo ${Qt5Core_LIBRARIES})
>>
>> etc.
>>
>> Or you might use qt5_use_package:
>> http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3083
>>
>> qt5_use_package(foo Core)
>> # That's it! Nothing more to do.
>>
>> The variables all map fairly well. There is also a Qt5Transitional package
>> which might help with that (If it gets released, which I'm not certain it
>> will be): https://projects.kde.org/projects/kdesupport/extra-cmake-
>>
> modules/repository/revisions/master/entry/modules/FindQt5Transitional.cmake
>>
>> The Qt5<module>Config.cmake files are generated and installed by Qt
>> itself.
>>
>> I'd like a review of them by people familiar enough with how CMake works
>> and an API review from people familiar with how it is used.
>>
>> The generation of them is platform specific, and configure options
>> specific, eg whether you use -framework on mac, whether you use MinGW or
>> MSVC, whether building with an infix or a namespace. The easiest way for
>> you to generate the config files is:
>>
>> # Note: Don't bother cloning qt5.git unless you already have it.
>> # That takes forever.
>> git clone git://gitorious.org/qt/qtbase.git
>> cd qtbase
>> ./configure
>> ls lib/cmake
>>
>> Make sure you have at least commit
>> c470999329ee576038c50573314699f972f48909.
>>
>> You can go on to build and test them if you wish. The ctest unit tests are
>> in qtbase/tests/manual/cmake. These tests are not part of any
>> multi-platform CI system.
>>
>> Compared to the last time I emailed about this, the generated Config files
>> have become more simple. I discovered that qmake can have conditionals in
>> its configure_file equivalent.
>>
>> Things that work:
>> * Finding Qt with an infix.
>>
>> * Building against Qt with a namespace.
>>
>> * Finding statically built Qt (though when linking you have to list the
>> dependencies yourself currently)
>>
>> * Finding a particular version should work as ConfigVersion files are
>> installed, but I have not tested it.
>>
>> Things to review:
>>
>> * Are the Config files created correctly for your platform and
>> configuration?
>>
>> * Do the unit tests pass on your platform?
>>
>> * Currently there is no Qt5Config.cmake.
>> Such a thing could probably exist and use the FIND_COMPONENTS to find what
>> was requested. [...]

Absolutely, I would greatly appreciate a well-designed and component-
aware Qt5Config.cmake. In general, there might be reasons why a multi-
component package's components that are to be used together should not
be requested in separate FIND_PACKAGE() invocations, see [1] and look
for package X with components A and B. However, I don't know if Qt5
will be the first example of that kind.

>> [...] However, because there is no way to signal from a Config
>> file that a component was not found [...]

No: See [2] and look for the XXX_YY_FOUND variables. They fit perfectly
to indicate if component YY of package XXX has been found or not, and
they can be set conveniently by FPHSA(XXX_YY ...), see [3].

>> [...] (that is, find_package(Qt5 REQUIRED
>> Gui Xml) might not find QtXml, but Qt5_FOUND would still be true if the
>> Qt5Config file is found, whether the component is or not), [...]

No: FIND_PACKAGE(Qt5 REQUIRED ...) is expected to bail out if any of
the required components aren't found, so the value of the Qt5_FOUND
variable doesn't matter in this case. BTW, note that FIND_PACKAGE()
must not bail out if a component which the user hasn't requested is
not found, regardless of the REQUIRED flag, unless the component is
an immediate or mediate prerequisite of a required one.

Regarding Qt5_FOUND, FIND_PACKAGE(Qt5 COMPONENTS ...), i.e. without
REQUIRED, is more interesting, see [4]. In short: Qt5_FOUND indicates
if Qt5 is basically present but says nothing about any component; the
user must refer to the component-specific FOUND variables, and those
must even be protected by the package-specific one:

FIND_PACKAGE(Qt5 COMPONENTS XYZ)
IF(Qt5_FOUND AND Qt5_XYZ_FOUND)
    ...
ENDIF()

Referring to Qt5_XYZ_FOUND alone is not reliable because this variable
wouldn't have received a definite value if Qt5Config.cmake hasn't been
found by FIND_PACKAGE(). I.e., the user would refer to this variable's
value before the FIND_PACKAGE() call; probably, that's not expected.

Note that you should also think about handling components requested
by the user but unknown to Qt5Config.cmake. Consider the following
use case: One day, a component - aka module - XYZ is added to Qt5,
and there's an application that can make use of it but needn't to:

FIND_PACKAGE(Qt5 COMPONENTS XYZ)
IF(Qt5_FOUND AND Qt5_XYZ_FOUND)
    ....
ENDIF()

What should happen if this application is configured against a Qt5
installation which does not know XYZ yet? IMO, the user can expect
that Qt5_XYZ_FOUND receives a definite value during the processing
of Qt5Config.cmake in any case. I.e., the Qt5Config.cmake of a Qt5
installation unaware of XYZ must nevertheless set Qt5_XYZ_FOUND to
FALSE if XYZ has been requested by the user.

For the latter, this means to refer only to components which have
been requested explicitly with FIND_PACKAGE(), and to not rely on
the assumption that any component is searched automatically, but
that's another story...

>> [...] I'm not sure
>> it's a good idea, [...]

Personally, I'm quite sure it is.

>> [...] or at least it's more complicated. [...]

Probably, but I think it's worth the effort, even for the reason alone
that a comprehensive Qt5Config.cmake could handle the possibly subtle
relations among Qt5 modules far better than distinct module-specific
configuration files ever can.

>> [...] At least, I think
>> something like qt5_use_package is a better idea anyway.

First of all, I definitely agree to your criticism of UseQt4.cmake in
[5], and I appreciate your attempt to get rid of such a use file for
Qt5. However, I'd choose a Qt5Config.cmake over qt5_use_package(),
and my current vision for the former's usage is roughly, e.g.,

FIND_PACKAGE(Qt5 REQUIRED Xml)
SET_TARGET_PROPERTIES(t1 PROPERTIES
    COMPILE_DEFINITIONS ${Qt5_DEFINITIONS}
    INCLUDE_DIRECTORIES ${Qt5_INCLUDE_DIRS})  # If available one day.
TARGET_LINK_LIBRARIES(t1 ${Qt5_LIBRARIES})

FIND_PACKAGE(Qt5 REQUIRED Sql)
SET_TARGET_PROPERTIES(t2 PROPERTIES
    COMPILE_DEFINITIONS ${Qt5_DEFINITIONS}
    INCLUDE_DIRECTORIES ${Qt5_INCLUDE_DIRS})  # If available one day.
TARGET_LINK_LIBRARIES(t2 ${Qt5_LIBRARIES})

with t2 not being overlinked due to accumulated results.

Some spontaneous questions/remarks on {cmake,qt5}_use_package():

- Do they need to be macros?
- What if "_package" is a multi-component one?
- What about COMPILE_DEFINITIONS_<CONFIG> properties?
- Target property INCLUDE_DIRECTORIES coming up in 2.8.8?
- Using SET_PROPERTY(... APPEND_STRING ...) on COMPILE_FLAGS?
- Do

  list(APPEND _flags ...)
  ...
  set_target_properties(... PROPERTIES COMPILE_FLAGS ${_flags})

  not provide for the semicolons that are undesired in COMPILE_FLAGS?

>> We could have a small FindQt5.cmake in CMake which could do that however
>> maybe.

Usually, a find module is more complicated than a config file because
the latter "knows" its package - along with information about static/
shared libraries, infixes, namespaces etc. ;-) - whereas the former
needs to figure out such things on its own during the search.

Regards,

Michael

[1] http://www.mail-archive.com/cmake@cmake.org/msg38482.html
[2] ${CMAKE_ROOT}/Modules/readme.txt
[3] http://www.mail-archive.com/cmake@cmake.org/msg32993.html
[4] http://www.mail-archive.com/cmake@cmake.org/msg28431.html
[5] http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3083

>> * Do you see any issues related to cross compiling?
>>
>> * Try my port of the CMake Gui dialog to Qt5, or review the patches (most
>> of the patches make sense in CMake master anyway IMO):
>> https://gitorious.org/~steveire/cmake/steveires-cmake/commits/qt5-port
>>
>> * API Review -
>> Do the variable names make sense? For example, to find a regular Qt5Gui,
>> you use find_package(Qt5Gui) and then you can use ${Qt5Gui_LIBRARIES}.
>>
>> To find a Qt which was built with a namespace you use find_package(Qt5Gui)
>> and then you can use ${Qt5Gui_LIBRARIES}. That is - it's exactly the same.
>>
>> To find a Qt that was built with an infix in the library names, you use
>> find_package(Qt5Gui) and then you can use ${Qt5Gui_LIBRARIES}. That is -
>> it's exactly the same.
>>
>> Is there any reason for it not to be the exact same in all these cases?
>>
>> I'd imagine if using an infix-ed Qt, that's an implementation detail. If
>> using a namespaced Qt, it might be an uninteresting implementation detail.
>> I'm not really certain of the use-cases for a namespaced Qt and how that
>> might affect this CMake API.
>>
>> * Is anything missing? One thing that is missing is the qt4_automoc macro
>> (all other macros are available with qt5 equivalents). Alex say's it's
>> broken. The CMAKE_AUTOMOC feature is a not-completely-source-compatible
>> replacement.
>>
>>
>> Thanks,
>>
>> --
>> Stephen Kelly <stephen.kelly at kdab.com> | Software Engineer
>> KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
>> www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
>> KDAB - Qt Experts - Platform-Independent Software Solutions