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

[Michael Hertling]

On 03/01/2012 10:08 PM, Alexander Neundorf wrote:
> On Thursday 01 March 2012, Michael Hertling wrote:
>> On 02/28/2012 10:03 PM, Alexander Neundorf wrote:
>>> ...will reply later in detail.
>>>
>>> Could you please go through the existing find-modules shipped with cmake
>>> which support COMPONENTS and make a summary of how they handle them ?
>>>
>>> At least FindQt4.cmake be default searches all components.
>>>
>>> Thanks
>>> Alex
>>
>> The following CMakeLists.txt can be used to systematically investigate
>> the results of the component-aware find modules currently shipped with
>> CMake, these are Find{Boost,GTK2,HDF5,ImageMagick,Java,OpenSceneGraph,
>> Qt4,wxWidgets,XMLRPC}:
>>
>> CMAKE_MINIMUM_REQUIRED(VERSION 2.8 FATAL_ERROR)
>> PROJECT(FINDRESULTVARIABLES C CXX)
>> SET(CMAKE_VERBOSE_MAKEFILE ON)
>>
>> FUNCTION(FindResultVariables pkg)
>>     SET(prefix ${pkg})
>>     IF(DEFINED ARGV1)
>>         SET(prefix ${ARGV1})
>>     ENDIF()
>>     UNSET(REQUIRED)
>>     IF(${pkg}_REQUIRED)
>>         SET(REQUIRED REQUIRED)
>>     ENDIF()
>>     FIND_PACKAGE(${pkg} COMPONENTS ${${pkg}_COMPONENTS} ${REQUIRED})
>>     MESSAGE("Begin: ${pkg} result variables")
>>     GET_DIRECTORY_PROPERTY(v VARIABLES)
>>     FOREACH(i IN LISTS v)
>>         IF(i MATCHES "^${prefix}.*_FOUND$")
>>             MESSAGE("${i}=${${i}}")
>>         ENDIF()
>>     ENDFOREACH()
>>     MESSAGE("${prefix}_LIBRARIES: ${${prefix}_LIBRARIES}")
>>     MESSAGE("End: ${pkg} result variables")
>> ENDFUNCTION()
>>
>> FindResultVariables(Boost)
>> FindResultVariables(GTK2)
>> FindResultVariables(HDF5)
>> FindResultVariables(ImageMagick)
>> FindResultVariables(Java)
>> FindResultVariables(OpenSceneGraph)
>> FindResultVariables(Qt4 QT)
>> FindResultVariables(wxWidgets)
>> FindResultVariables(XMLRPC)
> 
> Thanks :-)
>  
>> HDF5, OpenSceneGraph and XMLRPC aren't installed on my system; my
>> preliminary findings for the remaining packages are as follows:
> 
> Maybe you can have a look at the code of those ?

Perhaps, someone who has these packages already installed can take a
look at them in the meantime and run the above-noted CMakeLists.txt.
Probably, this will provide for faster results.

>> (1) Searching unrequested components:
>>
>> Qt4: Yes.
>> GTK2,ImageMagick: Partially.
>> wxWidgets: All components if none are requested.
>>
>> The remaining modules don't search unrequested components.
>>
>> (2) Assigning *_*_FOUND variables:
>>
>> Qt4: Only for modules which are known and found.
>> ImageMagick: Also for requested but unknown components.
>> wxWidgets: No *_*_FOUND variables at all but forwards unknown
>>            components to *_LIBRARIES variable without an error.
>>
>> The remaining modules assign only to *_*_FOUND variables for
>> components which they know and which have been requested.
>>
>> (3) Respecting the REQUIRED flag:
>>
>> wxWidgets: REQUIRED ignored completely.
> 
> This is clearly a bug.
> 
>> Boost: SEND_ERROR instead of FATAL_ERROR.
> 
> This is somewhere between acceptable and bug.
> 
>> GTK2,Java: Bail out on unknown components even if not REQUIRED.
> 
> Depending on how unknown components should be handled, this is either ok or 
> not.
> I'm leaning towards supporting only known components, so the find-module or 
> config file knows what it is doing.
> In this case this would be correct behaviour, the programmer would see this 
> error, not the user.

