Review Request: Don't lose the original CMAKE_MODULE_PATH

[Patrick Spendrin]

Am 08.08.2012 22:52, schrieb 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.

Well, kdewin has no other use case than providing additional include
headers in constrast to KDE4 and KF5.
I will change the Config.cmake file again tomorrow, although I really
hate that change as it will increase the amount of code each project has
to include, thus increasing error likeliness etc.

> 
> Alex
> 

regards,
Patrick