Review Request: Don't lose the original CMAKE_MODULE_PATH

[Alexander Neundorf]

On Tuesday 07 August 2012, Patrick Spendrin wrote:
> Am 07.08.2012 22:42, schrieb Alexander Neundorf:
...
> > Setting CMAKE_REQUIRED_INCLUDES and CMAKE_REQUIRED_LIBRARIES within the
> > Config.cmake file is not good style.
> > 
> > Finding a package should not change the behaviour of cmake, just provide
> > information about the package which has been found. So I'd prefer to
> > remove lines 26 and 27.
> 
> Since kdewin is a special library (a compatibility layer) I would like
> to keep that in place. Otherwise I would have to set this in each and
> every dependent package. The code has been copied over from the old
> FindKDEWin.cmake, and I think that the behaviour should not be changed
> here.

Strictly speaking, from the CMake POV, this is wrong.

Doing 
find_package(KDEWin)
should not change any current build settings, but only provide information 
about the installed project.
A user has to do anyway

find_package(KDEWin)

so it shouldn't be a big problem to do the following:

find_package(KDEWin)
set(CMAKE_REQUIRED_INCLUDES ${KDEWIN_INCLUDES} )
set(CMAKE_REQUIRED_LIBRARIES ${KDEWIN_LIBRARIES} )

This would be the clean way.


If there is a chance to break compatibility, this is now with the switch to 
KF5/e-c-m.

I'm strongly in favor of this.

I know that 
find_package(KDE4)
does not follow this rule.
This was not a good decision, but we had to stay with it to keep source 
compatibility. And it is more convenient for our KDE developers, but it is not 
how cmake stuff should behave.

With KF5 this changes somewhat. If you do
find_package(KF5)
no settings will be modified, you have to request that explicitely:
find_package(KF5 COMPONENT CMake Compiler)
By requesting the components "CMake" and "Compiler" the cmake- and compiler-
settings suggested by KDE will be applied. So it is at least a somewhat 
explicit request by the user.

Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20120808/e6b74ce1/attachment.html>