On 06/07/2011 09:13 PM, Bjørn Forsman wrote: > 2011/6/7 Michael Wild <them...@gmail.com>: >> 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). > > Ok, I see the point now. However, if find_package is called only in > the subdirectories that actually need it the above problem is > non-existent. No? (At least add_definitions only work on the current > dir and below.) But if the policy is they way it is, then OK, I get > the point and I'll leave it at that.
As you've said yourself in this thread earlier, FIND_PACKAGE()'s point is to "set up variables containing necessary includes and libs" - and nothing should happen behind your back. For this reason, I think that FIND_PACKAGE() should be safe to invoke from *anywhere* in a project, with results solely depending on the COMPONENTS, well-known variables like CMAKE_PREFIX_PATH and well-documented ones like XXX_ROOT; taking into account the results of previous FIND_PACKAGE() invocations might have undesired effects, e.g. overlinking. > However, according to Modules/readme.txt, FindXXX.cmake files should > set XXX_LIBRARIES, XXX_INCLUDE_DIRS and XXX_DEFINITIONS. FindQt4.cmake > sets neither. I can try to come up with a patch to fix this, but I > won't complain if someone beats me to it ;-) My suggestions for an improvement of FindQt4.cmake are: 1) Introduce symbolic names, e.g. QT_QT*_DEFINITIONS, for the modules' preprocessor definitions like -DQT_*_LIB, so one has a chance to set up a complete compilation environment w.r.t. Qt modules without the need to include the QT_USE_FILE. This is important for find modules using FindQt4.cmake and should be feasible without harming backward compatibility. 2) Correct/complete the management of Qt's inter-module dependencies and check if it can be added to FindQt4.cmake. AFAICS, it means no harm if this is done in FindQt4.cmake, too, along with UseQt4.cmake. 3) Populate QT_{LIBRARIES,INCLUDE_DIRS,DEFINITIONS} with settings according only to the COMPONENTS and their prerequisites, cf (2). QT_INCLUDE_DIRS is new, i.e. no harm, and QT_LIBRARIES is reset by UseQt4.cmake, so the latter's users should not see any differences with regard to the current behavior. Admittedly, the enhancement of QT_DEFINITIONS might result in some doublings among the preprocessor definitions, but this should also mean no harm and could possibly be taken into account by UseQt4.cmake. Alternatively, one might use the variables QT4_{LIBRARIES,INCLUDE_DIRS,DEFINITIONS} for this purpose. ^ Regards, Michael _______________________________________________ Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake