On Thursday 06 November 2008 00:46:47 Alexander Neundorf wrote:
> Hi,
>
> FindPlasma.cmake as it is currently in kdelibs doesn't make sense.
> Each cmake module should (or must) be self-contained, i.e. you should be
> able to do simply
>
> find_package(Foo)
>
> and you should get the expected results.
> Now FindPlasma.cmake is right now only this:
>
> --------8<-------------8<--------------------8<-----------
> set(PLASMA_LIBS ${KDE4_PLASMA_LIBS} )
> set(PLASMA_INCLUDE_DIR ${KDE4_INCLUDE_DIR})
> set(PLASMA_FOUND true)
>
> find_file(PLASMA_OPENGL_FOUND plasma/glapplet.h
>           PATHS ${PLASMA_INCLUDE_DIR}
>           NO_DEFAULT_PATH)
>
> mark_as_advanced(PLASMA_INCLUDE_DIR PLASMA_LIBS)
> --------8<-------------8<--------------------8<-----------
>
>
> If I just do
> find_package(Plasma)
>
> I will PLASMA_FOUND TRUE as result, which would be clearly wrong (i.e.
> PLASMA_LIBS and PLASMA_INCLUDE_DIR would be empty).
>
> What would in some way work would be:
>
> find_package(KDE4)
> find_package(Plasma)
>
> Still PLASMA_FOUND would always be true then, also if KDE4 was *not* found.
>
> So, this needs to be fixed.
> How ?
> Does a separate FindPlasma.cmake still make sense ?
> It's probably good to keep it for compatibility.

yes

> The minimum would be to add something like this:
>
> if(NOT DEFINED KDE4_FOUND)
>    find_package(KDE4)
> endif(NOT DEFINED KDE4_FOUND)
>
> ...
> set(PLASMA_FOUND ${KDE4_FOUND})
> ...

By default FindPlasma will be installed when we install kde4. But it's right 
it's better to add it.

> This would make it self-contained and also report correct results.
>
> Somewhat harder would be to make it empty except a message like
> message(FATAL_ERROR "FindPlasma.cmake doesn't exist anymore, please use
> this and that instead")
>
> This would be cleaner but not be backward compatible. But OTOH, Plasma will
> just start staying compatible with the 4.2 release, right ?
> So anything relying on it since 4.1 may be broken anyway ?

I keep FindPlasma because some module as extragear can be compile yet with kde 
4.1 and kde 4.2 (I tested it previous week)
And I don't want that we create a hack in extragear module to change 
${PLASMA_LIBS} and ${PLASMA_INCLUDE_DIR} when we use kde4.1 or kde 4.2
it's better to keep compatibility with old cmake plasma variable.
Otherwise we must add a check in extragear module to disable plasma build when 
kdelibs < 4.2

Regards

> Alex



-- 
Laurent Montel,
KDE/KOffice developer, Qt consultancy projects
Klarälvdalens Datakonsult AB, Platform-independent software solutions
_______________________________________________
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Reply via email to