[CMake] FindQt4.cmake to automatically include QT_USE_FILE?

[Michael Hertling]

On 06/07/2011 06:58 PM, Michael Wild wrote:
> On 06/07/2011 06:23 PM, Bjørn Forsman wrote:
>> 2011/6/7 Michael Wild <themiwi at gmail.com>:
>>> On 06/07/2011 03:38 PM, Bjørn Forsman wrote:
>>>> Why not put find_package(Qt4) in the sub directories where it is actually used?
>>>
>>> And if it is used in multiple subdirectories?
>>
>> What's bad about that? (Sorry, I really don't know).
> 
> Then you would need to call find_package in all the subdirectories, with
> all the work that goes along with that.

Michael, with your approach, i.e. a FIND_PACKAGE(Qt4) once in the top-
level CMakeLists.txt and INCLUDE(${QT_USE_FILE}) where needed, you've
to set up the required QT_USE_* variables each time, and possibly, you
must even unset some of them in case they are inherited from previous
actions like that. Is this really less effort than an additional call
to FIND_PACKAGE()? IMO, the latter is neater and more expressive, and
if FindQt4.cmake would provide the variables recommended by Modules/
readme.txt, i.e. QT_{LIBRARIES,INCLUDE_DIRS,DEFINITIONS}, one could
completely drop the QT_USE_FILE and proceed as known from the
majority of packages.

>>> FindXXX.cmake modules should only do the minimum, i.e. finding the
>>> required libraries and include-directories, setting XXX_FOUND,
>>> XXX_INCLUDE_DIRS and XXX_LIBRARIES. Anything more complex, e.g. calling
>>> add_definitions(), include_directories(), function(), macro() etc.
>>> should go into a UseXXX.cmake module.
>>
>> Ok, I didn't know about that policy.
>>
>>> If you find a FindXXX.cmake module that calls add_definitions(),
>>> include_directories(), link_libraries() or some such, it is broken and
>>> needs to be fixed.
>>
>> Broken because of the above defined policy? Or of technical reasons?
>> Sorry, but I still don't see the reason why the current find_package +
>> use_file is better than a find_package that includes the use_file
>> itself.
> 
> If the FindXXX.cmake file called add_definitions(),
> include_directories() et al., that could be potentially harmful. Some
> sources might required that some define is not set, or a wrong header
> file might be #include'ed (thing of all the different X.h that exist).
> FindXXX.cmake modules should just define a few variables, *not* change
> the build system. That's the caller's responsibility, either by calling
> the functions himself or by including the UseXXX.cmake (if provided).

Absolutely, but note that FindXXX.cmake is possibly used in other find
modules, so it must provide *complete* information to set up the build
environment for XXX without a need to include UseXXX.cmake; otherwise,
the other package is forced to provide a use file, too, and this might
be inappropriate. The combination of {Find,Use}Qt4.cmake has such a
deficiency w.r.t. the QT_*_LIB preprocessor definitions.

>> What I see is that current usage goes something like this:
>>
>> 1) find_package(Qt4 REQUIRED) at the top level
>> 2) subdirs that actually need Qt: set what modules to enable/disable
>> and then include the use_file.
>>
>> How about doing it this way:
>> 1) subdirs that actually need Qt: call find_package(Qt4 COMPONENTS
>> QtCore [...] REQUIRED) with the modules it needs.
>>
>> Is there a problem with this approach? Other than breaking the policy ;-)
> 
> In principle not, except that all these redundant find_path() and
> find_library() calls will incur some slight overhead.

As Marcus has pointed out in the meantime, this should carry no weight
due to the cache. Personally, I also prefer to have multiple calls to
FIND_PACKAGE(Qt4), once in each CMakeLists.txt that's using Qt, and I
place them after any ADD_SUBDIRECTORY() command unless I actually want
to convey the results to the subdirectories. IMO, this allows for good
locality and self-contained subprojects and minimizes the chance of
overlinking, but probably, it's a matter of taste.

Regards,

Michael