Conditional installing of proxymodeltestsuite

[Stephen Kelly]

Alexander Neundorf wrote:

Hi,

Thanks for the feedback.

> On Wednesday 28 July 2010, Stephen Kelly wrote:
>> I guess I should attach the patch.
> 
> 
> diff --git a/kdeui/tests/proxymodeltestsuite/CMakeLists.txt
> b/kdeui/tests/proxymodeltestsuite/CMakeLists.txt
> index 5194511..dc4175e 100644
> --- a/kdeui/tests/proxymodeltestsuite/CMakeLists.txt
> +++ b/kdeui/tests/proxymodeltestsuite/CMakeLists.txt
> @@ -2,11 +2,22 @@ project(proxymodeltestsuite)
>  
>  set( EXECUTABLE_OUTPUT_PATH ${CMAKE_CURRENT_BINARY_DIR} )
>  
> +if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
> +
> +  # Building the test suite standalone
> +  set(proxymodeltestsuite_standalone TRUE)
> 
> -------------------------------------------------
> good idea to use a helper variable
> -------------------------------------------------
> 
> +
> +  find_package(Qt4 REQUIRED)
> +  find_package(Automoc4 REQUIRED)
> +  cmake_minimum_required(VERSION 2.6.3)
> +endif()
> +
> +include(${QT_USE_FILE})
> 
> -------------------------------------------------
> I think you don't need QT_USE_FILE here, I mean, you set the include
> directories explicitely and you also link against ${QT_QTTEST_LIBRARY}, so
> I think this is not necessary.

Ok. Actually I didn't even know what that does. At some point in Grantlee 
development I needed that to build, so now I put it in all CMake Qt only 
projects.

> -------------------------------------------------
>  
>  include_directories(
> -  ${CMAKE_CURRENT_BINARY_DIR}
> -  ${CMAKE_CURRENT_BINARY_DIR}/..
> -  ${CMAKE_CURRENT_SOURCE_DIR}/..
> +  ${QT_INCLUDES}
> +  ${proxymodeltestsuite_SOURCE_DIR}
> +  ${proxymodeltestsuite_BINARY_DIR}
>  )
> 
> -------------------------------------------------
> Isn't this the current directory now, while before it was the parent of
> the current directory ?

Yes. But those parent includes are historical and not needed anymore. I've 
already removed them in a separate patch.

> -------------------------------------------------
> 
>  @@ -51,15 +71,49 @@ if (Grantlee_FOUND)
>  
>  endif()
>  
> -kde4_add_library(proxymodeltestsuite SHARED
> -  ${proxymodeltestsuite_SRCS}
> -  ${eventlogger_RCS_SRCS}
> -)
> +if(proxymodeltestsuite_standalone)
> +  automoc4_add_library(proxymodeltestsuite SHARED
> +    ${proxymodeltestsuite_SRCS}
> +    ${eventlogger_RCS_SRCS}
> +  )
> +else ()
> +  kde4_add_library(proxymodeltestsuite SHARED
> +    ${proxymodeltestsuite_SRCS}
> +    ${eventlogger_RCS_SRCS}
> +  )
> +endif()
> 
> 
> -------------------------------------------------
> If it needs to build correctly also without KDE, I would remove the
> kde4_add_library() completely and always use the other branch.
> I just extended the documentation for kde4_add_library() in
> FindKDE4Internal.cmake a bit.
> kde4_add_library() adds enable_final (you don't need that), automoc (you
> already have that), 

> and it sets the LINK_INTERFACE_LIBRARIES property to
> empty (you may want to do this too).

I'm not sure how.

> If used inside of KDE, the global RPATH variables will still be active
> also when just using automoc4_add_library().
> -------------------------------------------------
> 
>  
>  target_link_libraries(proxymodeltestsuite
> -   ${KDE4_KDEUI_LIBS}
>     ${QT_QTTEST_LIBRARY}
>     ${Grantlee_CORE_LIBRARIES}
>  )
> 
> -------------------------------------------------
> I guess this can go in independent from the rest, right ?

Yes, I've committed that separately too.

> -------------------------------------------------
> 
>  
> +if(proxymodeltestsuite_standalone)
> +
> +  set (LIB_SUFFIX "" CACHE STRING "Define suffix of library directory
> name (eg. '64')")
> +  set( LIB_INSTALL_DIR ${CMAKE_INSTALL_PREFIX}/lib${LIB_SUFFIX} )
> +  set( BIN_INSTALL_DIR ${CMAKE_INSTALL_PREFIX}/bin )
> +  set( INCLUDE_INSTALL_DIR ${CMAKE_INSTALL_PREFIX}/include )
> +
> +  install(TARGETS proxymodeltestsuite
> +         RUNTIME DESTINATION ${BIN_INSTALL_DIR}
> +         LIBRARY DESTINATION ${LIB_INSTALL_DIR}
> +         ARCHIVE DESTINATION ${LIB_INSTALL_DIR}
> +         COMPONENT Devel
> +  )
> +
> +  install(FILES
> +    dynamictreemodel.h
> +    dynamictreewidget.h
> +    modelcommander.h
> +    modelspy.h
> +    modelselector.h
> +    modeltest.h
> +    proxymodeltest.h
> +    modeldumper.h
> +    modeleventlogger.h
> +    indexfinder.h
> +    DESTINATION ${INCLUDE_INSTALL_DIR}
> +  )
> 
> -------------------------------------------------
> I think you can just hardcode bin/, lib${LIB_SUFFIX}/ and include/ as the
> DESTINATIONs here, putting LIB_SUFFIX in the cache is probably still a
> good idea.

Done. I left the include one as a variable as it is used twice.

New patch attached.

> -------------------------------------------------
>  
> Alex
-------------- next part --------------
A non-text attachment was scrubbed...
Name: install_proxymodeltestsuite.patch
Type: text/x-patch
Size: 2571 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20100729/a57d8c7f/attachment.patch