Hello,
On 03/07/13 20:21, Brad King wrote:
> The open request's discussion identified some concerns about the
> SelectLibraryConfigurations module, so please start another thread
> to raise the concerns here (or respond to this message and change the
> subject line).
I was discussing with Brad about using SelectLibraryConfigurations in
FindGTK2.cmake, but there are a couple of things that I don't like in
this module:
SelectLibraryConfigurations module currently cache and mark as advanced
the variable ${basename}_LIBRARY.
${basename}_LIBRARY_RELEASE and ${basename}_LIBRARY_DEBUG are usually
cached, because they often come from find_library().
${basename}_LIBRARY on the other hand is always of type
"optimized;${${basename}_LIBRARY_RELEASE};debug;${${basename}_LIBRARY_DEBUG}"
or just "${basename}_LIBRARY_RELEASE" or "${basename}_LIBRARY_DEBUG" if
only one version of the library is not found, if both have the same
value, or if configuration types are not supported.
I believe that caching and marking as advanced just
${basename}_LIBRARY_RELEASE and ${basename}_LIBRARY_DEBUG is enough,
just by modifying these two variables, the user has enough control on
finding the library, and having 3 variables is IMO redundant and confusing.
So, is there a reason why ${basename}_LIBRARY is cached?
Another issue is that if one of the libraries (_DEBUG or _RELEASE) is
not set, the value is set to the value of the other one.
FindQt4, from which the macro is extracted, sets the values to
XXX_LIBRARY_{DEBUG,RELEASE}-NOTFOUND instead.
In both cases the XXX_LIBRARY is correct, but IMO using NOTFOUND makes
it easier to understand which one is missing.
I tried to fix these issues, you can find the patches attached.
Regards,
Daniele
>From bdc46bb45b395e0400302e36487dfbe81db69820 Mon Sep 17 00:00:00 2001
From: "Daniele E. Domenichelli" <[email protected]>
Date: Mon, 8 Jul 2013 14:37:23 +0200
Subject: [PATCH 1/2] SelectLibraryConfigurations: Do not cache the _LIBRARY
variable
---
Modules/SelectLibraryConfigurations.cmake | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/Modules/SelectLibraryConfigurations.cmake b/Modules/SelectLibraryConfigurations.cmake
index 5bca064..f475d87 100644
--- a/Modules/SelectLibraryConfigurations.cmake
+++ b/Modules/SelectLibraryConfigurations.cmake
@@ -73,15 +73,11 @@ macro( select_library_configurations basename )
endif()
endif()
- set( ${basename}_LIBRARY ${${basename}_LIBRARY} CACHE FILEPATH
- "The ${basename} library" )
-
if( ${basename}_LIBRARY )
set( ${basename}_FOUND TRUE )
endif()
- mark_as_advanced( ${basename}_LIBRARY
- ${basename}_LIBRARY_RELEASE
+ mark_as_advanced( ${basename}_LIBRARY_RELEASE
${basename}_LIBRARY_DEBUG
)
endmacro()
--
1.7.10.4
>From cb9a2c622b21edb18e1d13db73c4d53e244a3bba Mon Sep 17 00:00:00 2001
From: "Daniele E. Domenichelli" <[email protected]>
Date: Mon, 8 Jul 2013 17:37:18 +0200
Subject: [PATCH 2/2] SelectLibraryConfigurations: Use -NOTFOUND instead of
copying the vars
---
Modules/SelectLibraryConfigurations.cmake | 58 ++++++++++++-----------------
1 file changed, 23 insertions(+), 35 deletions(-)
diff --git a/Modules/SelectLibraryConfigurations.cmake b/Modules/SelectLibraryConfigurations.cmake
index f475d87..eebaf90 100644
--- a/Modules/SelectLibraryConfigurations.cmake
+++ b/Modules/SelectLibraryConfigurations.cmake
@@ -31,48 +31,36 @@
# This macro was adapted from the FindQt4 CMake module and is maintained by Will
# Dicharry <[email protected]>.
-# Utility macro to check if one variable exists while another doesn't, and set
-# one that doesn't exist to the one that exists.
-macro( _set_library_name basename GOOD BAD )
- if( ${basename}_LIBRARY_${GOOD} AND NOT ${basename}_LIBRARY_${BAD} )
- set( ${basename}_LIBRARY_${BAD} ${${basename}_LIBRARY_${GOOD}} )
- set( ${basename}_LIBRARY ${${basename}_LIBRARY_${GOOD}} )
- set( ${basename}_LIBRARIES ${${basename}_LIBRARY_${GOOD}} )
- endif()
-endmacro()
-
macro( select_library_configurations basename )
- # if only the release version was found, set the debug to be the release
- # version.
- _set_library_name( ${basename} RELEASE DEBUG )
- # if only the debug version was found, set the release value to be the
- # debug value.
- _set_library_name( ${basename} DEBUG RELEASE )
-
- # Set a default case, which will come into effect if
- # -no build type is set and the generator only supports one build type
- # at a time (i.e. CMAKE_CONFIGURATION_TYPES is false)
- # -${basename}_LIBRARY_DEBUG and ${basename}_LIBRARY_RELEASE are the same
- # -${basename}_LIBRARY_DEBUG and ${basename}_LIBRARY_RELEASE are both empty
- set( ${basename}_LIBRARY ${${basename}_LIBRARY_RELEASE} )
- set( ${basename}_LIBRARIES ${${basename}_LIBRARY_RELEASE} )
+ if(NOT ${basename}_LIBRARY_RELEASE)
+ set(${basename}_LIBRARY_RELEASE "${basename}_LIBRARY_RELEASE-NOTFOUND" CACHE FILEPATH "Path to a library.")
+ endif()
+ if(NOT ${basename}_LIBRARY_DEBUG)
+ set(${basename}_LIBRARY_DEBUG "${basename}_LIBRARY_DEBUG-NOTFOUND" CACHE FILEPATH "Path to a library.")
+ endif()
if( ${basename}_LIBRARY_DEBUG AND ${basename}_LIBRARY_RELEASE AND
- NOT ${basename}_LIBRARY_DEBUG STREQUAL ${basename}_LIBRARY_RELEASE )
+ NOT ${basename}_LIBRARY_DEBUG STREQUAL ${basename}_LIBRARY_RELEASE AND
+ ( CMAKE_CONFIGURATION_TYPES OR CMAKE_BUILD_TYPE ) )
# if the generator supports configuration types or CMAKE_BUILD_TYPE
# is set, then set optimized and debug options.
- if( CMAKE_CONFIGURATION_TYPES OR CMAKE_BUILD_TYPE )
- set( ${basename}_LIBRARY "" )
- foreach( _libname IN LISTS ${basename}_LIBRARY_RELEASE )
- list( APPEND ${basename}_LIBRARY optimized "${_libname}" )
- endforeach()
- foreach( _libname IN LISTS ${basename}_LIBRARY_DEBUG )
- list( APPEND ${basename}_LIBRARY debug "${_libname}" )
- endforeach()
- set( ${basename}_LIBRARIES "${${basename}_LIBRARY}" )
- endif()
+ set( ${basename}_LIBRARY "" )
+ foreach( _libname IN LISTS ${basename}_LIBRARY_RELEASE )
+ list( APPEND ${basename}_LIBRARY optimized "${_libname}" )
+ endforeach()
+ foreach( _libname IN LISTS ${basename}_LIBRARY_DEBUG )
+ list( APPEND ${basename}_LIBRARY debug "${_libname}" )
+ endforeach()
+ elseif( ${basename}_LIBRARY_RELEASE )
+ set( ${basename}_LIBRARY ${${basename}_LIBRARY_RELEASE} )
+ elseif( ${basename}_LIBRARY_DEBUG )
+ set( ${basename}_LIBRARY ${${basename}_LIBRARY_DEBUG} )
+ else()
+ set( ${basename}_LIBRARY "${basename}_LIBRARY-NOTFOUND")
endif()
+ set( ${basename}_LIBRARIES "${${basename}_LIBRARY}" )
+
if( ${basename}_LIBRARY )
set( ${basename}_FOUND TRUE )
endif()
--
1.7.10.4
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
