skipping the kdeinit magic on windows (Re: KDE/kdebase/apps/konqueror)

[Alexander Neundorf]

On Thursday 03 April 2008, David Faure wrote:
> On Thursday 03 April 2008, Ralf Habacker wrote:
> > David Faure schrieb:
> > > On Wednesday 02 April 2008, Ralf Habacker wrote:
> > >> SVN commit 793038 by habacker:
> > >>
> > >> kdeinit modules are not supported on win32
> > >>
> > >>  M  +10 -6     client/CMakeLists.txt
> > >>  M  +11 -5     src/CMakeLists.txt
> > >>
> > >>
> > >> --- trunk/KDE/kdebase/apps/konqueror/client/CMakeLists.txt
> > >> #793037:793038 @@ -10,13 +10,17 @@
> > >>
> > >>  kde4_add_app_icon(kfmclient_SRCS
> > >> "${KDE4_ICON_INSTALL_DIR}/oxygen/*/apps/system-file-manager.png")
> > >>
> > >> -kde4_add_kdeinit_executable( kfmclient NOGUI ${kfmclient_SRCS})
> > >> +if (WIN32)
> > >> +    add_definitions(-Dkdemain=main)
> > >> +    kde4_add_executable(kfmclient ${kfmclient_SRCS})
> > >> +    target_link_libraries(kfmclient  ${KDE4_KIO_LIBS} )
> > >> +else (WIN32)
> > >> +    kde4_add_kdeinit_executable( kfmclient NOGUI ${kfmclient_SRCS})
> > >> +    target_link_libraries(kdeinit_kfmclient  ${KDE4_KIO_LIBS} )
> > >> +    install(TARGETS kdeinit_kfmclient  DESTINATION
> > >>              ${INSTALL_TARGETS_DEFAULT_ARGS} ) 

The "DESTINATION" keyword here is wrong, everything is contained in 
INSTALL_TARGETS_DEFAULT_ARGS .

> > >> +    target_link_libraries(
> > >> kfmclient kdeinit_kfmclient )
> > >> +endif (WIN32)
> > >
> > > Ouch. But we define kdeinit modules everywhere in the source code.
> > > Maintaining CMakeLists.txt files like this one is going to be horrible.

Yes, we definitely don't want that.

> > > How about making kde4_add_kdeinit_executable do what's right for
> > > Windows, i.e. creating a static lib kdeinit_foo with -Dkdemain-main,
> > > and then the normal stuff from the CMakeLists.txt will work:
> > >
> > > target_link_libraries(kdeinit_kfmclient  ${KDE4_KIO_LIBS} )
> > > install(TARGETS kdeinit_kfmclient  DESTINATION

"DESTINATION" is wrong here, it's already included in the default args.

> > > ${INSTALL_TARGETS_DEFAULT_ARGS} )
> >
> > You are aware that you then install an obsolate static library ?
>
> Ah, indeed.

Is that a problem ? I also have a lot of static libraries on my system which I 
never ever use. If you create an installer for a non-development package you 
will skip the static libs anyway (I think). If not, maybe you could make your 
installation logic so intelligent that it ignores all kdeinit_*.lib files ?

> > > target_link_libraries( kfmclient kdeinit_kfmclient )
> > >
> > > (I think/hope that the dependencies of the static lib are used for the
> > > binary)

Yes, they are (if we didn't do anything to break it).

> > would it not be possible to have something like the following
> >
> > kde4_add_kdeinit_executable(kfmclient  ... )
> > kde4_target_link_library(kfmclient  ... )
> > kde4_install(TARGETS kfmclient ... )
> >
> > which would make it possible to hide all the internal platform depending
> > stuff into those cmake macros.
>
> I agree. But this is Alex's call really -> moving discussion to
> kde-buildsystem.

How should that macro work ?
Check each target if it's an executable, if so, check if there is a target 
kdeinit_<name_of_executable_target>, if there is one and we are on windows 
and it is a static library, don't add it to the install rule, otherwise do ?

No, I don't want to do that. Somebody has to maintain all that.

In case there are real problems due to the installed static library on 
Windows, how about 

install(TARGETS foo ${INSTALL_TARGETS_DEFAULT_ARGS} )
if(NOT WIN32)
   install(TARGETS kdeinit_foo ${INSTALL_TARGETS_DEFAULT_ARGS} )
endif(NOT WIN32)

This is not too ugly and it is very obvious what happens.

Alex