[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: Re: KDE/kdelibs/cmake/modules
From: Alexander Neundorf <neundorf () kde ! org>
Date: 2011-01-11 21:12:23
Message-ID: 201101112212.23221.neundorf () kde ! org
[Download RAW message or body]
Hi Gilles,
On Monday 03 January 2011, Gilles Caulier wrote:
> SVN commit 1211248 by cgilles:
>
> support local dir for digiKam 2.0.0
This looks like a good idea :-)
> M +32 -17 FindKexiv2.cmake
> M +28 -14 FindKipi.cmake
>
>
> --- trunk/KDE/kdelibs/cmake/modules/FindKexiv2.cmake #1211247:1211248
> @@ -1,4 +1,8 @@
> # - Try to find the KExiv2 library
> +#
> +# If you have put a local version of libkexiv2 into your source tree,
> +# set KEXIV2_LOCAL_DIR to the relative path to the local directory.
> +#
Making this documentation a bit more verbose would be a good idea :-)
Maybe also the variable could get an even more descriptive name, e.g.
KEXIV2_LOCAL_SOURCE_DIR or KEXIV2_LOCAL_SRC_DIR or KEXIV2_SOURCE_DIR or
KEXIV2_SOURCES_DIR or KEXIV2_IN_PROJECT_SOURCE_DIR or something...
Maybe also using "exact-case" for the name-part would be a good idea, since
this is what cmake does for these kind of input-variables, i.e. make it
KExiv2_LOCAL_SOURCE_DIR (see e.g. the documentation of find_package, there a
variable ExactCase_DIR can be set to point to an installed package).
Maybe my favourite would be KExiv2_SOURCES_DIR, and equivalent for kipi.
Some more small comments below.
> # Once done this will define
> #
> # KEXIV2_FOUND - system has libkexiv2
> @@ -12,34 +16,45 @@
> # Redistribution and use is allowed according to the terms of the BSD
> license. # For details see the accompanying COPYING-CMAKE-SCRIPTS file.
>
> -if (KEXIV2_INCLUDE_DIR AND KEXIV2_LIBRARIES)
> +if (KEXIV2_INCLUDE_DIR AND KEXIV2_LIBRARIES AND KEXIV2_DEFINITIONS)
>
> message(STATUS "Found Kexiv2 library in cache: ${KEXIV2_LIBRARIES}")
>
> # in cache already
> - SET(KEXIV2_FOUND TRUE)
> + set(KEXIV2_FOUND TRUE)
>
> -else (KEXIV2_INCLUDE_DIR AND KEXIV2_LIBRARIES)
> +else (KEXIV2_INCLUDE_DIR AND KEXIV2_LIBRARIES AND KEXIV2_DEFINITIONS)
>
> message(STATUS "Check Kexiv2 library in local sub-folder...")
>
> # Check if library is not in local sub-folder
>
> - FIND_FILE(KEXIV2_LOCAL_FOUND libkexiv2/version.h.cmake
> ${CMAKE_SOURCE_DIR}/libkexiv2 ${CMAKE_SOURCE_DIR}/libs/libkexiv2
> NO_DEFAULT_PATH) + if (KEXIV2_LOCAL_DIR)
> + set(KEXIV2_LOCAL_FOUND TRUE)
> + else (KEXIV2_LOCAL_DIR)
> + find_file(KEXIV2_LOCAL_FOUND libkexiv2/version.h.cmake
> ${CMAKE_SOURCE_DIR}/libkexiv2 ${CMAKE_SOURCE_DIR}/libs/libkexiv2
> NO_DEFAULT_PATH)
>
> if (KEXIV2_LOCAL_FOUND)
> -
> - FIND_FILE(KEXIV2_LOCAL_FOUND_IN_LIBS libkexiv2/version.h.cmake
> ${CMAKE_SOURCE_DIR}/libs/libkexiv2 NO_DEFAULT_PATH)
> + # Was it found in libkexiv2/ or in libs/libkexiv2?
> + find_file(KEXIV2_LOCAL_FOUND_IN_LIBS libkexiv2/version.h.cmake
> ${CMAKE_SOURCE_DIR}/libs/libkexiv2 NO_DEFAULT_PATH) if
> (KEXIV2_LOCAL_FOUND_IN_LIBS)
> - set(KEXIV2_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/libs/libkexiv2)
> + set(KEXIV2_LOCAL_DIR libs/libkexiv2)
> else (KEXIV2_LOCAL_FOUND_IN_LIBS)
> - set(KEXIV2_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/libkexiv2)
> + set(KEXIV2_LOCAL_DIR libkexiv2)
> endif (KEXIV2_LOCAL_FOUND_IN_LIBS)
> - set(KEXIV2_DEFINITIONS "-I${KEXIV2_INCLUDE_DIR}")
> + endif (KEXIV2_LOCAL_FOUND)
> +
> + endif (KEXIV2_LOCAL_DIR)
> +
> + if (KEXIV2_LOCAL_FOUND)
> + # we need two include directories: because the version.h file is put
> into the build directory
> + # TODO KEXIV2_INCLUDE_DIR sounds like it
> should contain only one directory...
> + set(KEXIV2_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/${KEXIV2_LOCAL_DIR}
> ${CMAKE_BINARY_DIR}/${KEXIV2_LOCAL_DIR})
> + set(KEXIV2_DEFINITIONS "-I${CMAKE_SOURCE_DIR}/${KEXIV2_LOCAL_DIR}"
> "-I${CMAKE_BINARY_DIR}/${KEXIV2_LOCAL_DIR}")
Why do you need to set KEXIV2_DEFINITIONS to contain the include dirs, when
you have them set already in KEXIV2_INCLUDE_DIR ?
> set(KEXIV2_LIBRARIES kexiv2)
> - message(STATUS "Found Kexiv2 library in local sub-folder:
> ${KEXIV2_INCLUDE_DIR}")
> + message(STATUS "Found Kexiv2 library in local
> sub-folder: ${CMAKE_SOURCE_DIR}/${KEXIV2_LOCAL_DIR}") set(KEXIV2_FOUND TRUE)
> - MARK_AS_ADVANCED(KEXIV2_INCLUDE_DIR KEXIV2_LIBRARIES)
> + mark_as_advanced(KEXIV2_INCLUDE_DIR KEXIV2_LIBRARIES
> KEXIV2_DEFINITIONS)
>
> else(KEXIV2_LOCAL_FOUND)
> if(NOT WIN32)
> @@ -47,13 +62,13 @@
>
> # use pkg-config to get the directories and then use these values
> # in the FIND_PATH() and FIND_LIBRARY() calls
> - INCLUDE(UsePkgConfig)
> + include(UsePkgConfig)
>
> PKGCONFIG(libkexiv2 _KEXIV2IncDir _KEXIV2LinkDir _KEXIV2LinkFlags
> _KEXIV2Cflags)
Please have a look at FindPkgConfig.cmake instead of using UsePkgConfig.cmake,
it is better maintained and more powerful.
> if(_KEXIV2LinkFlags)
> # query pkg-config asking for a libkexiv2 >= 0.2.0
> - EXEC_PROGRAM(${PKGCONFIG_EXECUTABLE} ARGS --atleast-version=0.2.0
> libkexiv2 RETURN_VALUE _return_VALUE OUTPUT_VARIABLE _pkgconfigDevNull )
> + exec_program(${PKGCONFIG_EXECUTABLE} ARGS --atleast-version=0.2.0
> libkexiv2 RETURN_VALUE _return_VALUE OUTPUT_VARIABLE _pkgconfigDevNull )
> if(_return_VALUE STREQUAL "0")
> message(STATUS "Found libkexiv2 release >= 0.2.0")
> set(KEXIV2_VERSION_GOOD_FOUND TRUE)
> @@ -73,11 +88,11 @@
> if(KEXIV2_VERSION_GOOD_FOUND)
> set(KEXIV2_DEFINITIONS "${_KEXIV2Cflags}")
>
> - FIND_PATH(KEXIV2_INCLUDE_DIR libkexiv2/version.h
> + find_path(KEXIV2_INCLUDE_DIR libkexiv2/version.h
> ${_KEXIV2IncDir}
> )
>
> - FIND_LIBRARY(KEXIV2_LIBRARIES NAMES kexiv2
> + find_library(KEXIV2_LIBRARIES NAMES kexiv2
> PATHS
> ${_KEXIV2LinkDir}
> )
> @@ -101,8 +116,8 @@
> endif (Kexiv2_FIND_REQUIRED)
> endif (KEXIV2_FOUND)
>
> - MARK_AS_ADVANCED(KEXIV2_INCLUDE_DIR KEXIV2_LIBRARIES)
> + mark_as_advanced(KEXIV2_INCLUDE_DIR KEXIV2_LIBRARIES
> KEXIV2_DEFINITIONS)
>
> endif(KEXIV2_LOCAL_FOUND)
>
> -endif (KEXIV2_INCLUDE_DIR AND KEXIV2_LIBRARIES)
> +endif (KEXIV2_INCLUDE_DIR AND KEXIV2_LIBRARIES AND KEXIV2_DEFINITIONS)
> --- trunk/KDE/kdelibs/cmake/modules/FindKipi.cmake #1211247:1211248
> @@ -1,4 +1,8 @@
> # - Try to find the Kipi library
> +#
> +# If you have put a local version of libkipi into your source tree,
> +# set KIPI_LOCAL_DIR to the relative path to the local directory.
> +#
> # Once done this will define
> #
> # KIPI_FOUND - system has libkipi
> @@ -12,34 +16,44 @@
> # Redistribution and use is allowed according to the terms of the BSD
> license. # For details see the accompanying COPYING-CMAKE-SCRIPTS file.
>
> -if (KIPI_INCLUDE_DIR AND KIPI_LIBRARIES)
> +if (KIPI_INCLUDE_DIR AND KIPI_LIBRARIES AND KIPI_DEFINITIONS)
>
> message(STATUS "Found Kipi library in cache: ${KIPI_LIBRARIES}")
>
> # in cache already
> - SET(KIPI_FOUND TRUE)
> + set(KIPI_FOUND TRUE)
>
> -else (KIPI_INCLUDE_DIR AND KIPI_LIBRARIES)
> +else (KIPI_INCLUDE_DIR AND KIPI_LIBRARIES AND KIPI_DEFINITIONS)
>
> message(STATUS "Check Kipi library in local sub-folder...")
>
> # Check if library is not in local sub-folder
>
> - find_file (KIPI_LOCAL_FOUND libkipi/version.h.cmake
> ${CMAKE_SOURCE_DIR}/libkipi ${CMAKE_SOURCE_DIR}/libs/libkipi
> NO_DEFAULT_PATH) + if (KIPI_LOCAL_DIR)
> + set (KIPI_LOCAL_FOUND TRUE)
> + else (KIPI_LOCAL_DIR)
> + find_file(KIPI_LOCAL_FOUND libkipi/kipi.h ${CMAKE_SOURCE_DIR}/libkipi
> ${CMAKE_SOURCE_DIR}/libs/libkipi NO_DEFAULT_PATH)
>
> if (KIPI_LOCAL_FOUND)
> -
> - find_file (KIPI_LOCAL_FOUND_IN_LIBS libkipi/version.h.cmake
> ${CMAKE_SOURCE_DIR}/libs/libkipi NO_DEFAULT_PATH) + # Was it found in
> libkdcraw/ or in libs/libkdcraw?
> + find_file(KIPI_LOCAL_FOUND_IN_LIBS libkipi/kipi.h
> ${CMAKE_SOURCE_DIR}/libs/libkipi NO_DEFAULT_PATH) if
> (KIPI_LOCAL_FOUND_IN_LIBS)
> - set(KIPI_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/libs/libkipi)
> + set(KIPI_LOCAL_DIR libs/libkipi)
> else (KIPI_LOCAL_FOUND_IN_LIBS)
> - set(KIPI_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/libkipi)
> + set(KIPI_LOCAL_DIR libkipi)
> endif (KIPI_LOCAL_FOUND_IN_LIBS)
> - set(KIPI_DEFINITIONS -I${KIPI_INCLUDE_DIR})
> + endif (KIPI_LOCAL_FOUND)
> + endif (KIPI_LOCAL_DIR)
> +
> + if (KIPI_LOCAL_FOUND)
> + # we need two include directories: because the version.h file is put
> into the build directory
> + # TODO KIPI_INCLUDE_DIR sounds like it should
> contain only one directory...
> + set(KIPI_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/${KIPI_LOCAL_DIR}
> ${CMAKE_BINARY_DIR}/${KIPI_LOCAL_DIR})
> + set(KIPI_DEFINITIONS "-I${CMAKE_SOURCE_DIR}/${KIPI_LOCAL_DIR}"
> "-I${CMAKE_BINARY_DIR}/${KIPI_LOCAL_DIR}")
Same as above, why do you need the include dirs in KIPI_DEFINITIONS ?
> set(KIPI_LIBRARIES kipi)
> - message(STATUS "Found Kipi library in local sub-folder:
> ${KIPI_INCLUDE_DIR}") + message(STATUS "Found Kipi library in local
> sub-folder: ${CMAKE_SOURCE_DIR}/${KIPI_LOCAL_DIR}") set(KIPI_FOUND TRUE)
> - mark_as_advanced(KIPI_INCLUDE_DIR KIPI_LIBRARIES)
> + mark_as_advanced(KIPI_INCLUDE_DIR KIPI_LIBRARIES KIPI_DEFINITIONS)
>
> else(KIPI_LOCAL_FOUND)
>
> @@ -48,7 +62,7 @@
>
> # use pkg-config to get the directories and then use these values
> # in the FIND_PATH() and FIND_LIBRARY() calls
> - INCLUDE(UsePkgConfig)
> + include(UsePkgConfig)
Same as above, please consider using FindPkgConfig.cmake (i.e.
find_package(PkgConfig) ) instead.
Alex
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic