[cmake-developers] [PATCH 1/3] wxWidgets module: allow specifying required version

[Rolf Eike Beer]

Am Samstag, 2. Mai 2015, 00:59:38 schrieb Simon Richter:
> ---
>  Modules/FindwxWidgets.cmake | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/Modules/FindwxWidgets.cmake b/Modules/FindwxWidgets.cmake
> index c16c011..ecce49c 100644
> --- a/Modules/FindwxWidgets.cmake
> +++ b/Modules/FindwxWidgets.cmake
> @@ -103,6 +103,12 @@
>  #    include(${wxWidgets_USE_FILE})
>  #    # and for each of your dependent executable/library targets:
>  #    target_link_libraries(<YourTarget> ${wxWidgets_LIBRARIES})
> +#
> +# If a wxWidgets version or greater is required:
> +#   find_package(wxWidgets 2.8.12 COMPONENTS net gl core base REQUIRED)
> +#
> +# If specific wxWidgets version is required:
> +#   find_package(wxWidgets 2.8.12 EXACT COMPONENTS net gl core base
> REQUIRED)

Drop that, it is already documented in the find_package command documentation. 
Otherwise we would end up replicating half of that documentation in every find 
module.

> @@ -856,6 +862,28 @@ else()
>    endif()
>  endif()
> 
> +# Check if a specfic version was requested by find_package().
> +if(wxWidgets_FOUND AND wxWidgets_FIND_VERSION)

Drop that if, just extract the version always. And just make it dependend on 
the include paths being found. That way even if no specific version was 
requested the version found will be shown in CMake output.

> +  find_file(_filename wx/version.h PATHS ${wxWidgets_INCLUDE_DIRS}
> NO_DEFAULT_PATH) +  dbg_msg("_filename:  ${_filename}")
> +
> +  if(NOT _filename)
> +    message(FATAL_ERROR "wxWidgets wx/version.h file not found in
> ${wxWidgets_INCLUDE_DIRS}.") +  endif()
> +
> +  file(READ ${_filename} _wx_version_h)
> +
> +  string(REGEX REPLACE "^(.*\n)?#define wxMAJOR_VERSION[ ]+([0-9]+).*"
> +    "\\2" wxWidgets_VERSION_MAJOR "${_wx_version_h}" )
> +  string(REGEX REPLACE "^(.*\n)?#define wxMINOR_VERSION[ ]+([0-9]+).*"
> +    "\\2" wxWidgets_VERSION_MINOR "${_wx_version_h}" )
> +  string(REGEX REPLACE "^(.*\n)?#define wxRELEASE_NUMBER[ ]+([0-9]+).*"
> +    "\\2" wxWidgets_VERSION_PATCH "${_wx_version_h}" )

You should allow any amount of spaces and tabs also between the #define and the 
constant name, if they decide to reformat their sources in the next version 
the CMake module will continue to work.

> +  set(wxWidgets_VERSION_STRING
> +   
> "${wxWidgets_VERSION_MAJOR}.${wxWidgets_VERSION_MINOR}.${wxWidgets_VERSION_
> PATCH}" ) +  dbg_msg("wxWidgets_VERSION_STRING:   
> ${wxWidgets_VERSION_STRING}") +endif()
> +
>  # Debug output:
>  DBG_MSG("wxWidgets_FOUND           : ${wxWidgets_FOUND}")
>  DBG_MSG("wxWidgets_INCLUDE_DIRS    : ${wxWidgets_INCLUDE_DIRS}")
> @@ -866,10 +894,14 @@ DBG_MSG("wxWidgets_USE_FILE        :
> ${wxWidgets_USE_FILE}")
> 
>  #=====================================================================
>  #=====================================================================
> -include(${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake)
> -FIND_PACKAGE_HANDLE_STANDARD_ARGS(wxWidgets DEFAULT_MSG wxWidgets_FOUND)
> -# Maintain consistency with all other variables.
> -set(wxWidgets_FOUND ${WXWIDGETS_FOUND})
> +
> +include(FindPackageHandleStandardArgs)

The other include is the correct one for an upstream CMake module.

> +
> +find_package_handle_standard_args(wxWidgets
> +  FOUND_VAR     wxWidgets_FOUND

You can drop that, this variable will always be set. On the other hand: the 
variable is already set above by hand, which probably is also wrong.

Greetings,

Eike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20150502/7ae1486c/attachment.sig>