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

[Alexander Neundorf]

On Sunday 06 April 2008, Ralf Habacker wrote:
> Ralf Habacker schrieb:
> > David Faure schrieb:
> >> 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} )
> >>>>> +    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.
> >>>>
> >>>> 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:

Or just build everything directly into the executable and keep the library 
just to keep it compatible.

...
> After some private discussion with Alex and Christian I have prepared a
> patch which adds the following platform independent cmake macros to
> KDE4Macros.cmake
>
> kde4_kdeinit_add_executable(target options sources)
> kde4_kdeinit_link_libraries(target options libraries)
> kde4_kdeinit_install(target options)
>
>
> This api makes it possible to skip the kdeinit magic on windows without
> any further patching of package CMakeLists.txt.  (See appended testcase
> for konquerors kfmclient)
>
> The appended patch contains the win32 and non win32 implementations of
> the above mentioned macros taken from the macro
> kde_add_kdeinit_executable which will be obsolate now. I've tried hard
> to make sure it will work on unix abe before by inspecting the macro
> kde_add_kdeinit_executable.
>
> Any problems with this patch ?

I don't like that it introduces two new macros.
The problem with the very first patch was that thetwo if()-branches contained 
significantly different code, which would be very hard to remember.
I would prefer the following which still has an if(), but it's a simple one:

kde4_add_kdeinit_executable( kfmclient NOGUI ${kfmclient_SRCS})
target_link_libraries(kdeinit_kfmclient  ${KDE4_KIO_LIBS} )

install(TARGETS kfmclient ${INSTALL_TARGETS_DEFAULT_ARGS} )
if(NOT WIN32)
   install(TARGETS kdeinit_kfmclient ${INSTALL_TARGETS_DEFAULT_ARGS} )
endif(NOT WIN32)


This is only one if() and it's an if() with easy logic (exclude one lib from 
installing), and if some linux developer forgets to put the install() into an 
if()-block, also no big problem then: then you will get a small unnecessary 
library installed on Windows (not nice, but it doesn't break anything, and it 
should be really tiny).
I really think you should try to do something in your installer-tools so such 
accidentally installed libs are excluded from the install package.

The attached patch should do this, but I didn't have time now to actually test 
it, it's late already...

Bye
Alex
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kdeinit.diff
Type: text/x-diff
Size: 4298 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20080407/3cb3a924/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kde4init_win32lib_dummy.cpp.in
Type: text/x-csrc
Size: 76 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20080407/3cb3a924/attachment-0001.bin