Please recall the use case of Qt5 with config file and a module XYZ
added in a later release. A FIND_PACKAGE(Qt5 COMPONENTS XYZ) against
a later Qt5 installation works flawlessly, but against an early one,
it faces Qt5Config.cmake with an unknown component XYZ, and this is
perfectly legal. How should one use the XYZ module optionally - i.e.
if available - given that Qt5Config.cmake bails out on a missing XYZ
although REQUIRED isn't flagged? IMO, without REQUIRED, config files
/ find modules should handle requested but unknown components grace-
fully by just assigning FALSE to the respective *_*_FOUND variable.

What's your opinion about the above-mentioned use case?

>> The remaining modules bail out on unavailable requested components.
>>
>> (4) Assinging the package-related *_FOUND variable:
>>
>> Java: No *_FOUND variable at all although it's documented.
> 
> Bug.
> 
>> wxWidgets: TRUE even if requested components aren't found, see above.
>>
>> The remaining modules return FALSE if a requested component isn't found.
>>
>> My comments on these, say, multifarious findings are:
> 
> ;-)
> 
>> Ad.(1): In general, automatically searching unrequested components
>> does not mean a harm, but it is also not beneficial at all events:
>>
>> - No guarantee to catch all components - consider a component added
>>   later to the package - so no guarantee that all later *_*_FOUND
>>   variables have been assigned a definite value by FIND_PACKAGE().
>> - Complicates find modules / config files due to REQUIRED/QUIET.
>> - Potentially higher costs due to unneeded search operations.
>>
>> For the latter loint, there is a not-so-uncommon use case: Suppose
>> a find module wants to check whether a library matches its header.
>> Putting away cross-compiling issues for the moment, this requires
>> building and running a test program. If it is to be performed for
>> each Qt4 module, e.g., a FIND_PACKAGE(Qt4) invocation would most
>> certainly be quite expensive. For these reasons, I usually advice
>> to search only requested components, request all components going
>> to be used and refer only to components having been requested.
> 
> In general this is not done in cmake, and also in general not recommened. 
> Find-modules should simply use find_path(), find_library() etc.

Usually, that's right, but I wouldn't deny a priori that searching
a component might consist of more - particularly more expensive -
operations than an invocation of FIND_PATH() and FIND_LIBRARY().

Anyway, my actual point in this regard is: Searching unrequested
components is OK but doesn't yield an advantage, or do you know
any? AFAICS, the only thing it's good for is assigning to FOUND
variables for unrequested components, and that's not sufficient
if the latters are to be enabled.

>> Ad.(2): Due to my guiding principle - refer only to FOUND variables
>> which have been assigned a definite value by FIND_PACKAGE() - I do
>> consider as important that a *_*_FOUND variable is assigned to for
>> each requested component. FindImageMagick.cmake does it right, but
>> the other modules - except for FindwxWidgets.cmake - have me check
>> *_*_FOUND variables without a definite value for requested but un-
>> known components. Again: The latters might become known one day.
> 
> I'd prefer not supporting unknown components. The find-module doesn't know 
> anything about them (since they are unknown), whether they have additional 
> dependencies, or whatnot.

One of my common find module / config file patterns is roughly:

  (i) Tweak X_FIND_COMPONENTS, e.g. regarding inter-component
      dependencies, inter-component incompatibilities etc.

 (ii) Enable each known and requested component; remove
      each enabled component from X_FIND_COMPONENTS.

(iii) Finally, handle unknown components:

      FOREACH(Y IN LISTS X_FIND_COMPONENTS)
          IF(X_FIND_REQUIRED)
              MESSAGE(FATAL_ERROR "Unknown component: ${Y}")
          ELSEIF(NOT X_FIND_QUIETLY)
              MESSAGE("Unknown component: ${Y}")
          ENDIF()
          SET(X_${Y}_FOUND FALSE)
      ENDFOREACH()

IMO, (iii) is quite simple, could be perfectly implemented as a
supplementary function and ensures that each unknown component
will have a FOUND variable with a definite value - that's all.

