KDE/kdelibs/cmake/modules

[Alexander Neundorf]

On Saturday 05 September 2009, Dario Freddi wrote:
> Gotta go out in a moment, I'll try to be quick
>
> In data sabato 05 settembre 2009 15:47:27, Alexander Neundorf ha scritto:
...
> > Hmm, now actually looking at it, I don't like the macro.
> > It does many things at once and that's not obvious from the name.
> >
> > E.g. I don't like that it does
> > target_link_libraries(... ${KDE4_KDECORE_LIBS})
> > No other macro we have adds libraries to an executable, this is always
> > done explicitely by the developer.
> > (only exceptions are kde4_add_kdeinit_executable() linking the
> >  helper-library and kde4_add_executable() linking QtMain under Windows).
>
> You are quite right, better off removing ittt
>
> > Also I don't like that it calls install.
> > You know, when I would see the code
> >
> > kde4_add_auth_helper_executable(foo ...)
> >
> > I would assume that this executable is not installed because there is no
> > install rule there.
> >
> > How about changing the macro to something like this:
> >
> > function(KDE4_INSTALL_AUTH_HELPER_FILES HELPER_ID HELPER_USER)
> >
> >   if (_kdeBootStrapping)
> >     set(_stubFilesDir  ${CMAKE_SOURCE_DIR}/kdecore/auth/backends/dbus/ )
> >   else (_kdeBootStrapping)
> >     set(_stubFilesDir  ${KDE4_DATA_INSTALL_DIR}/kauth/ )
> >   endif (_kdeBootStrapping)
> >
> >   configure_file(${_stubFilesDir}/dbus_policy.stub
> >                  ${CMAKE_CURRENT_BINARY_DIR}/${HELPER_ID}.conf)
> >   install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${HELPER_ID}.conf
> >           DESTINATION ${SYSCONF_INSTALL_DIR}/dbus-1/system.d/)
> >
> >   configure_file(${_stubFilesDir}/dbus_service.stub
> >                  ${CMAKE_CURRENT_BINARY_DIR}/${HELPER_ID}.service)
> >   install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${HELPER_ID}.service
> >           DESTINATION ${DBUS_SYSTEM_SERVICES_INSTALL_DIR})
> > endfunction(KDE4_INSTALL_AUTH_HELPER)
> >
> > As you see, I also replaced macro() by function() (new since cmake 2.6).
> >  This has the advantage that variables set inside a function are local to
> >  the function. And I think with a function the arguments are real
> >  variables, not basically text replacement as with macro (but I'm not
> > sure about this one, I didn't test it, please give it a try).
> >
> > Then the code in a CMakeLists.txt would look like:
> >
> > kde4_add_executable(foo ....)
> > target_link_libraries(foo ...)
> > install(TARGETS foo DESTINATION ${LIBEXEC_INSTALL_DIR} )
> >
> > kde4_install_auth_helper(...)
> >
> > That's more cmake code to to write, but it's more obvious what's going on
> > (i.e. less magic, more well-known commands).
>
> Uhm, is there any way we could rename the macro to make it more clear but
> keep the install and add_executable inside? The few developers that used
> this stuff yet provided a great feedback on how easy the helper creation
> is. But if you really think we would better be off this way, I will simply 
> change it 

Yes, please.
This would add another macro which creates an executable, but without 
providing real benefit. I mean, every KDE developer should know

kde4_add_executable()
target_link_libraries()
install(TARGETS)

So there's nothing new here.
Now to help with getting that stuff installed correctly we can provide a macro 
which helps with that. Which means there is 1 new thing to learn but the 
other code stays understandable for everybody else who don't know about 
KAuth.

Adding KDE4_ADD_AUTH_HELPER_EXECUTABLE() would also be 1 new thing to learn 
and it doesn't show that it will also install and link to kdecore. So the 
name would have to be something like 
KDE4_ADD_AUTH_HELPER_EXECUTABLE_LINK_TO_KDECORE_AND_INSTALL()
I guess we don't want that ;-)

> > Macro KDE4_AUTH_REGISTER_ACTIONS:
> >
> > Maybe an "install" in the name of the macro would be a good idea, since
> > it installs stuff.
>
> Uhm, you are right. KDE4_AUTH_INSTALL_ACTIONS? But it seems confusing to
> me. REGISTER_AND_INSTALL is making it too long IMHO. Any other suggestions?

I don't have a problem with long names.

Or just KDE4_INSTALL_AUTH_ACTIONS() ?
Do I understand correctly that the registering is part of a correct 
installation of this stuff ? Then one could argue that the register part is 
included in the install part. And the docs will say it anyway :-)
(having "install" in the name 1) makes it more obvious and 2) helps when 
grepping for "install" in the source tree).

Alex