kephal cmake files issue

[Alexander Neundorf]

On Friday 12 December 2008, Aike J Sommer wrote:
> Am Dienstag 09 Dezember 2008 22:43:06 schrieb Alexander Neundorf:
> > Hi,
> >
> > I just had a look at the CMakeLists.txt for kephal in kdebase/workspace/,
> > and they look a bit different.
> > I.e. they don't use the KDE specific macros.
> > Is there a reason for this, i.e. should it build also without KDE ?
> > If so, it's ok, we just have to make sure everything is set up correctly
> > (install dirs, RPATH, etc.).
> > If not, it should be converted to the "normal" KDE style using the KDE4
> > specific macros (since they handle e.g. RPATH setting and automoc
> > automatically).
> >
> > Alex
>
> Sorry for replying that late...
>
> libkephal actually does not link with any kde-libs, i didnt see any benefit
> in that, the kded-module obviously does, so there is no reason for its
> cmake- files to be different...
> kephald is a standalone version of the kded-module for development-purposes
> and is neither linked to kde-stuff nor built by default!!
>
> I dont really know my way around cmake tho, especially inside kde, so im
> not sure what macros and such you mean... Could you tell me what to look
> for?? Or if you want to, just correct what you think is wrong!!

libs/kephal/libkephal/CMakeLists.txt:

you are using add_library() instead of kde4_add_library().
This means you have to take care of moc yourself (you do) and you also have to 
take care of the RPATH yourself (I didn't see anything about this).
I suggest to set the following target properties:

set INSTALL_RPATH_USE_LINK_PATH to TRUE: so the RPATH of the installed library 
will automatically contain all directories of libraries which are outside the 
build dir
I think this should actually already be enough.
You may consider setting INSTALL_RPATH to LIB_INSTALL_DIR and the Qt library 
dir, but it seems this is not actually necessary.

In no other (AFAIK) place in KDE QT_USE_FILE is used. It would be nice if you 
could remove that and instead use the respective variables directly, like 
QT_QTDBUS_LIBRARIES etc.

You install FindKephal.cmake. Please move that file to 
workspace/cmake/modules/, so they are all in one place. And be aware that 
installing this file means 3rd party projects may use it and break if you 
change something in this file without keeping source compatibility.
Also that file doesn't have documentation at the top.
And please consider using the find_package_handle_standard_args() macro from 
FindPackageHandleStandardArgs.cmake, which takes care of the status output, 
the REQUIRED and QUIET keywords and the _FOUND variable.
Also add your name to it and please license it under BSD as the other cmake 
modules.

libs/kephal/kded_kephal/CMakeLists.txt:
Similar comments: please don't use QT_USE_FILE

Why do you use KDE4_ADD_PLUGIN() here ?
The KDE4_ADD_XXX() versions are basically the same as the original ones with 
the following additions:
-automoc handling
-support for enable final
-RPATH handling

KDE4_ADD_PLUGIN() additionally set the target property PREFIX to "", so the 
plugins don't start with "lib". For more details please have a look at 
KDE4Macros.cmake

If you take care of these things, they look good :-)
And, in general nowadays all-lowercase is preferred in KDEs CMakeLists.txt 
(but being consistent within a file has higher priority).

Alex