On Sunday 06 April 2008, Alexander Neundorf wrote:
>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.
I do not understand some things: There is a complete platform
independent solution which looks nice to the package developer, which
removes the if(...) issues in the CMakeLists.txt for ever
and it is rejected against a low level platform specific solution
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)
which is
- only full understandable by inspecting KDE4Macros.cmake,
- forces unix developer to know windows install behavior to setup the
the build system correctly,
- needs ongoing patch effort of kde windows developer in case linux devs
forgot to think about windows install behavior and
- needs patching of an outside tool
only to avoid introducing two macros, which are designed to use the
known call sequence(add_... _..link_libraries, __install) ????
Sorry, please take this not personally, but I do not think that this is
the right way :-P - I believe a build system should make it as much as
easy for package developers and to encapsulate platform issue as much as
possible into some internal macros and if it is required to introduce
new macros then it should be.
Anyway, appended is an updated patch - it needed some fixes and now does
not requires an additional .cpp dummy file. Feel free to apply it. I
will change the client code where required.
Bye
Ralf
Index: KDE4Macros.cmake
===================================================================
--- KDE4Macros.cmake (revision 794385)
+++ KDE4Macros.cmake (working copy)
@@ -718,33 +718,45 @@
kde4_check_executable_params(_SRCS _nogui _uninst _test ${ARGN})
-# if (WIN32)
-# # under windows, just build a normal executable
-# KDE4_ADD_EXECUTABLE(${_target_NAME}
${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp ${ARGN} )
-# else (WIN32)
- # under UNIX, create a shared library and a small executable, which
links to this library
+ kde4_handle_automoc(${_target_NAME} _SRCS)
- kde4_handle_automoc(kdeinit_${_target_NAME} _SRCS)
- if (KDE4_ENABLE_FINAL)
-
kde4_create_final_files(${CMAKE_CURRENT_BINARY_DIR}/kdeinit_${_target_NAME}_final_cpp.cpp
_separate_files ${_SRCS})
- add_library(kdeinit_${_target_NAME} SHARED
${CMAKE_CURRENT_BINARY_DIR}/kdeinit_${_target_NAME}_final_cpp.cpp
${_separate_files})
+ configure_file(${KDE4_MODULE_DIR}/kde4init_dummy.cpp.in
${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp)
- else (KDE4_ENABLE_FINAL)
- add_library(kdeinit_${_target_NAME} SHARED ${_SRCS})
- endif (KDE4_ENABLE_FINAL)
+ # under Windows, build a normal executable and additionally a dummy
kdeinit4_foo.lib, whose only purpose on windows is to
+ # keep the linking logic from the CMakeLists.txt on UNIX working (under
UNIX all necessary libs are linked against the kdeinit
+ # library instead against the executable, under windows we want to have
everything in the executable, but for compatibility we have to
+ # keep the library there-
+ if(WIN32)
+ set(_KDEINIT4_TARGET_NAME_ ${_target_NAME})
+ add_library(kdeinit_${_target_NAME} STATIC
${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp)
- kde4_handle_rpath_for_library(kdeinit_${_target_NAME})
- set_target_properties(kdeinit_${_target_NAME} PROPERTIES OUTPUT_NAME
kdeinit4_${_target_NAME})
+ if (KDE4_ENABLE_FINAL)
+
kde4_create_final_files(${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_final_cpp.cpp
_separate_files ${_SRCS})
+ kde4_add_executable(${_target_NAME} "${_nogui}" "${_uninst}"
${CMAKE_CURRENT_BINARY_DIR}/kdeinit_${_target_NAME}_final_cpp.cpp
${_separate_files} ${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp)
- configure_file(${KDE4_MODULE_DIR}/kde4init_dummy.cpp.in
${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp)
- kde4_add_executable(${_target_NAME} "${_nogui}" "${_uninst}"
${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp)
- target_link_libraries(${_target_NAME} kdeinit_${_target_NAME})
-# endif (WIN32)
+ else (KDE4_ENABLE_FINAL)
+ kde4_add_executable(${_target_NAME} "${_nogui}" "${_uninst}" ${_SRCS}
${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp)
+ endif (KDE4_ENABLE_FINAL)
- if (WIN32)
- target_link_libraries(${_target_NAME} ${QT_QTMAIN_LIBRARY})
- endif (WIN32)
+ set_target_properties(kdeinit_${_target_NAME} PROPERTIES OUTPUT_NAME
kdeinit4_${_target_NAME})
+ target_link_libraries(${_target_NAME} ${QT_QTMAIN_LIBRARY}
kdeinit_${_target_NAME})
+ else(WIN32)
+ if (KDE4_ENABLE_FINAL)
+
kde4_create_final_files(${CMAKE_CURRENT_BINARY_DIR}/kdeinit_${_target_NAME}_final_cpp.cpp
_separate_files ${_SRCS})
+ add_library(kdeinit_${_target_NAME} SHARED
${CMAKE_CURRENT_BINARY_DIR}/kdeinit_${_target_NAME}_final_cpp.cpp
${_separate_files})
+
+ else (KDE4_ENABLE_FINAL)
+ add_library(kdeinit_${_target_NAME} SHARED ${_SRCS})
+ endif (KDE4_ENABLE_FINAL)
+
+ kde4_handle_rpath_for_library(kdeinit_${_target_NAME})
+ set_target_properties(kdeinit_${_target_NAME} PROPERTIES OUTPUT_NAME
kdeinit4_${_target_NAME})
+
+ kde4_add_executable(${_target_NAME} "${_nogui}" "${_uninst}"
${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp)
+ target_link_libraries(${_target_NAME} kdeinit_${_target_NAME})
+ endif(WIN32)
+
endmacro (KDE4_ADD_KDEINIT_EXECUTABLE)
# add a unit test, which is executed when running make test
_______________________________________________
Kde-buildsystem mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-buildsystem