>> Ad.(3): After FIND_PACKAGE(... REQUIRED), I definitely do not want
>> to check if all requested components including their prerequisites
>> are actually present. OTOH, the user must have the oppurtunity to
>> handle a package's or a component's absence by h(is|er)self. Thus,
>> FIND_PACKAGE() without REQUIRED should never bail out because of
>> something being not found, but with REQUIRED, it must bail out.
> 
> Yes.

Exactly, and this is why the behavior of Find{GTK2,Java}.cmake
w.r.t. unknown components is wrong, IMO; see above. ;-)

No REQUIRED flag --> no bailing out.

>> Ad.(4): None of the modules returning *_FOUND==FALSE just because
>> a component is missing could be turned into a config file at the
>> moment without changing its behavior. In order to address this
>> issue in the future, one has to decide if either
>>
>> - a find module should behave like a config file in this regard and
>>   return *_FOUND==FALSE only if the package is totally unavailable;
>>   my favorite, or
>>
>> - FIND_PACKAGE() should should allow config files to assign FALSE to
>>   *_FOUND if a requested component is unavailable; your favorite, I
>>   guess.
> 
> This is on my TODO for 2.8.8.

I'm afraid, it's the latter? <8-|

>> In the latter case, the components' *_*_FOUND variables would need
>> to be assigned to also, and if no config file has been found, this
>> must be done by FIND_PACKAGE() autonomously. Though, the *_*_FOUND
>> variables are just a convention of Modules/readme.txt whereas the
>> package's *_FOUND variable is documented for FIND_PACKAGE(); this
>> is a different quality. Finally, consider the following use case:
>>
>> Suppose a package X with a non-(de)selectable core component and an
>> additionally selectable component Y; Qt's core module is a perfect
>> candidate for the former. If FIND_PACKAGE(X COMPONENTS Y) returns
>> with X_FOUND==FALSE and X_Y_FOUND!=TRUE, how do you determine the
>> core component's availability, following your favorite approach?
>> AFAICS, you can't: The results include "X totally unavailable" as
>> well as "X available without Y". Following my favorite approach,
>> by contrast, X_FOUND directly indicates the availability of the
>> core component. That's an example for the neccessity to reliably
>> detect a package's total presence/absence, and a multi-component
>> package needs not to be a pure additive entity made of its parts.

What's your opinion about the above-mentioned use case?

Suppose the Qt folks decide that Qt5's core module doesn't need to
be explicitly requested because it is prerequisite for everything
else. Suppose further your application needs Qt5Core and wants to
use Qt5Sql, provided the latter is available. Most certainly, you
would issue

FIND_PACKAGE(Qt5 COMPONENTS Sql)

wouldn't you? Now, if FIND_PACKAGE() returns QT5_FOUND==FALSE and
QT5_SQL_FOUND!=TRUE, how do you determine if Qt5Core is available
and just Qt5Sql is missing, or if Qt5 isn't available at all? For
your application, that's crucial. IMO, one must be able to detect
if a package is absent as a whole, or if merely components are
missing, and your favorite approach does not allow for that.

>> My main conclusion from the above-noted mess among CMake's current
>> component-aware find modules is that we urgently need a convention
>> how such modules and config files are intended to work. Hopefully,
>> we can take a step forward; Qt5's advent is a good opportunity.
> 
> I agree.

Sorry if I'm bugging, but I'm quite interested in your opinion about
my question concerning possibly undefined *_*_FOUND variables, so I
ask it once more: After FIND_PACKAGE() has returned, are you really
willing to query variables meant to indicate the package's and its
components' presence but that might - or actually - have not been
assigned a definite value by the find module / config file?

In other words: Are you really willing to rely on the assumption
that the relevant FOUND variables were actually undefined or had
already a suitable value before you invoked FIND_PACKAGE()?

IMO, w.r.t. the FOUND variables, this is right not acceptable. It
is for any other variable, but not for the FOUND ones as they're
expected to provide definite information about the availability
of the package and its components.

Besides, are there no comments on these issues by other people?
It is hard to imagine that only two persons have an opinion
about this topic.

Regards,

Michael